fix: ignore braces inside strings and comments when matching blocks - #613
fix: ignore braces inside strings and comments when matching blocks#613signalblur wants to merge 1 commit into
Conversation
The terraform, protobuf, graphql, and shell parsers locate a block's
extent by counting `{` and `}` as raw characters. A brace inside a string
literal or a comment decrements the depth early, so the block is
truncated: the reported `lineRange` stops short, and anything derived
from the body is sliced off with it. `extractMessageFields` returns an
empty array for a protobuf message whose first field carries an option
with a brace in its value, and `extractFields` loses GraphQL fields that
follow a default value containing one.
`graphql-parser.ts` had a second problem in `extractDefinitions`: it used
`indexOf("}")` with no depth counting at all, so the first brace in the
file ended every definition regardless of nesting.
Adds `brace-matcher.ts`, a state machine that tracks quote and
line-comment context, with a per-language `BraceSyntax` describing which
quote and comment forms to honor. `findClosingBrace` returns an index for
the parsers that slice by offset; `countBracesPerLine` returns per-line
deltas for the shell parser, which walks lines. Unbalanced input still
returns `content.length` and warns under the calling parser's name, as
the four private implementations did.
In `extractDefinitions` the brace scan now runs only when the definition's
header line opens a brace, so a bodyless `scalar` or `union` no longer
matches a later definition's closing brace.
Adds 11 regression tests. Each was confirmed to fail before the fix:
string and comment cases per parser, an escaped-quote case, a
backslash-continued line case, and the bodyless-scalar case.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review this |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b39d25e3af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const closeBrace = header.includes("{") | ||
| ? findClosingBrace(afterMatch, GRAPHQL_SYNTAX, "graphql-parser") | ||
| : -1; |
There was a problem hiding this comment.
For valid GraphQL definitions that place the opening brace on the following line (for example, type User\n{ ... }), this header-only check classifies the definition as bodyless. extractFields still finds its fields, but lineRange is returned as only the declaration line instead of extending through the closing brace. GraphQL treats line breaks as insignificant whitespace, so the scan needs to distinguish genuinely bodyless definitions without requiring { on the same line.
Useful? React with 👍 / 👎.
| export const SHELL_SYNTAX: BraceSyntax = { | ||
| quotes: ['"', "'"], | ||
| lineComments: ["#"], | ||
| backslashEscapes: true, |
There was a problem hiding this comment.
In shell code, # does not always begin a comment: unquoted parameter expansions commonly use forms such as ${path#*/} and ${value##prefix}. With # configured as an unconditional comment prefix, countBracesPerLine counts the expansion's opening { but ignores its closing }, so the function depth never returns to zero and extractFunctions reports the function as ending on its opening line. Shell comment recognition needs to account for token context or parameter expansion state.
Useful? React with 👍 / 👎.
2 participants
Summary
The terraform, protobuf, graphql, and shell parsers find a block's extent by counting
{and}as raw characters, so a brace inside a string literal or a comment decrements the depth early and truncates the block. This adds a shared brace matcher that tracks quote and comment context, and fixes a separate GraphQL bug whereextractDefinitionsdid no depth counting at all.What was wrong
The truncation costs more than line numbers.
extractMessageFieldsreturns an empty array for a protobuf message whose first field carries an option with a brace in its value, andextractFieldsdrops GraphQL fields that follow a default value containing one.extractDefinitionsingraphql-parser.tsusedindexOf("}")with no depth tracking, so the first brace in the file ended every definition regardless of nesting.The fix
brace-matcher.tsholds the state machine, parameterized by a per-languageBraceSyntaxnaming the quote and line-comment forms to honor.findClosingBracereturns an index for the parsers that slice by offset;countBracesPerLinereturns per-line deltas for the shell parser, which walks lines. Unbalanced input still returnscontent.lengthand warns under the calling parser's name, matching the four private implementations it replaces.The
extractDefinitionsscan now runs only when a definition's header line opens a brace, so a bodylessscalarorunionno longer matches a later definition's closing brace.Linked issue(s)
None.
How I tested this
11 regression tests in
parsers.test.ts: string and comment cases per parser, an escaped-quote case, a backslash-continued line case, and the bodyless-scalar case. Reverting the fix fails exactly those 11 and leaves the 61 pre-existing parser tests green, so each one pins the defect rather than the implementation.Expected values came from each language's own toolchain, not from reading the parser:
bash -nplus execution,terraform fmt -check -diff,buf build, andgraphql@16'sparse(), which reports lines 1-4 for the case the parser reported as 1-3.tsc --noEmitclean. Core suite 987 passed. Root suite 496 passed, 12 skipped.One unrelated note:
tests/benchmark/test_large_repo_benchmark.test.mjs:253asserts the benchmark output contains no/w, so it fails when the repo sits at a path that matches that substring. Passes at a normal checkout path.pnpm lintpnpm --filter @understand-anything/core testpnpm test.tf,.proto,.graphql, and.shfixtures outside the suite, comparing output against the reference toolchains aboveVersioning