Skip to content

address: LiteralsFirstInComparisons PMD rule #368

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.

Sign up for GitHub

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

Merged
timtebeek merged 64 commits into from
Oct 22, 2024
Merged

address: LiteralsFirstInComparisons PMD rule #368

Changes from 1 commit
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
78eb2e7
wip
pinguin3245678 Oct 14, 2024
db907e9
wip
pinguin3245678 Oct 14, 2024
44432a0
wip BAR_CONTENT_EQUALS
pinguin3245678 Oct 14, 2024
6bbb603
add compareToIgnoreCase
pinguin3245678 Oct 14, 2024
3b95e60
wip
pinguin3245678 Oct 14, 2024
b6f48c8
undo
pinguin3245678 Oct 14, 2024
b187593
fix name for direct matching
pinguin3245678 Oct 14, 2024
735ca39
EQUALS_IGNORE_CASE
pinguin3245678 Oct 14, 2024
111baaa
add CONTENT_EQUALS
pinguin3245678 Oct 14, 2024
709a488
add compareToInverted
pinguin3245678 Oct 14, 2024
983b7f5
tmp fix
pinguin3245678 Oct 14, 2024
d9f83f6
wip
pinguin3245678 Oct 14, 2024
5b30904
fix working
pinguin3245678 Oct 14, 2024
13d7057
wiß
pinguin3245678 Oct 14, 2024
cc2c022
wip
pinguin3245678 Oct 14, 2024
e94ff44
wip
pinguin3245678 Oct 14, 2024
8addad3
wip
pinguin3245678 Oct 14, 2024
21f37a4
wip
pinguin3245678 Oct 14, 2024
e424b6f
finalize
pinguin3245678 Oct 14, 2024
9470cd4
sort
pinguin3245678 Oct 14, 2024
35ed8cf
add doc
pinguin3245678 Oct 14, 2024
a519472
undo
pinguin3245678 Oct 14, 2024
2f0e9b8
Update src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullV…
Oct 14, 2024
931755a
Merge branch 'main' into EqualsAvoidsNullVisitor-extend-literalsfirst…
Oct 14, 2024
2c2b33a
fix Cast after
pinguin3245678 Oct 14, 2024
7e813dc
Merge remote-tracking branch 'origin/EqualsAvoidsNullVisitor-extend-l…
pinguin3245678 Oct 14, 2024
241262f
Update src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullV…
Oct 14, 2024
4d78d78
Revert "fix Cast after"
pinguin3245678 Oct 14, 2024
ca9d3d7
wip: error: pattern matching in instanceof is not supported in -source 8
pinguin3245678 Oct 14, 2024
55b8299
cast
pinguin3245678 Oct 14, 2024
bec2a8e
Revert build.gradle changes; rewrite-bom manages rewrite-java
timtebeek Oct 14, 2024
477820a
Merge remote-tracking branch 'origin/EqualsAvoidsNullVisitor-extend-l…
pinguin3245678 Oct 14, 2024
e1cdc55
format
pinguin3245678 Oct 14, 2024
0f85697
condense
pinguin3245678 Oct 14, 2024
f2b94d4
condense
pinguin3245678 Oct 14, 2024
396d43b
apply style
pinguin3245678 Oct 14, 2024
6c205af
wip
pinguin3245678 Oct 14, 2024
586d85e
format fin
pinguin3245678 Oct 14, 2024
d1b6c1d
fix
pinguin3245678 Oct 14, 2024
0715f12
isStringComparisonMethod
pinguin3245678 Oct 14, 2024
28f0313
remove null
pinguin3245678 Oct 14, 2024
65a46a6
undo
pinguin3245678 Oct 14, 2024
0948c77
Update src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullV…
Oct 14, 2024
32e089e
add doc
pinguin3245678 Oct 14, 2024
29ad521
Merge remote-tracking branch 'origin/EqualsAvoidsNullVisitor-extend-l…
pinguin3245678 Oct 14, 2024
4cc0405
fix getSuperIfSelectNull
pinguin3245678 Oct 14, 2024
5c54ac0
paranthese
pinguin3245678 Oct 14, 2024
b70d866
fix dry
pinguin3245678 Oct 14, 2024
0ee53b4
fix if
pinguin3245678 Oct 14, 2024
cae686e
fix if
pinguin3245678 Oct 14, 2024
cbe6400
fix literalsFirstInComparisons
pinguin3245678 Oct 14, 2024
f4e2a59
order
pinguin3245678 Oct 14, 2024
82ba719
dry firstArgument
pinguin3245678 Oct 14, 2024
34ced7e
naming
pinguin3245678 Oct 14, 2024
7767c42
Merge branch 'main' into EqualsAvoidsNullVisitor-extend-literalsfirst…
Oct 16, 2024
4ec7e3b
Update src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullV…
Oct 16, 2024
d279932
Fix earlier incorrect bot suggestion
timtebeek Oct 21, 2024
3e715de
Place binary operators on same line; remove nested ternaries
timtebeek Oct 21, 2024
a2c3c1f
Restore more deliberate naming of `potentialNullCheck`
timtebeek Oct 21, 2024
dd458ab
Collapse imports as per what's common elsewhere
timtebeek Oct 21, 2024
9fed309
Make the method matchers static final again
timtebeek Oct 21, 2024
2a582a1
add leftover
pinguin3245678 Oct 22, 2024
dadb102
Merge remote-tracking branch 'origin/EqualsAvoidsNullVisitor-extend-l…
pinguin3245678 Oct 22, 2024
726c5b0
Use JSpecify nullable annotations
timtebeek Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,11 @@
import org.openrewrite.java.JavaVisitor;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.style.EqualsAvoidsNullStyle;
import org.openrewrite.java.tree.Expression;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaType;
import org.openrewrite.java.tree.Space;
import org.openrewrite.java.tree.*;
import org.openrewrite.marker.Markers;

