Skip to content

Fix escaped char pattern in AvoidEscapedUnicodeCharactersCheck #9314

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

Merged
merged 1 commit into from
Mar 3, 2021

Conversation

strkkk
Copy link
Member

@strkkk strkkk commented Feb 20, 2021

AvoidEscapedUnicodeCharactersCheck pattern for escaped chars is a bit incorrect.
According to JLS char 'u' can be repeated many times, and it is still valid escape.

Note: pattern for whitespace escapes was not updated - it is causing sonar violation that require big refactoring of regex and tests.

Diff Regression config: https://gist.githubusercontent.com/strkkk/31749e0743553a606f69fdfc6670d780/raw/2031b8f52d40c522c5cd8c343ff2c8af476082d7/confg.xml
Diff Regression projects: https://gist.githubusercontent.com/strkkk/29c0f3a0ce06a681afdf8d3c417a4b30/raw/f2771cee1ba63f0236377c2b55b77a2fb1fe45e0/small_projects
Report label: only jdk9

strkkk added a commit to strkkk/checkstyle that referenced this pull request Feb 20, 2021
@strkkk strkkk force-pushed the correct_avoidunicode branch from 559c606 to 17fdba3 Compare February 20, 2021 13:36
@strkkk
Copy link
Member Author

strkkk commented Feb 20, 2021

Github, generate report

strkkk added a commit to strkkk/checkstyle that referenced this pull request Feb 20, 2021
@strkkk strkkk force-pushed the correct_avoidunicode branch from 17fdba3 to f5b9257 Compare February 20, 2021 15:59
+ "(00[0-1][0-9A-Fa-f]"
+ "|00[8-9][0-9A-Fa-f]"
private static final Pattern UNICODE_CONTROL = Pattern.compile("\\\\u+"
+ "(00[0189][0-9A-Fa-f]"
Copy link
Member Author

Choose a reason for hiding this comment

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

small simplification

@strkkk strkkk force-pushed the correct_avoidunicode branch from f5b9257 to c2699f2 Compare February 21, 2021 08:28
@checkstyle checkstyle deleted a comment from github-actions bot Feb 21, 2021
@github-actions
Copy link
Contributor

@strkkk
Copy link
Member Author

strkkk commented Feb 24, 2021

Github, generate report

@github-actions
Copy link
Contributor

@strkkk
Copy link
Member Author

strkkk commented Feb 24, 2021

GitHub, generate report

@checkstyle checkstyle deleted a comment from github-actions bot Feb 24, 2021
@github-actions
Copy link
Contributor

Report generation job failed on phase , step .
Link: https://github.com/checkstyle/checkstyle/actions/runs/595589659

@strkkk
Copy link
Member Author

strkkk commented Feb 25, 2021

Openjdk report failed by timeout (job was interrupted after 6 hours), but for other reps looks ok.
@romani please take a look

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

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

It would be good to place example of problem in PR description in format we demand for issues. As main code is changed, it is bug.

Small item before merge:

@romani romani assigned strkkk and unassigned romani Feb 28, 2021
@romani romani added bug and removed miscellaneous labels Feb 28, 2021
@strkkk strkkk assigned romani and unassigned strkkk Feb 28, 2021
@romani romani merged commit b80e542 into checkstyle:master Mar 3, 2021
@romani romani added this to the 8.42 milestone Mar 3, 2021
@strkkk strkkk deleted the correct_avoidunicode branch March 3, 2021 15:43
SGanguly1999 pushed a commit to SGanguly1999/checkstyle that referenced this pull request Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants