Fix TAG lexer rule to allow a parenthesized value suffix - #149
Open
lukiod wants to merge 1 commit into
Open
Conversation
The TAG rule (@' ~[@ )]+) excluded ')' entirely, so a tag literal ending in a parenthesized value (e.g. @id("<uuid>")) had its closing paren misread as the grouping R_PAREN token instead of part of the tag, truncating the tag text by one character and leaving a stray ')' that never matches the stored pickle tag. Fix: the base tag body now also excludes '(' (forcing it into the new optional group instead of being silently absorbed), and an optional trailing ('(' ~[)]* ')')? segment captures a parenthesized value verbatim, including its closing paren, as part of the same TAG token. Verified: reproduced the original failure directly (confirmed the unpatched grammar tokenizes @id("uuid") as TAG=@id("uuid" plus a stray R_PAREN, so tagExpressionMatchesPickle never matches), then confirmed the fix resolves it and that existing grouping-paren expressions ((@taga and @TAGB), nested parens) are unaffected. Note on test execution: this repo's own `mvn test -Ptest` currently reports 0 tests run for this JUnit4-style class (no junit-vintage-engine on the classpath), a pre-existing gap unrelated to this change. Verified directly instead by compiling against the generated parser and running the exact scenarios from the added tests standalone.
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
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.
Fixes #72.
The
TAGlexer rule ('@' ~[@ )]+) excluded)entirely, so a tag literal ending in a parenthesized value (e.g.@id("<uuid>")) had its closing paren misread as the groupingR_PARENtoken instead of part of the tag. That truncates the tag text by one character and leaves a stray), sotagExpressionMatchesPicklenever matches the tag against itself.Fix: the base tag body now also excludes
((forcing it into a new optional group instead of being silently absorbed), and an optional trailing('(' ~[)]* ')')?segment captures a parenthesized value verbatim, including its closing paren, as part of the sameTAGtoken. Existing grouping-paren expressions ((@tagA and @tagB), nested parens) are unaffected, since a bare(not immediately following@tagNamestill tokenizes asL_PARENas before.Verification: reproduced the failure directly against the unpatched grammar first — confirmed it tokenizes
@id("uuid")asTAG=@id("uuid"(missing the final)) plus a strayR_PAREN, so the match fails — then confirmed the fix resolves it and that the existing grouping-paren test cases are unaffected.One disclosure: this repo's own
mvn test -Ptestcurrently reports 0 tests run forGherkinTagFiltererTest(nojunit-vintage-engineon the classpath for this JUnit4-style class) — a pre-existing gap unrelated to this change, so I verified the fix and the three new regression tests directly by compiling against the ANTLR-generated parser and running the exact same assertions standalone rather than through the (currently non-functional) Maven test target.