Address feedback

PicnicSupermarket · Sep 12, 2022 · 6f199a3 · 6f199a3
1 parent 83629ce
commit 6f199a3
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 13 deletions.
diff --git a/...ne/refaster/util/AbstractTestChecker.java → ...ster/util/AbstractMatcherTestChecker.java b/...ne/refaster/util/AbstractTestChecker.java → ...ster/util/AbstractMatcherTestChecker.java
@@ -13,12 +13,19 @@
 import com.sun.source.util.TreeScanner;
 import javax.annotation.Nullable;
 
-abstract class AbstractTestChecker extends BugChecker implements CompilationUnitTreeMatcher {
+/**
+ * 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;
 
-  AbstractTestChecker(Matcher<ExpressionTree> delegate) {
+  AbstractMatcherTestChecker(Matcher<ExpressionTree> delegate) {
     this.delegate = delegate;
   }
 
@@ -27,34 +34,34 @@ public Description matchCompilationUnit(CompilationUnitTree compilationUnit, Vis
     new TreeScanner<Void, Void>() {
       @Nullable
       @Override
-      public Void scan(Tree tree, @Nullable Void p) {
+      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, p);
+        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.
+         * can be `ExpressionTree`s, they will never be matched by Refaster.
          */
         return null;
       }
 
       @Nullable
       @Override
-      public Void visitMethod(MethodTree node, @Nullable Void p) {
+      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(), p);
+        return scan(node.getBody(), unused);
       }
     }.scan(compilationUnit, null);
 

diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/util/IsArrayTest.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/util/IsArrayTest.java
@@ -10,7 +10,7 @@
 final class IsArrayTest {
   @Test
   void matches() {
-    CompilationTestHelper.newInstance(TestChecker.class, getClass())
+    CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass())
         .addSourceLines(
             "A.java",
             "class A {",
@@ -50,13 +50,13 @@ void matches() {
 
   /** A {@link BugChecker} which simply delegates to {@link IsArray}. */
   @BugPattern(summary = "Flags expressions matched by `IsArray`", severity = ERROR)
-  public static final class TestChecker extends AbstractTestChecker {
+  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 TestChecker() {
+    public MatcherTestChecker() {
       super(new IsArray());
     }
   }

diff --git a/...upport/src/test/java/tech/picnic/errorprone/refaster/util/ThrowsCheckedExceptionTest.java b/...upport/src/test/java/tech/picnic/errorprone/refaster/util/ThrowsCheckedExceptionTest.java
@@ -10,7 +10,7 @@
 final class ThrowsCheckedExceptionTest {
   @Test
   void matches() {
-    CompilationTestHelper.newInstance(TestChecker.class, getClass())
+    CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass())
         .addSourceLines(
             "A.java",
             "import java.util.concurrent.Callable;",
@@ -81,13 +81,13 @@ void matches() {
 
   /** A {@link BugChecker} which simply delegates to {@link ThrowsCheckedException}. */
   @BugPattern(summary = "Flags expressions matched by `ThrowsCheckedException`", severity = ERROR)
-  public static final class TestChecker extends AbstractTestChecker {
+  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 TestChecker() {
+    public MatcherTestChecker() {
       super(new ThrowsCheckedException());
     }
   }