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

Do not rewrite String.replaceAll with special chars in replacement string #306

Merged
merged 3 commits into from
Jun 29, 2024

Conversation

protocol7
Copy link
Contributor

@protocol7 protocol7 commented Jun 29, 2024

If the replacement string of String.replaceAll contains $ or , we should not rewrite it as these indicate special replacements: https://docs.oracle.com/en/java/javase/22/docs/api/java.base/java/util/regex/Matcher.html#replaceAll(java.lang.String)

…ring

If the replacement string of String.replaceAll contains $ or \, we
should not rewrite it as these indicate special replacements:
https://docs.oracle.com/en/java/javase/22/docs/api/java.base/java/util/regex/Matcher.html#replaceAll(java.lang.String)

Fixes openrewrite#301
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Some suggestions could not be made:

  • src/main/java/org/openrewrite/staticanalysis/MaskCreditCardNumbers.java
    • lines 50-51
  • src/test/java/org/openrewrite/staticanalysis/MaskCreditCardNumbersTest.java
    • lines 19-18
    • lines 32-32
    • lines 53-53

Copy link
Contributor

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

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

Thanks for taking this on @protocol7 ! I've expanded on your work just a bit to also cover the case where the second argument isn't a literal, as that variable/field/returnValue might then contain special characters. Hope you agree!

@timtebeek timtebeek added the bug Something isn't working label Jun 29, 2024
@timtebeek timtebeek merged commit 7f1ec7f into openrewrite:main Jun 29, 2024
2 checks passed
@protocol7 protocol7 deleted the ngn/fix-301 branch July 1, 2024 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

UseStringReplace should not rewrite when special replacement string
3 participants