Skip to content

Commit cf13ce7

Browse files
committed
Fix nil bug
1 parent a5dbff6 commit cf13ce7

File tree

3 files changed

+72
-10
lines changed

3 files changed

+72
-10
lines changed

AGENTS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@
8383

8484
- For AI review panes, re-rendering existing split buffers during sync should preserve current window sizes; reusing `open()` without a layout-preserving option causes background refreshes to resize walkthrough splits.
8585

86+
## Learned while hardening comment export against JSON nulls
87+
88+
- `vim.json.decode()` turns JSON `null` into truthy `vim.NIL` userdata, so `comment.start_line or comment.line` is unsafe on decoded PR comment payloads. Normalize optional numeric fields with an explicit `type(value) == "number"` check before formatting ranges or applying suggestions.
89+
8690
## Project Overview
8791

8892
`neo-reviewer` is a Neovim plugin for reviewing GitHub PRs. Hybrid architecture: Rust CLI for GitHub API/diff parsing, Lua plugin for UI.

lua/neo_reviewer/ui/comments.lua

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -104,15 +104,26 @@ local function build_line_spec(line, end_line)
104104
return tostring(line)
105105
end
106106

107+
---@param value any
108+
---@return integer?
109+
local function coerce_integer(value)
110+
-- vim.json.decode represents JSON null as vim.NIL, which is truthy userdata.
111+
if type(value) == "number" then
112+
return value
113+
end
114+
return nil
115+
end
116+
107117
---@param comment NRComment
108118
---@return string?
109119
local function build_comment_location(comment)
110-
if type(comment.path) ~= "string" or type(comment.line) ~= "number" then
120+
local end_line = coerce_integer(comment.line)
121+
if type(comment.path) ~= "string" or not end_line then
111122
return nil
112123
end
113124

114-
local start_line = comment.start_line or comment.line
115-
local location = string.format("%s:%s", comment.path, build_line_spec(start_line, comment.line))
125+
local start_line = coerce_integer(comment.start_line) or end_line
126+
local location = string.format("%s:%s", comment.path, build_line_spec(start_line, end_line))
116127
local end_side = type(comment.side) == "string" and comment.side or nil
117128
local start_side = type(comment.start_side) == "string" and comment.start_side or end_side
118129

@@ -144,9 +155,10 @@ local function format_local_comments_markdown()
144155
local has_comments = false
145156

146157
for _, comment in ipairs(get_local_root_comments()) do
147-
if type(comment.id) == "number" and type(comment.line) == "number" and type(comment.path) == "string" then
148-
local start_line = comment.start_line or comment.line
149-
table.insert(chunks, build_comment_heading(comment.id, comment.path, start_line, comment.line))
158+
local end_line = coerce_integer(comment.line)
159+
if type(comment.id) == "number" and end_line and type(comment.path) == "string" then
160+
local start_line = coerce_integer(comment.start_line) or end_line
161+
table.insert(chunks, build_comment_heading(comment.id, comment.path, start_line, end_line))
150162
table.insert(chunks, (comment.body or "") .. "\n\n")
151163
has_comments = true
152164
end
@@ -1229,8 +1241,8 @@ end
12291241
---@param comment NRComment
12301242
---@return string[]
12311243
local function get_current_code_lines(source_bufnr, comment)
1232-
local start_line = comment.start_line or comment.line
1233-
local end_line = comment.line
1244+
local end_line = coerce_integer(comment.line)
1245+
local start_line = coerce_integer(comment.start_line) or end_line
12341246
if not start_line or not end_line then
12351247
return {}
12361248
end
@@ -1314,8 +1326,8 @@ local function apply_suggestion(source_bufnr, suggestions, file_path)
13141326
end
13151327

13161328
local comment = sugg.comment
1317-
local start_line = comment.start_line or comment.line
1318-
local end_line = comment.line
1329+
local end_line = coerce_integer(comment.line)
1330+
local start_line = coerce_integer(comment.start_line) or end_line
13191331

13201332
if not start_line or not end_line then
13211333
vim.notify("Invalid suggestion line range", vim.log.levels.ERROR)

lua/tests/plenary/ui/comments_spec.lua

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,6 +1057,52 @@ describe("neo_reviewer.ui.comments", function()
10571057
)
10581058
end)
10591059

1060+
it("ignores vim.NIL line metadata when formatting PR exports", function()
1061+
state.set_review({
1062+
pr = { number = 125, title = "Null metadata export", author = "author" },
1063+
files = {},
1064+
comments = {
1065+
{
1066+
id = 30,
1067+
path = "src/nulls.lua",
1068+
line = 12,
1069+
start_line = vim.NIL,
1070+
side = "RIGHT",
1071+
start_side = vim.NIL,
1072+
body = "Root comment with null range metadata",
1073+
author = "reviewer",
1074+
created_at = "2024-01-04T09:00:00Z",
1075+
in_reply_to_id = vim.NIL,
1076+
},
1077+
{
1078+
id = 31,
1079+
path = "src/nulls.lua",
1080+
line = 12,
1081+
start_line = vim.NIL,
1082+
side = "RIGHT",
1083+
start_side = vim.NIL,
1084+
body = "Reply with null range metadata",
1085+
author = "author",
1086+
created_at = "2024-01-04T10:00:00Z",
1087+
in_reply_to_id = 30,
1088+
},
1089+
},
1090+
})
1091+
1092+
local content = comments.format_comments_markdown()
1093+
1094+
assert.are.equal(
1095+
"# PR comments\n\n"
1096+
.. "## Comment 30 (src/nulls.lua:12)\n"
1097+
.. "@reviewer 2024-01-04 09:00\n\n"
1098+
.. "Root comment with null range metadata\n\n"
1099+
.. "### Reply 31 (src/nulls.lua:12)\n"
1100+
.. "@author 2024-01-04 10:00\n\n"
1101+
.. "Reply with null range metadata\n\n",
1102+
content
1103+
)
1104+
end)
1105+
10601106
it("keeps orphan PR replies in the export", function()
10611107
state.set_review({
10621108
pr = { number = 124, title = "Orphan reply export", author = "author" },

0 commit comments

Comments
 (0)