Skip to content

Commit

Permalink
PSM-671 Extend checker to also flag illegal modifiers in Junit method…
Browse files Browse the repository at this point in the history
… declarations
  • Loading branch information
anicolasgar committed Dec 24, 2020
1 parent 3a08bc6 commit 710576d
Show file tree
Hide file tree
Showing 3 changed files with 201 additions and 98 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,45 +3,58 @@
import static com.google.errorprone.fixes.SuggestedFixes.renameMethod;

import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.fixes.SuggestedFixes;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.MethodTree;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.util.Name;
import javax.lang.model.element.Modifier;

/** A {@link BugChecker} which flags redundant 'test' prefixes in test method names. */
/** A {@link BugChecker} which flags improvements in Junit method declarations. */
@AutoService(BugChecker.class)
@BugPattern(
name = "TestMethodNameCheck",
summary = "Method names starting with a 'test' prefix can likely be removed",
name = "JunitMethodDeclarationCheck",
summary = "Junit method declaration can likely be improved",
linkType = BugPattern.LinkType.NONE,
severity = BugPattern.SeverityLevel.WARNING,
tags = BugPattern.StandardTags.SIMPLIFICATION)
public final class TestMethodNameCheck extends BugChecker implements MethodTreeMatcher {
public final class JunitMethodDeclarationCheck extends BugChecker implements MethodTreeMatcher {
private static final long serialVersionUID = 1L;
private static final String TEST_PREFIX = "test";
private static final ImmutableSet<Modifier> ILLEGAL_MODIFIERS =
ImmutableSet.of(Modifier.PRIVATE, Modifier.PROTECTED, Modifier.PUBLIC);

@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
MethodSymbol sym = ASTHelpers.getSymbol(tree);

if (sym == null
|| !ASTHelpers.isJUnitTestCode(state)
|| !hasTestPrefix(sym.getQualifiedName())) {
|| (!hasTestPrefix(sym.getQualifiedName()) && !hasIllegalModifiers(tree))) {
return Description.NO_MATCH;
}

String originalMethodName = sym.getQualifiedName().toString();
String newMethodName = removeTestPrefix(originalMethodName);
SuggestedFix.Builder builder = SuggestedFix.builder();

/* The new method name is not valid */
if (Character.isDigit(newMethodName.charAt(0))) {
return Description.NO_MATCH;
/* Remove all illegal modifiers, if has. */
SuggestedFixes.removeModifiers(tree, state, ILLEGAL_MODIFIERS.toArray(Modifier[]::new))
.ifPresent(builder::merge);

/* The new method name is valid. */
if (hasTestPrefix(sym.getQualifiedName()) && !Character.isDigit(newMethodName.charAt(0))) {
builder.merge(renameMethod(tree, newMethodName, state));
}
return describeMatch(tree, renameMethod(tree, newMethodName, state));

return describeMatch(tree, builder.build());
}

/** Determines whether the provided method name starts with the word 'test'. */
Expand All @@ -57,4 +70,9 @@ private static String removeTestPrefix(String methodName) {

return sb.toString();
}

/** Determines whether the method tree has any modifier that is not allowed. */
private static boolean hasIllegalModifiers(MethodTree tree) {
return ASTHelpers.getModifiers(tree).getFlags().stream().anyMatch(ILLEGAL_MODIFIERS::contains);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
package tech.picnic.errorprone.bugpatterns;

import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;

public final class JunitMethodDeclarationCheckTest {
private final CompilationTestHelper compilationTestHelper =
CompilationTestHelper.newInstance(JunitMethodDeclarationCheck.class, getClass());
private final BugCheckerRefactoringTestHelper refactoringTestHelper =
BugCheckerRefactoringTestHelper.newInstance(new JunitMethodDeclarationCheck(), getClass());

@Test
public void testIdentification() {
compilationTestHelper
.addSourceLines(
"A.java",
"import org.junit.jupiter.api.Test;",
"",
"final class A {",
" @Test void method1() {}",
"",
" // BUG: Diagnostic contains:",
" @Test void testMethod2() {}",
"",
" // BUG: Diagnostic contains:",
" @Test public void method3() {}",
"",
" // BUG: Diagnostic contains:",
" @Test protected void method4() {}",
"",
" // BUG: Diagnostic contains:",
" @Test private void method5() {}",
"}")
.addSourceLines(
"B.java",
"import org.junit.jupiter.params.ParameterizedTest;",
"",
"final class B {",
" @ParameterizedTest void method6() {}",
"",
" // BUG: Diagnostic contains:",
" @ParameterizedTest void testMethod7() {}",
"",
"}")
.doTest();
}

@Test
public void testReplacementTestPrefix() {
refactoringTestHelper
.addInputLines(
"in/Container.java",
"import org.junit.jupiter.api.Test;",
"import org.junit.jupiter.params.ParameterizedTest;",
"",
"interface Container {",
" class A {",
" @Test void test() {}",
" @Test void testMethod1() {}",
" @Test void test2() {}",
" @Test void method3() {}",
" void method4() {}",
" }",
"",
" class B {",
" @ParameterizedTest void test() {}",
" @ParameterizedTest void testMethod1() {}",
" @ParameterizedTest void test2() {}",
" @ParameterizedTest void method3() {}",
" void method4() {}",
" }",
"}")
.addOutputLines(
"out/Container.java",
"import org.junit.jupiter.api.Test;",
"import org.junit.jupiter.params.ParameterizedTest;",
"",
"interface Container {",
" class A {",
" @Test void test() {}",
" @Test void method1() {}",
" @Test void test2() {}",
" @Test void method3() {}",
" void method4() {}",
" }",
"",
" class B {",
" @ParameterizedTest void test() {}",
" @ParameterizedTest void method1() {}",
" @ParameterizedTest void test2() {}",
" @ParameterizedTest void method3() {}",
" void method4() {}",
" }",
"}")
.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
}

@Test
public void testDropModifiers() {
refactoringTestHelper
.addInputLines(
"in/Container.java",
"import org.junit.jupiter.api.AfterAll;",
"import org.junit.jupiter.api.AfterEach;",
"import org.junit.jupiter.api.BeforeAll;",
"import org.junit.jupiter.api.BeforeEach;",
"import org.junit.jupiter.api.Test;",
"interface Container {",
" class A {",
" @BeforeEach public void setUp1() {}",
" @BeforeEach protected void setUp2() {}",
" @BeforeEach private void setUp3() {}",
" @BeforeEach void setUp4() {}",
" @BeforeAll public void setUp5() {}",
" @BeforeAll protected void setUp6() {}",
" @BeforeAll private void setUp7() {}",
" @BeforeAll void setUp8() {}",
" @AfterEach public void tearDown1() {}",
" @AfterEach protected void tearDown2() {}",
" @AfterEach private void tearDown3() {}",
" @AfterEach void tearDown4() {}",
" @AfterAll public void tearDown5() {}",
" @AfterAll protected void tearDown6() {}",
" @AfterAll private void tearDown7() {}",
" @AfterAll void tearDown8() {}",
" @Test void method1() {}",
" @Test public void method2() {}",
" @Test protected void method3() {}",
" @Test private void method4() {}",
" void method5() {}",
" public void method6() {}",
" protected void method7() {}",
" private void method8() {}",
" }",
"}")
.addOutputLines(
"out/Container.java",
"import org.junit.jupiter.api.AfterAll;",
"import org.junit.jupiter.api.AfterEach;",
"import org.junit.jupiter.api.BeforeAll;",
"import org.junit.jupiter.api.BeforeEach;",
"import org.junit.jupiter.api.Test;",
"interface Container {",
" class A {",
" @BeforeEach void setUp1() {}",
" @BeforeEach void setUp2() {}",
" @BeforeEach void setUp3() {}",
" @BeforeEach void setUp4() {}",
" @BeforeAll void setUp5() {}",
" @BeforeAll void setUp6() {}",
" @BeforeAll void setUp7() {}",
" @BeforeAll void setUp8() {}",
" @AfterEach void tearDown1() {}",
" @AfterEach void tearDown2() {}",
" @AfterEach void tearDown3() {}",
" @AfterEach void tearDown4() {}",
" @AfterAll void tearDown5() {}",
" @AfterAll void tearDown6() {}",
" @AfterAll void tearDown7() {}",
" @AfterAll void tearDown8() {}",
" @Test void method1() {}",
" @Test void method2() {}",
" @Test void method3() {}",
" @Test void method4() {}",
" void method5() {}",
" public void method6() {}",
" protected void method7() {}",
" private void method8() {}",
" }",
"}")
.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
}
}

This file was deleted.

0 comments on commit 710576d

Please sign in to comment.