import static java.util.Collections.singletonList;
import static java.util.Objects.requireNonNull;
import static org.openrewrite.java.tree.JLeftPadded.build;
import static org.openrewrite.java.tree.Space.SINGLE_SPACE;

/**
* A visitor that identifies and addresses potential issues related to
Expand Down Expand Up @@ -97,8 +92,8 @@ private static J.Binary literalsFirstInComparisonsNull(J.MethodInvocation m, Exp
m.getPrefix(),
Markers.EMPTY,
requireNonNull(m.getSelect()),
build(J.Binary.Type.Equal).withBefore(SINGLE_SPACE),
firstArgument.withPrefix(SINGLE_SPACE),
JLeftPadded.build(J.Binary.Type.Equal).withBefore(Space.SINGLE_SPACE),
Copy link
Contributor Author

@Pankraz76 Pankraz76 Oct 22, 2024

Choose a reason for hiding this comment

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

Build maybe okay, but the Space is DRY Space. SPACE

If i need unwanted context i move my mouse and not clutter the code with redundancy, but its a style where most dev´s use full qualified imports.

image

Copy link
Member

@timtebeek timtebeek Oct 22, 2024

Choose a reason for hiding this comment

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

Often classes use a mix of the two constants in Space, only one of which has _SPACE in the name for the repetition. I'd prefer then to be consistent with prior use, and i/fwhen we dedice to change that, do so through our enforced patterns in rewrite-recommendations. I hope that context helps!

Copy link
Contributor

@knutwannheden knutwannheden Oct 22, 2024

Choose a reason for hiding this comment

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

I suspect the idea with the name was to emphasize that the object represents a single space character. The class Space represents any sequence of whitespace and comments, so Space.SINGLE would be a bit ambiguous (a single space, tab, newline, or comment?), so Space.SINGLE_SPACE uses "space" in two different meanings.

firstArgument.withPrefix(Space.SINGLE_SPACE),
JavaType.Primitive.Boolean);
}

Expand Down