-
Notifications
You must be signed in to change notification settings - Fork 135
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
new JUnit5SuiteMisuse error-prone check (#843)
ErrorProne will now detect dangerous usage of `@RunWith(Suite.class)` that references JUnit5 classes, as this can cause tests to silently not run!
- Loading branch information
1 parent
150de77
commit 53e3d92
Showing
14 changed files
with
319 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
117 changes: 117 additions & 0 deletions
117
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/JUnit5SuiteMisuse.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| /* | ||
| * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.palantir.baseline.errorprone; | ||
|
|
||
| import com.google.auto.service.AutoService; | ||
| import com.google.common.collect.ImmutableList; | ||
| import com.google.errorprone.BugPattern; | ||
| import com.google.errorprone.VisitorState; | ||
| import com.google.errorprone.bugpatterns.BugChecker; | ||
| import com.google.errorprone.matchers.AnnotationMatcherUtils; | ||
| import com.google.errorprone.matchers.Description; | ||
| import com.google.errorprone.matchers.Matchers; | ||
| import com.google.errorprone.util.ASTHelpers; | ||
| import com.sun.source.tree.AnnotationTree; | ||
| import com.sun.source.tree.ClassTree; | ||
| import com.sun.source.tree.ExpressionTree; | ||
| import com.sun.tools.javac.code.Type; | ||
| import com.sun.tools.javac.tree.JCTree; | ||
| import java.util.Collections; | ||
| import java.util.HashSet; | ||
| import java.util.List; | ||
| import java.util.Set; | ||
|
|
||
| @AutoService(BugChecker.class) | ||
| @BugPattern( | ||
| name = "JUnit5SuiteMisuse", | ||
| link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks", | ||
| linkType = BugPattern.LinkType.CUSTOM, | ||
| severity = BugPattern.SeverityLevel.ERROR, | ||
| summary = "Referencing JUnit5 tests from JUnit4 Suites will silently not work") | ||
| public final class JUnit5SuiteMisuse extends BugChecker | ||
| implements BugChecker.ClassTreeMatcher, BugChecker.AnnotationTreeMatcher { | ||
|
|
||
| private static final long serialVersionUID = 1L; | ||
|
|
||
| // We remember classes and validate them later because error-prone doesn't let us arbitrarily explore classes we | ||
| // discover when reading the @SuiteClasses annotation. | ||
| private static final Set<Type.ClassType> knownJUnit5TestClasses = new HashSet<>(); | ||
| private static final Set<Type.ClassType> referencedBySuites = new HashSet<>(); | ||
|
|
||
| @Override | ||
| public Description matchClass(ClassTree tree, VisitorState state) { | ||
| if (!JUnit5RuleUsage.hasJunit5TestCases.matches(tree, state)) { | ||
| return Description.NO_MATCH; | ||
| } | ||
|
|
||
| Type.ClassType type = ASTHelpers.getType(tree); | ||
| knownJUnit5TestClasses.add(type); // accumulate these so we can check them when visiting suiteclasses | ||
|
|
||
| if (referencedBySuites.contains(type)) { | ||
| return buildDescription(tree) | ||
| .setMessage("Class uses JUnit5 tests but is referenced by a JUnit4 SuiteClasses annotation") | ||
| .build(); | ||
| } | ||
|
|
||
| return Description.NO_MATCH; | ||
| } | ||
|
|
||
| @Override | ||
| public Description matchAnnotation(AnnotationTree tree, VisitorState state) { | ||
| if (!Matchers.isSameType("org.junit.runners.Suite.SuiteClasses").matches(tree, state)) { | ||
| return Description.NO_MATCH; | ||
| } | ||
|
|
||
| for (Type referencedClass : getReferencedClasses(tree, state)) { | ||
| Type.ClassType classType = (Type.ClassType) referencedClass; | ||
| referencedBySuites.add(classType); | ||
|
|
||
| if (knownJUnit5TestClasses.contains(classType)) { | ||
| return buildDescription(tree) | ||
| .setMessage("Don't reference JUnit5 test classes from JUnit4 SuiteClasses annotation") | ||
| .build(); | ||
| } | ||
| } | ||
|
|
||
| return Description.NO_MATCH; | ||
| } | ||
|
|
||
| private static List<Type> getReferencedClasses(AnnotationTree tree, VisitorState state) { | ||
| ExpressionTree value = AnnotationMatcherUtils.getArgument(tree, "value"); | ||
|
|
||
| if (value == null) { | ||
| return Collections.emptyList(); | ||
| } | ||
|
|
||
| if (value instanceof JCTree.JCFieldAccess) { | ||
| return Collections.singletonList(((JCTree.JCFieldAccess) value).selected.type); | ||
| } | ||
|
|
||
| if (value instanceof JCTree.JCNewArray) { | ||
| ImmutableList.Builder<Type> list = ImmutableList.builder(); | ||
| for (JCTree.JCExpression elem : ((JCTree.JCNewArray) value).elems) { | ||
| list.add(((JCTree.JCFieldAccess) elem).selected.type); | ||
| } | ||
| return list.build(); | ||
| } | ||
|
|
||
| throw new UnsupportedOperationException(String.format( | ||
| "Unable to get referenced classes for %s of type %s", | ||
| state.getSourceForNode(tree), | ||
| value.getClass())); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
174 changes: 174 additions & 0 deletions
174
...ine-error-prone/src/test/java/com/palantir/baseline/errorprone/JUnit5SuiteMisuseTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,174 @@ | ||
| /* | ||
| * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.palantir.baseline.errorprone; | ||
|
|
||
| import com.google.errorprone.CompilationTestHelper; | ||
| import org.junit.Before; | ||
| import org.junit.Test; | ||
|
|
||
| public class JUnit5SuiteMisuseTest { | ||
|
|
||
| private CompilationTestHelper compilationHelper; | ||
|
|
||
| @Before | ||
| public void before() { | ||
| compilationHelper = CompilationTestHelper.newInstance(JUnit5SuiteMisuse.class, getClass()); | ||
| } | ||
|
|
||
| @Test | ||
| public void multiple_junit4_references_pass() { | ||
| compilationHelper.addSourceLines( | ||
| "Container.java", | ||
| "import org.junit.runner.RunWith;", | ||
| "import org.junit.runners.Suite;", | ||
| "class Container {", | ||
| " @RunWith(Suite.class)", | ||
| " @Suite.SuiteClasses({FooTest.class, BarTest.class})", | ||
| " public static class MySuite {}", | ||
| "", | ||
| " public static class FooTest {", | ||
| " @org.junit.Test public void my_test() {}", | ||
| " }", | ||
| "", | ||
| " public static class BarTest {", | ||
| " @org.junit.Test public void my_test() {}", | ||
| " }", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| @Test | ||
| public void single_junit4_reference_passes() { | ||
| compilationHelper.addSourceLines( | ||
| "Container.java", | ||
| "import org.junit.runner.RunWith;", | ||
| "import org.junit.runners.Suite;", | ||
| "class Container {", | ||
| " @RunWith(Suite.class)", | ||
| " @Suite.SuiteClasses(FooTest.class)", | ||
| " public static class MySuite {}", | ||
| "", | ||
| " public static class FooTest {", | ||
| " @org.junit.Test public void my_test() {}", | ||
| " }", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| @Test | ||
| public void single_junit4_reference_passes_different_order() { | ||
| compilationHelper.addSourceLines( | ||
| "Container.java", | ||
| "import org.junit.runner.RunWith;", | ||
| "import org.junit.runners.Suite;", | ||
| "class Container {", | ||
| " public static class FooTest {", | ||
| " @org.junit.Test public void my_test() {}", | ||
| " }", | ||
| "", | ||
| " @RunWith(Suite.class)", | ||
| " @Suite.SuiteClasses(FooTest.class)", | ||
| " public static class MySuite {}", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| @Test | ||
| public void multiple_junit5_references_fail() { | ||
| compilationHelper.addSourceLines( | ||
| "Container.java", | ||
| "import org.junit.runner.RunWith;", | ||
| "import org.junit.runners.Suite;", | ||
| "class Container {", | ||
| " @RunWith(Suite.class)", | ||
| " @Suite.SuiteClasses({FooTest.class, BarTest.class})", | ||
| " public static class MySuite {}", | ||
| "", | ||
| " // BUG: Diagnostic contains: JUnit5SuiteMisuse", | ||
| " public static class FooTest {", | ||
| " @org.junit.jupiter.api.Test public void my_test() {}", | ||
| " }", | ||
| "", | ||
| " // BUG: Diagnostic contains: JUnit5SuiteMisuse", | ||
| " public static class BarTest {", | ||
| " @org.junit.jupiter.api.Test public void my_test() {}", | ||
| " }", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| @Test | ||
| public void multiple_junit5_references_fail_different_order() { | ||
| compilationHelper.addSourceLines( | ||
| "Container.java", | ||
| "import org.junit.runner.RunWith;", | ||
| "import org.junit.runners.Suite;", | ||
| "class Container {", | ||
| " public static class FooTest {", | ||
| " @org.junit.jupiter.api.Test public void my_test() {}", | ||
| " }", | ||
| "", | ||
| " public static class BarTest {", | ||
| " @org.junit.jupiter.api.Test public void my_test() {}", | ||
| " }", | ||
| "", | ||
| " @RunWith(Suite.class)", | ||
| " // BUG: Diagnostic contains: JUnit5SuiteMisuse", | ||
| " @Suite.SuiteClasses({FooTest.class, BarTest.class})", | ||
| " public static class MySuite {}", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| @Test | ||
| public void single_junit5_reference_fails() { | ||
| compilationHelper.addSourceLines( | ||
| "Container.java", | ||
| "import org.junit.runner.RunWith;", | ||
| "import org.junit.runners.Suite;", | ||
| "class Container {", | ||
| " @RunWith(Suite.class)", | ||
| " @Suite.SuiteClasses(FooTest.class)", | ||
| " public static class MySuite {}", | ||
| "", | ||
| " // BUG: Diagnostic contains: JUnit5SuiteMisuse", | ||
| " public static class FooTest {", | ||
| " @org.junit.jupiter.api.Test public void my_test() {}", | ||
| " }", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| @Test | ||
| public void single_junit5_reference_fails_different_order() { | ||
| compilationHelper.addSourceLines( | ||
| "Container.java", | ||
| "import org.junit.runner.RunWith;", | ||
| "import org.junit.runners.Suite;", | ||
| "class Container {", | ||
| " public static class FooTest {", | ||
| " @org.junit.jupiter.api.Test public void my_test() {}", | ||
| " }", | ||
| "", | ||
| " @RunWith(Suite.class)", | ||
| " // BUG: Diagnostic contains: JUnit5SuiteMisuse", | ||
| " @Suite.SuiteClasses(FooTest.class)", | ||
| " public static class MySuite {}", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
| } |
Oops, something went wrong.