WIP: Fix PMD rule LiteralsFirstInComparisons for compareTo* and contentEquals #448
Conversation
punkratz312
force-pushed
the
Fix-PMD-rule-LiteralsFirstInComparisons-for-compareTo-and-contentEquals
branch
from
1f33b73 to
b38ff0a
Compare
|
I need to fix my setup to get this running. It should be almost there, right? I think it's better to deliver something now rather than waiting longer. Feel free to take over and provide early feedback if this seems like the right direction. Thanks |
…ns-for-compareTo-and-contentEquals' into Fix-PMD-rule-LiteralsFirstInComparisons-for-compareTo-and-contentEquals
| style = Checkstyle.equalsAvoidsNull(); | ||
| return Preconditions.check( | ||
| // new UsesMethod<>("java.lang.String *quals*(..)"), | ||
| new UsesMethod<>("java.lang.String *(..)"), |
There was a problem hiding this comment.
Hi @timtebeek,
I need help finding a better regex for equals and compareTo. The current one is just a temporary fix—it might work, but it's inefficient.
| } | ||
| }; | ||
| return Preconditions.check( | ||
| new UsesMethod<>("java.lang.String *quals*(..)"), |
There was a problem hiding this comment.
I noticed this by accident while wondering why the recipe wasn’t applied. I think it would be better to prioritize a functional approach—putting the most important things first. In a functional style, it’s easier to see and understand the flow right away.
Thanks for considering this!
| @@ -447,6 +499,15 @@ public class A { | |||
| System.out.println(s.compareToIgnoreCase("test")); | |||
| } | |||
| } | |||
| """, | |||
| """ | |||
| public class A { | |||
There was a problem hiding this comment.
I’m not sure why we don’t have a second comparison method here—you guys probably know better. Looking at the code, it seems like the goal is to avoid an NPE.
This test was the only one failing, which caught my attention, so I decided to conclude it and ask the team for their thoughts.
Thanks!
#362