From 11403ced42ce71cdbc155e7e4cdacfb777321107 Mon Sep 17 00:00:00 2001 From: smnatale Date: Thu, 16 Apr 2026 16:51:17 +0100 Subject: [PATCH 1/3] extract utils --- lua/coderabbit/health.lua | 6 ++-- lua/coderabbit/history.lua | 6 ++-- lua/coderabbit/parser.lua | 8 ++---- lua/coderabbit/review.lua | 39 ++++++++++++------------- lua/coderabbit/show.lua | 10 +++---- lua/coderabbit/storage.lua | 29 +++++++------------ lua/coderabbit/utils.lua | 58 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 102 insertions(+), 54 deletions(-) create mode 100644 lua/coderabbit/utils.lua diff --git a/lua/coderabbit/health.lua b/lua/coderabbit/health.lua index 2c836d9..2c96579 100644 --- a/lua/coderabbit/health.lua +++ b/lua/coderabbit/health.lua @@ -1,5 +1,7 @@ local M = {} +local utils = require("coderabbit.utils") + function M.check() vim.health.start("coderabbit.nvim") @@ -41,8 +43,8 @@ function M.check() -- Authentication local auth_raw = vim.fn.system({ binary, "auth", "status", "--agent" }) if vim.v.shell_error == 0 then - local ok, auth = pcall(vim.json.decode, auth_raw) - if ok and auth.authenticated then + local auth = utils.json_decode(auth_raw) + if auth and auth.authenticated then local user = auth.user and auth.user.username or "unknown" local org = auth.currentOrg and auth.currentOrg.name or "none" vim.health.ok("Authenticated as " .. user .. " (org: " .. org .. ")") diff --git a/lua/coderabbit/history.lua b/lua/coderabbit/history.lua index 5a0b46f..56ad7db 100644 --- a/lua/coderabbit/history.lua +++ b/lua/coderabbit/history.lua @@ -1,5 +1,7 @@ local M = {} +local utils = require("coderabbit.utils") + local function format_time(timestamp) if not timestamp then return "unknown" @@ -20,7 +22,7 @@ function M.format_entry(entry) if ctx.review_type then table.insert(parts, ctx.review_type) end - table.insert(parts, string.format("%d finding%s", entry.finding_count, entry.finding_count == 1 and "" or "s")) + table.insert(parts, utils.pluralize(entry.finding_count, "finding")) return table.concat(parts, " │ ") end @@ -29,7 +31,7 @@ function M.open() local entries = storage.list() if #entries == 0 then - vim.notify("CodeRabbit: No saved reviews yet", vim.log.levels.INFO) + utils.notify("No saved reviews yet") return end diff --git a/lua/coderabbit/parser.lua b/lua/coderabbit/parser.lua index 87447d8..e538afc 100644 --- a/lua/coderabbit/parser.lua +++ b/lua/coderabbit/parser.lua @@ -1,16 +1,14 @@ local M = {} +local utils = require("coderabbit.utils") + --- Parse a single NDJSON line from `cr review --agent` output. --- Returns a table with at least a `type` field, or nil on parse failure. function M.parse_line(line) if not line or line == "" then return nil end - local ok, data = pcall(vim.json.decode, line) - if not ok or type(data) ~= "table" then - return nil - end - return data + return utils.json_decode(line) end --- Strip the boilerplate prefix from codegenInstructions. diff --git a/lua/coderabbit/review.lua b/lua/coderabbit/review.lua index 073d6bf..98e8f76 100644 --- a/lua/coderabbit/review.lua +++ b/lua/coderabbit/review.lua @@ -4,6 +4,7 @@ local config = require("coderabbit.config") local cli = require("coderabbit.cli") local parser = require("coderabbit.parser") local diagnostics = require("coderabbit.diagnostics") +local utils = require("coderabbit.utils") local spinner_frames = { "⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏" } local FRAME_MS = 80 @@ -98,13 +99,13 @@ function M.run(opts) opts = opts or {} if M.is_running() then - vim.notify("CodeRabbit: Review already in progress", vim.log.levels.WARN) + utils.notify("Review already in progress", vim.log.levels.WARN) return end if not cli.is_available() then - vim.notify( - "CodeRabbit: CLI not found. Install with: curl -fsSL https://cli.coderabbit.ai/install.sh | sh", + utils.notify( + "CLI not found. Install with: curl -fsSL https://cli.coderabbit.ai/install.sh | sh", vim.log.levels.ERROR ) return @@ -116,7 +117,7 @@ function M.run(opts) local finding_count = 0 local got_error = false - vim.notify("CodeRabbit: Reviewing...") + utils.notify("Reviewing...") state.start_time = os.time() state.fidget_handle = fidget_start() @@ -144,7 +145,7 @@ function M.run(opts) local now = os.time() if not state.last_notify_time or (now - state.last_notify_time) >= 20 then state.last_notify_time = now - vim.notify("CodeRabbit: " .. msg, vim.log.levels.INFO) + utils.notify(msg) end end elseif event.type == "finding" then @@ -171,7 +172,7 @@ function M.run(opts) if msg:match("[Aa]uth") then msg = msg .. "\nRun: cr auth login" end - vim.notify("CodeRabbit: " .. msg, vim.log.levels.ERROR) + utils.notify(msg, vim.log.levels.ERROR) end end, @@ -182,7 +183,7 @@ function M.run(opts) if code == -1 then fidget_finish("timed out") - vim.notify("CodeRabbit: Review timed out", vim.log.levels.ERROR) + utils.notify("Review timed out", vim.log.levels.ERROR) return end @@ -193,18 +194,14 @@ function M.run(opts) msg = msg .. "\nRun: cr auth login" end fidget_finish("failed") - vim.notify("CodeRabbit: " .. msg, vim.log.levels.ERROR) + utils.notify(msg, vim.log.levels.ERROR) return end if not got_error then - local summary = string.format( - "CodeRabbit: Review complete. %d finding%s. Run :CodeRabbitShow to view.", - finding_count, - finding_count == 1 and "" or "s" - ) - fidget_finish(string.format("done — %d finding%s", finding_count, finding_count == 1 and "" or "s")) - vim.notify(summary, vim.log.levels.INFO) + local fc = utils.pluralize(finding_count, "finding") + utils.notify("Review complete. " .. fc .. ". Run :CodeRabbitShow to view.") + fidget_finish("done — " .. fc) else fidget_finish("done (with errors)") end @@ -223,16 +220,16 @@ function M.stop() fidget_finish("cancelled") cli.cancel(state.job_id) state.job_id = nil - vim.notify("CodeRabbit: Review cancelled", vim.log.levels.INFO) + utils.notify("Review cancelled") else - vim.notify("CodeRabbit: No review in progress", vim.log.levels.WARN) + utils.notify("No review in progress", vim.log.levels.WARN) end end function M.restore(id) local entries = storage.list() if #entries == 0 then - vim.notify("CodeRabbit: No saved reviews found", vim.log.levels.WARN) + utils.notify("No saved reviews found", vim.log.levels.WARN) return end @@ -242,7 +239,7 @@ function M.restore(id) local review = storage.load(id) if not review then - vim.notify("CodeRabbit: Review #" .. id .. " not found", vim.log.levels.WARN) + utils.notify("Review #" .. id .. " not found", vim.log.levels.WARN) return end @@ -253,7 +250,7 @@ function M.restore(id) diagnostics.set(finding.filepath, { finding.diagnostic }) end - vim.notify(string.format("CodeRabbit: Restored %d findings from review #%d", #findings, id), vim.log.levels.INFO) + utils.notify(string.format("Restored %d findings from review #%d", #findings, id)) end) end @@ -266,7 +263,7 @@ function M.clear() state.current_branch = nil state.base_branch = nil state.base_commit = nil - vim.notify("CodeRabbit: Cleared", vim.log.levels.INFO) + utils.notify("Cleared") end return M diff --git a/lua/coderabbit/show.lua b/lua/coderabbit/show.lua index 8800b55..3082f21 100644 --- a/lua/coderabbit/show.lua +++ b/lua/coderabbit/show.lua @@ -1,5 +1,6 @@ local M = {} +local utils = require("coderabbit.utils") local buf_id = nil local severity_labels = vim.diagnostic.severity @@ -174,7 +175,7 @@ function M.open(id) if id then local entry = review.get_review(id) if not entry then - vim.notify("CodeRabbit: Review #" .. id .. " not found", vim.log.levels.WARN) + utils.notify("Review #" .. id .. " not found", vim.log.levels.WARN) return end findings = entry.findings @@ -187,7 +188,7 @@ function M.open(id) end if #findings == 0 and not running and not context then - vim.notify("CodeRabbit: No review results. Run :CodeRabbitReview first", vim.log.levels.WARN) + utils.notify("No review results. Run :CodeRabbitReview first", vim.log.levels.WARN) return end @@ -198,9 +199,8 @@ function M.open(id) content, 1, string.format( - "> **Review in progress...** Showing %d finding%s so far. Run `:CodeRabbitShow` again to refresh.", - #findings, - #findings == 1 and "" or "s" + "> **Review in progress...** Showing %s so far. Run `:CodeRabbitShow` again to refresh.", + utils.pluralize(#findings, "finding") ) ) end diff --git a/lua/coderabbit/storage.lua b/lua/coderabbit/storage.lua index e6f7773..7debd70 100644 --- a/lua/coderabbit/storage.lua +++ b/lua/coderabbit/storage.lua @@ -1,5 +1,6 @@ local M = {} +local utils = require("coderabbit.utils") local base_dir = vim.fn.stdpath("data") .. "/coderabbit" --- Override the base directory (for testing only). @@ -79,13 +80,7 @@ function M.save(findings, context) path = dir .. "/" .. filename suffix = suffix + 1 end - local file = io.open(path, "w") - if not file then - return nil - end - local ok = file:write(json) - file:close() - if not ok then + if not utils.write_file(path, json) then return nil end return filename @@ -103,12 +98,10 @@ function M.list() local entries = {} for i, path in ipairs(files) do - local file = io.open(path, "r") - if file then - local content = file:read("*a") - file:close() - local ok, data = pcall(vim.json.decode, content) - if ok and type(data) == "table" then + local content = utils.read_file(path) + if content then + local data = utils.json_decode(content) + if data then table.insert(entries, { id = i, timestamp = data.timestamp, @@ -134,14 +127,12 @@ function M.load(id) if not path then return nil end - local file = io.open(path, "r") - if not file then + local content = utils.read_file(path) + if not content then return nil end - local content = file:read("*a") - file:close() - local ok, data = pcall(vim.json.decode, content) - if not ok or type(data) ~= "table" then + local data = utils.json_decode(content) + if not data then return nil end data.id = id diff --git a/lua/coderabbit/utils.lua b/lua/coderabbit/utils.lua new file mode 100644 index 0000000..78e22d9 --- /dev/null +++ b/lua/coderabbit/utils.lua @@ -0,0 +1,58 @@ +local M = {} + +--- Safely decode a JSON string into a table. +--- @param raw string +--- @return table|nil +function M.json_decode(raw) + local ok, data = pcall(vim.json.decode, raw) + if not ok or type(data) ~= "table" then + return nil + end + return data +end + +--- Read an entire file into a string. +--- @param path string +--- @return string|nil +function M.read_file(path) + local file = io.open(path, "r") + if not file then + return nil + end + local content = file:read("*a") + file:close() + return content +end + +--- Write a string to a file, replacing its contents. +--- @param path string +--- @param content string +--- @return boolean success +function M.write_file(path, content) + local file = io.open(path, "w") + if not file then + return false + end + local ok = file:write(content) + file:close() + return ok ~= nil +end + +--- Show a notification prefixed with "CodeRabbit: ". +--- @param msg string +--- @param level number|nil vim.log.levels value (default: INFO) +function M.notify(msg, level) + vim.notify("CodeRabbit: " .. msg, level or vim.log.levels.INFO) +end + +--- Format a count with a word, adding "s" for plurals. +--- e.g. pluralize(1, "finding") -> "1 finding" +--- pluralize(3, "finding") -> "3 findings" +--- @param n number +--- @param word string +--- @return string +function M.pluralize(n, word) + return string.format("%d %s", n, n == 1 and word or word .. "s") +end + +return M From 280452b123181e26da150bdb8721f3ddce58479e Mon Sep 17 00:00:00 2001 From: Sam Natale Date: Fri, 17 Apr 2026 09:54:25 +0100 Subject: [PATCH 2/3] quickfix utils --- lua/coderabbit/quickfix.lua | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lua/coderabbit/quickfix.lua b/lua/coderabbit/quickfix.lua index 3c02da8..9a07776 100644 --- a/lua/coderabbit/quickfix.lua +++ b/lua/coderabbit/quickfix.lua @@ -1,5 +1,7 @@ local M = {} +local utils = require("coderabbit.utils") + local severity_types = { [vim.diagnostic.severity.ERROR] = "E", [vim.diagnostic.severity.WARN] = "W", @@ -56,7 +58,7 @@ function M.populate(id) if id then local entry = review.get_review(id) if not entry then - vim.notify("CodeRabbit: Review #" .. id .. " not found", vim.log.levels.WARN) + utils.notify("Review #" .. id .. " not found", vim.log.levels.WARN) return end findings = type(entry.findings) == "table" and entry.findings or {} @@ -64,7 +66,7 @@ function M.populate(id) else findings = review.get_results() if #findings == 0 and not review.get_context() then - vim.notify("CodeRabbit: No review results. Run :CodeRabbitReview first", vim.log.levels.WARN) + utils.notify("No review results. Run :CodeRabbitReview first", vim.log.levels.WARN) return end title = "CodeRabbit Review" From 53f29892255d28609a6c75a383a7daef024c333a Mon Sep 17 00:00:00 2001 From: Sam Natale Date: Fri, 17 Apr 2026 09:59:56 +0100 Subject: [PATCH 3/3] add test coverage --- tests/coderabbit/utils_spec.lua | 127 ++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 tests/coderabbit/utils_spec.lua diff --git a/tests/coderabbit/utils_spec.lua b/tests/coderabbit/utils_spec.lua new file mode 100644 index 0000000..536862b --- /dev/null +++ b/tests/coderabbit/utils_spec.lua @@ -0,0 +1,127 @@ +local utils = require("coderabbit.utils") +local h = require("tests.helpers") +local test, eq = h.test, h.eq + +-- ────────────────────────────────────────────────────────── +-- Tests: json_decode +-- ────────────────────────────────────────────────────────── + +for _, case in ipairs({ + { "valid object", '{"a":1}', { a = 1 } }, + { "valid array", "[1,2,3]", { 1, 2, 3 } }, + { "empty object", "{}", true }, + { "nil input", nil, nil }, + { "empty string", "", nil }, + { "invalid JSON", "not json", nil }, + { "JSON string (non-table)", '"hello"', nil }, + { "JSON number (non-table)", "42", nil }, + { "JSON boolean (non-table)", "true", nil }, +}) do + test("json_decode: " .. case[1], function() + local result = utils.json_decode(case[2]) + if case[3] == nil then + eq(result, nil) + elseif case[3] == true then + -- just check it returned a table (e.g. vim.empty_dict) + assert(type(result) == "table", "expected table, got " .. type(result)) + else + eq(vim.inspect(result), vim.inspect(case[3])) + end + end) +end + +-- ────────────────────────────────────────────────────────── +-- Tests: read_file +-- ────────────────────────────────────────────────────────── + +test("read_file: reads existing file", function() + local path = vim.fn.tempname() + vim.fn.writefile({ "hello", "world" }, path) + local content = utils.read_file(path) + assert(content ~= nil, "expected content") + assert(content:find("hello"), "expected 'hello' in content") + assert(content:find("world"), "expected 'world' in content") + os.remove(path) +end) + +test("read_file: returns nil for non-existent file", function() + eq(utils.read_file("/tmp/does_not_exist_" .. os.time()), nil) +end) + +test("read_file: reads empty file", function() + local path = vim.fn.tempname() + vim.fn.writefile({}, path) + local content = utils.read_file(path) + assert(content ~= nil, "expected non-nil for empty file") + eq(content, "") + os.remove(path) +end) + +-- ────────────────────────────────────────────────────────── +-- Tests: write_file +-- ────────────────────────────────────────────────────────── + +test("write_file: writes and read back matches", function() + local path = vim.fn.tempname() + eq(utils.write_file(path, "test content"), true) + eq(utils.read_file(path), "test content") + os.remove(path) +end) + +test("write_file: overwrites existing content", function() + local path = vim.fn.tempname() + utils.write_file(path, "first") + utils.write_file(path, "second") + eq(utils.read_file(path), "second") + os.remove(path) +end) + +test("write_file: returns false for invalid path", function() + eq(utils.write_file("/no/such/directory/file.txt", "data"), false) +end) + +-- ────────────────────────────────────────────────────────── +-- Tests: notify +-- ────────────────────────────────────────────────────────── + +test("notify: prefixes message with CodeRabbit", function() + local captured_msg, captured_level + local orig = vim.notify + vim.notify = function(msg, level) + captured_msg = msg + captured_level = level + end + utils.notify("test message") + vim.notify = orig + eq(captured_msg, "CodeRabbit: test message") + eq(captured_level, vim.log.levels.INFO) +end) + +test("notify: passes custom level", function() + local captured_level + local orig = vim.notify + vim.notify = function(_, level) + captured_level = level + end + utils.notify("error!", vim.log.levels.ERROR) + vim.notify = orig + eq(captured_level, vim.log.levels.ERROR) +end) + +-- ────────────────────────────────────────────────────────── +-- Tests: pluralize +-- ────────────────────────────────────────────────────────── + +for _, case in ipairs({ + { 0, "finding", "0 findings" }, + { 1, "finding", "1 finding" }, + { 2, "finding", "2 findings" }, + { 100, "item", "100 items" }, + { 1, "error", "1 error" }, +}) do + test("pluralize: " .. case[3], function() + eq(utils.pluralize(case[1], case[2]), case[3]) + end) +end + +h.summary()