Repository navigation
Fix: Update parseSyntaxError to use lossless-json for accurate error locations - #181
sameerakhtar406 wants to merge 5 commits into
Conversation
anurag629
left a comment
There was a problem hiding this comment.
Thanks @sameerakhtar406, the lossless-json fallback works. All four inputs from the table now show the right line and column, and the screenshot is there. A few things before it can go in:
- Please keep the first two branches as they were (
if (posMatch)andelse if (lineMatch)) and only add the lossless-json part as the finalelse. Chrome and Node put both in the same message ("at position 13 (line 1 column 14)"), so with the new conditions every message that already has a location goes to lossless-json too. That breaks one case: lossless-json stops at a duplicate key, so{"a":1,"a":2,}shows line 1, column 9 with this PR, whiledevelopcorrectly shows column 14 (the trailing comma). Can you add that as a test? - The multi-line test uses a different input from the issue. The table's second row is three lines,
{"a": 1,then"b": truthen}, so'{"a": 1,\n"b": tru\n}'with line 2, column 6 (your code already gets that one right). Please drop the "Adjust column" comment with it. - Small cleanup: remove the commented
//let message;and the extra blank lines, fix the indentation of the closing braces, and addvalidateto the import list at the top of the test file instead of a second import further down.
The red "Says how it was tested" check is only about the format. It looks for the section names as headings, so put ## in front of "How I tested this" and "AI assistance" and it turns green. What you wrote in them is fine.
|
Sure @anurag629, I'll just fix these right away |
|
Hi @anurag629, I've fixed a sometime back, kindly check :) |
anurag629
left a comment
There was a problem hiding this comment.
Thanks @sameerakhtar406, the logic is right now. I checked it: the four inputs from the issue give the right line and column, {"a":1,"a":2,} stays at column 14, {"a":1,} stays at column 8, and the four new tests fail with develop's json-utils.ts like they should. Lint and all 243 tests pass.
Only the formatting from point 3 is left. A few lines have trailing spaces, the else/try/catch aren't formatted like the rest of the file, and the function's closing brace ended up indented. I put the fixed lines as suggestions below, so you can commit them from this page ("Add suggestion to batch" on each one, then "Commit suggestions"). I'll merge once CI is green after that.
| } | ||
| else{ | ||
| try{ | ||
| llParse(raw); | ||
| } | ||
| catch(llError) { | ||
| if (llError instanceof Error) { | ||
| const llPosMatch = llError.message.match(/position (\d+)/i); | ||
| if (llPosMatch) { | ||
| position = parseInt(llPosMatch[1], 10); | ||
| const lc = computeLineCol(raw, position); | ||
| line = lc.line; | ||
| column = lc.column; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const suggestion = suggestFix(raw); | ||
| return { message: msg.replace(/in JSON.*$/, "").trim() || "Invalid JSON", line, column, position, suggestion }; | ||
| } | ||
|
|
||
| } | ||
There was a problem hiding this comment.
| } | |
| else{ | |
| try{ | |
| llParse(raw); | |
| } | |
| catch(llError) { | |
| if (llError instanceof Error) { | |
| const llPosMatch = llError.message.match(/position (\d+)/i); | |
| if (llPosMatch) { | |
| position = parseInt(llPosMatch[1], 10); | |
| const lc = computeLineCol(raw, position); | |
| line = lc.line; | |
| column = lc.column; | |
| } | |
| } | |
| } | |
| } | |
| const suggestion = suggestFix(raw); | |
| return { message: msg.replace(/in JSON.*$/, "").trim() || "Invalid JSON", line, column, position, suggestion }; | |
| } | |
| } | |
| } else { | |
| try { | |
| llParse(raw); | |
| } catch (llError) { | |
| if (llError instanceof Error) { | |
| const llPosMatch = llError.message.match(/position (\d+)/i); | |
| if (llPosMatch) { | |
| position = parseInt(llPosMatch[1], 10); | |
| const lc = computeLineCol(raw, position); | |
| line = lc.line; | |
| column = lc.column; | |
| } | |
| } | |
| } | |
| } | |
| const suggestion = suggestFix(raw); | |
| return { message: msg.replace(/in JSON.*$/, "").trim() || "Invalid JSON", line, column, position, suggestion }; | |
| } | |
Co-authored-by: Anurag Verma <78868769+anurag629@users.noreply.github.com>
Co-authored-by: Anurag Verma <78868769+anurag629@users.noreply.github.com>
|
Thanks @anurag629, I think I have applied all the batched formatting suggestions! Ready for the CI approval whenever you get a moment |
anurag629
left a comment
There was a problem hiding this comment.
Thanks @sameerakhtar406, two of the three went in. The one for the else/try/catch block didn't make it, GitHub sometimes marks a suggestion as outdated once another one moves the lines. Here it is again for the current lines, so it's one more click. I'll merge after that.
| } | ||
| else{ | ||
| try{ | ||
| llParse(raw); | ||
| } | ||
| catch(llError) { | ||
| if (llError instanceof Error) { | ||
| const llPosMatch = llError.message.match(/position (\d+)/i); | ||
| if (llPosMatch) { | ||
| position = parseInt(llPosMatch[1], 10); | ||
| const lc = computeLineCol(raw, position); | ||
| line = lc.line; | ||
| column = lc.column; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const suggestion = suggestFix(raw); | ||
| return { message: msg.replace(/in JSON.*$/, "").trim() || "Invalid JSON", line, column, position, suggestion }; | ||
| } | ||
|
|
||
| } | ||
There was a problem hiding this comment.
| } | |
| else{ | |
| try{ | |
| llParse(raw); | |
| } | |
| catch(llError) { | |
| if (llError instanceof Error) { | |
| const llPosMatch = llError.message.match(/position (\d+)/i); | |
| if (llPosMatch) { | |
| position = parseInt(llPosMatch[1], 10); | |
| const lc = computeLineCol(raw, position); | |
| line = lc.line; | |
| column = lc.column; | |
| } | |
| } | |
| } | |
| } | |
| const suggestion = suggestFix(raw); | |
| return { message: msg.replace(/in JSON.*$/, "").trim() || "Invalid JSON", line, column, position, suggestion }; | |
| } | |
| } | |
| } else { | |
| try { | |
| llParse(raw); | |
| } catch (llError) { | |
| if (llError instanceof Error) { | |
| const llPosMatch = llError.message.match(/position (\d+)/i); | |
| if (llPosMatch) { | |
| position = parseInt(llPosMatch[1], 10); | |
| const lc = computeLineCol(raw, position); | |
| line = lc.line; | |
| column = lc.column; | |
| } | |
| } | |
| } | |
| } | |
| const suggestion = suggestFix(raw); | |
| return { message: msg.replace(/in JSON.*$/, "").trim() || "Invalid JSON", line, column, position, suggestion }; | |
| } | |
|
Hey @anurag629 , GitHub was giving an error applying the inline suggestion as I think it lost track of line numbers after earlier commits due to shifted lines, so I fixed it locally in VS Code and pushed it. I hope it will work now. Let me know if there's still any issue :) |
Closes #163
How I tested this:
Added Vitest test cases in lib/tests/json-utils.test.ts to cover the exact edge cases and multi-line formatting specified in the issue.
Ran npm run test and verified all 242 tests pass green.
Ran npm run lint and
npm run buildto ensure no regressions.Booted the local dev server using npm run dev verified the UI correctly renders "line 1, column 7" for {"a": x} (screenshot attached) and all the other test cases mentioned in Issue #163
AI assistance:
Used LLMs to help structure the TypeScript try...catch fallback logic, navigate strict type narrowing for unknown errors and be sure of my work