Fix PMD rule `LiteralsFirstInComparisons` for `compareTo*` and `contentEquals`

[punkratz312]

Simply switch the arguments; that should be a very small change to make, just similar to the Inline Variable recipe.

What problem are you trying to solve?

https://pmd.github.io/pmd/pmd_rules_java_bestpractices.html#literalsfirstincomparisons

Describe the situation before applying the recipe

class A {
    boolean bar(String x) {
        return x.equals("2"); // should be "2".equals(x)
    }
}

Describe the situation after applying the recipe

class A {
    boolean bar(String x) {
        return "2".equals(x);
    }
}

Further examples from the PMD:

class Foo {
    boolean bar(String x) {
        return x.equals("2"); // should be "2".equals(x)
    }
    boolean bar(String x) {
        return x.equalsIgnoreCase("2"); // should be "2".equalsIgnoreCase(x)
    }
    boolean bar(String x) {
        return (x.compareTo("bar") > 0); // should be: "bar".compareTo(x) < 0
    }
    boolean bar(String x) {
        return (x.compareToIgnoreCase("bar") > 0); // should be: "bar".compareToIgnoreCase(x) < 0
    }
    boolean bar(String x) {
        return x.contentEquals("bar"); // should be "bar".contentEquals(x)
    }
}

[punkratz312]

I would love to volunteer and implement it if you could provide me with the snippet to flip the arguments. It should be very similar to my favorite rule, 1488 InlineVariable. https://docs.openrewrite.org/recipes/staticanalysis/inlinevariable.

[knutwannheden]

For the equals() case there is already a recipe here: https://github.com/openrewrite/rewrite-static-analysis/blob/main/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNull.java. The others are not covered as far as I know.

[timtebeek]

Thanks for the offer to help @punkratz312 ! As Knut has indicated String#equals and String#equalsIgnoreCase are already handled in this existing recipe: https://github.com/openrewrite/rewrite-static-analysis/blob/2163f0f05dd6fe7f4762e3486c8f16a9a5787036/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java#L32-L34

I suppose contentEquals could easily be added there as well, and we'd welcome such a change. Once that's covered a separate recipe can likely do the same for compareTo and compareToIgnoreCase.

Great to see you join the project and actively look for areas to improve. :)

[punkratz312]

Thanks for the quick win! I'm looking forward to this fix in our codebase. When will it be available after the merge?

• https://github.com/openrewrite/rewrite-static-analysis/pull/365

[timtebeek]

Once merged it'll be available from our snapshot versions in a matter of minutes (provided the build succeeds). From there it'll get included in the next release; which is every other week on Wednesday, with the next one on the 23rd. Thanks again!

[timtebeek]

• Fixed in #368. Thanks again!