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

[MNG-6829] Replace StringUtils#isEmpty(String) & #isNotEmpty(String) #58

Conversation

timtebeek
Copy link
Contributor

Last batch of is(Not)Empty for https://issues.apache.org/jira/browse/MNG-6829
These are the smallest change sets, hence why I opened more at the same time.
After this we can target the next most often used method from the StringUtils classes.


Use this link to re-run the recipe: https://public.moderne.io/recipes/org.openrewrite.java.migrate.apache.commons.lang.IsNotEmptyToJdk?organizationId=QXBhY2hlIE1hdmVu

Co-authored-by: Moderne <[email protected]>
@timtebeek
Copy link
Contributor Author

@elharo I notice that my commit message didn't make it into the created pull request message, so you might not be aware of some of these last open pull requests. Would you mind going through this last batch?

Last batch of is(Not)Empty for https://issues.apache.org/jira/browse/MNG-6829
These are the smallest change sets, hence why I opened more at the same time.
After this we can target the next most often used method from the StringUtils classes.

@elharo elharo merged commit 429ad5b into apache:master May 24, 2023
@@ -218,7 +218,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
ArtifactType artifactType =
repositorySystemSession.getArtifactTypeRegistry().get(packaging);
if (artifactType != null
&& StringUtils.isEmpty(classifier)
&& (classifier == null || classifier.isEmpty())
&& !StringUtils.isEmpty(artifactType.getClassifier())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Noticed it missed one in line 222

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's surprising! Usually when that happens there's some missing type attribution; I'll do a final sweep to see if it pops up again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at this again: the line was not missed, but intentionally left out in the recipe, as we can't be certain that the argument method invocation is safe to call twice; once for the null check, and again for the isEmpty call. Hence why I don't convert those automatically yet, even though in this case it's probably safe to do.

@timtebeek timtebeek deleted the refactor/replace-any-string-utils-is-empty-string-and-is-not-empty-string branch June 1, 2023 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants