[java] Fix #2996 - CommentSize/CommentContent suppression

pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/TokenDocument.java

@@ -10,8 +10,13 @@
 
 /**
  * Token layer of a parsed file.
+ * This object is used to store state global to all tokens of a single file,
+ * e.g. the text document. Not all languages currently have an implementation
+ * of a token document.
+ *
+ * @see net.sourceforge.pmd.lang.ast.impl.javacc.JavaccTokenDocument
  */
-public abstract class TokenDocument<T extends GenericToken> {
+public abstract class TokenDocument<T extends GenericToken<T>> {
 
     private final TextDocument textDocument;
 

pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/javacc/AbstractJjtreeNode.java

@@ -90,25 +90,23 @@ protected void insertChild(B child, int index) {
      */
     protected void fitTokensToChildren(int index) {
         if (index == 0) {
-            enlargeLeft((B) getChild(index));
+            enlargeLeft(getChild(index).getFirstToken());
         }
         if (index == getNumChildren()) {
-            enlargeRight((B) getChild(index));
+            enlargeRight(getChild(index).getLastToken());
         }
     }
 
-    private void enlargeLeft(B child) {
+    protected void enlargeLeft(JavaccToken childFst) {
         JavaccToken thisFst = this.getFirstToken();
-        JavaccToken childFst = child.getFirstToken();
 
         if (childFst.compareTo(thisFst) < 0) {
             this.setFirstToken(childFst);
         }
     }
 
-    private void enlargeRight(B child) {
+    private void enlargeRight(JavaccToken childLast) {
         JavaccToken thisLast = this.getLastToken();
-        JavaccToken childLast = child.getLastToken();
 
         if (childLast.compareTo(thisLast) > 0) {
             this.setLastToken(childLast);

pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/internal/NodeFindingUtil.java

@@ -0,0 +1,113 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.ast.internal;
+
+import java.util.Optional;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import net.sourceforge.pmd.lang.ast.Node;
+import net.sourceforge.pmd.lang.document.TextRegion;
+
+/**
+ * Utilities to find a node at specific text coordinates in a tree.
+ */
+public final class NodeFindingUtil {
+
+    private NodeFindingUtil() {
+        // utility class
+    }
+
+
+    /**
+     * Locates the innermost node in the subtree rooted in the given node
+     * that contains the given offset.
+     */
+    public static Optional<Node> findNodeAt(Node root, int offset) {
+        return Optional.ofNullable(findNodeImpl(root, offset));
+    }
+
+
+    /**
+     * Simple recursive search algo. Assumes that the regions of siblings
+     * do not overlap and that parents contain regions of children entirely.
+     * - We only have to explore one node at each level of the tree, and we quickly
+     * hit the bottom (average depth of a Java AST ~20-25, with 6.x.x grammar).
+     * - At each level, the next node to explore is chosen via binary search.
+     */
+    private static @Nullable Node findNodeImpl(Node subject, int offset) {
+        // deepest node containing the target offset
+        Node deepestNode = subject;
+        if (!deepestNode.getTextRegion().contains(offset)) {
+            return null;
+        }
+        while (true) {
+            Node child = binarySearchInChildren(deepestNode, offset);
+            if (child == null) {
+                // no more specific child contains the node
+                return deepestNode;
+            }
+            deepestNode = child;
+        }
+    }
+
+    // returns the child of the [parent] that contains the target
+    // it's assumed to be unique
+    private static Node binarySearchInChildren(Node parent, int offset) {
+
+        int low = 0;
+        int high = parent.getNumChildren() - 1;
+
+        while (low <= high) {
+            int mid = (low + high) / 2;
+            Node child = parent.getChild(mid);
+            TextRegion childRegion = child.getTextRegion();
+            int cmp = Integer.compare(childRegion.getStartOffset(), offset);
+
+            if (cmp < 0) {
+                // node start is before target
+                low = mid + 1;
+                if (childRegion.getEndOffset() > offset) {
+                    // node end is after target
+                    return child;
+                }
+            } else if (cmp > 0) {
+                high = mid - 1;
+            } else {
+                // target is node start position
+                return child; // key found
+            }
+        }
+        return null;  // key not found
+    }
+
+    /**
+     * Returns the innermost node that covers the entire given text range
+     * in the given tree.
+     *
+     * @param root  Root of the tree
+     * @param range Range to find
+     * @param exact If true, will return the *outermost* node whose range
+     *              is *exactly* the given text range, otherwise it may be larger.
+     */
+    public static Optional<Node> findNodeCovering(Node root, TextRegion range, boolean exact) {
+        return findNodeAt(root, range.getStartOffset()).map(innermost -> {
+            for (Node parent : innermost.ancestorsOrSelf()) {
+                TextRegion parentRange = parent.getTextRegion();
+                if (!exact && parentRange.contains(range)) {
+                    return parent;
+                } else if (exact && parentRange.equals(range)) {
+                    return parent;
+                } else if (exact && parentRange.contains(range)) {
+                    // if it isn't the same, then we can't find better so better stop looking
+                    return null;
+                }
+            }
+            return null;
+        });
+    }
+
+
+}

pmd-core/src/main/java/net/sourceforge/pmd/lang/document/TextRegion.java

@@ -184,8 +184,8 @@ public static TextRegion fromOffsetLength(int startOffset, int length) {
     /**
      * Builds a new region from start and end offset.
      *
-     * @param startOffset Start offset
-     * @param endOffset   End offset
+     * @param startOffset Start offset (inclusive)
+     * @param endOffset   End offset (exclusive)
      *
      * @throws AssertionError If either offset is negative, or the two
      *                        offsets are not ordered
@@ -230,7 +230,7 @@ public int compareTo(@NonNull TextRegion o) {
 
     @Override
     public String toString() {
-        return "Region(start=" + startOffset + ", len=" + length + ")";
+        return "Region(start=" + startOffset + ", len=" + length + ", end=" + getEndOffset() + ")";
     }
 
     @Override

pmd-core/src/main/java/net/sourceforge/pmd/reporting/RuleContext.java

@@ -10,13 +10,18 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 
 import org.apache.commons.lang3.StringUtils;
 import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
+import net.sourceforge.pmd.annotation.Experimental;
 import net.sourceforge.pmd.lang.LanguageVersionHandler;
+import net.sourceforge.pmd.lang.ast.AstInfo;
 import net.sourceforge.pmd.lang.ast.Node;
+import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken;
+import net.sourceforge.pmd.lang.ast.internal.NodeFindingUtil;
 import net.sourceforge.pmd.lang.document.FileLocation;
 import net.sourceforge.pmd.lang.document.TextRange2d;
 import net.sourceforge.pmd.lang.rule.AbstractRule;
@@ -29,7 +34,7 @@
  * This forwards events to a {@link FileAnalysisListener}. It implements
  * violation suppression by filtering some violations out, according to
  * the {@link ViolationSuppressor}s for the language.
- *
+ * <p>
  * A RuleContext contains a Rule instance and violation reporting methods
  * implicitly report only for that rule. Contrary to PMD 6, RuleContext is
  * not unique throughout the analysis, a separate one is used per file and rule.
@@ -130,22 +135,48 @@ public void addViolationWithMessage(Node location, String message, Object... for
      * @param formatArgs Format arguments for the message
      */
     public void addViolationWithPosition(Node node, int beginLine, int endLine, String message, Object... formatArgs) {
-        Objects.requireNonNull(node, "Node was null");
+        FileLocation location;
+        if (beginLine != -1 && endLine != -1) {
+            location = FileLocation.range(node.getTextDocument().getFileId(),
+                                          TextRange2d.range2d(beginLine, 1, endLine, 1));
+        } else {
+            location = node.getReportLocation();
+        }
+        addViolationWithPosition(node, node.getAstInfo(), location, message, formatArgs);
+    }
+
+    /**
+     * Record a new violation of the contextual rule, at the given location (node or token).
+     * The position is refined using the given begin and end line numbers.
+     * The given violation message ({@link Rule#getMessage()}) is treated
+     * as a format string for a {@link MessageFormat} and should hence use
+     * appropriate escapes. The given formatting arguments are used.
+     *
+     * @param reportable Location of the violation (node or token)
+     * @param astInfo    Info about the root of the tree ({@link Node#getAstInfo()})
+     * @param message    Violation message
+     * @param formatArgs Format arguments for the message
+     *
+     * @experimental This will probably never be stabilized, will instead be
+     * replaced by a fluent API or something to report violations. Do not use
+     * this outside of the PMD codebase. See https://github.com/pmd/pmd/issues/5039.
+     */
+    @Experimental
+    public void addViolationWithPosition(Reportable reportable, AstInfo<?> astInfo, FileLocation location,
+                                         String message, Object... formatArgs) {
+        Objects.requireNonNull(reportable, "Node was null");
         Objects.requireNonNull(message, "Message was null");
         Objects.requireNonNull(formatArgs, "Format arguments were null, use an empty array");
 
-        LanguageVersionHandler handler = node.getAstInfo().getLanguageProcessor().services();
+        LanguageVersionHandler handler = astInfo.getLanguageProcessor().services();
 
-        FileLocation location = node.getReportLocation();
-        if (beginLine != -1 && endLine != -1) {
-            location = FileLocation.range(location.getFileId(), TextRange2d.range2d(beginLine, 1, endLine, 1));
-        }
+        Node suppressionNode = getNearestNode(reportable, astInfo);
 
-        final Map<String, String> extraVariables = ViolationDecorator.apply(handler.getViolationDecorator(), node);
-        final String description = makeMessage(message, formatArgs, extraVariables);
-        final RuleViolation violation = new ParametricRuleViolation(rule, location, description, extraVariables);
+        Map<String, String> extraVariables = ViolationDecorator.apply(handler.getViolationDecorator(), suppressionNode);
+        String description = makeMessage(message, formatArgs, extraVariables);
+        RuleViolation violation = new ParametricRuleViolation(rule, location, description, extraVariables);
 
-        final SuppressedViolation suppressed = suppressOrNull(node, violation, handler);
+        SuppressedViolation suppressed = suppressOrNull(suppressionNode, violation, handler);
 
         if (suppressed != null) {
             listener.onSuppressedRuleViolation(suppressed);
@@ -154,6 +185,24 @@ public void addViolationWithPosition(Node node, int beginLine, int endLine, Stri
         }
     }
 
+    private Node getNearestNode(Reportable reportable, AstInfo<?> astInfo) {
+        if (reportable instanceof Node) {
+            return (Node) reportable;
+        }
+        int startOffset = getStartOffset(reportable, astInfo);
+        Optional<Node> foundNode = NodeFindingUtil.findNodeAt(astInfo.getRootNode(), startOffset);
+        // default to the root node
+        return foundNode.orElse(astInfo.getRootNode());
+    }
+
+    private static int getStartOffset(Reportable reportable, AstInfo<?> astInfo) {
+        if (reportable instanceof JavaccToken) {
+            return ((JavaccToken) reportable).getRegion().getStartOffset();
+        }
+        FileLocation loc = reportable.getReportLocation();
+        return astInfo.getTextDocument().offsetAtLineColumn(loc.getStartPos());
+    }
+
     private static @Nullable SuppressedViolation suppressOrNull(Node location, RuleViolation rv, LanguageVersionHandler handler) {
         SuppressedViolation suppressed = ViolationSuppressor.suppressOrNull(handler.getExtraViolationSuppressors(), rv, location);
         if (suppressed == null) {

pmd-core/src/test/java/net/sourceforge/pmd/lang/DummyLanguageModule.java

@@ -159,7 +159,7 @@ public static DummyRootNode readLispNode(ParserTask task) {
                     throw new ParseException("Unbalanced parentheses: " + text);
                 }
 
-                top.setRegion(TextRegion.fromBothOffsets(top.getTextRegion().getStartOffset(), i));
+                top.setRegion(TextRegion.fromBothOffsets(top.getTextRegion().getStartOffset(), i + 1));
 
                 if (top.getImage() == null) {
                     // cut out image (if node doesn't have children it hasn't been populated yet)

pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/internal/NodeFindingUtilTest.java

@@ -0,0 +1,72 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.ast.internal;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Optional;
+
+import org.junit.jupiter.api.Test;
+
+import net.sourceforge.pmd.lang.DummyLanguageModule;
+import net.sourceforge.pmd.lang.LanguageProcessor;
+import net.sourceforge.pmd.lang.LanguageProcessorRegistry;
+import net.sourceforge.pmd.lang.ast.DummyNode;
+import net.sourceforge.pmd.lang.ast.Node;
+import net.sourceforge.pmd.lang.ast.Parser;
+import net.sourceforge.pmd.lang.ast.RootNode;
+import net.sourceforge.pmd.lang.ast.SemanticErrorReporter;
+import net.sourceforge.pmd.lang.document.TextDocument;
+
+class NodeFindingUtilTest {
+
+    @Test
+    void testFindNode() {
+        DummyNode.DummyRootNode root = parseLispish("(a(b)x(c))");
+
+        assertFinds("a", 1, root);
+        assertFinds("b", 2, root);
+        assertFinds("b", 3, root);
+        assertFinds("b", 4, root);
+        assertFinds("a", 5, root);
+        assertFinds("c", 6, root);
+        assertFinds("c", 7, root);
+        assertFinds("c", 8, root);
+        assertFinds("a", 9, root);
+        assertDoesNotFind(10, root);
+    }
+
+    private static void assertDoesNotFind(int offset, DummyNode.DummyRootNode root) {
+        assertFalse(NodeFindingUtil.findNodeAt(root, offset).isPresent());
+    }
+
+    static void assertFinds(String nodeImage, int offset, Node root) {
+        Optional<Node> found = NodeFindingUtil.findNodeAt(root, offset);
+
+        assertTrue(found.isPresent(), "Node not found: " + nodeImage + " at offset " + offset);
+        assertThat(found.get().getImage(), equalTo(nodeImage));
+    }
+
+    static DummyNode.DummyRootNode parseLispish(String source) {
+
+        DummyLanguageModule lang = DummyLanguageModule.getInstance();
+        try (LanguageProcessor processor = lang.createProcessor(lang.newPropertyBundle())) {
+            Parser.ParserTask task = new Parser.ParserTask(
+                TextDocument.readOnlyString(source, lang.getDefaultVersion()),
+                SemanticErrorReporter.noop(),
+                LanguageProcessorRegistry.singleton(processor)
+            );
+            RootNode root = processor.services().getParser().parse(task);
+
+            return (DummyNode.DummyRootNode) root;
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+
+    }
+}