-
Notifications
You must be signed in to change notification settings - Fork 39
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
) This rule is implemented using a Refaster template, relying on the new `ThrowsCheckedException` matcher. While there, introduce `AbstractTestChecker` to simplify the test setup for Refaster `Matcher`s. This base class flags all `ExpressionTree`s matched by the `Matcher` under test.
- Loading branch information
1 parent
f7ec283
commit 62fe10f
Showing
10 changed files
with
281 additions
and
12 deletions.
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
60 changes: 60 additions & 0 deletions
60
...er-support/src/main/java/tech/picnic/errorprone/refaster/util/ThrowsCheckedException.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,60 @@ | ||
| package tech.picnic.errorprone.refaster.util; | ||
|
|
||
| import com.google.common.collect.ImmutableSet; | ||
| import com.google.errorprone.VisitorState; | ||
| import com.google.errorprone.matchers.Matcher; | ||
| import com.google.errorprone.util.ASTHelpers; | ||
| import com.sun.source.tree.ExpressionTree; | ||
| import com.sun.source.tree.LambdaExpressionTree; | ||
| import com.sun.source.tree.MemberReferenceTree; | ||
| import com.sun.tools.javac.code.Symbol; | ||
| import com.sun.tools.javac.code.Type; | ||
| import com.sun.tools.javac.code.Types.FunctionDescriptorLookupError; | ||
| import java.util.Collection; | ||
|
|
||
| /** | ||
| * A matcher of functional interface expressions for which execution of the functional interface | ||
| * method may throw a checked exception. | ||
| */ | ||
| public final class ThrowsCheckedException implements Matcher<ExpressionTree> { | ||
| private static final long serialVersionUID = 1L; | ||
|
|
||
| @Override | ||
| public boolean matches(ExpressionTree tree, VisitorState state) { | ||
| return containsCheckedException(getThrownTypes(tree, state), state); | ||
| } | ||
|
|
||
| private static Collection<Type> getThrownTypes(ExpressionTree tree, VisitorState state) { | ||
| if (tree instanceof LambdaExpressionTree) { | ||
| return ASTHelpers.getThrownExceptions(((LambdaExpressionTree) tree).getBody(), state); | ||
| } | ||
|
|
||
| if (tree instanceof MemberReferenceTree) { | ||
| Symbol symbol = ASTHelpers.getSymbol(tree); | ||
| if (symbol == null) { | ||
| return ImmutableSet.of(); | ||
| } | ||
|
|
||
| return symbol.type.getThrownTypes(); | ||
| } | ||
|
|
||
| Type type = ASTHelpers.getType(tree); | ||
| if (type == null) { | ||
| return ImmutableSet.of(); | ||
| } | ||
|
|
||
| try { | ||
| return state.getTypes().findDescriptorType(type).getThrownTypes(); | ||
| } catch (FunctionDescriptorLookupError e) { | ||
| return ImmutableSet.of(); | ||
| } | ||
| } | ||
|
|
||
| private static boolean containsCheckedException(Collection<Type> types, VisitorState state) { | ||
| return !types.stream() | ||
| .allMatch( | ||
| t -> | ||
| ASTHelpers.isSubtype(t, state.getSymtab().runtimeExceptionType, state) | ||
| || ASTHelpers.isSubtype(t, state.getSymtab().errorType, state)); | ||
| } | ||
| } |
3 changes: 2 additions & 1 deletion
3
refaster-support/src/main/java/tech/picnic/errorprone/refaster/util/package-info.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
70 changes: 70 additions & 0 deletions
70
...upport/src/test/java/tech/picnic/errorprone/refaster/util/AbstractMatcherTestChecker.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,70 @@ | ||
| package tech.picnic.errorprone.refaster.util; | ||
|
|
||
| import com.google.errorprone.VisitorState; | ||
| import com.google.errorprone.bugpatterns.BugChecker; | ||
| import com.google.errorprone.bugpatterns.BugChecker.CompilationUnitTreeMatcher; | ||
| import com.google.errorprone.matchers.Description; | ||
| import com.google.errorprone.matchers.Matcher; | ||
| import com.sun.source.tree.CompilationUnitTree; | ||
| import com.sun.source.tree.ExpressionTree; | ||
| import com.sun.source.tree.ImportTree; | ||
| import com.sun.source.tree.MethodTree; | ||
| import com.sun.source.tree.Tree; | ||
| import com.sun.source.util.TreeScanner; | ||
| import javax.annotation.Nullable; | ||
|
|
||
| /** | ||
| * An abstract {@link BugChecker} that reports a match for each expression matched by the given | ||
| * {@link Matcher}. | ||
| * | ||
| * <p>Only {@link ExpressionTree}s that represent proper Java expressions (i.e. {@link | ||
| * ExpressionTree}s that may be matched by Refaster) are considered. | ||
| */ | ||
| abstract class AbstractMatcherTestChecker extends BugChecker implements CompilationUnitTreeMatcher { | ||
| private static final long serialVersionUID = 1L; | ||
|
|
||
| private final Matcher<ExpressionTree> delegate; | ||
|
|
||
| AbstractMatcherTestChecker(Matcher<ExpressionTree> delegate) { | ||
| this.delegate = delegate; | ||
| } | ||
|
|
||
| @Override | ||
| public Description matchCompilationUnit(CompilationUnitTree compilationUnit, VisitorState state) { | ||
| new TreeScanner<Void, Void>() { | ||
| @Nullable | ||
| @Override | ||
| public Void scan(Tree tree, @Nullable Void unused) { | ||
| if (tree instanceof ExpressionTree && delegate.matches((ExpressionTree) tree, state)) { | ||
| state.reportMatch( | ||
| Description.builder(tree, canonicalName(), null, defaultSeverity(), message()) | ||
| .build()); | ||
| } | ||
|
|
||
| return super.scan(tree, unused); | ||
| } | ||
|
|
||
| @Nullable | ||
| @Override | ||
| public Void visitImport(ImportTree node, @Nullable Void unused) { | ||
| /* | ||
| * We're not interested in matching import statements. While components of these | ||
| * can be `ExpressionTree`s, they will never be matched by Refaster. | ||
| */ | ||
| return null; | ||
| } | ||
|
|
||
| @Nullable | ||
| @Override | ||
| public Void visitMethod(MethodTree node, @Nullable Void unused) { | ||
| /* | ||
| * We're not interested in matching e.g. parameter and return type declarations. While these | ||
| * can be `ExpressionTree`s, they will never be matched by Refaster. | ||
| */ | ||
| return scan(node.getBody(), unused); | ||
| } | ||
| }.scan(compilationUnit, null); | ||
|
|
||
| return Description.NO_MATCH; | ||
| } | ||
| } |
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
94 changes: 94 additions & 0 deletions
94
...upport/src/test/java/tech/picnic/errorprone/refaster/util/ThrowsCheckedExceptionTest.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,94 @@ | ||
| package tech.picnic.errorprone.refaster.util; | ||
|
|
||
| import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; | ||
|
|
||
| import com.google.errorprone.BugPattern; | ||
| import com.google.errorprone.CompilationTestHelper; | ||
| import com.google.errorprone.bugpatterns.BugChecker; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| final class ThrowsCheckedExceptionTest { | ||
| @Test | ||
| void matches() { | ||
| CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass()) | ||
| .addSourceLines( | ||
| "A.java", | ||
| "import java.util.concurrent.Callable;", | ||
| "import java.util.function.Supplier;", | ||
| "", | ||
| "class A {", | ||
| " void negative1() {", | ||
| " callableSink(null);", | ||
| " }", | ||
| "", | ||
| " void negative2() {", | ||
| " supplierSink(null);", | ||
| " }", | ||
| "", | ||
| " void negative3() {", | ||
| " callableSink(() -> toString());", | ||
| " }", | ||
| "", | ||
| " void negative4() {", | ||
| " supplierSink(() -> toString());", | ||
| " }", | ||
| "", | ||
| " void negative5() {", | ||
| " callableSink(this::toString);", | ||
| " }", | ||
| "", | ||
| " void negative6() {", | ||
| " supplierSink(this::toString);", | ||
| " }", | ||
| "", | ||
| " void negative7() {", | ||
| " supplierSink(", | ||
| " new Supplier<>() {", | ||
| " @Override", | ||
| " public Object get() {", | ||
| " return getClass();", | ||
| " }", | ||
| " });", | ||
| " }", | ||
| "", | ||
| " void positive1() {", | ||
| " // BUG: Diagnostic contains:", | ||
| " callableSink(() -> getClass().getDeclaredConstructor());", | ||
| " }", | ||
| "", | ||
| " void positive2() {", | ||
| " // BUG: Diagnostic contains:", | ||
| " callableSink(getClass()::getDeclaredConstructor);", | ||
| " }", | ||
| "", | ||
| " void positive3() {", | ||
| " callableSink(", | ||
| " // BUG: Diagnostic contains:", | ||
| " new Callable<>() {", | ||
| " @Override", | ||
| " public Object call() throws NoSuchMethodException {", | ||
| " return getClass().getDeclaredConstructor();", | ||
| " }", | ||
| " });", | ||
| " }", | ||
| "", | ||
| " private static void callableSink(Callable<?> callable) {}", | ||
| "", | ||
| " private static void supplierSink(Supplier<?> supplier) {}", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| /** A {@link BugChecker} which simply delegates to {@link ThrowsCheckedException}. */ | ||
| @BugPattern(summary = "Flags expressions matched by `ThrowsCheckedException`", severity = ERROR) | ||
| public static final class MatcherTestChecker extends AbstractMatcherTestChecker { | ||
| private static final long serialVersionUID = 1L; | ||
|
|
||
| // XXX: This is a false positive reported by Checkstyle. See | ||
| // https://github.com/checkstyle/checkstyle/issues/10161#issuecomment-1242732120. | ||
| @SuppressWarnings("RedundantModifier") | ||
| public MatcherTestChecker() { | ||
| super(new ThrowsCheckedException()); | ||
| } | ||
| } | ||
| } |