fix(reusable-lint): parse JSONC-by-specification files with a JSONC parser - #57
Merged
Conversation
…arser
`lint / JSON validity` parsed every *.json with a strict json.load(). But
".json" is two different formats. TypeScript documents and supports `//`
comments in tsconfig.json, as do VS Code's settings files, devcontainer
and .eslintrc. Pointing a strict parser at those is simply the wrong
parser, and the only way to satisfy it is to delete the comments.
It has already cost real information once. openNTL/ntl's
mcp/ntl-postgres-mcp-server/tsconfig.json carries a four-line comment
explaining why exactOptionalPropertyTypes is deliberately off - that the
MCP SDK's own Transport and CallToolResult types are not written for it.
Nothing about that file is broken; the checker was.
So: two parsers, chosen by filename. Ordinary .json stays strict. The
by-specification JSONC names - tsconfig*.json, jsconfig*.json,
.vscode/*.json, devcontainer.json, .eslintrc.json, *.jsonc - get a JSONC
parse that strips comments and trailing commas without touching string
contents. This TIGHTENS the gate rather than weakening it: a genuine
syntax error in a tsconfig still fails, *.jsonc files are now validated
where previously the `-name '*.json'` find never saw them at all, and
nothing is exempted.
Comments are stripped before trailing commas, in two passes, because a
trailing comma can be separated from its brace by a comment as in
`{ "a": true, /* note */ }`. Newlines inside block comments are preserved
so reported line numbers still point at the real line. The `-type f`
behaviour from #54 is kept - os.walk lists a DIRECTORY named *.json under
dirs rather than files, and an isfile() guard covers symlinks to one.
Proven both ways before landing. PASS: legal comments in tsconfig.json
and .vscode/settings.json; a string containing "// not a comment and , }";
a directory named server-card.json; openNTL/ntl in full; this repo.
FAIL as required: a missing ':' in a commented tsconfig (line 4), an
unterminated string in a commented tsconfig (line 3), and a `//` comment
in package.json, which is NOT JSONC by specification (line 2).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
lint / JSON validityparsed every*.jsonwith a strictjson.load(). But ".json" is two different formats.The bug, found in production
openNTL/ntl'smcp/ntl-postgres-mcp-server/tsconfig.jsonfails the gate. It contains a four-line comment explaining whyexactOptionalPropertyTypesis deliberately off — that the MCP SDK's ownTransportandCallToolResulttypes are not written for it. TypeScript documents and supports comments intsconfig.json. Nothing about that file is broken; the checker was. The only way to satisfy a strict parser there is to delete a correct explanation.The fix
Two parsers, chosen by filename. Ordinary
.jsonstays strict. The by-specification JSONC names get a JSONC parse:tsconfig*.json,jsconfig*.json,.vscode/*.json,devcontainer.json,.eslintrc.json,*.jsonc.This tightens the gate. A genuine syntax error in a tsconfig still fails;
*.jsoncfiles are now validated at all, where the old-name '*.json'find never saw them; nothing is exempted. The-type fbehaviour from #54 is preserved.Comments are stripped before trailing commas, in two passes, because a trailing comma can be separated from its brace by a comment —
{ "a": true, /* note */ }. My first attempt got this wrong and the test caught it. Newlines inside block comments are preserved so reported line numbers still point at the real line.Proven both ways
Passes (correctly):
//and/* */comments intsconfig.json.vscode/settings.json"// not a comment and , } inside"server-card.json(the #54 case)openNTL/ntlin full, 7 filesStill fails (correctly):
:in a commentedtsconfig.jsonline=4 invalid JSONC: Expecting ':' delimitertsconfig.jsonline=3 invalid JSONC: Invalid control character//comment inpackage.json— not JSONC by specline=2 invalid JSON: Expecting property nameUnblocks openNTL/ntl#30, which has auto-merge armed.
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com