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

Suppress this-escape warning for JDK21 #101519

Merged
merged 2 commits into from
Oct 31, 2023

Conversation

benwtrent
Copy link
Member

follow up to: #99848

@benwtrent benwtrent added >non-issue :Delivery/Build Build or test infrastructure v8.12.0 labels Oct 30, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-delivery (Team:Delivery)

@elasticsearchmachine elasticsearchmachine added the Team:Delivery Meta label for Delivery team label Oct 30, 2023
@benwtrent
Copy link
Member Author

@elastic/es-esql Are the files I changed automatically generated? When building with JDK21 with -Werror its angry. So I just added some suppressions, however intellij did a bunch of whitespace changes as well.

@nik9000
Copy link
Member

nik9000 commented Oct 30, 2023

DocVector is fine to change. The lexer and parser are automatically generated. But I don't think you added a warning there.

@benwtrent
Copy link
Member Author

@nik9000 where is the code that generates EsqlBaseLexer & EsqlBaseParser? Those will cause the warnings to trip every time they are regenerated as the generated code doesn't suppress the warnings. I did indeed add suppression to those two classes.

@nik9000
Copy link
Member

nik9000 commented Oct 30, 2023

@nik9000 where is the code that generates EsqlBaseLexer & EsqlBaseParser? Those will cause the warnings to trip every time they are regenerated as the generated code doesn't suppress the warnings. I did indeed add suppression to those two classes.

It's in x-pack/plugin/esql/build.gradle and starts with tasks.register("regen") { er, well, it's actually in antlr, but you can modify the generated files with that gradle code.

@benwtrent
Copy link
Member Author

@nik9000 I added some regex to that regen task. It now adds @SuppressWarnings("this-escape") to every ctor it can find. This seemed easier than trying to figure out the exact ones that had issues, and since its generated, I didn't think that would be an issue.

I re-ran regen locally, and that is what is in the PR now.

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

LGTM

@benwtrent benwtrent merged commit 1722c0a into elastic:main Oct 31, 2023
@benwtrent benwtrent deleted the fix-this-escape-SearchResponse branch October 31, 2023 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure >non-issue Team:Delivery Meta label for Delivery team v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants