diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java index 00d6d9bdcb..c99b496d32 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java @@ -32,7 +32,7 @@ /** A {@link BugChecker} that flags redundant identity conversions. */ // XXX: Consider detecting cases where a flagged expression is passed to a method, and where removal -// of the identify conversion would cause a different method overload to be selected. Depending on +// of the identity conversion would cause a different method overload to be selected. Depending on // the target method such a modification may change the code's semantics or performance. @AutoService(BugChecker.class) @BugPattern( @@ -45,6 +45,13 @@ public final class IdentityConversion extends BugChecker implements MethodInvoca private static final long serialVersionUID = 1L; private static final Matcher IS_CONVERSION_METHOD = anyOf( + staticMethod() + .onClassAny( + Primitives.allWrapperTypes().stream() + .map(Class::getName) + .collect(toImmutableSet())) + .named("valueOf"), + staticMethod().onClass(String.class.getName()).named("valueOf"), staticMethod() .onClassAny( "com.google.common.collect.ImmutableBiMap", @@ -60,12 +67,8 @@ public final class IdentityConversion extends BugChecker implements MethodInvoca "com.google.common.collect.ImmutableTable") .named("copyOf"), staticMethod() - .onClassAny( - Primitives.allWrapperTypes().stream() - .map(Class::getName) - .collect(toImmutableSet())) - .named("valueOf"), - staticMethod().onClass(String.class.getName()).named("valueOf"), + .onClass("com.google.errorprone.matchers.Matchers") + .namedAnyOf("allOf", "anyOf"), staticMethod().onClass("reactor.adapter.rxjava.RxJava2Adapter"), staticMethod() .onClass("reactor.core.publisher.Flux") diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IdentityConversionTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IdentityConversionTest.java index a8f867502a..baa74babdc 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IdentityConversionTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IdentityConversionTest.java @@ -16,7 +16,10 @@ final class IdentityConversionTest { void identification() { compilationTestHelper .addSourceLines( - "Foo.java", + "A.java", + "import static com.google.errorprone.matchers.Matchers.instanceMethod;", + "import static com.google.errorprone.matchers.Matchers.staticMethod;", + "", "import com.google.common.collect.ImmutableBiMap;", "import com.google.common.collect.ImmutableList;", "import com.google.common.collect.ImmutableListMultimap;", @@ -28,12 +31,14 @@ void identification() { "import com.google.common.collect.ImmutableSet;", "import com.google.common.collect.ImmutableSetMultimap;", "import com.google.common.collect.ImmutableTable;", + "import com.google.errorprone.matchers.Matcher;", + "import com.google.errorprone.matchers.Matchers;", "import reactor.adapter.rxjava.RxJava2Adapter;", "import reactor.core.publisher.Flux;", "import reactor.core.publisher.Mono;", "", - "public final class Foo {", - " public void foo() {", + "public final class A {", + " public void m() {", " // BUG: Diagnostic contains:", " Boolean b1 = Boolean.valueOf(Boolean.FALSE);", " // BUG: Diagnostic contains:", @@ -143,7 +148,15 @@ void identification() { " ImmutableTable o11 = ImmutableTable.copyOf(ImmutableTable.of());", "", " // BUG: Diagnostic contains:", + " Matcher allOf1 = Matchers.allOf(instanceMethod());", + " Matcher allOf2 = Matchers.allOf(instanceMethod(), staticMethod());", + " // BUG: Diagnostic contains:", + " Matcher anyOf1 = Matchers.anyOf(staticMethod());", + " Matcher anyOf2 = Matchers.anyOf(instanceMethod(), staticMethod());", + "", + " // BUG: Diagnostic contains:", " Flux flux1 = Flux.just(1).flatMap(e -> RxJava2Adapter.fluxToFlowable(Flux.just(2)));", + "", " // BUG: Diagnostic contains:", " Flux flux2 = Flux.concat(Flux.just(1));", " // BUG: Diagnostic contains:", @@ -154,9 +167,9 @@ void identification() { " Flux flux5 = Flux.merge(Flux.just(1));", "", " // BUG: Diagnostic contains:", - " Mono m1 = Mono.from(Mono.just(1));", + " Mono mono1 = Mono.from(Mono.just(1));", " // BUG: Diagnostic contains:", - " Mono m2 = Mono.fromDirect(Mono.just(1));", + " Mono mono2 = Mono.fromDirect(Mono.just(1));", " }", "}") .doTest(); @@ -167,12 +180,15 @@ void replacementFirstSuggestedFix() { refactoringTestHelper .setFixChooser(FixChoosers.FIRST) .addInputLines( - "Foo.java", + "A.java", + "import static com.google.errorprone.matchers.Matchers.staticMethod;", "import static org.mockito.Mockito.when;", "", "import com.google.common.collect.ImmutableCollection;", "import com.google.common.collect.ImmutableList;", "import com.google.common.collect.ImmutableSet;", + "import com.google.errorprone.matchers.Matcher;", + "import com.google.errorprone.matchers.Matchers;", "import java.util.ArrayList;", "import java.util.Collection;", "import org.reactivestreams.Publisher;", @@ -180,8 +196,8 @@ void replacementFirstSuggestedFix() { "import reactor.core.publisher.Flux;", "import reactor.core.publisher.Mono;", "", - "public final class Foo {", - " public void foo() {", + "public final class A {", + " public void m() {", " ImmutableSet set1 = ImmutableSet.copyOf(ImmutableSet.of());", " ImmutableSet set2 = ImmutableSet.copyOf(ImmutableList.of());", "", @@ -206,18 +222,23 @@ void replacementFirstSuggestedFix() { " Object o1 = ImmutableSet.copyOf(ImmutableList.of());", " Object o2 = ImmutableSet.copyOf(ImmutableSet.of());", "", + " Matcher matcher = Matchers.allOf(staticMethod());", + "", " when(\"foo\".contains(\"f\")).thenAnswer(inv -> ImmutableSet.copyOf(ImmutableList.of(1)));", " }", "", " void bar(Publisher publisher) {}", "}") .addOutputLines( - "Foo.java", + "A.java", + "import static com.google.errorprone.matchers.Matchers.staticMethod;", "import static org.mockito.Mockito.when;", "", "import com.google.common.collect.ImmutableCollection;", "import com.google.common.collect.ImmutableList;", "import com.google.common.collect.ImmutableSet;", + "import com.google.errorprone.matchers.Matcher;", + "import com.google.errorprone.matchers.Matchers;", "import java.util.ArrayList;", "import java.util.Collection;", "import org.reactivestreams.Publisher;", @@ -225,8 +246,8 @@ void replacementFirstSuggestedFix() { "import reactor.core.publisher.Flux;", "import reactor.core.publisher.Mono;", "", - "public final class Foo {", - " public void foo() {", + "public final class A {", + " public void m() {", " ImmutableSet set1 = ImmutableSet.of();", " ImmutableSet set2 = ImmutableSet.copyOf(ImmutableList.of());", "", @@ -251,6 +272,8 @@ void replacementFirstSuggestedFix() { " Object o1 = ImmutableSet.copyOf(ImmutableList.of());", " Object o2 = ImmutableSet.of();", "", + " Matcher matcher = staticMethod();", + "", " when(\"foo\".contains(\"f\")).thenAnswer(inv -> ImmutableSet.copyOf(ImmutableList.of(1)));", " }", "", @@ -264,14 +287,14 @@ void replacementSecondSuggestedFix() { refactoringTestHelper .setFixChooser(FixChoosers.SECOND) .addInputLines( - "Foo.java", + "A.java", "import com.google.common.collect.ImmutableCollection;", "import com.google.common.collect.ImmutableList;", "import com.google.common.collect.ImmutableSet;", "import java.util.ArrayList;", "", - "public final class Foo {", - " public void foo() {", + "public final class A {", + " public void m() {", " ImmutableSet set1 = ImmutableSet.copyOf(ImmutableSet.of());", " ImmutableSet set2 = ImmutableSet.copyOf(ImmutableList.of());", "", @@ -280,14 +303,14 @@ void replacementSecondSuggestedFix() { " }", "}") .addOutputLines( - "Foo.java", + "A.java", "import com.google.common.collect.ImmutableCollection;", "import com.google.common.collect.ImmutableList;", "import com.google.common.collect.ImmutableSet;", "import java.util.ArrayList;", "", - "public final class Foo {", - " public void foo() {", + "public final class A {", + " public void m() {", " @SuppressWarnings(\"IdentityConversion\")", " ImmutableSet set1 = ImmutableSet.copyOf(ImmutableSet.of());", " ImmutableSet set2 = ImmutableSet.copyOf(ImmutableList.of());",