Skip to content

Commit

Permalink
Add CharSequence to UndefinedEquals, and provide a suggested fix for it.
Browse files Browse the repository at this point in the history 
RELNOTES: Add CharSequence to UndefinedEquals.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=208980767
  • Loading branch information
@graememorgan @cushon
graememorgan authored and cushon committed Aug 17, 2018
1 parent d57070d commit 000aaaf
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 34 deletions.
105 changes: 79 additions & 26 deletions core/src/main/java/com/google/errorprone/bugpatterns/UndefinedEquals.java
View file
Expand Up @@ -29,40 +29,37 @@
import static com.google.errorprone.matchers.Matchers.staticMethod; import static com.google.errorprone.matchers.Matchers.staticMethod;
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod; import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
import static com.google.errorprone.util.ASTHelpers.getReceiver; import static com.google.errorprone.util.ASTHelpers.getReceiver;
import static com.google.errorprone.util.ASTHelpers.getReceiverType;
import static com.google.errorprone.util.ASTHelpers.getType; import static com.google.errorprone.util.ASTHelpers.getType;
import static com.google.errorprone.util.ASTHelpers.isSameType; import static com.google.errorprone.util.ASTHelpers.isSameType;


import com.google.common.collect.ImmutableList;
import com.google.errorprone.BugPattern; import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.ProvidesFix;
import com.google.errorprone.VisitorState; import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher; import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Type;
import java.util.Arrays;
import java.util.List; import java.util.List;
import java.util.Optional;
import java.util.regex.Pattern; import java.util.regex.Pattern;


/** /**
* Collection, Iterable, Multimap, and Queue do not have well-defined equals behavior. * Flags types which do not have well-defined equals behavior.
* *
* @author eleanorh@google.com (Eleanor Harris) * @author eleanorh@google.com (Eleanor Harris)
*/ */
@BugPattern( @BugPattern(
name = "UndefinedEquals", name = "UndefinedEquals",
summary = "Collection, Iterable, Multimap, and Queue do not have well-defined equals behavior", summary = "Collection, Iterable, Multimap, and Queue do not have well-defined equals behavior",
severity = WARNING) severity = WARNING,
providesFix = ProvidesFix.REQUIRES_HUMAN_ATTENTION)
public final class UndefinedEquals extends BugChecker implements MethodInvocationTreeMatcher { public final class UndefinedEquals extends BugChecker implements MethodInvocationTreeMatcher {


private static final ImmutableList<String> BAD_CLASS_LIST =
ImmutableList.of(
"com.google.common.collect.Multimap",
"java.lang.Iterable",
"java.util.Collection",
"java.util.Queue");

private static final Matcher<MethodInvocationTree> ASSERT_THAT_EQUALS = private static final Matcher<MethodInvocationTree> ASSERT_THAT_EQUALS =
allOf( allOf(
instanceMethod() instanceMethod()
Expand All @@ -77,38 +74,94 @@ public final class UndefinedEquals extends BugChecker implements MethodInvocatio


@Override @Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
Type receiverType; Tree receiver;
Type argumentType; Tree argument;
List<? extends ExpressionTree> arguments = tree.getArguments(); List<? extends ExpressionTree> arguments = tree.getArguments();


if (staticEqualsInvocation().matches(tree, state) if (staticEqualsInvocation().matches(tree, state)
|| assertEqualsInvocation().matches(tree, state) || assertEqualsInvocation().matches(tree, state)
|| assertNotEqualsInvocation().matches(tree, state)) { || assertNotEqualsInvocation().matches(tree, state)) {
receiverType = getType(arguments.get(arguments.size() - 2)); receiver = arguments.get(arguments.size() - 2);
argumentType = getType(getLast(arguments)); argument = getLast(arguments);
} else if (instanceEqualsInvocation().matches(tree, state)) { } else if (instanceEqualsInvocation().matches(tree, state)) {
receiverType = getReceiverType(tree); receiver = getReceiver(tree);
argumentType = getType(arguments.get(0)); argument = arguments.get(0);
} else if (ASSERT_THAT_EQUALS.matches(tree, state)) { } else if (ASSERT_THAT_EQUALS.matches(tree, state)) {
receiverType = getType(getOnlyElement(arguments)); receiver = getOnlyElement(arguments);
argumentType = argument = getOnlyElement(((MethodInvocationTree) getReceiver(tree)).getArguments());
getType(getOnlyElement(((MethodInvocationTree) getReceiver(tree)).getArguments()));
} else { } else {
return Description.NO_MATCH; return Description.NO_MATCH;
} }


return BAD_CLASS_LIST return Arrays.stream(BadClass.values())
.stream()
.filter( .filter(
t -> b ->
isSameType(receiverType, state.getTypeFromString(t), state) isSameType(getType(receiver), b.type(state), state)
|| isSameType(argumentType, state.getTypeFromString(t), state)) || isSameType(getType(argument), b.type(state), state))
.findFirst() .findFirst()
.map( .map(
b -> b ->
buildDescription(tree) buildDescription(tree)
.setMessage(b + " does not have well-defined equals behavior") .setMessage(b.typeName() + " does not have well-defined equals behavior")
.addFix(b.generateFix(receiver, argument, state))
.build()) .build())
.orElse(Description.NO_MATCH); .orElse(Description.NO_MATCH);
} }

private enum BadClass {
MULTIMAP("com.google.common.collect.Multimap") {
@Override
Optional<SuggestedFix> generateFix(Tree receiver, Tree argument, VisitorState state) {
return Optional.empty();
}
},
CHARSEQUENCE("java.lang.CharSequence") {
@Override
Optional<SuggestedFix> generateFix(Tree receiver, Tree argument, VisitorState state) {
if (isSameType(getType(receiver), CHARSEQUENCE.type(state), state)
&& isSameType(getType(argument), state.getSymtab().stringType, state)) {
return Optional.of(SuggestedFix.postfixWith(receiver, ".toString()"));
}
if (isSameType(getType(argument), CHARSEQUENCE.type(state), state)
&& isSameType(getType(receiver), state.getSymtab().stringType, state)) {
return Optional.of(SuggestedFix.postfixWith(argument, ".toString()"));
}
return Optional.empty();
}
},
ITERABLE("java.lang.Iterable") {
@Override
Optional<SuggestedFix> generateFix(Tree receiver, Tree argument, VisitorState state) {
return Optional.empty();
}
},
COLLECTION("java.util.Collection") {
@Override
Optional<SuggestedFix> generateFix(Tree receiver, Tree argument, VisitorState state) {
return Optional.empty();
}
},
QUEUE("java.util.Queue") {
@Override
Optional<SuggestedFix> generateFix(Tree receiver, Tree argument, VisitorState state) {
return Optional.empty();
}
};

private final String typeName;

BadClass(String typeName) {
this.typeName = typeName;
}

abstract Optional<SuggestedFix> generateFix(Tree receiver, Tree argument, VisitorState state);

private Type type(VisitorState state) {
return state.getTypeFromString(typeName);
}

private String typeName() {
return typeName;
}
}
} }
35 changes: 27 additions & 8 deletions core/src/test/java/com/google/errorprone/bugpatterns/UndefinedEqualsTest.java
View file
Expand Up @@ -16,8 +16,8 @@


package com.google.errorprone.bugpatterns; package com.google.errorprone.bugpatterns;


import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.CompilationTestHelper; import com.google.errorprone.CompilationTestHelper;
import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.junit.runners.JUnit4; import org.junit.runners.JUnit4;
Expand All @@ -29,13 +29,8 @@
*/ */
@RunWith(JUnit4.class) @RunWith(JUnit4.class)
public final class UndefinedEqualsTest { public final class UndefinedEqualsTest {

private final CompilationTestHelper compilationHelper =
private CompilationTestHelper compilationHelper; CompilationTestHelper.newInstance(UndefinedEquals.class, getClass());

@Before
public void setUp() {
compilationHelper = CompilationTestHelper.newInstance(UndefinedEquals.class, getClass());
}


@Test @Test
public void positiveInstanceEquals() { public void positiveInstanceEquals() {
Expand Down Expand Up @@ -136,4 +131,28 @@ public void negative() {
"}") "}")
.doTest(); .doTest();
} }

@Test
public void charSequenceFix() throws Exception {
BugCheckerRefactoringTestHelper.newInstance(new UndefinedEquals(), getClass())
.addInputLines(
"Test.java",
"import static com.google.common.truth.Truth.assertThat;",
"class Test {",
" void f(CharSequence a, String b) {",
" assertThat(a).isEqualTo(b);",
" assertThat(b).isEqualTo(a);",
" }",
"}")
.addOutputLines(
"Test.java",
"import static com.google.common.truth.Truth.assertThat;",
"class Test {",
" void f(CharSequence a, String b) {",
" assertThat(a.toString()).isEqualTo(b);",
" assertThat(b).isEqualTo(a.toString());",
" }",
"}")
.doTest();
}
} }

0 comments on commit 000aaaf

Please sign in to comment.