Escape method and property names when rendering interaction mismatches#2394
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughAdds Groovy-safe escaping for mocked method and property names in mismatch diagnostics, with regression tests for dollar signs and related characters. Release notes document the fix. ChangesMismatch rendering
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes interaction mismatch rendering for mocked names that contain Groovy string-special characters.
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (1): Last reviewed commit: "Escape method and property names in inte..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@spock-specs/src/test/groovy/org/spockframework/smoke/mock/TooFewInvocations.groovy`:
- Around line 747-813: Add reverse-direction tests alongside the existing
dollar-sign mismatch cases in the feature methods covering method and property
name mismatches: invoke the method/property whose actual name contains “$”,
while declaring the expected interaction with the non-dollar name. Assert the
resulting TooFewInvocationsError message includes the correct unmatched
invocation, escaped name comparison, and similarity report, confirming no lexer
error occurs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 418a60ad-541d-486f-b859-56f97dc0d99d
📒 Files selected for processing (6)
docs/release_notes.adocspock-core/src/main/java/org/spockframework/mock/constraint/EqualMethodNameConstraint.javaspock-core/src/main/java/org/spockframework/mock/constraint/EqualPropertyNameConstraint.javaspock-core/src/main/java/org/spockframework/util/TextUtil.javaspock-specs/src/test/groovy/org/spockframework/smoke/mock/TooFewInvocations.groovyspock-specs/src/test/groovy/org/spockframework/util/TextUtilSpec.groovy
AndreasTu
left a comment
There was a problem hiding this comment.
Thank you for your contribution.
Just two small things.
We annotate test, which cover issues with the @Issue annotation.
This comment has been minimized.
This comment has been minimized.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2394 +/- ##
============================================
+ Coverage 82.28% 82.30% +0.01%
- Complexity 4878 4882 +4
============================================
Files 474 474
Lines 15251 15258 +7
Branches 1959 1961 +2
============================================
+ Hits 12550 12558 +8
Misses 2002 2002
+ Partials 699 698 -1
🚀 New features to boost your workflow:
|
EqualMethodNameConstraint and EqualPropertyNameConstraint build a synthetic `methodName == "<name>"` condition and re-parse it as Groovy to render the similarity of an unmatched invocation. A `$` in the name starts a GString interpolation, so the Groovy lexer fails with LexerNoViableAltException (printed to stderr) and the similarity report is dropped. Escape the name for a double-quoted Groovy literal before formatting it, so the condition parses. Names without special characters are unchanged. Fixes spockframework#2364 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
432c4f2 to
52b87ef
Compare
|
Thanks for the review, @AndreasTu! I've added the Regarding the reverse-direction test coderabbit suggested: I left it out, since only the declared name is baked into the re-parsed Groovy source, while the actual invoked name only flows through as a runtime value and never reaches the lexer — so a reverse case wouldn't exercise the fix. |
AndreasTu
left a comment
There was a problem hiding this comment.
Thank you again for the fix!
Fixes #2364
Problem
When a mock interaction is unsatisfied, Spock lists the unmatched invocations
"ordered by similarity". For a name mismatch it builds a synthetic condition
(
methodName == "<name>"orpropertyName == "<name>") and re-parses it asGroovy to produce the aligned similarity report.
If the name contains a
$, that$inside the double-quoted literal starts aGString interpolation, so the Groovy lexer fails with
unknown recognition error type: ...LexerNoViableAltExceptionon stderr and thesimilarity report is dropped:
Fix
Escape the name for a double-quoted Groovy literal (
",$,\, and controlcharacters) before formatting the condition, in both
EqualMethodNameConstraintand
EqualPropertyNameConstraint. The condition parses again and the reportrenders:
Names without special characters format exactly as before, so existing output
is unchanged.
Tests
TooFewInvocations: two functional specs for a method name and a propertyname containing
$, red before the fix and green after.TextUtilSpec: unit coverage for the newTextUtil.escapeGroovyDoubleQuotedStringhelper.Verified on Groovy variants 4.0 and 5.0; the mock, condition, and util specs
pass.
AI assistance: developed with help from Claude. I have reviewed the change and
understand and stand behind every line.