report syntax errors instead of parser codes by srivastava-diya · Pull Request #115 · hyperjump-io/json-language-server

srivastava-diya · 2026-09-23T18:11:56Z

Closes #109.

What this does

We currently show the user whatever jsonc-parser called the error ValueExpected, PropertyNameExpected which isn't friendly, and often isn't even what they did wrong. The same code covers several different mistakes, so there's no way to turn it into a helpful message.

This replaces jsonc.parseTree with a parser that reads the document token by token using jsonc-parser's scanner underneath, and reports what the user actually did. It still produces the same tree parseTree did, so nothing else in the codebase changes.

The problems it fixes

One mistake used to produce several errors.

{ "a": [1, 2 }

Four errors before, one now, pointing at the [ that was never closed.

{ a: 1 }

Three before, one now, pointing at the key.

Errors landed on the wrong thing. A trailing comma was reported at the closing brace. Now it points at the comma.

Some errors had no length, so there was nothing for the editor to underline. Unclosed brackets and braces now point at the opening bracket, which also tells you which one is unclosed. The only one left is an empty file, where there's genuinely nothing to point at.

The error didn't say what was wrong. ValueExpected alone covered trailing comma, single-quoted value, missing value, unclosed bracket, NaN, True and an empty file. There are now 16 codes that each name one thing trailing-comma, property-key-not-quoted, string-not-closed, number-invalid and so on.

Errors can also carry the text that caused them, so a message can say "0x1F" is not a valid number rather than something generic.

Multiple genuine mistakes still give multiple errors { a: 1, "b" 2, 'c': 3 } gives three.

What this doesn't do

No message text or translation. The diagnostic currently shows the code itself as a placeholder, which is what #110 replaces.

Two things I found along the way and deliberately left, since neither needs parser work: we still accept // comments in .json files, and we still don't report duplicate keys. I'll raise those separately.

Notes

A few judgement calls worth a second opinion:

A truncated document like {"a":{"b":{"c":[1,2 reports every container left open, so four errors. Arguably right, since four closers really are missing, but it's the one place "one mistake, one error" doesn't hold. Same count as before.
An empty file gives an error with no length, as above.
A number put against a string, { "a": 1"b" }, still gives three errors. Odd input, but it's the one case I didn't get down to one.
comment-not-closed exists but is unreachable today, because we skip comments while scanning. It becomes live if we start reporting comments.

tests

This is a first pass and I expect to restructure it based on feedback. I wrote the cases first as a way of pinning down what each mistake should produce, so the 40 tests are organised by mistake rather than by anything about how the parser is built. Happy to reorganise, rename, split or trim I wanted the behaviour agreed before polishing the layout.

jdesrosiers

Looks great! I make a couple suggestions inline.

A truncated document like {"a":{"b":{"c":[1,2 reports every container left open, so four errors.

👍 These are all distinct problems and we should show them all.

An empty file gives an error with no length

Does an error with no length even show up? Not sure what we could do about it in any case, so it probably doesn't matter.

A number put against a string, { "a": 1"b" }, still gives three errors. Odd input, but it's the one case I didn't get down to one.

I think that makes sense. It looks simple, but it's actually multiple errors. There should be a comma after the number, then the string is a property name, so it needs a colon and a value.

So, there's actually a simpler case where there's more than one error: {"a"}. This produces two errors: colon-expected and value-expected. I think that's fine too. There really are two things missing here. The property name has two required things that must always come after it.

comment-not-closed exists but is unreachable today, because we skip comments while scanning. It becomes live if we start reporting comments.

This language server should support JSONC as well as JSON. I think the parser needs to have an option whether or not to allow comments based on the file extension. If it's .json, it should report a syntax error. If it's .jsonc, it should catch things like comment-not-closed.

jdesrosiers · 2026-09-26T17:48:40Z

+  test("an unclosed array points at the bracket that was never closed", () => {
+    const { errors } = parse(`{ "a": [1, 2 }`);
+
+    expect(errors).toEqual([
+      { code: "bracket-not-closed", offset: 7, length: 1 }
+    ]);
+  });


Could this point to the whole array, or maybe the last item? My thought is that if I have a large array covering many lines, knowing where the array started doesn't do much to help me find where I need to close the array. It would be a lot more helpful if the error could tell me where the missing bracket should be.

Went with the last item, since that's where the bracket goes. Did the same for unclosed objects, which point at the last property. If it's empty, it falls back to the opening bracket since there's nothing else to point at.

jdesrosiers · 2026-09-26T18:00:20Z

+  test("a second top level value points at where the document should have ended", () => {
+    const { errors } = parse(`{"a":1} {"b":2}`);
+
+    expect(errors).toEqual([
+      { code: "end-of-file-expected", offset: 8, length: 1 }
+    ]);
+  });


Can we make this point at everything after the document should have ended? All of that is invalid, no just the first character.

Done, it now covers everything from where the document should have ended to the end of the file, not counting trailing whitespace.

jdesrosiers · 2026-09-26T19:08:30Z

+  test("a missing comma points at the member that needed one before it", () => {
+    const { errors } = parse(`{ "a": 1 "b": 2 }`);
+
+    expect(errors).toEqual([
+      { code: "comma-expected", offset: 9, length: 3 }
+    ]);
+  });
+
+  test("a missing colon points at the value that needed one before it", () => {
+    const { errors } = parse(`{ "a" 1 }`);
+
+    expect(errors).toEqual([
+      { code: "colon-expected", offset: 6, length: 1 }
+    ]);
+  });
+
+  test("a property with no value points at the token found instead", () => {
+    const { errors } = parse(`{ "a": }`);
+
+    expect(errors).toEqual([
+      { code: "value-expected", offset: 7, length: 1 }
+    ]);
+  });


The positioning of these errors doesn't feel right to me. By positioning them after the error, it feels like it's not pointing at the thing that's actually wrong. I think my feeling is influenced by where whitespace usually goes. For example, when I see the first test, I think, "the 1 needs a comma". The comma is needed because "b" is present, but it's the 1 that gets the comma. Of course the comma isn't really part of the 1, but that's what it looks like with typical whitespace conventions.

I feel like the error should be on the thing before the error instead of after the error. In the first, there should be a comma after 1. In the second, there should be a colon after "a". In the third, there should be value after :.

I think the last test makes the case best if you put it on multiple lines.

{ "a": }

The error would appear on the }, which means the error doesn't appear on the line you'd make the change on. If the error is on the :, it's more clear where the fix goes.

Makes sense, moved them. Comma and colon now point at what they should follow, and a missing value points at the colon. I added your multiline example as a test too. I did the same for gaps like [1, , 2], which now point at the first comma.

jdesrosiers

Here are a couple of case I've found that look like they could be better.

[,]

value-expected on [ -- I'd expect the error to be trailing-comma on ,

--

Here's another case I found. This is a common case if a line gets removed accidentally.

{
  "a": 1,
  "b": [
    2
  ], // <-- remove this line
  "c": 3
}

This produces a lot of errors. On the : after "c", I see all these.

brace-not-closed
comma-expected
bracket-not-closed
comma-expected
value-expected
value-expected
end-of-file-expected

Ideally the error would be just brace-not-closed and comma-expected on 2.

srivastava-diya · 2026-10-03T17:14:43Z

sure i'm looking at this one

srivastava-diya · 2026-10-03T18:19:20Z

Hi @jdesrosiers this PR has also become huge, please have a look whenever you have time.

jdesrosiers

I found a few places where we can recover better and capture more of the intended JSON in a few error situations.

I also some performance testing comparing to jsonc-parser. It's only sightly slower, which is plenty good enough. I expected it to be a little slower because what we're doing is more complex.

I think we're ready!

When a property's value was missing, the parser took the next property's key as the value. The colon after that key ended the object early, which dropped every later property from the tree. Now, if the value position holds something that looks like a property, the value is treated as missing. A property with no value also ended past the next token, so its range included the following comma or closing brace, and findNodeAtOffset on that token returned the property. It now ends where the next token starts. It still covers the whitespace after the colon, which value completion needs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

invalidEscapeIndex treated every \u as a valid escape, so for a malformed unicode escape it found nothing and the error pointed at the start of the string. It now checks for four hex digits after \u. The error covers the backslash, the u, and any valid hex digits that come before the bad character. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A token that couldn't start a property or an array item stopped parsing the whole container, which dropped the rest of the document from the tree. Now recovery keeps going: - Every token in an object's key position is read as part of a property. A number or keyword key (1, true, null) is property-key-not-quoted, with its text as the key, like an unquoted word. A colon with no key, or a container where a key should be, is property-key-expected and is read as a property with an empty "" key, so its value is still parsed. - A stray colon in an array, or a doubled colon after a key, is reported as unexpected-token covering the stray colons, and parsing continues with the next value. - Both containers remember where the last item ended. If the next item starts with no comma, comma-expected is reported there, even when stray tokens were skipped in between. A comma right after skipped text counts as a separator. comma-expected is not reported after a property with no value. So {"a": 1 : 2} reads as "a": 1, a missing comma, then "": 2, and [1 : 2] reads as [1, 2] with a missing comma and an unexpected colon. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Refactors with no change in behavior: - parseScalar replaces parseString, parseLiteral and the invalid-literal branch, which each built a node, set its value, finished it at the token end and advanced. parseValue sets each value directly. - parseComma replaces parseSeparator and skipExtraComma. It reports value-expected only for an extra comma. - parseContainer replaces parseObject and parseArray, which shared the opening, comma and separator handling, and the not-closed check. It branches on the container type only where they differ. - report always sets data, the startsKey wrapper is gone, and looksLikeProperty moves into the parse closure and takes no arguments. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Comma-expected and value-expected errors were spliced into the error list to keep it in document order, because some are found after later errors. A stable sort before returning does that instead, which removes Separator.errorIndex and both splices. Not-closed errors used to come innermost container first. They are now in document order too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

jdesrosiers requested changes Sep 26, 2026

View reviewed changes

srivastava-diya force-pushed the parser branch from 5047a1a to 0136bac Compare September 26, 2026 23:03

jdesrosiers reviewed Sep 29, 2026

View reviewed changes

srivastava-diya force-pushed the parser branch from 0136bac to 2fbbabd Compare October 3, 2026 17:26

jdesrosiers approved these changes Oct 3, 2026

View reviewed changes

srivastava-diya and others added 8 commits October 4, 2026 04:15

report syntax errors instead of parser codes

42474ce

refactor error positions, JSONC comments, MutableNode type etc

a43abb2

improve recovery for stray commas and unclosed arrays

805d383

srivastava-diya force-pushed the parser branch from ac4e703 to b561722 Compare October 3, 2026 22:56

srivastava-diya merged commit 90ab5fe into hyperjump-io:main Oct 3, 2026
3 checks passed

srivastava-diya mentioned this pull request Oct 4, 2026

Translate syntax error messages with Fluent #110

Closed

Navigation Menu

Repository navigation

report syntax errors instead of parser codes - #115

report syntax errors instead of parser codes#115
srivastava-diya merged 8 commits into
hyperjump-io:mainfrom
srivastava-diya:parser

srivastava-diya commented Sep 23, 2026

Uh oh!

jdesrosiers left a comment

Uh oh!

jdesrosiers Sep 26, 2026

Uh oh!

srivastava-diya Sep 26, 2026

Uh oh!

jdesrosiers Sep 26, 2026

Uh oh!

srivastava-diya Sep 26, 2026

Uh oh!

Uh oh!

jdesrosiers Sep 26, 2026

Uh oh!

srivastava-diya Sep 26, 2026

Uh oh!

jdesrosiers left a comment

Uh oh!

srivastava-diya commented Oct 3, 2026

Uh oh!

srivastava-diya commented Oct 3, 2026

Uh oh!

jdesrosiers left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

srivastava-diya commented Sep 23, 2026

What this does

The problems it fixes

What this doesn't do

Notes

tests

Uh oh!

jdesrosiers left a comment

Choose a reason for hiding this comment

Uh oh!

jdesrosiers Sep 26, 2026

Choose a reason for hiding this comment

Uh oh!

srivastava-diya Sep 26, 2026

Choose a reason for hiding this comment

Uh oh!

jdesrosiers Sep 26, 2026

Choose a reason for hiding this comment

Uh oh!

srivastava-diya Sep 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jdesrosiers Sep 26, 2026

Choose a reason for hiding this comment

Uh oh!

srivastava-diya Sep 26, 2026

Choose a reason for hiding this comment

Uh oh!

jdesrosiers left a comment

Choose a reason for hiding this comment

Uh oh!

srivastava-diya commented Oct 3, 2026

Uh oh!

srivastava-diya commented Oct 3, 2026

Uh oh!

jdesrosiers left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants