Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dangling commas in JSONC #135

Closed
hildjj opened this issue Oct 9, 2024 · 4 comments · Fixed by #136 or #137
Closed

Dangling commas in JSONC #135

hildjj opened this issue Oct 9, 2024 · 4 comments · Fixed by #136 or #137
Labels
bug Something isn't working

Comments

@hildjj
Copy link

hildjj commented Oct 9, 2024

Tell us about your environment

  • Momoa Version:
    3.2.2

  • Node Version:
    v22.9.0

  • npm Version:
    10.9.0

Which function(s) is causing a problem?

[ ] interpret()
[ ] iterator()
[ ] parse()
[x] print()
[ ] traverse()
[ ] tokenize()

Example JSON code that demonstrates the problem:

{
  "foo": 1,
}

What did you do?

parse('{"foo": 1,}', {mode: "jsonc"})

What did you expect to happen?

{
  type: 'Document',
  body: {
    type: 'Object',
    members: [
      {
        type: 'Member',
        name: {
          type: 'String',
          value: 'foo',
          loc: {
            start: { line: 1, column: 2, offset: 1 },
            end: { line: 1, column: 7, offset: 6 }
          }
        },
        value: {
          type: 'Number',
          value: 1,
          loc: {
            start: { line: 1, column: 9, offset: 8 },
            end: { line: 1, column: 10, offset: 9 }
          }
        },
        loc: {
          start: { line: 1, column: 2, offset: 1 },
          end: { line: 1, column: 10, offset: 9 }
        }
      }
    ],
    loc: {
      start: { line: 1, column: 1, offset: 0 },
      end: { line: 1, column: 11, offset: 10 }
    }
  },
  loc: {
    start: { line: 1, column: 1, offset: 0 },
    end: { line: 1, column: 11, offset: 10 }
  }
}

(with end positions slightly different. I just parsed without the trailing comma.)

What actually happened?

Uncaught:
UnexpectedToken [Error]: Unexpected token RBrace found. (1:11)
    at assertTokenType (file:///Users/hildjj/track/@eslint/json/node_modules/.pnpm/@humanwhocodes+momoa@3.2.2/node_modules/@humanwhocodes/momoa/dist/momoa.js:1716:19)
    at parseProperty (file:///Users/hildjj/track/@eslint/json/node_modules/.pnpm/@humanwhocodes+momoa@3.2.2/node_modules/@humanwhocodes/momoa/dist/momoa.js:1855:13)
    at parseObject (file:///Users/hildjj/track/@eslint/json/node_modules/.pnpm/@humanwhocodes+momoa@3.2.2/node_modules/@humanwhocodes/momoa/dist/momoa.js:1918:30)
    at parseValue (file:///Users/hildjj/track/@eslint/json/node_modules/.pnpm/@humanwhocodes+momoa@3.2.2/node_modules/@humanwhocodes/momoa/dist/momoa.js:2063:24)
    at Module.parse (file:///Users/hildjj/track/@eslint/json/node_modules/.pnpm/@humanwhocodes+momoa@3.2.2/node_modules/@humanwhocodes/momoa/dist/momoa.js:2075:21) {
  line: 1,
  column: 11,
  offset: 10
}

What do you think the solution is?

JSONC doesn't have a formal definition that I could find, but the one that @eslint/json points to is Microsoft's implementation. Both typescript and vscode accept trailing commas in their config files, so I believe allowTrailingComma should at least be an option for jsonc mode.

@hildjj hildjj added the bug Something isn't working label Oct 9, 2024
hildjj added a commit to hildjj/peggy that referenced this issue Oct 11, 2024
Updated lint config, and fixed a few spurious lint
issues, particularly with trailing commas in JSONC.
See: humanwhocodes/momoa#135

This moved us to eleventy@3.0.0.  I tested with their
update tooling, which got me to include their
info in a meta generator tag, and made me notice
that the README.md file needed to be ignored.
@nzakas
Copy link
Collaborator

nzakas commented Oct 15, 2024

I hadn't realized this, but it appears you are correct:
https://github.com/microsoft/node-jsonc-parser/blob/3c9b4203d663061d87d4d34dd0004690aef94db5/src/test/json.test.ts#L308

I'll take a look at adding this.

@hildjj
Copy link
Author

hildjj commented Oct 15, 2024

Are you likely to want to make it an option, or just always accept trailing commas? I'd prefer always-accept, or for the option to default to always-accept, so that I don't have to ask @eslint/json for extra work. I would understand the opposite preference, however.

@nzakas
Copy link
Collaborator

nzakas commented Oct 15, 2024

I think always on is the right choice as that is probably what most people expect.

@nzakas
Copy link
Collaborator

nzakas commented Oct 15, 2024

Actually, it looks like the default is to not allow trailing commas. 🤔
https://github.com/microsoft/node-jsonc-parser/blob/3c9b4203d663061d87d4d34dd0004690aef94db5/src/impl/parser.ts#L22

I tested this in VS Code by switching a normal JSON file to JSONC and it flagged the trailing comma as a problem.

So, I think I should match that behavior and have an option that is disabled by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants