Make suppression work in NotJavadoc.

Sprinkling SuppressibleTreePathScanner around isn't a panacea, who knew. <facepalm> PiperOrigin-RevId: 546954297
google · Jul 10, 2023 · 703c965 · 703c965
1 parent 15e8e01
commit 703c965
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 5 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/javadoc/NotJavadoc.java b/core/src/main/java/com/google/errorprone/bugpatterns/javadoc/NotJavadoc.java
@@ -26,6 +26,8 @@
 import static com.sun.tools.javac.parser.Tokens.Comment.CommentStyle.JAVADOC;
 
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableRangeSet;
+import com.google.common.collect.Range;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
@@ -41,6 +43,7 @@
 import com.sun.source.tree.PackageTree;
 import com.sun.source.tree.Tree;
 import com.sun.source.tree.VariableTree;
+import com.sun.source.util.TreePathScanner;
 import com.sun.tools.javac.code.Symbol.MethodSymbol;
 import com.sun.tools.javac.parser.Tokens.Comment;
 import java.util.HashMap;
@@ -55,7 +58,8 @@
 public final class NotJavadoc extends BugChecker implements CompilationUnitTreeMatcher {
   @Override
   public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState state) {
-    ImmutableMap<Integer, Tree> javadocableTrees = getJavadoccableTrees(tree, state);
+    ImmutableMap<Integer, Tree> javadocableTrees = getJavadoccableTrees(tree);
+    ImmutableRangeSet<Integer> suppressedRegions = suppressedRegions(state);
     for (ErrorProneToken token : getTokens(state.getSourceCode().toString(), state.context)) {
       for (Comment comment : token.comments()) {
         if (!comment.getStyle().equals(JAVADOC) || comment.getText().equals("/**/")) {
@@ -65,11 +69,16 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s
           continue;
         }
 
+        if (suppressedRegions.intersects(
+            Range.closed(
+                comment.getSourcePos(0), comment.getSourcePos(comment.getText().length() - 1)))) {
+          continue;
+        }
+
         int endPos = 2;
         while (comment.getText().charAt(endPos) == '*') {
           endPos++;
         }
-
         state.reportMatch(
             describeMatch(
                 getDiagnosticPosition(comment.getSourcePos(0), tree),
@@ -79,10 +88,9 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s
     return NO_MATCH;
   }
 
-  private ImmutableMap<Integer, Tree> getJavadoccableTrees(
-      CompilationUnitTree tree, VisitorState state) {
+  private ImmutableMap<Integer, Tree> getJavadoccableTrees(CompilationUnitTree tree) {
     Map<Integer, Tree> javadoccablePositions = new HashMap<>();
-    new SuppressibleTreePathScanner<Void, Void>(state) {
+    new TreePathScanner<Void, Void>() {
       @Override
       public Void visitPackage(PackageTree packageTree, Void unused) {
         javadoccablePositions.put(getStartPosition(packageTree), packageTree);

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/javadoc/NotJavadocTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/javadoc/NotJavadocTest.java
@@ -157,4 +157,19 @@ public void moduleLevel() {
         .expectUnchanged()
         .doTest(TEXT_MATCH);
   }
+
+  @Test
+  public void suppression() {
+    helper
+        .addInputLines(
+            "Test.java", //
+            "class Test {",
+            "  @SuppressWarnings(\"NotJavadoc\")",
+            "  void test() {",
+            "    /** Not Javadoc. */",
+            "  }",
+            "}")
+        .expectUnchanged()
+        .doTest(TEXT_MATCH);
+  }
 }