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
anicolasgar committed Dec 28, 2020
1 parent 3a08bc6 commit 86f7f2a
Show file tree
Hide file tree
Showing 4 changed files with 303 additions and 149 deletions.
99 changes: 99 additions & 0 deletions ...contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JunitMethodDeclarationCheck.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package tech.picnic.errorprone.bugpatterns;

import static com.google.errorprone.matchers.ChildMultiMatcher.MatchType.AT_LEAST_ONE;
import static com.google.errorprone.matchers.Matchers.annotations;
import static com.google.errorprone.matchers.Matchers.isType;

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.matchers.MultiMatcher;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import java.util.Optional;
import javax.lang.model.element.Modifier;

/** A {@link BugChecker} which flags improvements in Junit method declarations. */
@AutoService(BugChecker.class)
@BugPattern(
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 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);
private static final MultiMatcher<Tree, AnnotationTree> OVERRIDE_ANNOTATION =
annotations(AT_LEAST_ONE, isType("java.lang.Override"));

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

if (sym == null
|| !OVERRIDE_ANNOTATION.multiMatchResult(tree, state).matchingNodes().isEmpty()
|| !ASTHelpers.isJUnitTestCode(state)
|| (!hasTestPrefix(sym.getQualifiedName().toString()) && !hasIllegalModifiers(tree))) {
return Description.NO_MATCH;
}

SuggestedFix.Builder builder = SuggestedFix.builder();
/* Remove all illegal modifiers, if has. */
SuggestedFixes.removeModifiers(tree, state, ILLEGAL_MODIFIERS.toArray(Modifier[]::new))
.ifPresent(builder::merge);
/* Remove the 'test' prefix, if possible. */
renameMethod(tree, state, sym.getQualifiedName().toString()).ifPresent(builder::merge);

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

/** Renames the method removing the 'test' prefix, if possible. */
private static Optional<SuggestedFix> renameMethod(
MethodTree tree, VisitorState state, String methodName) {
if (hasTestPrefix(methodName)) {
String newMethodName = removeTestPrefix(methodName);

return isValidMethodName(newMethodName)
? Optional.of(SuggestedFixes.renameMethod(tree, newMethodName, state))
: Optional.empty();
}

return Optional.empty();
}

/** Removes the 'test' prefix from the method name, if possible. */
private static String removeTestPrefix(String methodName) {
StringBuilder sb = new StringBuilder(methodName);
sb.delete(0, 4);
sb.setCharAt(0, Character.toLowerCase(sb.charAt(0)));

return sb.toString();
}

/** Determines whether the provided method name is valid. */
private static boolean isValidMethodName(String methodName) {
return !methodName.isEmpty() && !Character.isDigit(methodName.charAt(0));
}

/** Determines whether the provided method name starts with the word 'test'. */
private static boolean hasTestPrefix(String methodName) {
return methodName.length() > 4 && methodName.startsWith(TEST_PREFIX);
}

/** 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);
}
}
60 changes: 0 additions & 60 deletions ...r-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/TestMethodNameCheck.java
View file

This file was deleted.

204 changes: 204 additions & 0 deletions ...rib/src/test/java/tech/picnic/errorprone/bugpatterns/JunitMethodDeclarationCheckTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
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 foo() {}",
" @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() {}",
"",
" // BUG: Diagnostic contains:",
" @Test private void method6() { ",
" new Foo() {",
" @Override",
" public void bar() {}",
" };",
" }",
"",
" // BUG: Diagnostic contains:",
" @Test void testMethod7() { ",
" new Foo() {",
" @Override",
" public void bar() {}",
" };",
" }",
"",
"",
" @Test void method8() { ",
" new Foo() {",
" @Override",
" public void bar() {}",
" };",
" }",
"",
"}",
"",
"abstract class Foo { ",
" abstract public void bar();",
"}")
.addSourceLines(
"B.java",
"import org.junit.jupiter.params.ParameterizedTest;",
"",
"final class B {",
" @ParameterizedTest void method1() {}",
"",
" // BUG: Diagnostic contains:",
" @ParameterizedTest void testMethod2() {}",
"",
"}")
.doTest();
}

@Test
public void testReplaceTestPrefix() {
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);
}
}
Loading

0 comments on commit 86f7f2a

Please sign in to comment.