[java] Fix #2996 - CommentSize 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);
+ }
+
+ }
+}