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

prefer JDK String replace methods to Plexus ones #193

Merged
merged 1 commit into from
May 31, 2023
Merged

prefer JDK String replace methods to Plexus ones #193

merged 1 commit into from
May 31, 2023

Conversation

elharo
Copy link
Contributor

@elharo elharo commented May 31, 2023

@elharo elharo requested a review from slachiewicz May 31, 2023 11:17
@elharo elharo changed the title prefer JDK methods to Plexus ones prefer JDK replace methods to Plexus ones May 31, 2023
@elharo elharo changed the title prefer JDK replace methods to Plexus ones prefer JDK String replace methods to Plexus ones May 31, 2023
@slachiewicz
Copy link
Member

Is this null safe?

@elharo
Copy link
Contributor Author

elharo commented May 31, 2023

Yes. None of the strings involved can be null here. They're either literals or null-checked earlier in the method.

@elharo elharo merged commit f744036 into master May 31, 2023
@elharo elharo deleted the string branch May 31, 2023 14:09
@timtebeek
Copy link
Contributor

@timtebeek

Appreciate the update @elharo ; do I understand that this is one you'd like to tackle next?
I had a quick look; there's some 190 occurrences of StringUtils.replace(..) left in Apache Maven;
45 of those are in src/test; most involve replacing some variant of slashes:
https://public.moderne.io/results/kTDTf/data-tables (temporary link) / Google Sheets (less temporary)

Since we can't yet detect nullability, we'd likely have to insert a null check and fallback, which might look slightly awkward in some. Potentially we'd be able to clean that up in the future, but that would be some time away. Let me know if this is something you'd want to proceed with.

Alternatively there's another few common methods that can be found through:
https://public.moderne.io/recipes/org.openrewrite.java.search.FindMethods
With method pattern *..StringUtils *(..) run against the Apache Maven organization (top right dropdown).
Or we can have a look at some of the FileUtils/IOUtils, although I suspect those would require Java 11+ for JDK replacements.

@elharo
Copy link
Contributor Author

elharo commented May 31, 2023

Not necessarily next. Just thought I'd call your attention to some methods that are really, really out of date

@elharo
Copy link
Contributor Author

elharo commented May 31, 2023

A lot of FileUtils and IOUtils can be fixed in Java 8. I'm not sure it's a simple search and replace though. For instance, I've removed a lot of IOUtils.close() methods by using try with resources.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants