diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationCheck.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationCheck.java index 400c7e747f3..258d2bc8cf3 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationCheck.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationCheck.java @@ -5,6 +5,7 @@ import static com.google.errorprone.matchers.Matchers.anyOf; import static com.google.errorprone.matchers.Matchers.isType; import static java.util.function.Predicate.not; +import static tech.picnic.errorprone.bugpatterns.JavaKeywords.isJavaKeyword; import com.google.auto.service.AutoService; import com.google.common.collect.ImmutableSet; @@ -85,35 +86,41 @@ public Description matchMethod(MethodTree tree, VisitorState state) { if (isTestMethod) { tryCanonicalizeMethodName(tree) - .filter(methodName -> !reportDescriptionForPossibleNameClash(tree, methodName, state)) + .filter(methodName -> isValidMethodName(tree, methodName, state)) .ifPresent( methodName -> builder.merge(SuggestedFixes.renameMethod(tree, methodName, state))); } return builder.isEmpty() ? Description.NO_MATCH : describeMatch(tree, builder.build()); } - private boolean reportDescriptionForPossibleNameClash( - MethodTree tree, String methodName, VisitorState state) { + private boolean isValidMethodName(MethodTree tree, String methodName, VisitorState state) { if (isMethodNameInClass(methodName, state)) { - state.reportMatch( - buildDescription(tree) - .setMessage( - String.format("A method with name %s already exists in the class.", methodName)) - .build()); - return true; + reportIncorrectMethodName( + methodName, tree, "A method with name %s already exists in the class.", state); + return false; } if (isMethodNameStaticallyImported(methodName, state)) { - state.reportMatch( - buildDescription(tree) - .setMessage( - String.format( - "A method with name %s is already statically imported.", methodName)) - .build()); - return true; + reportIncorrectMethodName( + methodName, tree, "A method with name %s is already statically imported.", state); + return false; } - return false; + if (isJavaKeyword(methodName)) { + reportIncorrectMethodName( + methodName, + tree, + "Method name `%s` is not possible because it is a Java keyword.", + state); + return false; + } + return true; + } + + private void reportIncorrectMethodName( + String methodName, MethodTree tree, String message, VisitorState state) { + state.reportMatch( + buildDescription(tree).setMessage(String.format(message, methodName)).build()); } private static boolean isMethodNameInClass(String methodName, VisitorState state) { diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JavaKeywords.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JavaKeywords.java new file mode 100644 index 00000000000..a86e1ff6035 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JavaKeywords.java @@ -0,0 +1,77 @@ +package tech.picnic.errorprone.bugpatterns; + +import com.google.common.collect.ImmutableSet; + +@SuppressWarnings("DeclarationOrder" /* The private constructor should come first. */) +final class JavaKeywords { + private JavaKeywords() {} + + /** + * List of all Java Language Keywords. + * + *

See: the Oracle + * Documentation on Java Keywords. + */ + private static final ImmutableSet JAVA_KEYWORDS = + ImmutableSet.of( + "assert", + "boolean", + "break", + "byte", + "case", + "catch", + "char", + "class", + "const", + "continue", + "default", + "do", + "double", + "else", + "enum", + "extends", + "final", + "finally", + "float", + "for", + "goto", + "if", + "implements", + "import", + "instanceof", + "int", + "interface", + "long", + "native", + "new", + "package", + "private", + "protected", + "public", + "return", + "short", + "static", + "strictfp", + "super", + "switch", + "synchronized", + "this", + "throw", + "throws", + "transient", + "try", + "void", + "volatile", + "while"); + + /** + * Check whether the String is a Java Language Keyword. + * + * @param possibleKeyword a possible Java keyword + * @return whether the String is a Java keyword + */ + public static boolean isJavaKeyword(String possibleKeyword) { + return JAVA_KEYWORDS.contains(possibleKeyword); + } +} diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationCheckTest.java index d2faf47ee5e..9c25eb0d2e8 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationCheckTest.java @@ -267,4 +267,44 @@ void methodAlreadyInStaticImports() { "}") .doTest(TestMode.TEXT_MATCH); } + + @Test + void methodHasJavaKeyword() { + refactoringTestHelper + .addInputLines( + "A.java", + "import org.junit.jupiter.api.Test;", + "", + "class A {", + " @Test", + " void testClass() {}", + "", + " @Test", + " void testClazz() {}", + "", + " @Test", + " void testThrow() {}", + "", + " @Test", + " void testThrowww() {}", + "}") + .addOutputLines( + "A.java", + "import org.junit.jupiter.api.Test;", + "", + "class A {", + " @Test", + " void testClass() {}", + "", + " @Test", + " void clazz() {}", + "", + " @Test", + " void testThrow() {}", + "", + " @Test", + " void throwww() {}", + "}") + .doTest(TestMode.TEXT_MATCH); + } }