diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ExplicitArgumentEnumeration.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ExplicitArgumentEnumeration.java index 625f52bdf4..7770efe93c 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ExplicitArgumentEnumeration.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ExplicitArgumentEnumeration.java @@ -5,9 +5,11 @@ import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; import static com.google.errorprone.BugPattern.StandardTags.PERFORMANCE; import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION; +import static com.google.errorprone.matchers.Matchers.allOf; import static com.google.errorprone.matchers.Matchers.anyOf; import static com.google.errorprone.matchers.Matchers.instanceMethod; import static com.google.errorprone.matchers.Matchers.staticMethod; +import static com.google.errorprone.matchers.Matchers.symbolMatcher; import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL; import com.google.auto.service.AutoService; @@ -28,11 +30,13 @@ import com.google.errorprone.util.Visibility; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.MethodTree; import com.sun.tools.javac.code.Symbol.MethodSymbol; import com.sun.tools.javac.code.Symbol.VarSymbol; import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import javax.lang.model.element.Element; @@ -74,6 +78,19 @@ public final class ExplicitArgumentEnumeration extends BugChecker List.class.getCanonicalName(), Set.class.getCanonicalName()) .named("of"), + allOf( + staticMethod() + .onClassAny( + ImmutableList.class.getCanonicalName(), + ImmutableMultiset.class.getCanonicalName(), + ImmutableSet.class.getCanonicalName()) + .named("copyOf"), + symbolMatcher( + (symbol, state) -> + state + .getSymtab() + .arrayClass + .equals(((MethodSymbol) symbol).params().get(0).type.tsym))), staticMethod().onClass(Arrays.class.getCanonicalName()).named("asList")); private static final Matcher IMMUTABLE_COLLECTION_BUILDER = instanceMethod().onDescendantOf(ImmutableCollection.Builder.class.getCanonicalName()); @@ -109,7 +126,12 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState } MethodSymbol method = ASTHelpers.getSymbol(tree); - if (!isUnaryIterableAcceptingMethod(method, state)) { + if (!isUnaryIterableAcceptingMethod(method, state) || isLocalOverload(method, state)) { + /* + * This isn't a method invocation we can simplify, or it's an invocation of a local overload. + * The latter type of invocation we do not suggest replacing, as this is fairly likely to + * introduce an unbounded recursive call chain. + */ return Description.NO_MATCH; } @@ -131,6 +153,17 @@ private static boolean isUnaryIterableAcceptingMethod(MethodSymbol method, Visit && ASTHelpers.isSubtype(params.get(0).type, state.getSymtab().iterableType, state); } + private static boolean isLocalOverload(MethodSymbol calledMethod, VisitorState state) { + MethodTree enclosingMethod = state.findEnclosing(MethodTree.class); + if (enclosingMethod == null) { + return false; + } + + MethodSymbol callingMethod = ASTHelpers.getSymbol(enclosingMethod); + return Objects.equals(callingMethod.getEnclosingElement(), calledMethod.getEnclosingElement()) + && callingMethod.getSimpleName().equals(calledMethod.getSimpleName()); + } + private static Optional trySuggestCallingVarargsOverload( MethodSymbol method, MethodInvocationTree argument, VisitorState state) { /* diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ExplicitArgumentEnumerationTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ExplicitArgumentEnumerationTest.java index 3366248f70..1ac3e047eb 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ExplicitArgumentEnumerationTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ExplicitArgumentEnumerationTest.java @@ -21,6 +21,9 @@ void identification() { "import reactor.test.StepVerifier;", "", "class A {", + " // BUG: Diagnostic contains:", + " private final int value = unaryMethod(ImmutableList.of(1, 2));", + "", " void m() {", " ImmutableList list = ImmutableList.of();", " assertThat(ImmutableList.of()).containsAnyElementsOf(list);", @@ -37,6 +40,9 @@ void identification() { " ImmutableList.builder()", " // BUG: Diagnostic contains:", " .addAll(ImmutableList.of())", + " // BUG: Diagnostic contains:", + " .addAll(ImmutableList.copyOf(new String[0]))", + " .addAll(ImmutableList.copyOf(ImmutableList.of()))", " .build();", "", " assertThat(ImmutableList.of(1))", @@ -57,17 +63,25 @@ void identification() { " .withClasspath();", " }", "", - " private void unaryMethod(ImmutableList args) {}", + " private int unaryMethod(ImmutableList args) {", + " return 0;", + " }", "", - " private void unaryMethod(Integer... args) {}", + " private int unaryMethod(Integer... args) {", + " return unaryMethod(ImmutableList.copyOf(args));", + " }", "", " void unaryMethodWithLessVisibleOverload(ImmutableList args) {}", "", - " private void unaryMethodWithLessVisibleOverload(Integer... args) {}", + " private void unaryMethodWithLessVisibleOverload(Integer... args) {", + " unaryMethodWithLessVisibleOverload(ImmutableList.copyOf(args));", + " }", "", " private void binaryMethod(ImmutableList args, int extraArg) {}", "", - " private void binaryMethod(Integer... args) {}", + " private void binaryMethod(Integer... args) {", + " binaryMethod(ImmutableList.copyOf(args), 0);", + " }", "}") .doTest(); }