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

Groovy issues with Strings in parentheses #4431

Merged
merged 2 commits into from
Aug 21, 2024
Merged

Conversation

timtebeek
Copy link
Contributor

What's changed?

What's your motivation?

Resolve Groovy parser issues affecting Gradle projects ;
some of the tests here fare worse on Groovy 2.5.

Anything in particular you'd like reviewers to focus on?

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

Any additional context

@timtebeek timtebeek added bug Something isn't working parser-groovy labels Aug 20, 2024
@timtebeek timtebeek self-assigned this Aug 20, 2024
// On Java 8 _INSIDE_PARENTHESES_LEVEL is a regular Integer
insideParenthesesLevel = (Integer) rawIpl;
}
Integer insideParenthesesLevel = getInsideParenthesesLevel(node);
Copy link
Contributor Author

@timtebeek timtebeek Aug 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where we still see a difference between Groovy 2.5 and 3+;

  • on 3+ the ConstantExpression has a metaDataMap containing _INSIDE_PARENTHESES_LEVEL: 1
  • on 2.5 the ConstantExpression lacks a metaDataMap

We then either enter (for 3+) the below if, or skip it for 2.5, meaning the cursor positions are not updated there through the call to whitespace().

Eventually the 2.5 parser then fails with:

java.lang.AssertionError: Source file was parsed into an LST that contains non-whitespace characters in its whitespace. This is indicative of a bug in the parser. 
def a = (~~(non-whitespace)~~>"-")<~~

return ((AtomicInteger) rawIpl).get();
} else {
// On Java 8 _INSIDE_PARENTHESES_LEVEL is a regular Integer
return (Integer) rawIpl;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not seeing any evidence that this is ever triggered on Groovy 2.5; might need to change how we detect and handle parentheses for that version.

@timtebeek timtebeek marked this pull request as ready for review August 20, 2024 22:12
@timtebeek
Copy link
Contributor Author

I'd like to get this smaller increment in already, as this fixes things for Gradle 7+, before continuing to explore a fix for older gradle versions too. Then we can already do a patch release to get the first fixes out.

@timtebeek timtebeek requested a review from sambsnyd August 21, 2024 07:10
@timtebeek
Copy link
Contributor Author

Merging already for the fix to LiteralTest.stringLiteralInParentheses() on Groovy 3+.

@timtebeek timtebeek merged commit ed3525b into main Aug 21, 2024
2 checks passed
@timtebeek timtebeek deleted the groovy-parentheses-issue branch August 21, 2024 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working parser-groovy
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

1 participant