Skip to content

Commit

Permalink
Revert "Update EqualsAvoidsNullVisitor.java"
Browse files Browse the repository at this point in the history
This reverts commit 4d0fece.
  • Loading branch information
Vincent Potucek committed Dec 27, 2024
1 parent 4d0fece commit c8225b7
Showing 1 changed file with 34 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/
package org.openrewrite.staticanalysis;

import lombok.EqualsAndHashCode;
import lombok.Value;
import org.jspecify.annotations.Nullable;
Expand All @@ -23,8 +24,10 @@
import org.openrewrite.java.style.EqualsAvoidsNullStyle;
import org.openrewrite.java.tree.*;
import org.openrewrite.marker.Markers;

import static java.util.Collections.singletonList;
import static java.util.Objects.requireNonNull;

/**
* A visitor that identifies and addresses potential issues related to
* the use of {@code equals} methods in Java, particularly to avoid
Expand All @@ -44,46 +47,35 @@
public class EqualsAvoidsNullVisitor<P> extends JavaVisitor<P> {

private static final String JAVA_LANG_STRING = "java.lang.String ";
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(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)");
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(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;

@Override
public J visitMethodInvocation(J.MethodInvocation method, P p) {
J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, p);
if (m.getSelect() != null && !(m.getSelect() instanceof J.Literal) &&
isStringComparisonMethod(m) && hasCompatibleArgument(m)) {
isStringComparisonMethod(m) && hasCompatibleArgument(m)) {

maybeHandleParentBinary(m);








Expand Down





Expand Up

@@ -91,20 +96,22 @@ private boolean hasCompatibleArgument(J.MethodInvocation m) {

Expression firstArgument = m.getArguments().get(0);
return firstArgument.getType() == JavaType.Primitive.Null ?
literalsFirstInComparisonsNull(m, firstArgument) :
literalsFirstInComparisons(m, firstArgument);
}
return m;
}

private boolean hasCompatibleArgument(J.MethodInvocation m) {
if (m.getArguments().isEmpty()) {
return false;
Expand All @@ -104,43 +96,37 @@ private boolean hasCompatibleArgument(J.MethodInvocation m) {

private boolean isStringComparisonMethod(J.MethodInvocation methodInvocation) {
return EQUALS.matches(methodInvocation) ||
!style.getIgnoreEqualsIgnoreCase() &&
EQUALS_IGNORE_CASE.matches(methodInvocation) ||
COMPARE_TO.matches(methodInvocation) ||
COMPARE_TO_IGNORE_CASE.matches(methodInvocation) ||
CONTENT_EQUALS.matches(methodInvocation);
!style.getIgnoreEqualsIgnoreCase() &&
EQUALS_IGNORE_CASE.matches(methodInvocation) ||
COMPARE_TO.matches(methodInvocation) ||
COMPARE_TO_IGNORE_CASE.matches(methodInvocation) ||
CONTENT_EQUALS.matches(methodInvocation);
}

private void maybeHandleParentBinary(J.MethodInvocation m) {
P parent = getCursor().getParentTreeCursor().getValue();
if (parent instanceof J.Binary) {
if (((J.Binary) parent).getOperator() == J.Binary.Type.And && ((J.Binary) parent).getLeft() instanceof J.Binary) {
J.Binary potentialNullCheck = (J.Binary) ((J.Binary) parent).getLeft();
if (isNullLiteral(potentialNullCheck.getLeft()) && matchesSelect(potentialNullCheck.getRight(), requireNonNull(m.getSelect())) ||
isNullLiteral(potentialNullCheck.getRight()) && matchesSelect(potentialNullCheck.getLeft(), requireNonNull(m.getSelect()))) {
if (isNullLiteral(potentialNullCheck.getLeft()) && matchesSelect(potentialNullCheck.getRight(),
requireNonNull(m.getSelect())) ||
isNullLiteral(potentialNullCheck.getRight()) && matchesSelect(potentialNullCheck.getLeft(),
requireNonNull(m.getSelect()))) {
doAfterVisit(new RemoveUnnecessaryNullCheck<>((J.Binary) parent));
}
}







Expand Down



}
}

private boolean isNullLiteral(Expression expression) {
return expression instanceof J.Literal && ((J.Literal) expression).getType() == JavaType.Primitive.Null;
}

private boolean matchesSelect(Expression expression, Expression select) {
return expression.printTrimmed(getCursor()).replaceAll("\\s", "")
.equals(select.printTrimmed(getCursor()).replaceAll("\\s", ""));
}

private static J.Binary literalsFirstInComparisonsNull(J.MethodInvocation m, Expression firstArgument) {
return new J.Binary(Tree.randomId(),
m.getPrefix(),
Expand All @@ -150,23 +136,30 @@ private static J.Binary literalsFirstInComparisonsNull(J.MethodInvocation m, Exp
firstArgument.withPrefix(Space.SINGLE_SPACE),
JavaType.Primitive.Boolean);
}

private static J.MethodInvocation literalsFirstInComparisons(J.MethodInvocation m, Expression firstArgument) {
return m.withSelect(firstArgument.withPrefix(requireNonNull(m.getSelect()).getPrefix()))
.withArguments(singletonList(m.getSelect().withPrefix(Space.EMPTY)));
}

private static class RemoveUnnecessaryNullCheck<P> extends JavaVisitor<P> {

private final J.Binary scope;

boolean done;

public RemoveUnnecessaryNullCheck(J.Binary scope) {
this.scope = scope;
}

@Override
public @Nullable J visit(@Nullable Tree tree, P p) {
if (done) {
return (J) tree;
}
return super.visit(tree, p);
}

@Override
public J visitBinary(J.Binary binary, P p) {
if (scope.isScope(binary)) {
Expand Down

0 comments on commit c8225b7

Please sign in to comment.