From a0da2074205368d929dfbc6561df59128eaf5f44 Mon Sep 17 00:00:00 2001 From: ghm Date: Fri, 28 Feb 2020 02:00:11 -0800 Subject: [PATCH] Hardcode JUnit4TestNotRun:DoNotRequireRunWith=true. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=297796637 --- .../bugpatterns/JUnit4TestNotRun.java | 36 +++---------------- .../bugpatterns/JUnit4TestNotRunTest.java | 9 +---- 2 files changed, 6 insertions(+), 39 deletions(-) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/JUnit4TestNotRun.java b/core/src/main/java/com/google/errorprone/bugpatterns/JUnit4TestNotRun.java index 74d3d43bd31..f7364c81c11 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/JUnit4TestNotRun.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/JUnit4TestNotRun.java @@ -22,7 +22,6 @@ import static com.google.errorprone.matchers.JUnitMatchers.isJUnit4TestClass; import static com.google.errorprone.matchers.JUnitMatchers.isJunit3TestCase; import static com.google.errorprone.matchers.Matchers.allOf; -import static com.google.errorprone.matchers.Matchers.enclosingClass; import static com.google.errorprone.matchers.Matchers.hasModifier; import static com.google.errorprone.matchers.Matchers.methodHasParameters; import static com.google.errorprone.matchers.Matchers.methodReturns; @@ -34,7 +33,6 @@ import com.google.errorprone.BugPattern; import com.google.errorprone.BugPattern.ProvidesFix; -import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher; import com.google.errorprone.fixes.SuggestedFix; @@ -70,18 +68,6 @@ public class JUnit4TestNotRun extends BugChecker implements ClassTreeMatcher { private static final String TEST_ANNOTATION = "@Test "; private static final String IGNORE_ANNOTATION = "@Ignore "; - private static final Matcher OLD_POSSIBLE_TEST_METHOD = - allOf( - hasModifier(PUBLIC), - methodReturns(VOID_TYPE), - methodHasParameters(), - not(JUnitMatchers::hasJUnitAnnotation), - // Use old logic to determine if this is a JUnit4 test class. - // Only classes with @RunWith, don't check for other @Test methods. - not(enclosingClass(hasModifier(Modifier.ABSTRACT))), - not(enclosingClass(JUnitMatchers.isTestCaseDescendant)), - enclosingClass(JUnitMatchers.hasJUnit4TestRunner)); - private static final Matcher POSSIBLE_TEST_METHOD = allOf( hasModifier(PUBLIC), @@ -92,26 +78,18 @@ public class JUnit4TestNotRun extends BugChecker implements ClassTreeMatcher { private static final Matcher NOT_STATIC = not(hasModifier(STATIC)); - private final boolean improvedHeuristic; - - public JUnit4TestNotRun(ErrorProneFlags flags) { - improvedHeuristic = flags.getBoolean("JUnit4TestNotRun:DoNotRequireRunWith").orElse(true); - } - @Override public Description matchClass(ClassTree tree, VisitorState state) { - if (improvedHeuristic && !isJUnit4TestClass.matches(tree, state)) { + if (!isJUnit4TestClass.matches(tree, state)) { return NO_MATCH; } - Matcher matcher = - improvedHeuristic ? POSSIBLE_TEST_METHOD : OLD_POSSIBLE_TEST_METHOD; Map suspiciousMethods = new HashMap<>(); for (Tree member : tree.getMembers()) { if (!(member instanceof MethodTree) || isSuppressed(member)) { continue; } MethodTree methodTree = (MethodTree) member; - if (matcher.matches(methodTree, state) && !isSuppressed(tree)) { + if (POSSIBLE_TEST_METHOD.matches(methodTree, state) && !isSuppressed(tree)) { suspiciousMethods.put(getSymbol(methodTree), methodTree); } } @@ -142,19 +120,15 @@ public Void visitMethodInvocation( *
  • The method is public, void, and has no parameters; *
  • the method is not already annotated with {@code @Test}, {@code @Before}, {@code @After}, * {@code @BeforeClass}, or {@code @AfterClass}; - *
  • the enclosing class appears to be intended to run in JUnit4, that is: - *
      - *
    1. it is non-abstract, - *
    2. it does not extend JUnit3 {@code TestCase}, - *
    3. it has an {@code @RunWith} annotation or at least one other method annotated - * {@code @Test}; - *
    *
  • and, the method appears to be a test method, that is: *
      *
    1. or, the method body contains a method call with a name that contains "assert", * "verify", "check", "fail", or "expect". *
    * + * + * Assumes that we have reason to believe we're in a test class (i.e. has a {@code RunWith} + * annotation; has other {@code @Test} methods, etc). */ private Optional handleMethod(MethodTree methodTree, VisitorState state) { // Method appears to be a JUnit 3 test case (name prefixed with "test"), probably a test. diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/JUnit4TestNotRunTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/JUnit4TestNotRunTest.java index fc853ebfc02..1e11466385d 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/JUnit4TestNotRunTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/JUnit4TestNotRunTest.java @@ -16,11 +16,9 @@ package com.google.errorprone.bugpatterns; -import com.google.common.collect.ImmutableList; import com.google.errorprone.BugCheckerRefactoringTestHelper; import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers; import com.google.errorprone.CompilationTestHelper; -import com.google.errorprone.ErrorProneFlags; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -33,8 +31,7 @@ public class JUnit4TestNotRunTest { CompilationTestHelper.newInstance(JUnit4TestNotRun.class, getClass()); private final BugCheckerRefactoringTestHelper refactoringHelper = - BugCheckerRefactoringTestHelper.newInstance( - new JUnit4TestNotRun(ErrorProneFlags.empty()), getClass()); + BugCheckerRefactoringTestHelper.newInstance(new JUnit4TestNotRun(), getClass()); @Test public void testPositiveCase1() { @@ -317,7 +314,6 @@ public void shouldNotDetectMethodsOnAbstractClass() { "public abstract class Test {", " public void testDoSomething() {}", "}") - .setArgs(ImmutableList.of("-XepOpt:JUnit4TestNotRun:DoNotRequireRunWith=false")) .doTest(); } @@ -505,7 +501,6 @@ public void suppression() { public void testNegativeCase1() { compilationHelper .addSourceFile("JUnit4TestNotRunNegativeCase1.java") - .setArgs(ImmutableList.of("-XepOpt:JUnit4TestNotRun:DoNotRequireRunWith=false")) .doTest(); } @@ -513,7 +508,6 @@ public void testNegativeCase1() { public void testNegativeCase2() { compilationHelper .addSourceFile("JUnit4TestNotRunNegativeCase2.java") - .setArgs(ImmutableList.of("-XepOpt:JUnit4TestNotRun:DoNotRequireRunWith=false")) .doTest(); } @@ -526,7 +520,6 @@ public void testNegativeCase3() { public void testNegativeCase4() { compilationHelper .addSourceFile("JUnit4TestNotRunNegativeCase4.java") - .setArgs(ImmutableList.of("-XepOpt:JUnit4TestNotRun:DoNotRequireRunWith=false")) .doTest(); }