From 78eb2e702964307bcbddf26aabd5c67c0e5abfd6 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 11:23:16 +0200 Subject: [PATCH 01/28] wip --- .../EqualsAvoidsNullVisitor.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index 420d4eacb..cf3e9db35 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -31,7 +31,10 @@ @EqualsAndHashCode(callSuper = false) public class EqualsAvoidsNullVisitor

extends JavaVisitor

{ private static final MethodMatcher STRING_EQUALS = new MethodMatcher("String equals(java.lang.Object)"); - private static final MethodMatcher STRING_EQUALS_IGNORE_CASE = new MethodMatcher("String equalsIgnoreCase(java.lang.String)"); + private static final MethodMatcher STRING_EQUALS_IGNORE_CASE = new MethodMatcher( + "String equalsIgnoreCase(java.lang.String)"); + private static final MethodMatcher STRING_EQUALS_IGNORE_CASE = new MethodMatcher( + "String equalsIgnoreCase(java.lang.String)"); EqualsAvoidsNullStyle style; @@ -46,16 +49,19 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { return m; } - if ((STRING_EQUALS.matches(m) || (!Boolean.TRUE.equals(style.getIgnoreEqualsIgnoreCase()) && STRING_EQUALS_IGNORE_CASE.matches(m))) && - m.getArguments().get(0) instanceof J.Literal && - !(m.getSelect() instanceof J.Literal)) { + if (STRING_EQUALS.matches(m) + || !style.getIgnoreEqualsIgnoreCase() + && STRING_EQUALS_IGNORE_CASE.matches(m) + && m.getArguments().get(0) instanceof J.Literal + && !(m.getSelect() instanceof J.Literal)) { Tree parent = getCursor().getParentTreeCursor().getValue(); if (parent instanceof J.Binary) { J.Binary binary = (J.Binary) parent; if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof J.Binary) { J.Binary potentialNullCheck = (J.Binary) binary.getLeft(); - if ((isNullLiteral(potentialNullCheck.getLeft()) && matchesSelect(potentialNullCheck.getRight(), m.getSelect())) || - (isNullLiteral(potentialNullCheck.getRight()) && matchesSelect(potentialNullCheck.getLeft(), m.getSelect()))) { + if ((isNullLiteral(potentialNullCheck.getLeft()) && matchesSelect(potentialNullCheck.getRight(), + m.getSelect())) || + (isNullLiteral(potentialNullCheck.getRight()) && matchesSelect(potentialNullCheck.getLeft(), m.getSelect()))) { doAfterVisit(new RemoveUnnecessaryNullCheck<>(binary)); } } From db907e9e46cbfa7114338866e7f0acdba0ab97d3 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 11:25:23 +0200 Subject: [PATCH 02/28] wip --- .../EqualsAvoidsNullVisitor.java | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index cf3e9db35..2eb63912f 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -30,11 +30,18 @@ @Value @EqualsAndHashCode(callSuper = false) public class EqualsAvoidsNullVisitor

extends JavaVisitor

{ - private static final MethodMatcher STRING_EQUALS = new MethodMatcher("String equals(java.lang.Object)"); - private static final MethodMatcher STRING_EQUALS_IGNORE_CASE = new MethodMatcher( - "String equalsIgnoreCase(java.lang.String)"); - private static final MethodMatcher STRING_EQUALS_IGNORE_CASE = new MethodMatcher( + + private static final MethodMatcher EQUALS = new MethodMatcher("String equals(java.lang.Object)"); + private static final MethodMatcher EQUALS_IGNORE_CASE = new MethodMatcher( "String equalsIgnoreCase(java.lang.String)"); + private static final MethodMatcher BAR_EQUALS = new MethodMatcher("String bar(java.lang.String)"); + private static final MethodMatcher BAR_EQUALS_IGNORE_CASE = + new MethodMatcher("String barIgnoreCase(java.lang.String)"); + private static final MethodMatcher BAR_COMPARE_TO = new MethodMatcher("String barCompareTo(java.lang.String)"); + private static final MethodMatcher BAR_COMPARE_TO_IGNORE_CASE = + new MethodMatcher("String barCompareToIgnoreCase(java.lang.String)"); + private static final MethodMatcher BAR_CONTENT_EQUALS = + new MethodMatcher("String barContentEquals(java.lang.String)"); EqualsAvoidsNullStyle style; @@ -49,9 +56,9 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { return m; } - if (STRING_EQUALS.matches(m) + if (EQUALS.matches(m) || !style.getIgnoreEqualsIgnoreCase() - && STRING_EQUALS_IGNORE_CASE.matches(m) + && EQUALS_IGNORE_CASE.matches(m) && m.getArguments().get(0) instanceof J.Literal && !(m.getSelect() instanceof J.Literal)) { Tree parent = getCursor().getParentTreeCursor().getValue(); From 44432a0b183aeb3e765d52bf3293b039f3d9f991 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 11:26:23 +0200 Subject: [PATCH 03/28] wip BAR_CONTENT_EQUALS --- .../openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index 2eb63912f..0b05e3615 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -59,6 +59,11 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { if (EQUALS.matches(m) || !style.getIgnoreEqualsIgnoreCase() && EQUALS_IGNORE_CASE.matches(m) + && BAR_EQUALS.matches(m) + && BAR_EQUALS_IGNORE_CASE.matches(m) + && BAR_COMPARE_TO.matches(m) + && BAR_COMPARE_TO_IGNORE_CASE.matches(m) + && BAR_CONTENT_EQUALS.matches(m) && m.getArguments().get(0) instanceof J.Literal && !(m.getSelect() instanceof J.Literal)) { Tree parent = getCursor().getParentTreeCursor().getValue(); From 6bbb603a2fc1be3ad6f38b483bbe545281708d64 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 11:29:44 +0200 Subject: [PATCH 04/28] add compareToIgnoreCase --- .../EqualsAvoidsNullVisitor.java | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index 0b05e3615..b70ab3e75 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -31,17 +31,15 @@ @EqualsAndHashCode(callSuper = false) public class EqualsAvoidsNullVisitor

extends JavaVisitor

{ - private static final MethodMatcher EQUALS = new MethodMatcher("String equals(java.lang.Object)"); - private static final MethodMatcher EQUALS_IGNORE_CASE = new MethodMatcher( - "String equalsIgnoreCase(java.lang.String)"); - private static final MethodMatcher BAR_EQUALS = new MethodMatcher("String bar(java.lang.String)"); - private static final MethodMatcher BAR_EQUALS_IGNORE_CASE = - new MethodMatcher("String barIgnoreCase(java.lang.String)"); - private static final MethodMatcher BAR_COMPARE_TO = new MethodMatcher("String barCompareTo(java.lang.String)"); - private static final MethodMatcher BAR_COMPARE_TO_IGNORE_CASE = - new MethodMatcher("String barCompareToIgnoreCase(java.lang.String)"); - private static final MethodMatcher BAR_CONTENT_EQUALS = - new MethodMatcher("String barContentEquals(java.lang.String)"); + private static final String STRING_PREFIX = "String "; + private static final MethodMatcher EQUALS = new MethodMatcher(STRING_PREFIX + "equals(java.lang.Object)"); + private static final MethodMatcher EQUALS_IGNORE_CASE = new MethodMatcher(STRING_PREFIX + "equalsIgnoreCase(java" + + ".lang.String)"); + private static final MethodMatcher COMPARE_TO = new MethodMatcher(STRING_PREFIX + "compareTo(java.lang.String)"); + private static final MethodMatcher COMPARE_TO_IGNORE_CASE = new MethodMatcher(STRING_PREFIX + + "compareToIgnoreCase(java.lang.String)"); + private static final MethodMatcher CONTENT_EQUALS = new MethodMatcher(STRING_PREFIX + + "contentEquals(java.lang.String)"); EqualsAvoidsNullStyle style; @@ -59,11 +57,9 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { if (EQUALS.matches(m) || !style.getIgnoreEqualsIgnoreCase() && EQUALS_IGNORE_CASE.matches(m) - && BAR_EQUALS.matches(m) - && BAR_EQUALS_IGNORE_CASE.matches(m) - && BAR_COMPARE_TO.matches(m) - && BAR_COMPARE_TO_IGNORE_CASE.matches(m) - && BAR_CONTENT_EQUALS.matches(m) + && COMPARE_TO.matches(m) + && COMPARE_TO_IGNORE_CASE.matches(m) + && CONTENT_EQUALS.matches(m) && m.getArguments().get(0) instanceof J.Literal && !(m.getSelect() instanceof J.Literal)) { Tree parent = getCursor().getParentTreeCursor().getValue(); From 3b95e60ef4f74d1143898731794ea2c7493e5ff7 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 11:35:42 +0200 Subject: [PATCH 05/28] wip --- .../EqualsAvoidsNullVisitor.java | 37 +++++++++++-------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index b70ab3e75..ee7fc42ae 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -17,6 +17,7 @@ import lombok.EqualsAndHashCode; import lombok.Value; +import org.jetbrains.annotations.NotNull; import org.jspecify.annotations.Nullable; import org.openrewrite.Tree; import org.openrewrite.java.JavaVisitor; @@ -26,6 +27,8 @@ import org.openrewrite.marker.Markers; import static java.util.Collections.singletonList; +import static org.openrewrite.java.tree.Space.EMPTY; +import static org.openrewrite.java.tree.Space.SINGLE_SPACE; @Value @EqualsAndHashCode(callSuper = false) @@ -45,16 +48,17 @@ public class EqualsAvoidsNullVisitor

extends JavaVisitor

{ @Override public J visitMethodInvocation(J.MethodInvocation method, P p) { - J j = super.visitMethodInvocation(method, p); - if (!(j instanceof J.MethodInvocation)) { - return j; - } - J.MethodInvocation m = (J.MethodInvocation) j; - if (m.getSelect() == null) { + J m = super.visitMethodInvocation(method, p); + if (!(m instanceof J.MethodInvocation)) { return m; } + return visitMethodInvocation((J.MethodInvocation) m); + } - if (EQUALS.matches(m) + private @NotNull TypedTree visitMethodInvocation(final J.MethodInvocation m) { + if (m.getSelect() == null) { + return m; + } else if (EQUALS.matches(m) || !style.getIgnoreEqualsIgnoreCase() && EQUALS_IGNORE_CASE.matches(m) && COMPARE_TO.matches(m) @@ -74,19 +78,21 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { } } } - if (m.getArguments().get(0).getType() == JavaType.Primitive.Null) { - return new J.Binary(Tree.randomId(), m.getPrefix(), Markers.EMPTY, + return new J.Binary(Tree.randomId(), + m.getPrefix(), + Markers.EMPTY, m.getSelect(), - JLeftPadded.build(J.Binary.Type.Equal).withBefore(Space.SINGLE_SPACE), - m.getArguments().get(0).withPrefix(Space.SINGLE_SPACE), + JLeftPadded.build(J.Binary.Type.Equal).withBefore(SINGLE_SPACE), + m.getArguments().get(0).withPrefix(SINGLE_SPACE), JavaType.Primitive.Boolean); } else { - m = m.withSelect(((J.Literal) m.getArguments().get(0)).withPrefix(m.getSelect().getPrefix())) - .withArguments(singletonList(m.getSelect().withPrefix(Space.EMPTY))); + return m.withSelect(m.getArguments() + .get(0) + .withPrefix(m.getSelect().getPrefix())) + .withArguments(singletonList(m.getSelect().withPrefix(EMPTY))); } } - return m; } @@ -118,9 +124,8 @@ public RemoveUnnecessaryNullCheck(J.Binary scope) { public J visitBinary(J.Binary binary, P p) { if (scope.isScope(binary)) { done = true; - return binary.getRight().withPrefix(Space.EMPTY); + return binary.getRight().withPrefix(EMPTY); } - return super.visitBinary(binary, p); } } From b6f48c890c57fb9e26374e6f9d6108a9733df0da Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 11:38:24 +0200 Subject: [PATCH 06/28] undo --- .../EqualsAvoidsNullVisitor.java | 59 +++++++------------ 1 file changed, 21 insertions(+), 38 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index ee7fc42ae..b89db4a13 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -17,7 +17,6 @@ import lombok.EqualsAndHashCode; import lombok.Value; -import org.jetbrains.annotations.NotNull; import org.jspecify.annotations.Nullable; import org.openrewrite.Tree; import org.openrewrite.java.JavaVisitor; @@ -27,45 +26,30 @@ import org.openrewrite.marker.Markers; import static java.util.Collections.singletonList; -import static org.openrewrite.java.tree.Space.EMPTY; -import static org.openrewrite.java.tree.Space.SINGLE_SPACE; @Value @EqualsAndHashCode(callSuper = false) public class EqualsAvoidsNullVisitor

extends JavaVisitor

{ - - private static final String STRING_PREFIX = "String "; - private static final MethodMatcher EQUALS = new MethodMatcher(STRING_PREFIX + "equals(java.lang.Object)"); - private static final MethodMatcher EQUALS_IGNORE_CASE = new MethodMatcher(STRING_PREFIX + "equalsIgnoreCase(java" + - ".lang.String)"); - private static final MethodMatcher COMPARE_TO = new MethodMatcher(STRING_PREFIX + "compareTo(java.lang.String)"); - private static final MethodMatcher COMPARE_TO_IGNORE_CASE = new MethodMatcher(STRING_PREFIX - + "compareToIgnoreCase(java.lang.String)"); - private static final MethodMatcher CONTENT_EQUALS = new MethodMatcher(STRING_PREFIX - + "contentEquals(java.lang.String)"); + private static final MethodMatcher STRING_EQUALS = new MethodMatcher("String equals(java.lang.Object)"); + private static final MethodMatcher STRING_EQUALS_IGNORE_CASE = new MethodMatcher("String equalsIgnoreCasewerewr" + + "(java.lang.String)"); EqualsAvoidsNullStyle style; @Override public J visitMethodInvocation(J.MethodInvocation method, P p) { - J m = super.visitMethodInvocation(method, p); - if (!(m instanceof J.MethodInvocation)) { - return m; + J j = super.visitMethodInvocation(method, p); + if (!(j instanceof J.MethodInvocation)) { + return j; } - return visitMethodInvocation((J.MethodInvocation) m); - } - - private @NotNull TypedTree visitMethodInvocation(final J.MethodInvocation m) { + J.MethodInvocation m = (J.MethodInvocation) j; if (m.getSelect() == null) { return m; - } else if (EQUALS.matches(m) - || !style.getIgnoreEqualsIgnoreCase() - && EQUALS_IGNORE_CASE.matches(m) - && COMPARE_TO.matches(m) - && COMPARE_TO_IGNORE_CASE.matches(m) - && CONTENT_EQUALS.matches(m) - && m.getArguments().get(0) instanceof J.Literal - && !(m.getSelect() instanceof J.Literal)) { + } + + if ((STRING_EQUALS.matches(m) || (!Boolean.TRUE.equals(style.getIgnoreEqualsIgnoreCase()) && STRING_EQUALS_IGNORE_CASE.matches(m))) && + m.getArguments().get(0) instanceof J.Literal && + !(m.getSelect() instanceof J.Literal)) { Tree parent = getCursor().getParentTreeCursor().getValue(); if (parent instanceof J.Binary) { J.Binary binary = (J.Binary) parent; @@ -78,21 +62,19 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { } } } + if (m.getArguments().get(0).getType() == JavaType.Primitive.Null) { - return new J.Binary(Tree.randomId(), - m.getPrefix(), - Markers.EMPTY, + return new J.Binary(Tree.randomId(), m.getPrefix(), Markers.EMPTY, m.getSelect(), - JLeftPadded.build(J.Binary.Type.Equal).withBefore(SINGLE_SPACE), - m.getArguments().get(0).withPrefix(SINGLE_SPACE), + JLeftPadded.build(J.Binary.Type.Equal).withBefore(Space.SINGLE_SPACE), + m.getArguments().get(0).withPrefix(Space.SINGLE_SPACE), JavaType.Primitive.Boolean); } else { - return m.withSelect(m.getArguments() - .get(0) - .withPrefix(m.getSelect().getPrefix())) - .withArguments(singletonList(m.getSelect().withPrefix(EMPTY))); + m = m.withSelect(((J.Literal) m.getArguments().get(0)).withPrefix(m.getSelect().getPrefix())) + .withArguments(singletonList(m.getSelect().withPrefix(Space.EMPTY))); } } + return m; } @@ -124,8 +106,9 @@ public RemoveUnnecessaryNullCheck(J.Binary scope) { public J visitBinary(J.Binary binary, P p) { if (scope.isScope(binary)) { done = true; - return binary.getRight().withPrefix(EMPTY); + return binary.getRight().withPrefix(Space.EMPTY); } + return super.visitBinary(binary, p); } } From b18759318eea3a7fb347de06264c5e4b589db1f3 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 11:39:18 +0200 Subject: [PATCH 07/28] fix name for direct matching --- ...sNullTest.java => EqualsAvoidsNullVisitorTest.java} | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) rename src/test/java/org/openrewrite/staticanalysis/{EqualsAvoidsNullTest.java => EqualsAvoidsNullVisitorTest.java} (95%) diff --git a/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullTest.java b/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java similarity index 95% rename from src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullTest.java rename to src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java index 68bb55236..4e2a79301 100644 --- a/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java @@ -23,7 +23,7 @@ import static org.openrewrite.java.Assertions.java; @SuppressWarnings({"ClassInitializerMayBeStatic", "StatementWithEmptyBody", "ConstantConditions"}) -class EqualsAvoidsNullTest implements RewriteTest { +class EqualsAvoidsNullVisitorTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { @@ -88,8 +88,8 @@ public class A { @Test void nullLiteral() { rewriteRun( - //language=java - java(""" + //language=java + java(""" public class A { void foo(String s) { if(s.equals(null)) { @@ -97,8 +97,8 @@ void foo(String s) { } } """, - """ - + """ + public class A { void foo(String s) { if(s == null) { From 735ca39b3bdd19bd8302cfc1047b8d034eb579da Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 11:40:49 +0200 Subject: [PATCH 08/28] EQUALS_IGNORE_CASE --- .../EqualsAvoidsNullVisitor.java | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index b89db4a13..f9b237313 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -30,9 +30,16 @@ @Value @EqualsAndHashCode(callSuper = false) public class EqualsAvoidsNullVisitor

extends JavaVisitor

{ - private static final MethodMatcher STRING_EQUALS = new MethodMatcher("String equals(java.lang.Object)"); - private static final MethodMatcher STRING_EQUALS_IGNORE_CASE = new MethodMatcher("String equalsIgnoreCasewerewr" + - "(java.lang.String)"); + + private static final String STRING_PREFIX = "String "; + private static final MethodMatcher EQUALS = new MethodMatcher(STRING_PREFIX + "equals(java.lang.Object)"); + private static final MethodMatcher EQUALS_IGNORE_CASE = new MethodMatcher(STRING_PREFIX + "equalsIgnoreCase(java" + + ".lang.String)"); + private static final MethodMatcher COMPARE_TO = new MethodMatcher(STRING_PREFIX + "compareTo(java.lang.String)"); + private static final MethodMatcher COMPARE_TO_IGNORE_CASE = new MethodMatcher(STRING_PREFIX + + "compareToIgnoreCase(java.lang.String)"); + private static final MethodMatcher CONTENT_EQUALS = new MethodMatcher(STRING_PREFIX + + "contentEquals(java.lang.String)"); EqualsAvoidsNullStyle style; @@ -47,7 +54,7 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { return m; } - if ((STRING_EQUALS.matches(m) || (!Boolean.TRUE.equals(style.getIgnoreEqualsIgnoreCase()) && STRING_EQUALS_IGNORE_CASE.matches(m))) && + if (EQUALS.matches(m) || !Boolean.TRUE.equals(style.getIgnoreEqualsIgnoreCase()) && EQUALS_IGNORE_CASE.matches(m) && m.getArguments().get(0) instanceof J.Literal && !(m.getSelect() instanceof J.Literal)) { Tree parent = getCursor().getParentTreeCursor().getValue(); @@ -55,8 +62,8 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { J.Binary binary = (J.Binary) parent; if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof J.Binary) { J.Binary potentialNullCheck = (J.Binary) binary.getLeft(); - if ((isNullLiteral(potentialNullCheck.getLeft()) && matchesSelect(potentialNullCheck.getRight(), - m.getSelect())) || + if (isNullLiteral(potentialNullCheck.getLeft()) && matchesSelect(potentialNullCheck.getRight(), + m.getSelect()) || (isNullLiteral(potentialNullCheck.getRight()) && matchesSelect(potentialNullCheck.getLeft(), m.getSelect()))) { doAfterVisit(new RemoveUnnecessaryNullCheck<>(binary)); } @@ -70,7 +77,7 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { m.getArguments().get(0).withPrefix(Space.SINGLE_SPACE), JavaType.Primitive.Boolean); } else { - m = m.withSelect(((J.Literal) m.getArguments().get(0)).withPrefix(m.getSelect().getPrefix())) + m = m.withSelect(m.getArguments().get(0).withPrefix(m.getSelect().getPrefix())) .withArguments(singletonList(m.getSelect().withPrefix(Space.EMPTY))); } } From 111baaadbfca1b4572d6dd5e5e0f8596db307511 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 11:41:21 +0200 Subject: [PATCH 09/28] add CONTENT_EQUALS --- .../staticanalysis/EqualsAvoidsNullVisitor.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index f9b237313..315f2a08d 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -54,9 +54,14 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { return m; } - if (EQUALS.matches(m) || !Boolean.TRUE.equals(style.getIgnoreEqualsIgnoreCase()) && EQUALS_IGNORE_CASE.matches(m) && - m.getArguments().get(0) instanceof J.Literal && - !(m.getSelect() instanceof J.Literal)) { + if (EQUALS.matches(m) + || !Boolean.TRUE.equals(style.getIgnoreEqualsIgnoreCase()) + && EQUALS_IGNORE_CASE.matches(m) + && COMPARE_TO.matches(m) + && COMPARE_TO_IGNORE_CASE.matches(m) + && CONTENT_EQUALS.matches(m) + && m.getArguments().get(0) instanceof J.Literal + && !(m.getSelect() instanceof J.Literal)) { Tree parent = getCursor().getParentTreeCursor().getValue(); if (parent instanceof J.Binary) { J.Binary binary = (J.Binary) parent; From 709a48856f31cec74e4d49f6d7b74c18d3bbbc3b Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 11:46:49 +0200 Subject: [PATCH 10/28] add compareToInverted --- .../EqualsAvoidsNullVisitor.java | 30 ++----- .../EqualsAvoidsNullVisitorTest.java | 78 +++++++++++++++++++ 2 files changed, 86 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index 315f2a08d..420d4eacb 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -30,16 +30,8 @@ @Value @EqualsAndHashCode(callSuper = false) public class EqualsAvoidsNullVisitor

extends JavaVisitor

{ - - private static final String STRING_PREFIX = "String "; - private static final MethodMatcher EQUALS = new MethodMatcher(STRING_PREFIX + "equals(java.lang.Object)"); - private static final MethodMatcher EQUALS_IGNORE_CASE = new MethodMatcher(STRING_PREFIX + "equalsIgnoreCase(java" + - ".lang.String)"); - private static final MethodMatcher COMPARE_TO = new MethodMatcher(STRING_PREFIX + "compareTo(java.lang.String)"); - private static final MethodMatcher COMPARE_TO_IGNORE_CASE = new MethodMatcher(STRING_PREFIX - + "compareToIgnoreCase(java.lang.String)"); - private static final MethodMatcher CONTENT_EQUALS = new MethodMatcher(STRING_PREFIX - + "contentEquals(java.lang.String)"); + private static final MethodMatcher STRING_EQUALS = new MethodMatcher("String equals(java.lang.Object)"); + private static final MethodMatcher STRING_EQUALS_IGNORE_CASE = new MethodMatcher("String equalsIgnoreCase(java.lang.String)"); EqualsAvoidsNullStyle style; @@ -54,22 +46,16 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { return m; } - if (EQUALS.matches(m) - || !Boolean.TRUE.equals(style.getIgnoreEqualsIgnoreCase()) - && EQUALS_IGNORE_CASE.matches(m) - && COMPARE_TO.matches(m) - && COMPARE_TO_IGNORE_CASE.matches(m) - && CONTENT_EQUALS.matches(m) - && m.getArguments().get(0) instanceof J.Literal - && !(m.getSelect() instanceof J.Literal)) { + if ((STRING_EQUALS.matches(m) || (!Boolean.TRUE.equals(style.getIgnoreEqualsIgnoreCase()) && STRING_EQUALS_IGNORE_CASE.matches(m))) && + m.getArguments().get(0) instanceof J.Literal && + !(m.getSelect() instanceof J.Literal)) { Tree parent = getCursor().getParentTreeCursor().getValue(); if (parent instanceof J.Binary) { J.Binary binary = (J.Binary) parent; if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof J.Binary) { J.Binary potentialNullCheck = (J.Binary) binary.getLeft(); - if (isNullLiteral(potentialNullCheck.getLeft()) && matchesSelect(potentialNullCheck.getRight(), - m.getSelect()) || - (isNullLiteral(potentialNullCheck.getRight()) && matchesSelect(potentialNullCheck.getLeft(), m.getSelect()))) { + if ((isNullLiteral(potentialNullCheck.getLeft()) && matchesSelect(potentialNullCheck.getRight(), m.getSelect())) || + (isNullLiteral(potentialNullCheck.getRight()) && matchesSelect(potentialNullCheck.getLeft(), m.getSelect()))) { doAfterVisit(new RemoveUnnecessaryNullCheck<>(binary)); } } @@ -82,7 +68,7 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { m.getArguments().get(0).withPrefix(Space.SINGLE_SPACE), JavaType.Primitive.Boolean); } else { - m = m.withSelect(m.getArguments().get(0).withPrefix(m.getSelect().getPrefix())) + m = m.withSelect(((J.Literal) m.getArguments().get(0)).withPrefix(m.getSelect().getPrefix())) .withArguments(singletonList(m.getSelect().withPrefix(Space.EMPTY))); } } diff --git a/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java b/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java index 4e2a79301..ea4934235 100644 --- a/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java @@ -108,4 +108,82 @@ void foo(String s) { """) ); } + + @Test + void compareToInverted() { + rewriteRun( + //language=java + java( + """ + public class A { + { + String s = null; + if(s.compareTo("test") == 0) {} + } + } + """, + """ + public class A { + { + String s = null; + if("test".compareTo(s) == 0) {} + } + } + """ + ) + ); + } + + @Test + void compareToIgnoreCaseInverted() { + rewriteRun( + //language=java + java( + """ + public class A { + { + String s = null; + if(s.compareToIgnoreCase("test") == 0) {} + } + } + """, + """ + public class A { + { + String s = null; + if("test".compareToIgnoreCase(s) == 0) {} + } + } + """ + ) + ); + } + + @Test + void contentEqualsInverted() { + rewriteRun( + //language=java + java( + """ + public class A { + { + String s = null; + CharSequence cs = "test"; + if(s.contentEquals(cs)) {} + } + } + """, + """ + public class A { + { + String s = null; + CharSequence cs = "test"; + if(cs.equals(s)) {} + } + } + """ + ) + ); + } + } From 983b7f542053cdb6d94b862d2ed10e196fc7481f Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 11:47:55 +0200 Subject: [PATCH 11/28] tmp fix --- .../EqualsAvoidsNullVisitor.java | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index 420d4eacb..9c742122d 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -30,8 +30,16 @@ @Value @EqualsAndHashCode(callSuper = false) public class EqualsAvoidsNullVisitor

extends JavaVisitor

{ - private static final MethodMatcher STRING_EQUALS = new MethodMatcher("String equals(java.lang.Object)"); - private static final MethodMatcher STRING_EQUALS_IGNORE_CASE = new MethodMatcher("String equalsIgnoreCase(java.lang.String)"); + + private static final String STRING_PREFIX = "String "; + private static final MethodMatcher EQUALS = new MethodMatcher(STRING_PREFIX + "equals(java.lang.Object)"); + private static final MethodMatcher EQUALS_IGNORE_CASE = new MethodMatcher(STRING_PREFIX + "equalsIgnoreCase(java" + + ".lang.String)"); + private static final MethodMatcher COMPARE_TO = new MethodMatcher(STRING_PREFIX + "compareTo(java.lang.String)"); + private static final MethodMatcher COMPARE_TO_IGNORE_CASE = new MethodMatcher(STRING_PREFIX + + "compareToIgnoreCase(java.lang.String)"); + private static final MethodMatcher CONTENT_EQUALS = new MethodMatcher(STRING_PREFIX + + "contentEquals(java.lang.String)"); EqualsAvoidsNullStyle style; @@ -46,16 +54,22 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { return m; } - if ((STRING_EQUALS.matches(m) || (!Boolean.TRUE.equals(style.getIgnoreEqualsIgnoreCase()) && STRING_EQUALS_IGNORE_CASE.matches(m))) && - m.getArguments().get(0) instanceof J.Literal && - !(m.getSelect() instanceof J.Literal)) { + if (EQUALS.matches(m) + || !style.getIgnoreEqualsIgnoreCase() + && EQUALS_IGNORE_CASE.matches(m) + && COMPARE_TO.matches(m) + && COMPARE_TO_IGNORE_CASE.matches(m) + && CONTENT_EQUALS.matches(m) + && m.getArguments().get(0) instanceof J.Literal + && !(m.getSelect() instanceof J.Literal)) { Tree parent = getCursor().getParentTreeCursor().getValue(); if (parent instanceof J.Binary) { J.Binary binary = (J.Binary) parent; if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof J.Binary) { J.Binary potentialNullCheck = (J.Binary) binary.getLeft(); - if ((isNullLiteral(potentialNullCheck.getLeft()) && matchesSelect(potentialNullCheck.getRight(), m.getSelect())) || - (isNullLiteral(potentialNullCheck.getRight()) && matchesSelect(potentialNullCheck.getLeft(), m.getSelect()))) { + if (isNullLiteral(potentialNullCheck.getLeft()) && matchesSelect(potentialNullCheck.getRight(), + m.getSelect()) || + (isNullLiteral(potentialNullCheck.getRight()) && matchesSelect(potentialNullCheck.getLeft(), m.getSelect()))) { doAfterVisit(new RemoveUnnecessaryNullCheck<>(binary)); } } @@ -68,7 +82,7 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { m.getArguments().get(0).withPrefix(Space.SINGLE_SPACE), JavaType.Primitive.Boolean); } else { - m = m.withSelect(((J.Literal) m.getArguments().get(0)).withPrefix(m.getSelect().getPrefix())) + m = m.withSelect(m.getArguments().get(0).withPrefix(m.getSelect().getPrefix())) .withArguments(singletonList(m.getSelect().withPrefix(Space.EMPTY))); } } From d9f83f64476902d3ac6a68f3b03aa95c7140ca69 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 12:03:04 +0200 Subject: [PATCH 12/28] wip --- .../EqualsAvoidsNullVisitor.java | 10 +- .../EqualsAvoidsNullVisitorTest.java | 97 +++---------------- 2 files changed, 18 insertions(+), 89 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index 9c742122d..d281f7143 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -55,11 +55,11 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { } if (EQUALS.matches(m) - || !style.getIgnoreEqualsIgnoreCase() - && EQUALS_IGNORE_CASE.matches(m) - && COMPARE_TO.matches(m) - && COMPARE_TO_IGNORE_CASE.matches(m) - && CONTENT_EQUALS.matches(m) + && !style.getIgnoreEqualsIgnoreCase() + || EQUALS_IGNORE_CASE.matches(m) + || COMPARE_TO.matches(m) + || COMPARE_TO_IGNORE_CASE.matches(m) + || CONTENT_EQUALS.matches(m) && m.getArguments().get(0) instanceof J.Literal && !(m.getSelect() instanceof J.Literal)) { Tree parent = getCursor().getParentTreeCursor().getValue(); diff --git a/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java b/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java index ea4934235..2f0c30bad 100644 --- a/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java @@ -37,13 +37,16 @@ void invertConditional() { //language=java java( """ - public class A { - { - String s = null; - if(s.equals("test")) {} - if(s.equalsIgnoreCase("test")) {} - } - } + public class A { + { + String s = null; + if(s.equals("test")) {} + if(s.equalsIgnoreCase("test")) {} + System.out.println(s.compareTo("test")); + System.out.println(s.compareToIgnoreCase("test")); + System.out.println(s.contentEquals("test")); + } + } """, """ public class A { @@ -51,6 +54,9 @@ public class A { String s = null; if("test".equals(s)) {} if("test".equalsIgnoreCase(s)) {} + System.out.println("test".compareTo(s)); + System.out.println("test".compareToIgnoreCase(s)); + System.out.println("test".contentEquals(s)); } } """ @@ -109,81 +115,4 @@ void foo(String s) { ); } - @Test - void compareToInverted() { - rewriteRun( - //language=java - java( - """ - public class A { - { - String s = null; - if(s.compareTo("test") == 0) {} - } - } - """, - """ - public class A { - { - String s = null; - if("test".compareTo(s) == 0) {} - } - } - """ - ) - ); - } - - @Test - void compareToIgnoreCaseInverted() { - rewriteRun( - //language=java - java( - """ - public class A { - { - String s = null; - if(s.compareToIgnoreCase("test") == 0) {} - } - } - """, - """ - public class A { - { - String s = null; - if("test".compareToIgnoreCase(s) == 0) {} - } - } - """ - ) - ); - } - - @Test - void contentEqualsInverted() { - rewriteRun( - //language=java - java( - """ - public class A { - { - String s = null; - CharSequence cs = "test"; - if(s.contentEquals(cs)) {} - } - } - """, - """ - public class A { - { - String s = null; - CharSequence cs = "test"; - if(cs.equals(s)) {} - } - } - """ - ) - ); - } - } From 5b3090443d87e1a941cf02fcf460e31a99b60b97 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 12:10:12 +0200 Subject: [PATCH 13/28] fix working --- .../EqualsAvoidsNullVisitor.java | 67 +++++++------------ 1 file changed, 24 insertions(+), 43 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index d281f7143..73a04882d 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -1,23 +1,7 @@ -/* - * Copyright 2020 the original author or authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - *

- * https://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ package org.openrewrite.staticanalysis; import lombok.EqualsAndHashCode; import lombok.Value; -import org.jspecify.annotations.Nullable; import org.openrewrite.Tree; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.MethodMatcher; @@ -31,15 +15,14 @@ @EqualsAndHashCode(callSuper = false) public class EqualsAvoidsNullVisitor

extends JavaVisitor

{ - private static final String STRING_PREFIX = "String "; - private static final MethodMatcher EQUALS = new MethodMatcher(STRING_PREFIX + "equals(java.lang.Object)"); - private static final MethodMatcher EQUALS_IGNORE_CASE = new MethodMatcher(STRING_PREFIX + "equalsIgnoreCase(java" + + private static final MethodMatcher EQUALS = new MethodMatcher("java.lang.String equals(java.lang.Object)"); + private static final MethodMatcher EQUALS_IGNORE_CASE = new MethodMatcher("java.lang.String equalsIgnoreCase(java" + ".lang.String)"); - private static final MethodMatcher COMPARE_TO = new MethodMatcher(STRING_PREFIX + "compareTo(java.lang.String)"); - private static final MethodMatcher COMPARE_TO_IGNORE_CASE = new MethodMatcher(STRING_PREFIX - + "compareToIgnoreCase(java.lang.String)"); - private static final MethodMatcher CONTENT_EQUALS = new MethodMatcher(STRING_PREFIX - + "contentEquals(java.lang.String)"); + private static final MethodMatcher COMPARE_TO = new MethodMatcher("java.lang.String compareTo(java.lang.String)"); + private static final MethodMatcher COMPARE_TO_IGNORE_CASE = new MethodMatcher("java.lang.String " + + "compareToIgnoreCase(java.lang.String)"); + private static final MethodMatcher CONTENT_EQUALS = new MethodMatcher("java.lang.String contentEquals(java.lang" + + ".CharSequence)"); EqualsAvoidsNullStyle style; @@ -54,27 +37,33 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { return m; } - if (EQUALS.matches(m) - && !style.getIgnoreEqualsIgnoreCase() - || EQUALS_IGNORE_CASE.matches(m) - || COMPARE_TO.matches(m) - || COMPARE_TO_IGNORE_CASE.matches(m) - || CONTENT_EQUALS.matches(m) - && m.getArguments().get(0) instanceof J.Literal - && !(m.getSelect() instanceof J.Literal)) { + boolean isLiteralArgument = m.getArguments().get(0) instanceof J.Literal; + boolean isNotLiteralSelect = !(m.getSelect() instanceof J.Literal); + + // Perform the swap for equals, equalsIgnoreCase, compareTo, compareToIgnoreCase + if (isNotLiteralSelect && isLiteralArgument && + (EQUALS.matches(m) + || (!Boolean.TRUE.equals(style.getIgnoreEqualsIgnoreCase()) && EQUALS_IGNORE_CASE.matches(m)) + || COMPARE_TO.matches(m) + || COMPARE_TO_IGNORE_CASE.matches(m) + // Exclude contentEquals() from swapping, as it's a safe call with a literal on the left. + || (CONTENT_EQUALS.matches(m) && isNotLiteralSelect))) { + Tree parent = getCursor().getParentTreeCursor().getValue(); + // Check for null checks if (parent instanceof J.Binary) { J.Binary binary = (J.Binary) parent; if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof J.Binary) { J.Binary potentialNullCheck = (J.Binary) binary.getLeft(); if (isNullLiteral(potentialNullCheck.getLeft()) && matchesSelect(potentialNullCheck.getRight(), - m.getSelect()) || - (isNullLiteral(potentialNullCheck.getRight()) && matchesSelect(potentialNullCheck.getLeft(), m.getSelect()))) { + m.getSelect()) + || isNullLiteral(potentialNullCheck.getRight()) && matchesSelect(potentialNullCheck.getLeft(), m.getSelect())) { doAfterVisit(new RemoveUnnecessaryNullCheck<>(binary)); } } } + // If the argument is null, replace with a binary null check if (m.getArguments().get(0).getType() == JavaType.Primitive.Null) { return new J.Binary(Tree.randomId(), m.getPrefix(), Markers.EMPTY, m.getSelect(), @@ -82,6 +71,7 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { m.getArguments().get(0).withPrefix(Space.SINGLE_SPACE), JavaType.Primitive.Boolean); } else { + // Swap the select and argument, maintaining prefixes m = m.withSelect(m.getArguments().get(0).withPrefix(m.getSelect().getPrefix())) .withArguments(singletonList(m.getSelect().withPrefix(Space.EMPTY))); } @@ -102,14 +92,6 @@ private static class RemoveUnnecessaryNullCheck

extends JavaVisitor

{ private final J.Binary scope; boolean done; - @Override - public @Nullable J visit(@Nullable Tree tree, P p) { - if (done) { - return (J) tree; - } - return super.visit(tree, p); - } - public RemoveUnnecessaryNullCheck(J.Binary scope) { this.scope = scope; } @@ -120,7 +102,6 @@ public J visitBinary(J.Binary binary, P p) { done = true; return binary.getRight().withPrefix(Space.EMPTY); } - return super.visitBinary(binary, p); } } From 13d7057b141b9ac56d4b0ab6c5c61fa59c7ef43b Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 12:12:51 +0200 Subject: [PATCH 14/28] =?UTF-8?q?wi=C3=9F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../EqualsAvoidsNullVisitor.java | 36 ++++++++----------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index 73a04882d..227ab5aed 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -35,35 +35,29 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { J.MethodInvocation m = (J.MethodInvocation) j; if (m.getSelect() == null) { return m; - } - - boolean isLiteralArgument = m.getArguments().get(0) instanceof J.Literal; - boolean isNotLiteralSelect = !(m.getSelect() instanceof J.Literal); - - // Perform the swap for equals, equalsIgnoreCase, compareTo, compareToIgnoreCase - if (isNotLiteralSelect && isLiteralArgument && - (EQUALS.matches(m) - || (!Boolean.TRUE.equals(style.getIgnoreEqualsIgnoreCase()) && EQUALS_IGNORE_CASE.matches(m)) - || COMPARE_TO.matches(m) - || COMPARE_TO_IGNORE_CASE.matches(m) - // Exclude contentEquals() from swapping, as it's a safe call with a literal on the left. - || (CONTENT_EQUALS.matches(m) && isNotLiteralSelect))) { + } else if (!(m.getSelect() instanceof J.Literal) + && m.getArguments().get(0) instanceof J.Literal + && EQUALS.matches(m) + || !Boolean.TRUE.equals(style.getIgnoreEqualsIgnoreCase()) + && EQUALS_IGNORE_CASE.matches(m) + || COMPARE_TO.matches(m) + || COMPARE_TO_IGNORE_CASE.matches(m) + || CONTENT_EQUALS.matches(m)) { Tree parent = getCursor().getParentTreeCursor().getValue(); - // Check for null checks if (parent instanceof J.Binary) { J.Binary binary = (J.Binary) parent; if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof J.Binary) { - J.Binary potentialNullCheck = (J.Binary) binary.getLeft(); - if (isNullLiteral(potentialNullCheck.getLeft()) && matchesSelect(potentialNullCheck.getRight(), + J.Binary left = (J.Binary) binary.getLeft(); + if (isNullLiteral(left.getLeft()) + && matchesSelect(left.getRight(), m.getSelect()) - || isNullLiteral(potentialNullCheck.getRight()) && matchesSelect(potentialNullCheck.getLeft(), m.getSelect())) { + || isNullLiteral(left.getRight()) + && matchesSelect(left.getLeft(), m.getSelect())) { doAfterVisit(new RemoveUnnecessaryNullCheck<>(binary)); } } } - - // If the argument is null, replace with a binary null check if (m.getArguments().get(0).getType() == JavaType.Primitive.Null) { return new J.Binary(Tree.randomId(), m.getPrefix(), Markers.EMPTY, m.getSelect(), @@ -71,12 +65,10 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { m.getArguments().get(0).withPrefix(Space.SINGLE_SPACE), JavaType.Primitive.Boolean); } else { - // Swap the select and argument, maintaining prefixes - m = m.withSelect(m.getArguments().get(0).withPrefix(m.getSelect().getPrefix())) + return m.withSelect(m.getArguments().get(0).withPrefix(m.getSelect().getPrefix())) .withArguments(singletonList(m.getSelect().withPrefix(Space.EMPTY))); } } - return m; } From cc2c0223c08f7dc1f9c3e6418690a65aa15b5505 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 12:14:13 +0200 Subject: [PATCH 15/28] wip --- .../EqualsAvoidsNullVisitor.java | 51 ++++++++++--------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index 227ab5aed..86760e35b 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -2,6 +2,7 @@ import lombok.EqualsAndHashCode; import lombok.Value; +import org.jetbrains.annotations.NotNull; import org.openrewrite.Tree; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.MethodMatcher; @@ -10,6 +11,7 @@ import org.openrewrite.marker.Markers; import static java.util.Collections.singletonList; +import static java.util.Objects.requireNonNull; @Value @EqualsAndHashCode(callSuper = false) @@ -43,33 +45,36 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { || COMPARE_TO.matches(m) || COMPARE_TO_IGNORE_CASE.matches(m) || CONTENT_EQUALS.matches(m)) { + return visitMethodInvocation(m); + } + return m; + } - Tree parent = getCursor().getParentTreeCursor().getValue(); - if (parent instanceof J.Binary) { - J.Binary binary = (J.Binary) parent; - if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof J.Binary) { - J.Binary left = (J.Binary) binary.getLeft(); - if (isNullLiteral(left.getLeft()) - && matchesSelect(left.getRight(), - m.getSelect()) - || isNullLiteral(left.getRight()) - && matchesSelect(left.getLeft(), m.getSelect())) { - doAfterVisit(new RemoveUnnecessaryNullCheck<>(binary)); - } + private @NotNull Expression visitMethodInvocation(final J.MethodInvocation m) { + Tree parent = getCursor().getParentTreeCursor().getValue(); + if (parent instanceof J.Binary) { + J.Binary binary = (J.Binary) parent; + if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof J.Binary) { + J.Binary left = (J.Binary) binary.getLeft(); + if (isNullLiteral(left.getLeft()) + && matchesSelect(left.getRight(), + requireNonNull(m.getSelect())) + || isNullLiteral(left.getRight()) + && matchesSelect(left.getLeft(), requireNonNull(m.getSelect()))) { + doAfterVisit(new RemoveUnnecessaryNullCheck<>(binary)); } } - if (m.getArguments().get(0).getType() == JavaType.Primitive.Null) { - return new J.Binary(Tree.randomId(), m.getPrefix(), Markers.EMPTY, - m.getSelect(), - JLeftPadded.build(J.Binary.Type.Equal).withBefore(Space.SINGLE_SPACE), - m.getArguments().get(0).withPrefix(Space.SINGLE_SPACE), - JavaType.Primitive.Boolean); - } else { - return m.withSelect(m.getArguments().get(0).withPrefix(m.getSelect().getPrefix())) - .withArguments(singletonList(m.getSelect().withPrefix(Space.EMPTY))); - } } - return m; + if (m.getArguments().get(0).getType() == JavaType.Primitive.Null) { + return new J.Binary(Tree.randomId(), m.getPrefix(), Markers.EMPTY, + requireNonNull(m.getSelect()), + JLeftPadded.build(J.Binary.Type.Equal).withBefore(Space.SINGLE_SPACE), + m.getArguments().get(0).withPrefix(Space.SINGLE_SPACE), + JavaType.Primitive.Boolean); + } else { + return m.withSelect(m.getArguments().get(0).withPrefix(m.getSelect().getPrefix())) + .withArguments(singletonList(m.getSelect().withPrefix(Space.EMPTY))); + } } private boolean isNullLiteral(Expression expression) { From e94ff44a4a3a7c3b36d40755beb046266a1e5029 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 12:14:52 +0200 Subject: [PATCH 16/28] wip --- .../org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index 86760e35b..31ac237f5 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -72,7 +72,7 @@ && matchesSelect(left.getLeft(), requireNonNull(m.getSelect()))) { m.getArguments().get(0).withPrefix(Space.SINGLE_SPACE), JavaType.Primitive.Boolean); } else { - return m.withSelect(m.getArguments().get(0).withPrefix(m.getSelect().getPrefix())) + return m.withSelect(m.getArguments().get(0).withPrefix(requireNonNull(m.getSelect()).getPrefix())) .withArguments(singletonList(m.getSelect().withPrefix(Space.EMPTY))); } } From 8addad312f785d4a082cf8cbffe6da81ae45be19 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 12:16:17 +0200 Subject: [PATCH 17/28] wip --- .../EqualsAvoidsNullVisitor.java | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index 31ac237f5..a8e74a58f 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -39,12 +39,11 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { return m; } else if (!(m.getSelect() instanceof J.Literal) && m.getArguments().get(0) instanceof J.Literal - && EQUALS.matches(m) - || !Boolean.TRUE.equals(style.getIgnoreEqualsIgnoreCase()) - && EQUALS_IGNORE_CASE.matches(m) + && (EQUALS.matches(m) + || (!Boolean.TRUE.equals(style.getIgnoreEqualsIgnoreCase()) && EQUALS_IGNORE_CASE.matches(m)) || COMPARE_TO.matches(m) || COMPARE_TO_IGNORE_CASE.matches(m) - || CONTENT_EQUALS.matches(m)) { + || CONTENT_EQUALS.matches(m))) { return visitMethodInvocation(m); } return m; @@ -56,25 +55,21 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { J.Binary binary = (J.Binary) parent; if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof J.Binary) { J.Binary left = (J.Binary) binary.getLeft(); - if (isNullLiteral(left.getLeft()) - && matchesSelect(left.getRight(), - requireNonNull(m.getSelect())) - || isNullLiteral(left.getRight()) - && matchesSelect(left.getLeft(), requireNonNull(m.getSelect()))) { + if ((isNullLiteral(left.getLeft()) && matchesSelect(left.getRight(), requireNonNull(m.getSelect()))) + || (isNullLiteral(left.getRight()) && matchesSelect(left.getLeft(), + requireNonNull(m.getSelect())))) { doAfterVisit(new RemoveUnnecessaryNullCheck<>(binary)); } } - } - if (m.getArguments().get(0).getType() == JavaType.Primitive.Null) { + } else if (m.getArguments().get(0).getType() == JavaType.Primitive.Null) { return new J.Binary(Tree.randomId(), m.getPrefix(), Markers.EMPTY, requireNonNull(m.getSelect()), JLeftPadded.build(J.Binary.Type.Equal).withBefore(Space.SINGLE_SPACE), m.getArguments().get(0).withPrefix(Space.SINGLE_SPACE), JavaType.Primitive.Boolean); - } else { - return m.withSelect(m.getArguments().get(0).withPrefix(requireNonNull(m.getSelect()).getPrefix())) - .withArguments(singletonList(m.getSelect().withPrefix(Space.EMPTY))); } + return m.withSelect(m.getArguments().get(0).withPrefix(requireNonNull(m.getSelect()).getPrefix())) + .withArguments(singletonList(m.getSelect().withPrefix(Space.EMPTY))); } private boolean isNullLiteral(Expression expression) { From 21f37a4439c2f59eb02515b443e7b6eb1898b004 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 12:17:57 +0200 Subject: [PATCH 18/28] wip --- .../EqualsAvoidsNullVisitor.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index a8e74a58f..09e34e146 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -2,6 +2,7 @@ import lombok.EqualsAndHashCode; import lombok.Value; +import lombok.val; import org.jetbrains.annotations.NotNull; import org.openrewrite.Tree; import org.openrewrite.java.JavaVisitor; @@ -30,31 +31,32 @@ public class EqualsAvoidsNullVisitor

extends JavaVisitor

{ @Override public J visitMethodInvocation(J.MethodInvocation method, P p) { - J j = super.visitMethodInvocation(method, p); + val j = super.visitMethodInvocation(method, p); if (!(j instanceof J.MethodInvocation)) { return j; } - J.MethodInvocation m = (J.MethodInvocation) j; + val m = (J.MethodInvocation) j; if (m.getSelect() == null) { return m; } else if (!(m.getSelect() instanceof J.Literal) && m.getArguments().get(0) instanceof J.Literal - && (EQUALS.matches(m) - || (!Boolean.TRUE.equals(style.getIgnoreEqualsIgnoreCase()) && EQUALS_IGNORE_CASE.matches(m)) + && EQUALS.matches(m) + || !style.getIgnoreEqualsIgnoreCase() + && EQUALS_IGNORE_CASE.matches(m) || COMPARE_TO.matches(m) || COMPARE_TO_IGNORE_CASE.matches(m) - || CONTENT_EQUALS.matches(m))) { + || CONTENT_EQUALS.matches(m)) { return visitMethodInvocation(m); } return m; } private @NotNull Expression visitMethodInvocation(final J.MethodInvocation m) { - Tree parent = getCursor().getParentTreeCursor().getValue(); + val parent = getCursor().getParentTreeCursor().getValue(); if (parent instanceof J.Binary) { - J.Binary binary = (J.Binary) parent; + val binary = (J.Binary) parent; if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof J.Binary) { - J.Binary left = (J.Binary) binary.getLeft(); + val left = (J.Binary) binary.getLeft(); if ((isNullLiteral(left.getLeft()) && matchesSelect(left.getRight(), requireNonNull(m.getSelect()))) || (isNullLiteral(left.getRight()) && matchesSelect(left.getLeft(), requireNonNull(m.getSelect())))) { From e424b6f778e1993510c558242e18f5a77d156805 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 12:20:26 +0200 Subject: [PATCH 19/28] finalize --- .../EqualsAvoidsNullVisitor.java | 45 +++++++++---------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index 09e34e146..84731d0f7 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -31,34 +31,33 @@ public class EqualsAvoidsNullVisitor

extends JavaVisitor

{ @Override public J visitMethodInvocation(J.MethodInvocation method, P p) { - val j = super.visitMethodInvocation(method, p); - if (!(j instanceof J.MethodInvocation)) { - return j; - } - val m = (J.MethodInvocation) j; - if (m.getSelect() == null) { - return m; - } else if (!(m.getSelect() instanceof J.Literal) - && m.getArguments().get(0) instanceof J.Literal - && EQUALS.matches(m) - || !style.getIgnoreEqualsIgnoreCase() - && EQUALS_IGNORE_CASE.matches(m) - || COMPARE_TO.matches(m) - || COMPARE_TO_IGNORE_CASE.matches(m) - || CONTENT_EQUALS.matches(m)) { - return visitMethodInvocation(m); + val superVisitMethodInvocation = super.visitMethodInvocation(method, p); + if (superVisitMethodInvocation instanceof J.MethodInvocation methodInvocation) { + if (methodInvocation.getSelect() == null) { + return methodInvocation; + } else if (!(methodInvocation.getSelect() instanceof J.Literal) + && methodInvocation.getArguments().get(0) instanceof J.Literal + && EQUALS.matches(methodInvocation) + || !style.getIgnoreEqualsIgnoreCase() + && EQUALS_IGNORE_CASE.matches(methodInvocation) + || COMPARE_TO.matches(methodInvocation) + || COMPARE_TO_IGNORE_CASE.matches(methodInvocation) + || CONTENT_EQUALS.matches(methodInvocation)) { + return visitMethodInvocation(methodInvocation); + } + return methodInvocation; } - return m; + return superVisitMethodInvocation; } private @NotNull Expression visitMethodInvocation(final J.MethodInvocation m) { val parent = getCursor().getParentTreeCursor().getValue(); - if (parent instanceof J.Binary) { - val binary = (J.Binary) parent; - if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof J.Binary) { - val left = (J.Binary) binary.getLeft(); - if ((isNullLiteral(left.getLeft()) && matchesSelect(left.getRight(), requireNonNull(m.getSelect()))) - || (isNullLiteral(left.getRight()) && matchesSelect(left.getLeft(), + if (parent instanceof final J.Binary binary) { + if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof final J.Binary left) { + if (isNullLiteral(left.getLeft()) + && matchesSelect(left.getRight(), requireNonNull(m.getSelect())) + || (isNullLiteral(left.getRight()) + && matchesSelect(left.getLeft(), requireNonNull(m.getSelect())))) { doAfterVisit(new RemoveUnnecessaryNullCheck<>(binary)); } From 9470cd425d665d7f0a48901dd59c47562fea68b0 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 12:23:19 +0200 Subject: [PATCH 20/28] sort --- build.gradle.kts | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index c6bb64e08..d08d46628 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -9,31 +9,26 @@ description = "The first Static Analysis and REMEDIATION tool" val rewriteVersion = rewriteRecipe.rewriteVersion.get() dependencies { - compileOnly("org.projectlombok:lombok:latest.release") - annotationProcessor("org.projectlombok:lombok:latest.release") - testImplementation("org.projectlombok:lombok:latest.release") - - implementation(platform("org.openrewrite:rewrite-bom:${rewriteVersion}")) - implementation("org.openrewrite:rewrite-java") - implementation("org.openrewrite:rewrite-groovy:${rewriteVersion}") - implementation("org.openrewrite:rewrite-kotlin:${rewriteVersion}") - implementation("org.openrewrite:rewrite-csharp:${rewriteVersion}") - implementation("org.openrewrite.meta:rewrite-analysis:${rewriteVersion}") - implementation("org.apache.commons:commons-text:latest.release") - annotationProcessor("org.openrewrite:rewrite-templating:${rewriteVersion}") - implementation("org.openrewrite:rewrite-templating:${rewriteVersion}") + annotationProcessor("org.projectlombok:lombok:latest.release") + compileOnly("org.projectlombok:lombok:latest.release") compileOnly("com.google.errorprone:error_prone_core:2.+:with-dependencies") { exclude("com.google.auto.service", "auto-service-annotations") } - + implementation("org.apache.commons:commons-text:latest.release") + implementation("org.openrewrite.meta:rewrite-analysis:${rewriteVersion}") + implementation("org.openrewrite:rewrite-csharp:${rewriteVersion}") + implementation("org.openrewrite:rewrite-groovy:${rewriteVersion}") + implementation("org.openrewrite:rewrite-java") + implementation("org.openrewrite:rewrite-kotlin:${rewriteVersion}") + implementation("org.openrewrite:rewrite-templating:${rewriteVersion}") + implementation(platform("org.openrewrite:rewrite-bom:${rewriteVersion}")) + testImplementation("com.google.code.gson:gson:latest.release") + testImplementation("junit:junit:4.13.2") testImplementation("org.jetbrains:annotations:24.+") - testImplementation("org.openrewrite:rewrite-groovy") testImplementation("org.junit-pioneer:junit-pioneer:2.0.1") - testImplementation("junit:junit:4.13.2") - - testImplementation("com.google.code.gson:gson:latest.release") - - testRuntimeOnly("org.openrewrite:rewrite-java-17") + testImplementation("org.openrewrite:rewrite-groovy") + testImplementation("org.projectlombok:lombok:latest.release") testRuntimeOnly("com.google.code.findbugs:jsr305:latest.release") + testRuntimeOnly("org.openrewrite:rewrite-java-17") } From 35ed8cfc0ffe2813c4b5a436062f1dd6b85f0809 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 12:25:37 +0200 Subject: [PATCH 21/28] add doc --- .../staticanalysis/EqualsAvoidsNullVisitor.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index 84731d0f7..e525c9d74 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -1,3 +1,18 @@ +/* + * Copyright 2020 the original author or authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.openrewrite.staticanalysis; import lombok.EqualsAndHashCode; From a51947200b944b5820b054179ae583015ef5809b Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 12:37:59 +0200 Subject: [PATCH 22/28] undo --- build.gradle.kts | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index d08d46628..c6bb64e08 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -9,26 +9,31 @@ description = "The first Static Analysis and REMEDIATION tool" val rewriteVersion = rewriteRecipe.rewriteVersion.get() dependencies { - annotationProcessor("org.openrewrite:rewrite-templating:${rewriteVersion}") - annotationProcessor("org.projectlombok:lombok:latest.release") compileOnly("org.projectlombok:lombok:latest.release") - compileOnly("com.google.errorprone:error_prone_core:2.+:with-dependencies") { - exclude("com.google.auto.service", "auto-service-annotations") - } - implementation("org.apache.commons:commons-text:latest.release") - implementation("org.openrewrite.meta:rewrite-analysis:${rewriteVersion}") - implementation("org.openrewrite:rewrite-csharp:${rewriteVersion}") - implementation("org.openrewrite:rewrite-groovy:${rewriteVersion}") + annotationProcessor("org.projectlombok:lombok:latest.release") + testImplementation("org.projectlombok:lombok:latest.release") + + implementation(platform("org.openrewrite:rewrite-bom:${rewriteVersion}")) implementation("org.openrewrite:rewrite-java") + implementation("org.openrewrite:rewrite-groovy:${rewriteVersion}") implementation("org.openrewrite:rewrite-kotlin:${rewriteVersion}") + implementation("org.openrewrite:rewrite-csharp:${rewriteVersion}") + implementation("org.openrewrite.meta:rewrite-analysis:${rewriteVersion}") + implementation("org.apache.commons:commons-text:latest.release") + + annotationProcessor("org.openrewrite:rewrite-templating:${rewriteVersion}") implementation("org.openrewrite:rewrite-templating:${rewriteVersion}") - implementation(platform("org.openrewrite:rewrite-bom:${rewriteVersion}")) - testImplementation("com.google.code.gson:gson:latest.release") - testImplementation("junit:junit:4.13.2") + compileOnly("com.google.errorprone:error_prone_core:2.+:with-dependencies") { + exclude("com.google.auto.service", "auto-service-annotations") + } + testImplementation("org.jetbrains:annotations:24.+") - testImplementation("org.junit-pioneer:junit-pioneer:2.0.1") testImplementation("org.openrewrite:rewrite-groovy") - testImplementation("org.projectlombok:lombok:latest.release") - testRuntimeOnly("com.google.code.findbugs:jsr305:latest.release") + testImplementation("org.junit-pioneer:junit-pioneer:2.0.1") + testImplementation("junit:junit:4.13.2") + + testImplementation("com.google.code.gson:gson:latest.release") + testRuntimeOnly("org.openrewrite:rewrite-java-17") + testRuntimeOnly("com.google.code.findbugs:jsr305:latest.release") } From 2f0e9b8aae30a442b8a48f16bc92398c918b3e87 Mon Sep 17 00:00:00 2001 From: punkratz312 Date: Mon, 14 Oct 2024 12:46:14 +0200 Subject: [PATCH 23/28] Update src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java Co-authored-by: Tim te Beek --- .../EqualsAvoidsNullVisitorTest.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java b/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java index 2f0c30bad..2f94ca709 100644 --- a/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java @@ -37,16 +37,16 @@ void invertConditional() { //language=java java( """ - public class A { - { - String s = null; - if(s.equals("test")) {} - if(s.equalsIgnoreCase("test")) {} - System.out.println(s.compareTo("test")); - System.out.println(s.compareToIgnoreCase("test")); - System.out.println(s.contentEquals("test")); - } - } + public class A { + { + String s = null; + if(s.equals("test")) {} + if(s.equalsIgnoreCase("test")) {} + System.out.println(s.compareTo("test")); + System.out.println(s.compareToIgnoreCase("test")); + System.out.println(s.contentEquals("test")); + } + } """, """ public class A { From 2c2b33a08b806069902e1ec122fccde8865c5730 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 13:08:59 +0200 Subject: [PATCH 24/28] fix Cast after // instanceof check --- .../staticanalysis/EqualsAvoidsNullVisitor.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index e525c9d74..ee12844ec 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -47,7 +47,8 @@ public class EqualsAvoidsNullVisitor

extends JavaVisitor

{ @Override public J visitMethodInvocation(J.MethodInvocation method, P p) { val superVisitMethodInvocation = super.visitMethodInvocation(method, p); - if (superVisitMethodInvocation instanceof J.MethodInvocation methodInvocation) { + if (superVisitMethodInvocation instanceof J.MethodInvocation) { + J.MethodInvocation methodInvocation = (J.MethodInvocation) superVisitMethodInvocation; if (methodInvocation.getSelect() == null) { return methodInvocation; } else if (!(methodInvocation.getSelect() instanceof J.Literal) @@ -67,8 +68,10 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { private @NotNull Expression visitMethodInvocation(final J.MethodInvocation m) { val parent = getCursor().getParentTreeCursor().getValue(); - if (parent instanceof final J.Binary binary) { - if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof final J.Binary left) { + if (parent instanceof J.Binary) { + J.Binary binary = (J.Binary) parent; + if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof J.Binary) { + J.Binary left = (J.Binary) binary.getLeft(); if (isNullLiteral(left.getLeft()) && matchesSelect(left.getRight(), requireNonNull(m.getSelect())) || (isNullLiteral(left.getRight()) From 241262fe877671e53ab6fe09473e5b5a2449e3e4 Mon Sep 17 00:00:00 2001 From: punkratz312 Date: Mon, 14 Oct 2024 13:10:51 +0200 Subject: [PATCH 25/28] Update src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java Co-authored-by: Tim te Beek --- .../openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java b/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java index 2f94ca709..620e296d6 100644 --- a/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitorTest.java @@ -23,7 +23,7 @@ import static org.openrewrite.java.Assertions.java; @SuppressWarnings({"ClassInitializerMayBeStatic", "StatementWithEmptyBody", "ConstantConditions"}) -class EqualsAvoidsNullVisitorTest implements RewriteTest { +class EqualsAvoidsNullTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { From 4d78d78c29715486a01d73daf2568f9b4572f82e Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 13:16:02 +0200 Subject: [PATCH 26/28] Revert "fix Cast after" This reverts commit 2c2b33a08b806069902e1ec122fccde8865c5730. --- .../staticanalysis/EqualsAvoidsNullVisitor.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java index ee12844ec..e525c9d74 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/EqualsAvoidsNullVisitor.java @@ -47,8 +47,7 @@ public class EqualsAvoidsNullVisitor

extends JavaVisitor

{ @Override public J visitMethodInvocation(J.MethodInvocation method, P p) { val superVisitMethodInvocation = super.visitMethodInvocation(method, p); - if (superVisitMethodInvocation instanceof J.MethodInvocation) { - J.MethodInvocation methodInvocation = (J.MethodInvocation) superVisitMethodInvocation; + if (superVisitMethodInvocation instanceof J.MethodInvocation methodInvocation) { if (methodInvocation.getSelect() == null) { return methodInvocation; } else if (!(methodInvocation.getSelect() instanceof J.Literal) @@ -68,10 +67,8 @@ public J visitMethodInvocation(J.MethodInvocation method, P p) { private @NotNull Expression visitMethodInvocation(final J.MethodInvocation m) { val parent = getCursor().getParentTreeCursor().getValue(); - if (parent instanceof J.Binary) { - J.Binary binary = (J.Binary) parent; - if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof J.Binary) { - J.Binary left = (J.Binary) binary.getLeft(); + if (parent instanceof final J.Binary binary) { + if (binary.getOperator() == J.Binary.Type.And && binary.getLeft() instanceof final J.Binary left) { if (isNullLiteral(left.getLeft()) && matchesSelect(left.getRight(), requireNonNull(m.getSelect())) || (isNullLiteral(left.getRight()) From ca9d3d7877968e0d2bb1e12ab1f92281f9014f31 Mon Sep 17 00:00:00 2001 From: I753089 Date: Mon, 14 Oct 2024 13:24:18 +0200 Subject: [PATCH 27/28] wip: error: pattern matching in instanceof is not supported in -source 8 --- build.gradle.kts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index c6bb64e08..99a70f28e 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -3,7 +3,13 @@ plugins { id("org.openrewrite.build.recipe-library") version "latest.release" } - +java { + toolchain { + languageVersion = JavaLanguageVersion.of(17) + } + sourceCompatibility = JavaVersion.VERSION_1_7 + targetCompatibility = JavaVersion.VERSION_1_7 +} group = "org.openrewrite.recipe" description = "The first Static Analysis and REMEDIATION tool" @@ -14,7 +20,7 @@ dependencies { testImplementation("org.projectlombok:lombok:latest.release") implementation(platform("org.openrewrite:rewrite-bom:${rewriteVersion}")) - implementation("org.openrewrite:rewrite-java") + implementation("org.openrewrite:rewrite-java:${rewriteVersion}") implementation("org.openrewrite:rewrite-groovy:${rewriteVersion}") implementation("org.openrewrite:rewrite-kotlin:${rewriteVersion}") implementation("org.openrewrite:rewrite-csharp:${rewriteVersion}") From bec2a8e32e0dab16b3cfb1997fa1474524df96e9 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 14 Oct 2024 13:56:12 +0200 Subject: [PATCH 28/28] Revert build.gradle changes; rewrite-bom manages rewrite-java --- build.gradle.kts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 99a70f28e..c6bb64e08 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -3,13 +3,7 @@ plugins { id("org.openrewrite.build.recipe-library") version "latest.release" } -java { - toolchain { - languageVersion = JavaLanguageVersion.of(17) - } - sourceCompatibility = JavaVersion.VERSION_1_7 - targetCompatibility = JavaVersion.VERSION_1_7 -} + group = "org.openrewrite.recipe" description = "The first Static Analysis and REMEDIATION tool" @@ -20,7 +14,7 @@ dependencies { testImplementation("org.projectlombok:lombok:latest.release") implementation(platform("org.openrewrite:rewrite-bom:${rewriteVersion}")) - implementation("org.openrewrite:rewrite-java:${rewriteVersion}") + implementation("org.openrewrite:rewrite-java") implementation("org.openrewrite:rewrite-groovy:${rewriteVersion}") implementation("org.openrewrite:rewrite-kotlin:${rewriteVersion}") implementation("org.openrewrite:rewrite-csharp:${rewriteVersion}")