From 9b4ffa0d13f52bd22db30e04623db23af8e009e6 Mon Sep 17 00:00:00 2001 From: stevegutz Date: Mon, 28 Jun 2021 16:53:47 -0700 Subject: [PATCH] [UnusedMethod] exempt @MethodSource with qualified method names As mentioned in [the JUnit 5 docs](https://junit.org/junit5/docs/current/api/org.junit.jupiter.params/org/junit/jupiter/params/provider/MethodSource.html), the value within `@MethodSource` can be a fully-qualified name. This updates the code to handle this fact and adds some tests to try to flex it. The particular case we were hitting is the one with the `@Nested` test class using a parameter method in its parent. Fixes #2411 COPYBARA_INTEGRATE_REVIEW=https://github.com/google/error-prone/pull/2411 from HubSpot:qualified-method-source-upstream 19ce7fb10293bbdc1c2ff63bc4cf6ca50dacf240 PiperOrigin-RevId: 381974583 --- .../errorprone/bugpatterns/UnusedMethod.java | 15 ++++-- .../bugpatterns/UnusedMethodTest.java | 52 +++++++++++++++++++ 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UnusedMethod.java b/core/src/main/java/com/google/errorprone/bugpatterns/UnusedMethod.java index 86e4362cb8a..9bfaa0d6a4b 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/UnusedMethod.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/UnusedMethod.java @@ -254,14 +254,21 @@ private void handleMethodSource(MethodTree tree) { .findAny() // get the annotation value array as a set of Names .flatMap(a -> getAnnotationValue(a, "value")) - .map(y -> asStrings(y).map(state::getName).collect(toImmutableSet())) - // remove all potentially unused methods whose simple name is referenced by the - // @MethodSource + .map( + y -> asStrings(y).map(state::getName).map(Name::toString).collect(toImmutableSet())) + // remove all potentially unused methods referenced by the @MethodSource .ifPresent( referencedNames -> unusedMethods .entrySet() - .removeIf(e -> referencedNames.contains(e.getKey().getSimpleName()))); + .removeIf( + e -> { + Symbol unusedSym = e.getKey(); + String simpleName = unusedSym.getSimpleName().toString(); + return referencedNames.contains(simpleName) + || referencedNames.contains( + unusedSym.owner.getQualifiedName() + "#" + simpleName); + })); } } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedMethodTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedMethodTest.java index 06238f73b5d..03393369ecd 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedMethodTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedMethodTest.java @@ -223,4 +223,56 @@ public void methodSource() { "}") .doTest(); } + + @Test + public void qualifiedMethodSource() { + helper + .addSourceLines( + "MethodSource.java", + "package org.junit.jupiter.params.provider;", + "public @interface MethodSource {", + " String[] value();", + "}") + .addSourceLines( + "Test.java", + "import java.util.stream.Stream;", + "import org.junit.jupiter.params.provider.MethodSource;", + "class Test {", + " @MethodSource(\"Test#parameters\")", + " void test() {}", + "", + "", + " private static Stream parameters() {", + " return Stream.of();", + " }", + "}") + .doTest(); + } + + @Test + public void nestedQualifiedMethodSource() { + helper + .addSourceLines( + "MethodSource.java", + "package org.junit.jupiter.params.provider;", + "public @interface MethodSource {", + " String[] value();", + "}") + .addSourceLines( + "Test.java", + "import java.util.stream.Stream;", + "import org.junit.jupiter.params.provider.MethodSource;", + "class Test {", + " // @Nested", + " public class NestedTest {", + " @MethodSource(\"Test#parameters\")", + " void test() {}", + " }", + "", + " private static Stream parameters() {", + " return Stream.of();", + " }", + "}") + .doTest(); + } }