From e4a5e4556c8408b19b814265f26fc6449d032135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 16 Nov 2020 15:54:57 +0100 Subject: [PATCH 01/20] Make Comment not implement Node --- .../pmd/lang/java/ast/Comment.java | 45 ++++++++++--------- .../pmd/lang/java/ast/FormalComment.java | 34 +++----------- .../pmd/lang/java/ast/MultiLineComment.java | 5 --- .../pmd/lang/java/ast/SingleLineComment.java | 5 --- 4 files changed, 31 insertions(+), 58 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java index a4bbde6182..c90cc6cc67 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java @@ -13,11 +13,10 @@ import java.util.regex.Pattern; import org.apache.commons.lang3.StringUtils; import net.sourceforge.pmd.PMD; -import net.sourceforge.pmd.lang.ast.impl.javacc.AbstractJjtreeNode; import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken; -import net.sourceforge.pmd.util.document.FileLocation; +import net.sourceforge.pmd.util.document.Chars; -public abstract class Comment extends AbstractJjtreeNode { +public abstract class Comment { // single regex, that captures: the start of a multi-line comment (/**|/*), the start of a single line comment (//) // or the start of line within a multiline comment (*). It removes the end of the comment (*/) if existing. @@ -29,28 +28,15 @@ public abstract class Comment extends AbstractJjtreeNode { private final JavaccToken token; protected Comment(JavaccToken t) { - super(0); this.token = t; - setFirstToken(t); - setLastToken(t); - } - - @Override - public FileLocation getReportLocation() { - return token.getReportLocation(); - } - - /** - * @deprecated Use {@link #getText()} - */ - @Override - @Deprecated - public String getImage() { - return super.getImage(); } public final JavaccToken getToken() { - return super.getFirstToken(); + return token; + } + + public int compareLocation(Comment other) { + return getToken().compareTo(other.getToken()); } /** @@ -90,11 +76,28 @@ public abstract class Comment extends AbstractJjtreeNode { return filteredLines; } + public Chars getText() { + return (Chars) getToken().getImageCs(); + } + + public int getBeginLine() { + return getToken().getBeginLine(); + } + + public int getEndLine() { + return getToken().getEndLine(); + } + + public String getImage() { + return getToken().getImage(); + } + /** * Similar to the String.trim() function, this one removes the leading and * trailing empty/blank lines from the line list. * * @param lines the list of lines, which might contain empty lines + * * @return the lines without leading or trailing blank lines. */ // note: this is only package private, since it is used by CommentUtil. Once CommentUtil is gone, this diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/FormalComment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/FormalComment.java index 9b5bb59c79..1a2f72a5d2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/FormalComment.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/FormalComment.java @@ -4,45 +4,25 @@ package net.sourceforge.pmd.lang.java.ast; -import java.util.ArrayList; -import java.util.List; -import java.util.regex.Matcher; -import java.util.regex.Pattern; +import org.checkerframework.checker.nullness.qual.Nullable; import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken; -import net.sourceforge.pmd.lang.java.javadoc.JavadocTag; public class FormalComment extends Comment { - private static final Pattern JAVADOC_TAG = Pattern.compile("@([A-Za-z0-9]+)"); + private JavadocCommentOwner owner; public FormalComment(JavaccToken t) { super(t); assert t.kind == JavaTokenKinds.FORMAL_COMMENT; - findJavadocs(); } - @Override - public String getXPathNodeName() { - return "FormalComment"; + void setOwner(JavadocCommentOwner owner) { + this.owner = owner; } - private void findJavadocs() { - List kids = new ArrayList<>(); - - Matcher javadocTagMatcher = JAVADOC_TAG.matcher(getFilteredComment()); - while (javadocTagMatcher.find()) { - JavadocTag tag = JavadocTag.tagFor(javadocTagMatcher.group(1)); - int tagStartIndex = javadocTagMatcher.start(1); - if (tag != null) { - kids.add(new JavadocElement(getFirstToken(), getBeginLine(), getBeginLine(), - // TODO valid? - tagStartIndex, tagStartIndex + tag.label.length() + 1, tag)); - } - } - - for (int i = kids.size() - 1; i >= 0; i--) { - addChild(kids.get(i), i); - } + public @Nullable JavadocCommentOwner getOwner() { + return owner; } + } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/MultiLineComment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/MultiLineComment.java index c8532e9a00..f504050b44 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/MultiLineComment.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/MultiLineComment.java @@ -13,9 +13,4 @@ public class MultiLineComment extends Comment { } - @Override - public String getXPathNodeName() { - return "MultiLineComment"; - } - } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/SingleLineComment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/SingleLineComment.java index f02ba836d0..6fb0c99b0d 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/SingleLineComment.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/SingleLineComment.java @@ -12,9 +12,4 @@ public class SingleLineComment extends Comment { super(t); } - - @Override - public String getXPathNodeName() { - return "SingleLineComment"; - } } From 5a1b898a85ee6126b26d24f9aabe0cbdb51714f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 16 Nov 2020 16:06:52 +0100 Subject: [PATCH 02/20] Refactor CommentSize --- .../rule/documentation/CommentSizeRule.java | 73 ++++++++++--------- 1 file changed, 40 insertions(+), 33 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java index a00d8a1771..efccc22cb2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java @@ -18,7 +18,7 @@ import net.sourceforge.pmd.lang.java.ast.Comment; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.properties.PropertyDescriptor; import net.sourceforge.pmd.properties.PropertyFactory; -import net.sourceforge.pmd.util.StringUtil; +import net.sourceforge.pmd.util.document.Chars; /** * A rule to manage those who just can't shut up... @@ -33,20 +33,24 @@ public class CommentSizeRule extends AbstractJavaRule { .require(positive()).defaultValue(6).build(); public static final PropertyDescriptor MAX_LINE_LENGTH - = PropertyFactory.intProperty("maxLineLength") - .desc("Maximum line length") - .require(positive()).defaultValue(80).build(); + = PropertyFactory.intProperty("maxLineLength") + .desc("Maximum line length") + .require(positive()).defaultValue(80).build(); private static final String CR = "\n"; - static final Set IGNORED_LINES = setOf("//", "/*", "/**", "*", "*/"); + static final Set IGNORED_LINES = setOf(Chars.wrap("//"), + Chars.wrap("/*"), + Chars.wrap("/**"), + Chars.wrap("*"), + Chars.wrap("*/")); public CommentSizeRule() { definePropertyDescriptor(MAX_LINES); definePropertyDescriptor(MAX_LINE_LENGTH); } - private static boolean hasRealText(String line) { + private static boolean hasRealText(Chars line) { if (StringUtils.isBlank(line)) { return false; @@ -57,30 +61,35 @@ public class CommentSizeRule extends AbstractJavaRule { private boolean hasTooManyLines(Comment comment) { - String[] lines = comment.getImage().split(CR); - - int start = 0; // start from top - for (; start < lines.length; start++) { - if (hasRealText(lines[start])) { - break; + int firstLineWithText = -1; + int lastLineWithText = 0; + int i = 0; + for (Chars line : comment.getText().lines()) { + boolean real = hasRealText(line); + if (real) { + lastLineWithText = i; + if (firstLineWithText == -1) { + firstLineWithText = i; + } } + i++; } - int end = lines.length - 1; // go up from bottom - for (; end > 0; end--) { - if (hasRealText(lines[end])) { - break; - } - } - - int lineCount = end - start + 1; + int lineCount = lastLineWithText - firstLineWithText + 1; return lineCount > getProperty(MAX_LINES); } - private String withoutCommentMarkup(String text) { - - return StringUtil.withoutPrefixes(text.trim(), "//", "*", "/**"); + private Chars withoutCommentMarkup(Chars text) { + text = text.trimStart(); + if (text.startsWith("//")) { + return text.subSequence(2, text.length()); + } else if (text.startsWith('*', 0)) { + return text.subSequence(1, text.length()); + } else if (text.startsWith("/**")) { + return text.subSequence(3, text.length()); + } + return text; } private List overLengthLineIndicesIn(Comment comment) { @@ -88,17 +97,13 @@ public class CommentSizeRule extends AbstractJavaRule { int maxLength = getProperty(MAX_LINE_LENGTH); List indices = new ArrayList<>(); - String[] lines = comment.getImage().split(CR); - - int offset = comment.getBeginLine(); - - for (int i = 0; i < lines.length; i++) { - String cleaned = withoutCommentMarkup(lines[i]); - if (cleaned.length() > maxLength) { - indices.add(i + offset); + int i = 0; + for (Chars line : comment.getText().lines()) { + line = withoutCommentMarkup(line); + if (line.length() > maxLength) { + indices.add(i); } } - return indices; } @@ -116,7 +121,9 @@ public class CommentSizeRule extends AbstractJavaRule { continue; } - for (Integer lineNum : lineNumbers) { + int offset = comment.getBeginLine(); + for (int lineNum : lineNumbers) { + lineNum += offset; addViolationWithMessage(data, cUnit, this.getMessage() + ": Line too long", lineNum, lineNum); } } From e926c9423daa4e6b29ae09e2b0e242865e3d610a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 16 Nov 2020 16:30:07 +0100 Subject: [PATCH 03/20] Refactor CommentDefaultAccessModifier --- .../pmd/lang/java/ast/ASTBlock.java | 2 +- .../pmd/lang/java/ast/Comment.java | 4 + .../CommentDefaultAccessModifierRule.java | 80 ++++++++----------- .../resources/category/java/codestyle.xml | 2 +- .../CommentDefaultAccessModifierTest.java | 1 - .../xml/CommentDefaultAccessModifier.xml | 17 ++-- 6 files changed, 48 insertions(+), 58 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTBlock.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTBlock.java index 3d9e998072..828e862088 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTBlock.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTBlock.java @@ -33,7 +33,7 @@ public final class ASTBlock extends ASTMaybeEmptyListOf implements public boolean containsComment() { JavaccToken t = getLastToken().getPreviousComment(); while (t != null) { - if (JavaTokenDocument.isComment(t)) { + if (Comment.isComment(t)) { return true; } t = t.getPreviousComment(); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java index c90cc6cc67..16e3def7c8 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java @@ -39,6 +39,10 @@ public abstract class Comment { return getToken().compareTo(other.getToken()); } + public static boolean isComment(JavaccToken token) { + return JavaTokenDocument.isComment(token); + } + /** * Filters the comment by removing the leading comment marker (like {@code *}) of each line * as well as the start markers ({@code //}, {@code /*} or {@code /**} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java index 257629321f..5f3e554954 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java @@ -6,23 +6,19 @@ package net.sourceforge.pmd.lang.java.rule.codestyle; import java.util.ArrayList; import java.util.Collection; -import java.util.HashSet; -import java.util.List; -import java.util.Set; import java.util.regex.Pattern; +import net.sourceforge.pmd.lang.ast.GenericToken; +import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken; import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclarator; -import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId; +import net.sourceforge.pmd.lang.java.ast.ASTModifierList; import net.sourceforge.pmd.lang.java.ast.AccessNode; -import net.sourceforge.pmd.lang.java.ast.Annotatable; import net.sourceforge.pmd.lang.java.ast.Comment; import net.sourceforge.pmd.lang.java.rule.AbstractIgnoredAnnotationRule; import net.sourceforge.pmd.properties.PropertyDescriptor; @@ -43,9 +39,6 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR private static final PropertyDescriptor TOP_LEVEL_TYPES = PropertyFactory.booleanProperty("checkTopLevelTypes") .desc("Check for default access modifier in top-level classes, annotations, and enums") .defaultValue(false).build(); - private static final String MESSAGE = "To avoid mistakes add a comment " - + "at the beginning of the %s %s if you want a default access modifier"; - private final Set interestingLineNumberComments = new HashSet<>(); public CommentDefaultAccessModifierRule() { definePropertyDescriptor(REGEX_DESCRIPTOR); @@ -60,23 +53,10 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR return ignoredStrings; } - @Override - public Object visit(final ASTCompilationUnit node, final Object data) { - interestingLineNumberComments.clear(); - final List comments = node.getComments(); - for (final Comment comment : comments) { - if (getProperty(REGEX_DESCRIPTOR).matcher(comment.getImage()).matches()) { - interestingLineNumberComments.add(comment.getBeginLine()); - } - } - return super.visit(node, data); - } - @Override public Object visit(final ASTMethodDeclaration decl, final Object data) { if (shouldReport(decl)) { - addViolationWithMessage(data, decl, - String.format(MESSAGE, decl.getFirstChildOfType(ASTMethodDeclarator.class).getImage(), "method")); + addViolation(data, decl, decl.getName(), "method"); } return super.visit(decl, data); } @@ -84,8 +64,7 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR @Override public Object visit(final ASTFieldDeclaration decl, final Object data) { if (shouldReport(decl)) { - addViolationWithMessage(data, decl, String.format(MESSAGE, - decl.getFirstDescendantOfType(ASTVariableDeclaratorId.class).getImage(), "field")); + addViolation(data, decl, decl.getVarIds().firstOrThrow().getName(), "field"); } return super.visit(decl, data); } @@ -93,7 +72,7 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR @Override public Object visit(final ASTAnnotationTypeDeclaration decl, final Object data) { if (!decl.isNested() && shouldReportTypeDeclaration(decl)) { // check for top-level annotation declarations - addViolationWithMessage(data, decl, String.format(MESSAGE, decl.getImage(), "top-level annotation")); + addViolation(data, decl, decl.getSimpleName(), "top-level annotation"); } return super.visit(decl, data); } @@ -101,7 +80,7 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR @Override public Object visit(final ASTEnumDeclaration decl, final Object data) { if (!decl.isNested() && shouldReportTypeDeclaration(decl)) { // check for top-level enums - addViolationWithMessage(data, decl, String.format(MESSAGE, decl.getImage(), "top-level enum")); + addViolation(data, decl, decl.getSimpleName(), "top-level enum"); } return super.visit(decl, data); } @@ -109,9 +88,9 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR @Override public Object visit(final ASTClassOrInterfaceDeclaration decl, final Object data) { if (decl.isNested() && shouldReport(decl)) { // check for nested classes - addViolationWithMessage(data, decl, String.format(MESSAGE, decl.getImage(), "nested class")); + addViolation(data, decl, decl.getSimpleName(), "nested class"); } else if (!decl.isNested() && shouldReportTypeDeclaration(decl)) { // and for top-level ones - addViolationWithMessage(data, decl, String.format(MESSAGE, decl.getImage(), "top-level class")); + addViolation(data, decl, decl.getSimpleName(), "top-level class"); } return super.visit(decl, data); } @@ -119,7 +98,7 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR @Override public Object visit(final ASTConstructorDeclaration decl, Object data) { if (shouldReport(decl)) { - addViolationWithMessage(data, decl, String.format(MESSAGE, decl.getImage(), "constructor")); + addViolation(data, decl, decl.getName(), "constructor"); } return super.visit(decl, data); } @@ -133,29 +112,36 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR return isConcreteClass && isMissingComment(decl); } - protected boolean hasIgnoredAnnotation(AccessNode node) { - if (node instanceof Annotatable) { - return hasIgnoredAnnotation((Annotatable) node); - } - return false; - } - private boolean isMissingComment(AccessNode decl) { // check if the class/method/field has a default access // modifier return decl.isPackagePrivate() - // if is a default access modifier check if there is a comment - // in this line - && !interestingLineNumberComments.contains(decl.getBeginLine()) - // that it is not annotated with e.g. @VisibleForTesting - && !hasIgnoredAnnotation(decl); + // if is a default access modifier check if there is a comment + // in this line + && !hasOkComment(decl) + // that it is not annotated with e.g. @VisibleForTesting + && !hasIgnoredAnnotation(decl); + } + + private boolean hasOkComment(AccessNode node) { + ASTModifierList modifiers = node.getModifiers(); + Iterable tokens = () -> GenericToken.range(modifiers.getFirstToken(), modifiers.getLastToken()); + Pattern regex = getProperty(REGEX_DESCRIPTOR); + for (JavaccToken token : tokens) { + for (JavaccToken special : GenericToken.previousSpecials(token)) { + if (Comment.isComment(special)) { + return regex.matcher(special.getImageCs()).matches(); + } + } + } + return false; } private boolean shouldReportTypeDeclaration(ASTAnyTypeDeclaration decl) { // don't report on interfaces - return !decl.isInterface() - && isMissingComment(decl) - // either nested or top level and we should check it - && (decl.isNested() || getProperty(TOP_LEVEL_TYPES)); + return !(decl.isInterface() && !decl.isAnnotation()) + && isMissingComment(decl) + // either nested or top level and we should check it + && (decl.isNested() || getProperty(TOP_LEVEL_TYPES)); } } diff --git a/pmd-java/src/main/resources/category/java/codestyle.xml b/pmd-java/src/main/resources/category/java/codestyle.xml index ad5337c4fd..17ece19544 100644 --- a/pmd-java/src/main/resources/category/java/codestyle.xml +++ b/pmd-java/src/main/resources/category/java/codestyle.xml @@ -268,7 +268,7 @@ public class Éléphant {} language="java" since="5.4.0" class="net.sourceforge.pmd.lang.java.rule.codestyle.CommentDefaultAccessModifierRule" - message="Missing commented default access modifier" + message="To avoid mistakes, add a comment at the beginning of the {1} ''{0}'' if you want a default access modifier" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_codestyle.html#commentdefaultaccessmodifier"> To avoid mistakes if we want that an Annotation, Class, Enum, Method, Constructor or Field have a default access modifier diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierTest.java index b175c0b70a..d0ad862dad 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierTest.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.codestyle; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") public class CommentDefaultAccessModifierTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/CommentDefaultAccessModifier.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/CommentDefaultAccessModifier.xml index 00d81cfa78..696cd8bb86 100755 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/CommentDefaultAccessModifier.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/CommentDefaultAccessModifier.xml @@ -119,6 +119,7 @@ public class Foo { Top-level annotations with default access modifier checks enabled with property true 2 - 1,6 + 2,7 @@ -154,7 +155,7 @@ public enum Foo {} @SomeAnnotation enum Baz {} -@VisibleForTesting +@com.google.common.annotations.VisibleForTesting enum Foobar {} /* default */ enum FoobarWithComment {} @@ -182,7 +183,7 @@ public class Foo {} @SomeAnnotation class Baz {} -@VisibleForTesting +@com.google.common.annotations.VisibleForTesting class Foobar {} /* default */ class FoobarWithComment {} @@ -245,7 +246,7 @@ public enum TestEnum { 0 @@ -328,7 +329,7 @@ public interface A { 1 6 - To avoid mistakes add a comment at the beginning of the method method if you want a default access modifier + To avoid mistakes, add a comment at the beginning of the method 'method' if you want a default access modifier 2 2,4 - To avoid mistakes add a comment at the beginning of the Foo constructor if you want a default access modifier - To avoid mistakes add a comment at the beginning of the Foo constructor if you want a default access modifier - + To avoid mistakes, add a comment at the beginning of the constructor 'Foo' if you want a default access modifier + To avoid mistakes, add a comment at the beginning of the constructor 'Foo' if you want a default access modifier + From 1baf797b7177c1ea59ce59af05d76c2a8bc40baf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 16 Nov 2020 16:37:27 +0100 Subject: [PATCH 04/20] Improve token range ergonomics --- .../net/sourceforge/pmd/lang/ast/GenericToken.java | 12 +++++++++--- .../pmd/lang/ast/impl/javacc/JjtreeNode.java | 8 ++++++++ .../pmd/lang/java/ast/AstDisambiguationPass.java | 2 +- .../sourceforge/pmd/lang/java/ast/TokenUtils.java | 4 ---- .../codestyle/CommentDefaultAccessModifierRule.java | 3 +-- .../sourceforge/pmd/lang/java/ast/TestExtensions.kt | 2 +- 6 files changed, 20 insertions(+), 11 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java index f8edf58f7a..7bdf1fb302 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java @@ -4,7 +4,7 @@ package net.sourceforge.pmd.lang.ast; -import java.util.Iterator; +import java.util.stream.Stream; import org.apache.commons.lang3.StringUtils; @@ -106,13 +106,19 @@ public interface GenericToken> extends Comparable, * * @throws IllegalArgumentException If the first token does not come before the other token */ - static > Iterator range(T from, T to) { + static > Iterable range(T from, T to) { if (from.compareTo(to) > 0) { throw new IllegalArgumentException(from + " must come before " + to); } - return IteratorUtil.generate(from, t -> t == to ? null : t.getNext()); + return () -> IteratorUtil.generate(from, t -> t == to ? null : t.getNext()); } + /** + * Returns a stream corresponding to {@link #range(GenericToken, GenericToken)}. + */ + static > Stream streamRange(T from, T to) { + return IteratorUtil.toStream(range(from, to).iterator()); + } /** * Returns an iterable that enumerates all special tokens belonging diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/javacc/JjtreeNode.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/javacc/JjtreeNode.java index 8d9ad2c53e..c79e3926f3 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/javacc/JjtreeNode.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/javacc/JjtreeNode.java @@ -4,6 +4,7 @@ package net.sourceforge.pmd.lang.ast.impl.javacc; +import net.sourceforge.pmd.lang.ast.GenericToken; import net.sourceforge.pmd.lang.ast.TextAvailableNode; import net.sourceforge.pmd.lang.ast.impl.GenericNode; import net.sourceforge.pmd.util.document.Chars; @@ -27,4 +28,11 @@ public interface JjtreeNode> extends GenericNode, Tex JavaccToken getLastToken(); + /** + * Returns a token range, that includes the first and last token. + */ + default Iterable tokens() { + return GenericToken.range(getFirstToken(), getLastToken()); + } + } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AstDisambiguationPass.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AstDisambiguationPass.java index b801c393ec..e508621d9e 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AstDisambiguationPass.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AstDisambiguationPass.java @@ -253,7 +253,7 @@ final class AstDisambiguationPass { * and in the worst case, the original {@link ASTAmbiguousName}. */ private static ASTExpression startResolve(ASTAmbiguousName name, ReferenceCtx ctx, boolean isPackageOrTypeOnly) { - Iterator tokens = TokenUtils.tokenRange(name); + Iterator tokens = name.tokens().iterator(); JavaccToken firstIdent = tokens.next(); TokenUtils.expectKind(firstIdent, JavaTokenKinds.IDENTIFIER); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TokenUtils.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TokenUtils.java index c0d48d1617..2c0ee6eb0e 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TokenUtils.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TokenUtils.java @@ -4,7 +4,6 @@ package net.sourceforge.pmd.lang.java.ast; -import java.util.Iterator; import java.util.NoSuchElementException; import java.util.Objects; @@ -91,7 +90,4 @@ final class TokenUtils { assert token.kind == kind : "Expected " + token.getDocument().describeKind(kind) + ", got " + token; } - public static Iterator tokenRange(JavaNode node) { - return GenericToken.range(node.getFirstToken(), node.getLastToken()); - } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java index 5f3e554954..2e8cbdb361 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java @@ -125,9 +125,8 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR private boolean hasOkComment(AccessNode node) { ASTModifierList modifiers = node.getModifiers(); - Iterable tokens = () -> GenericToken.range(modifiers.getFirstToken(), modifiers.getLastToken()); Pattern regex = getProperty(REGEX_DESCRIPTOR); - for (JavaccToken token : tokens) { + for (JavaccToken token : GenericToken.range(modifiers.getFirstToken(), modifiers.getLastToken())) { for (JavaccToken special : GenericToken.previousSpecials(token)) { if (Comment.isComment(special)) { return regex.matcher(special.getImageCs()).matches(); diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt index 63f67df989..deb478b4fb 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt @@ -45,7 +45,7 @@ fun haveVisibility(vis: AccessNode.Visibility): Matcher = object : M } fun JavaNode.tokenList(): List = - IteratorUtil.toList(TokenUtils.tokenRange(this)) + tokens().toList() fun String.addArticle() = when (this[0].toLowerCase()) { 'a', 'e', 'i', 'o', 'u' -> "an $this" From 066675cf44ca2c3519ebd7f7b975b31ddd7f258a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 16 Nov 2020 16:51:13 +0100 Subject: [PATCH 05/20] Update rule CommentContent --- .../sourceforge/pmd/util/document/Chars.java | 26 ++++++ .../documentation/CommentContentRule.java | 83 +++++-------------- 2 files changed, 48 insertions(+), 61 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/document/Chars.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/document/Chars.java index ebd4dc609e..73f9c2b3cc 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/document/Chars.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/document/Chars.java @@ -12,10 +12,13 @@ import java.nio.ByteBuffer; import java.nio.CharBuffer; import java.nio.charset.Charset; import java.util.Iterator; +import java.util.regex.Matcher; import java.util.regex.Pattern; import org.checkerframework.checker.nullness.qual.NonNull; +import net.sourceforge.pmd.internal.util.IteratorUtil.AbstractIterator; + /** * View on a string which doesn't copy the array for subsequence operations. * This view is immutable. Since it uses a string internally it benefits from @@ -444,6 +447,29 @@ public final class Chars implements CharSequence { }; } + /** + * Split this slice into subslices, like {@link String#split(String)}, + * except it's iterated lazily. Results excludes the delimiter + */ + public Iterable splits(Pattern regex) { + return () -> new AbstractIterator() { + final Matcher matcher = regex.matcher(Chars.this); + int lastPos = 0; + + @Override + protected void computeNext() { + if (matcher.find()) { + setNext(subSequence(lastPos, matcher.start())); + lastPos = matcher.end(); + } else { + if (lastPos != len) { + setNext(subSequence(lastPos, len)); + } + done(); + } + } + }; + } /** * Returns a new reader for the whole contents of this char sequence. diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java index eac9932f67..e20ba8e45b 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java @@ -9,19 +9,14 @@ import static net.sourceforge.pmd.properties.PropertyFactory.stringListProperty; import java.util.ArrayList; import java.util.Collections; -import java.util.HashSet; import java.util.List; -import java.util.Locale; -import java.util.Set; +import java.util.regex.Pattern; -import org.apache.commons.lang3.StringUtils; - -import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.Comment; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.properties.PropertyDescriptor; -import net.sourceforge.pmd.properties.PropertySource; +import net.sourceforge.pmd.util.document.Chars; /** * A rule that checks for illegal words in the comment text. @@ -32,9 +27,7 @@ import net.sourceforge.pmd.properties.PropertySource; */ public class CommentContentRule extends AbstractJavaRule { - private boolean caseSensitive; - private List originalBadWords; - private List currentBadWords; + private static final Pattern WHITESPACE = Pattern.compile("\\s+"); // ignored when property above == True public static final PropertyDescriptor CASE_SENSITIVE_DESCRIPTOR = booleanProperty("caseSensitive").defaultValue(false).desc("Case sensitive").build(); @@ -44,55 +37,32 @@ public class CommentContentRule extends AbstractJavaRule { .desc("Illegal terms or phrases") .defaultValues("idiot", "jerk").build(); // TODO make blank property? or add more defaults? - private static final Set> NON_REGEX_PROPERTIES; - - static { - NON_REGEX_PROPERTIES = new HashSet<>(1); - NON_REGEX_PROPERTIES.add(CASE_SENSITIVE_DESCRIPTOR); - } - public CommentContentRule() { definePropertyDescriptor(CASE_SENSITIVE_DESCRIPTOR); definePropertyDescriptor(DISSALLOWED_TERMS_DESCRIPTOR); } - /** - * Capture values and perform all the case-conversions once per run - */ - @Override - public void start(RuleContext ctx) { - originalBadWords = getProperty(DISSALLOWED_TERMS_DESCRIPTOR); - caseSensitive = getProperty(CASE_SENSITIVE_DESCRIPTOR); - if (caseSensitive) { - currentBadWords = originalBadWords; - } else { - currentBadWords = new ArrayList<>(); - for (String badWord : originalBadWords) { - currentBadWords.add(badWord.toUpperCase(Locale.ROOT)); - } - } - } + private List illegalTermsIn(Comment comment, List badWords, boolean caseSensitive) { - private List illegalTermsIn(Comment comment) { - - if (currentBadWords.isEmpty()) { + if (badWords.isEmpty()) { return Collections.emptyList(); } - String commentText = comment.getFilteredComment(); - if (StringUtils.isBlank(commentText)) { - return Collections.emptyList(); - } - - if (!caseSensitive) { - commentText = commentText.toUpperCase(Locale.ROOT); - } - List foundWords = new ArrayList<>(); + for (Chars word : comment.getText().splits(WHITESPACE)) { + if (word.length() <= 3 && + (word.contentEquals("*") + || word.contentEquals("//") + || word.contentEquals("/*") + || word.contentEquals("*/") + || word.contentEquals("/**"))) { + continue; + } - for (int i = 0; i < currentBadWords.size(); i++) { - if (commentText.contains(currentBadWords.get(i))) { - foundWords.add(originalBadWords.get(i)); + for (String badWord : badWords) { + if (word.contentEquals(badWord, !caseSensitive)) { + foundWords.add(badWord); + } } } @@ -118,12 +88,11 @@ public class CommentContentRule extends AbstractJavaRule { public Object visit(ASTCompilationUnit cUnit, Object data) { // NPE patch: Eclipse plugin doesn't call start() at onset? - if (currentBadWords == null) { - start(null); - } + List currentBadWords = getProperty(DISSALLOWED_TERMS_DESCRIPTOR); + boolean caseSensitive = getProperty(CASE_SENSITIVE_DESCRIPTOR); for (Comment comment : cUnit.getComments()) { - List badWords = illegalTermsIn(comment); + List badWords = illegalTermsIn(comment, currentBadWords, caseSensitive); if (badWords.isEmpty()) { continue; } @@ -139,16 +108,8 @@ public class CommentContentRule extends AbstractJavaRule { return !terms.isEmpty(); } - @Deprecated - public boolean hasDissallowedTerms() { - return this.hasDisallowedTerms(); - } - - /** - * @see PropertySource#dysfunctionReason() - */ @Override public String dysfunctionReason() { - return hasDissallowedTerms() ? null : "No disallowed terms specified"; + return hasDisallowedTerms() ? null : "No disallowed terms specified"; } } From f72953cefcbb9cbae693620476672b98dab52239 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 16 Nov 2020 17:06:12 +0100 Subject: [PATCH 06/20] Extract utilities --- .../sourceforge/pmd/util/document/Chars.java | 15 ++ .../pmd/lang/java/ast/Comment.java | 162 ++++++++---------- .../documentation/CommentContentRule.java | 7 +- .../rule/documentation/CommentSizeRule.java | 14 +- .../xpath/internal/GetCommentOnFunction.java | 2 +- .../lang/java/ast/CommentAssignmentTest.java | 5 +- .../pmd/lang/java/ast/CommentTest.java | 16 +- 7 files changed, 105 insertions(+), 116 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/document/Chars.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/document/Chars.java index 73f9c2b3cc..42044c7696 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/document/Chars.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/document/Chars.java @@ -53,6 +53,10 @@ public final class Chars implements CharSequence { return this.start + off; } + public boolean isEmpty() { + return len == 0; + } + /** * Wraps the given char sequence into a {@link Chars}. This may @@ -257,6 +261,17 @@ public final class Chars implements CharSequence { return trimStart().trimEnd(); } + /** + * Remove the suffix if it is present, otherwise returns this. + */ + public Chars removeSuffix(String charSeq) { + int trimmedLen = length() - charSeq.length(); + if (startsWith(charSeq, trimmedLen)) { + return slice(0, trimmedLen); + } + return this; + } + /** * Returns true if this char sequence is logically equal to the diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java index 16e3def7c8..8726199aea 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java @@ -4,131 +4,121 @@ package net.sourceforge.pmd.lang.java.ast; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -import org.apache.commons.lang3.StringUtils; - -import net.sourceforge.pmd.PMD; +import net.sourceforge.pmd.internal.util.IteratorUtil; import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken; import net.sourceforge.pmd.util.document.Chars; +import net.sourceforge.pmd.util.document.FileLocation; +import net.sourceforge.pmd.util.document.Reportable; -public abstract class Comment { - - // single regex, that captures: the start of a multi-line comment (/**|/*), the start of a single line comment (//) - // or the start of line within a multiline comment (*). It removes the end of the comment (*/) if existing. - private static final Pattern COMMENT_LINE_COMBINED = Pattern.compile("^(?://|/\\*\\*?|\\*)?(.*?)(?:\\*/|/)?$"); - - // Same as "\\R" - but \\R is only available with java8+ - static final Pattern NEWLINES_PATTERN = Pattern.compile("\\u000D\\u000A|[\\u000A\\u000B\\u000C\\u000D\\u0085\\u2028\\u2029]"); +/** + * Wraps a comment token to provide some utilities. + * This is not a node, it's not part of the tree anywhere, + * just convenient. + * + * TODO subclasses are useless + * TODO maybe move part of this into pmd core + */ +public abstract class Comment implements Reportable { private final JavaccToken token; - protected Comment(JavaccToken t) { + Comment(JavaccToken t) { this.token = t; } + /** The token underlying this comment. */ public final JavaccToken getToken() { return token; } + public boolean isSingleLine() { + return token.kind == JavaTokenKinds.SINGLE_LINE_COMMENT; + } + + public boolean hasJavadocContent() { + return token.kind == JavaTokenKinds.FORMAL_COMMENT; + } + + /** Returns the full text of the comment. */ + public Chars getText() { + // todo remove this cast + return (Chars) getToken().getImageCs(); + } + + @Override + public FileLocation getReportLocation() { + return getToken().getReportLocation(); + } + public int compareLocation(Comment other) { return getToken().compareTo(other.getToken()); } + /** + * Returns true if the given token has the kind + * of a comment token (there are three such kinds). + */ public static boolean isComment(JavaccToken token) { return JavaTokenDocument.isComment(token); } - /** - * Filters the comment by removing the leading comment marker (like {@code *}) of each line - * as well as the start markers ({@code //}, {@code /*} or {@code /**} - * and the end markers (*/). - * Also leading and trailing empty lines are removed. - * - * @return the filtered comment - */ - public String getFilteredComment() { - List lines = multiLinesIn(); - lines = trim(lines); - return StringUtils.join(lines, PMD.EOL); - } - /** * Removes the leading comment marker (like {@code *}) of each line * of the comment as well as the start marker ({@code //}, {@code /*} or {@code /**} * and the end markers (*/). * + *

Empty lines are removed. + * * @return List of lines of the comments */ - private List multiLinesIn() { - String[] lines = NEWLINES_PATTERN.split(getText()); - List filteredLines = new ArrayList<>(lines.length); - - for (String rawLine : lines) { - String line = rawLine.trim(); - - Matcher allMatcher = COMMENT_LINE_COMBINED.matcher(line); - if (allMatcher.matches()) { - filteredLines.add(allMatcher.group(1).trim()); + public Iterable filteredLines() { + return () -> IteratorUtil.mapNotNull( + getText().lines().iterator(), + line -> { + line = removeCommentMarkup(line); + return line.isEmpty() ? null : line; } - } - - return filteredLines; - } - - public Chars getText() { - return (Chars) getToken().getImageCs(); - } - - public int getBeginLine() { - return getToken().getBeginLine(); - } - - public int getEndLine() { - return getToken().getEndLine(); + ); } + /** + * @deprecated Use {@link #getText()} to avoid array copies + */ + @Deprecated public String getImage() { return getToken().getImage(); } /** - * Similar to the String.trim() function, this one removes the leading and - * trailing empty/blank lines from the line list. - * - * @param lines the list of lines, which might contain empty lines - * - * @return the lines without leading or trailing blank lines. + * True if this is a comment delimiter or an asterisk. This + * tests the whole parameter and not a prefix/suffix. */ - // note: this is only package private, since it is used by CommentUtil. Once CommentUtil is gone, this - // can be private - static List trim(List lines) { - if (lines == null) { - return Collections.emptyList(); - } + public static boolean isMarkupWord(Chars word) { + return word.length() <= 3 && + (word.contentEquals("*") + || word.contentEquals("//") + || word.contentEquals("/*") + || word.contentEquals("*/") + || word.contentEquals("/**")); + } - List result = new ArrayList<>(lines.size()); - List tempList = new ArrayList<>(); - boolean foundFirstNonEmptyLine = false; - for (String line : lines) { - if (StringUtils.isNotBlank(line)) { - // new non-empty line: add all previous empty lines occurred before - result.addAll(tempList); - tempList.clear(); - result.add(line); - - foundFirstNonEmptyLine = true; - } else { - if (foundFirstNonEmptyLine) { - // add the empty line to a temporary list first - tempList.add(line); - } + /** + * Trim the start of the provided line to remove a comment + * markup opener ({@code //, /*, /**, *}) or closer {@code * /}. + */ + public static Chars removeCommentMarkup(Chars line) { + line = line.trim().removeSuffix("*/"); + int subseqFrom = 0; + if (line.startsWith('/', 0)) { + if (line.startsWith("**", 1)) { + subseqFrom = 3; + } else if (line.startsWith('/', 1) + || line.startsWith('*', 1)) { + subseqFrom = 2; } + } else if (line.startsWith('*', 0)) { + subseqFrom = 1; } - return result; + return line.subSequence(subseqFrom, line.length()).trim(); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java index e20ba8e45b..0d71c34804 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java @@ -50,12 +50,7 @@ public class CommentContentRule extends AbstractJavaRule { List foundWords = new ArrayList<>(); for (Chars word : comment.getText().splits(WHITESPACE)) { - if (word.length() <= 3 && - (word.contentEquals("*") - || word.contentEquals("//") - || word.contentEquals("/*") - || word.contentEquals("*/") - || word.contentEquals("/**"))) { + if (Comment.isMarkupWord(word)) { continue; } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java index efccc22cb2..54c495cb6a 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java @@ -80,18 +80,6 @@ public class CommentSizeRule extends AbstractJavaRule { return lineCount > getProperty(MAX_LINES); } - private Chars withoutCommentMarkup(Chars text) { - text = text.trimStart(); - if (text.startsWith("//")) { - return text.subSequence(2, text.length()); - } else if (text.startsWith('*', 0)) { - return text.subSequence(1, text.length()); - } else if (text.startsWith("/**")) { - return text.subSequence(3, text.length()); - } - return text; - } - private List overLengthLineIndicesIn(Comment comment) { int maxLength = getProperty(MAX_LINE_LENGTH); @@ -99,7 +87,7 @@ public class CommentSizeRule extends AbstractJavaRule { List indices = new ArrayList<>(); int i = 0; for (Chars line : comment.getText().lines()) { - line = withoutCommentMarkup(line); + line = Comment.removeCommentMarkup(line); if (line.length() > maxLength) { indices.add(i); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/xpath/internal/GetCommentOnFunction.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/xpath/internal/GetCommentOnFunction.java index 25eb1a7ec5..feab32a8e7 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/xpath/internal/GetCommentOnFunction.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/xpath/internal/GetCommentOnFunction.java @@ -66,7 +66,7 @@ public class GetCommentOnFunction extends BaseJavaXPathFunction { List commentList = contextNode.getFirstParentOfType(ASTCompilationUnit.class).getComments(); for (Comment comment : commentList) { if (comment.getBeginLine() == codeBeginLine || comment.getEndLine() == codeEndLine) { - return new StringValue(comment.getImage()); + return new StringValue(comment.getText()); } } return EmptyAtomicSequence.INSTANCE; diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java index e81f8e9934..1a6dfaf209 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java @@ -10,6 +10,7 @@ import static org.junit.Assert.assertEquals; import java.util.List; +import org.apache.commons.lang3.StringUtils; import org.junit.Assert; import org.junit.Test; @@ -30,11 +31,11 @@ public class CommentAssignmentTest extends BaseNonParserTest { Comment comment = node.getComments().get(0); assertThat(comment, instanceOf(MultiLineComment.class)); - assertEquals("multi line comment with blank lines", comment.getFilteredComment()); + assertEquals("multi line comment with blank lines", StringUtils.join(comment.filteredLines(), ' ')); comment = node.getComments().get(1); assertThat(comment, instanceOf(FormalComment.class)); - assertEquals("a formal comment with blank lines", comment.getFilteredComment()); + assertEquals("a formal comment with blank lines", StringUtils.join(comment.filteredLines(), ' ')); } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentTest.java index 70e381897e..420d44f96b 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentTest.java @@ -8,7 +8,6 @@ import org.apache.commons.lang3.StringUtils; import org.junit.Assert; import org.junit.Test; -import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.lang.java.symboltable.BaseNonParserTest; public class CommentTest extends BaseNonParserTest { @@ -45,7 +44,7 @@ public class CommentTest extends BaseNonParserTest { + " */\n"; String filtered = filter(comment); Assert.assertEquals(2, lineCount(filtered)); - Assert.assertEquals("line 1" + PMD.EOL + "line 2", filtered); + Assert.assertEquals("line 1\nline 2", filtered); } @Test @@ -57,7 +56,7 @@ public class CommentTest extends BaseNonParserTest { + " */\r\n"; String filtered = filter(comment); Assert.assertEquals(2, lineCount(filtered)); - Assert.assertEquals("line 1" + PMD.EOL + "line 2", filtered); + Assert.assertEquals("line 1\nline 2", filtered); } @Test @@ -69,7 +68,7 @@ public class CommentTest extends BaseNonParserTest { + " */\n"; String filtered = filter(comment); Assert.assertEquals(2, lineCount(filtered)); - Assert.assertEquals("line 1" + PMD.EOL + "line 2", filtered); + Assert.assertEquals("line 1\nline 2", filtered); } @Test @@ -81,7 +80,7 @@ public class CommentTest extends BaseNonParserTest { + " */\r\n"; String filtered = filter(comment); Assert.assertEquals(2, lineCount(filtered)); - Assert.assertEquals("line 1" + PMD.EOL + "line 2", filtered); + Assert.assertEquals("line 1\nline 2", filtered); } @Test @@ -94,14 +93,15 @@ public class CommentTest extends BaseNonParserTest { + " */\n"; String filtered = filter(comment); Assert.assertEquals(2, lineCount(filtered)); - Assert.assertEquals("line 1" + PMD.EOL + "line 2", filtered); + Assert.assertEquals("line 1\nline 2", filtered); } private String filter(String comment) { - return java.parse(comment).getComments().get(0).getFilteredComment(); + Comment firstComment = java.parse(comment).getComments().get(0); + return StringUtils.join(firstComment.filteredLines(), '\n'); } private int lineCount(String filtered) { - return StringUtils.countMatches(filtered, PMD.EOL) + 1; + return StringUtils.countMatches(filtered, '\n') + 1; } } From de916626aabb87bc4e1b1066e7e6171fdf7bd3d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 16 Nov 2020 17:39:17 +0100 Subject: [PATCH 07/20] Remove subclasses of Comment --- .../pmd/util/document/Reportable.java | 4 ++++ pmd-java/etc/grammar/Java.jjt | 4 ++-- .../pmd/lang/java/ast/Comment.java | 24 +++++++++++++++---- .../pmd/lang/java/ast/FormalComment.java | 3 +++ .../pmd/lang/java/ast/MultiLineComment.java | 16 ------------- .../pmd/lang/java/ast/SingleLineComment.java | 15 ------------ .../CommentDefaultAccessModifierRule.java | 14 ++--------- .../rule/documentation/CommentSizeRule.java | 3 +-- .../lang/java/ast/CommentAssignmentTest.java | 11 ++++++--- 9 files changed, 39 insertions(+), 55 deletions(-) delete mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/MultiLineComment.java delete mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/SingleLineComment.java diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/document/Reportable.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/document/Reportable.java index e52f3d986f..1cb919ae35 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/document/Reportable.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/document/Reportable.java @@ -18,6 +18,10 @@ import net.sourceforge.pmd.lang.ast.Node; */ public interface Reportable { + // todo add optional method to get the nearest node, to implement + // suppression that depends on tree structure (eg annotations) for + // not just nodes, for example, for comments or individual tokens + /** * Returns the location at which this element should be reported. * diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index 6c2bd482ff..a1a8871549 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/pmd-java/etc/grammar/Java.jjt @@ -545,7 +545,7 @@ SPECIAL_TOKEN : if (startOfNOPMD != -1) { suppressMap.put(matchedToken.getBeginLine(), matchedToken.getImage().substring(startOfNOPMD + suppressMarker.length())); } - comments.add(new SingleLineComment(matchedToken)); + comments.add(new Comment(matchedToken)); } } @@ -566,7 +566,7 @@ SPECIAL_TOKEN : SPECIAL_TOKEN : { - { comments.add(new MultiLineComment(matchedToken)); } : DEFAULT + { comments.add(new Comment(matchedToken)); } : DEFAULT } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java index 8726199aea..9794988450 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java @@ -4,8 +4,12 @@ package net.sourceforge.pmd.lang.java.ast; +import java.util.stream.Stream; + import net.sourceforge.pmd.internal.util.IteratorUtil; +import net.sourceforge.pmd.lang.ast.GenericToken; import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken; +import net.sourceforge.pmd.lang.ast.impl.javacc.JjtreeNode; import net.sourceforge.pmd.util.document.Chars; import net.sourceforge.pmd.util.document.FileLocation; import net.sourceforge.pmd.util.document.Reportable; @@ -14,11 +18,9 @@ import net.sourceforge.pmd.util.document.Reportable; * Wraps a comment token to provide some utilities. * This is not a node, it's not part of the tree anywhere, * just convenient. - * - * TODO subclasses are useless - * TODO maybe move part of this into pmd core */ -public abstract class Comment implements Reportable { +public class Comment implements Reportable { + //TODO maybe move part of this into pmd core private final JavaccToken token; @@ -106,7 +108,7 @@ public abstract class Comment implements Reportable { * Trim the start of the provided line to remove a comment * markup opener ({@code //, /*, /**, *}) or closer {@code * /}. */ - public static Chars removeCommentMarkup(Chars line) { + private static Chars removeCommentMarkup(Chars line) { line = line.trim().removeSuffix("*/"); int subseqFrom = 0; if (line.startsWith('/', 0)) { @@ -121,4 +123,16 @@ public abstract class Comment implements Reportable { } return line.subSequence(subseqFrom, line.length()).trim(); } + + private static Stream getSpecialCommentsIn(JjtreeNode node) { + return GenericToken.streamRange(node.getFirstToken(), node.getLastToken()) + .flatMap(it -> IteratorUtil.toStream(GenericToken.previousSpecials(it).iterator())); + } + + public static Stream getLeadingComments(JavaNode node) { + if (node instanceof AccessNode) { + node = ((AccessNode) node).getModifiers(); + } + return getSpecialCommentsIn(node).filter(Comment::isComment); + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/FormalComment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/FormalComment.java index 1a2f72a5d2..77d9afb4dc 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/FormalComment.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/FormalComment.java @@ -8,6 +8,9 @@ import org.checkerframework.checker.nullness.qual.Nullable; import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken; +/** + * A wrapper for Javadoc {@link Comment}s. + */ public class FormalComment extends Comment { private JavadocCommentOwner owner; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/MultiLineComment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/MultiLineComment.java deleted file mode 100644 index f504050b44..0000000000 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/MultiLineComment.java +++ /dev/null @@ -1,16 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.ast; - -import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken; - -public class MultiLineComment extends Comment { - - public MultiLineComment(JavaccToken t) { - super(t); - } - - -} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/SingleLineComment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/SingleLineComment.java deleted file mode 100644 index 6fb0c99b0d..0000000000 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/SingleLineComment.java +++ /dev/null @@ -1,15 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.ast; - -import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken; - -public class SingleLineComment extends Comment { - - public SingleLineComment(JavaccToken t) { - super(t); - } - -} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java index 2e8cbdb361..1b236650f6 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java @@ -8,8 +8,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.regex.Pattern; -import net.sourceforge.pmd.lang.ast.GenericToken; -import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken; import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; @@ -17,7 +15,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTModifierList; import net.sourceforge.pmd.lang.java.ast.AccessNode; import net.sourceforge.pmd.lang.java.ast.Comment; import net.sourceforge.pmd.lang.java.rule.AbstractIgnoredAnnotationRule; @@ -124,16 +121,9 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR } private boolean hasOkComment(AccessNode node) { - ASTModifierList modifiers = node.getModifiers(); Pattern regex = getProperty(REGEX_DESCRIPTOR); - for (JavaccToken token : GenericToken.range(modifiers.getFirstToken(), modifiers.getLastToken())) { - for (JavaccToken special : GenericToken.previousSpecials(token)) { - if (Comment.isComment(special)) { - return regex.matcher(special.getImageCs()).matches(); - } - } - } - return false; + return Comment.getLeadingComments(node) + .anyMatch(it -> regex.matcher(it.getImageCs()).matches()); } private boolean shouldReportTypeDeclaration(ASTAnyTypeDeclaration decl) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java index 54c495cb6a..824e9afe33 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java @@ -86,8 +86,7 @@ public class CommentSizeRule extends AbstractJavaRule { List indices = new ArrayList<>(); int i = 0; - for (Chars line : comment.getText().lines()) { - line = Comment.removeCommentMarkup(line); + for (Chars line : comment.filteredLines()) { if (line.length() > maxLength) { indices.add(i); } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java index 1a6dfaf209..a6803fb015 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java @@ -7,6 +7,8 @@ package net.sourceforge.pmd.lang.java.ast; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.instanceOf; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import java.util.List; @@ -30,10 +32,13 @@ public class CommentAssignmentTest extends BaseNonParserTest { Comment comment = node.getComments().get(0); - assertThat(comment, instanceOf(MultiLineComment.class)); + assertFalse(comment.isSingleLine()); + assertFalse(comment.hasJavadocContent()); assertEquals("multi line comment with blank lines", StringUtils.join(comment.filteredLines(), ' ')); comment = node.getComments().get(1); + assertFalse(comment.isSingleLine()); + assertTrue(comment.hasJavadocContent()); assertThat(comment, instanceOf(FormalComment.class)); assertEquals("a formal comment with blank lines", StringUtils.join(comment.filteredLines(), ' ')); } @@ -51,7 +56,7 @@ public class CommentAssignmentTest extends BaseNonParserTest { + " /** Comment 3 */\n" + " public void method2() {}" + "}"); - List methods = node.findDescendantsOfType(ASTMethodDeclaration.class); + List methods = node.descendants(ASTMethodDeclaration.class).toList(); assertCommentEquals(methods.get(0), "/** Comment 1 */"); assertCommentEquals(methods.get(1), "/** Comment 3 */"); } @@ -69,7 +74,7 @@ public class CommentAssignmentTest extends BaseNonParserTest { + " /** Comment 3 */\n" + " public void method2() {}" + "}"); - List methods = node.findDescendantsOfType(ASTMethodDeclaration.class); + List methods = node.descendants(ASTMethodDeclaration.class).toList(); assertCommentEquals(methods.get(0), "/** Comment 1 */"); assertCommentEquals(methods.get(1), "/** Comment 2 */"); } From f16c56dbe8cbdc6da62d9e4162f5d05853fd3d35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 16 Nov 2020 19:57:01 +0100 Subject: [PATCH 08/20] Pmd warnings --- .../pmd/lang/java/rule/documentation/CommentSizeRule.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java index 824e9afe33..2d6cc62577 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java @@ -37,8 +37,6 @@ public class CommentSizeRule extends AbstractJavaRule { .desc("Maximum line length") .require(positive()).defaultValue(80).build(); - private static final String CR = "\n"; - static final Set IGNORED_LINES = setOf(Chars.wrap("//"), Chars.wrap("/*"), Chars.wrap("/**"), From 8f2054f4a4dececcbc2363f94ff2a43a281eafbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 16 Nov 2020 20:29:16 +0100 Subject: [PATCH 09/20] Test splits --- .../sourceforge/pmd/util/document/Chars.java | 23 ++++++--- .../pmd/util/document/CharsTest.java | 37 ++++++++++++++ .../pmd/lang/java/ast/JavaParser.java | 3 +- .../pmd/lang/java/ast/JavadocElement.java | 35 ------------- .../pmd/lang/java/ast/FormalCommentTest.java | 51 ------------------- 5 files changed, 55 insertions(+), 94 deletions(-) delete mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavadocElement.java delete mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/FormalCommentTest.java diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/document/Chars.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/document/Chars.java index 42044c7696..f2886decd5 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/document/Chars.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/document/Chars.java @@ -464,24 +464,35 @@ public final class Chars implements CharSequence { /** * Split this slice into subslices, like {@link String#split(String)}, - * except it's iterated lazily. Results excludes the delimiter + * except it's iterated lazily. Like splits the */ public Iterable splits(Pattern regex) { return () -> new AbstractIterator() { final Matcher matcher = regex.matcher(Chars.this); int lastPos = 0; - @Override - protected void computeNext() { + private boolean shouldRetry() { if (matcher.find()) { + if (matcher.start() == 0 && matcher.end() == 0 && lastPos != len) { + return true; // zero length match at the start, we should retry once + } setNext(subSequence(lastPos, matcher.start())); lastPos = matcher.end(); + } else if (lastPos != len) { + setNext(subSequence(lastPos, len)); } else { - if (lastPos != len) { - setNext(subSequence(lastPos, len)); - } done(); } + return false; + } + + @Override + protected void computeNext() { + if (matcher.hitEnd()) { + done(); + } else if (shouldRetry()) { + shouldRetry(); + } } }; } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/util/document/CharsTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/util/document/CharsTest.java index f0d91e422f..60f0a82586 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/util/document/CharsTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/util/document/CharsTest.java @@ -8,11 +8,15 @@ import static net.sourceforge.pmd.util.CollectionUtil.listOf; import java.io.IOException; import java.io.StringWriter; +import java.util.Arrays; +import java.util.Iterator; import java.util.List; +import java.util.regex.Pattern; import org.junit.Assert; import org.junit.Test; +import net.sourceforge.pmd.internal.util.IteratorUtil; import net.sourceforge.pmd.util.CollectionUtil; /** @@ -219,4 +223,37 @@ public class CharsTest { Assert.assertTrue(chars.contentEquals(Chars.wrap("A_B_C"), true)); } + @Test + public void testSplits() { + Chars chars = Chars.wrap("a_a_b_c_s").slice(2, 5); + Assert.assertEquals("a_b_c", chars.toString()); + + testSplits(chars, "_"); + testSplits(chars, "a"); + testSplits(chars, "b"); + testSplits(chars, "c"); + Assert.assertEquals(listOf("", "_b_c"), listSplits(chars, "a")); + + chars = chars.subSequence(1, 5); + Assert.assertEquals("_b_c", chars.toString()); + + Assert.assertEquals(listOf("", "b", "c"), listSplits(chars, "_")); + + + testSplits(Chars.wrap("abc"), ""); + testSplits(Chars.wrap(""), ""); + } + + private List listSplits(Chars chars, String regex) { + Pattern pattern = Pattern.compile(regex); + Iterator splits = chars.splits(pattern).iterator(); + return IteratorUtil.toList(IteratorUtil.map(splits, Chars::toString)); + } + + private void testSplits(Chars chars, String regex) { + List splitList = listSplits(chars, regex); + List expected = Arrays.asList(chars.toString().split(regex)); + Assert.assertEquals("Split should behave like String#split", expected, splitList); + } + } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaParser.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaParser.java index dc07926b0a..28bec7b696 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaParser.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaParser.java @@ -5,11 +5,10 @@ package net.sourceforge.pmd.lang.java.ast; import net.sourceforge.pmd.lang.ast.AstInfo; -import net.sourceforge.pmd.lang.ast.CharStream; import net.sourceforge.pmd.lang.ast.ParseException; +import net.sourceforge.pmd.lang.ast.impl.javacc.CharStream; import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccTokenDocument.TokenDocumentBehavior; import net.sourceforge.pmd.lang.ast.impl.javacc.JjtreeParserAdapter; -import net.sourceforge.pmd.lang.ast.impl.javacc.CharStream; import net.sourceforge.pmd.lang.java.ast.internal.LanguageLevelChecker; /** diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavadocElement.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavadocElement.java deleted file mode 100644 index 7472a9aeb3..0000000000 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavadocElement.java +++ /dev/null @@ -1,35 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.ast; - -import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken; -import net.sourceforge.pmd.lang.java.javadoc.JavadocTag; -import net.sourceforge.pmd.util.document.FileLocation; - -public class JavadocElement extends Comment { - - private final JavadocTag tag; - private final FileLocation reportLoc; - - public JavadocElement(JavaccToken t, int theBeginLine, int theEndLine, int theBeginColumn, int theEndColumn, JavadocTag theTag) { - super(t); - this.tag = theTag; - this.reportLoc = FileLocation.location("TODO", theBeginLine, theBeginColumn, theEndLine, theEndColumn); - } - - public JavadocTag tag() { - return tag; - } - - @Override - public FileLocation getReportLocation() { - return reportLoc; - } - - @Override - public String getXPathNodeName() { - return tag.label + " : " + tag.description; - } -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/FormalCommentTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/FormalCommentTest.java deleted file mode 100644 index 5c8f8b11da..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/FormalCommentTest.java +++ /dev/null @@ -1,51 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.ast; - -import static org.junit.Assert.assertEquals; - -import org.junit.Assert; -import org.junit.Test; - -import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken; - -public class FormalCommentTest extends BaseParserTest { - - @Test - public void testJavadocTagsAsChildren() { - ASTCompilationUnit acu = java.parse( - "interface Metric {" - + " /**\n" - + " * Checks if the metric can be computed on the node.\n" - + " *\n" - + " * @param node The node to check\n" - + " *\n" - + " * @return True if the metric can be computed\n" - + " */\n" - + " boolean supports(N node);\n" - + "}"); - - ASTType booleanT = acu.descendants(ASTType.class).firstOrThrow(); - JavaccToken firstToken = booleanT.getFirstToken(); - assertEquals("Boolean", JavaTokenKinds.BOOLEAN, firstToken.kind); - JavaccToken comment = firstToken.getPreviousComment(); - assertEquals("Implicit modifier list", JavaccToken.IMPLICIT_TOKEN, comment.kind); - comment = comment.getPreviousComment(); - assertEquals("Whitespace", JavaTokenKinds.WHITESPACE, comment.kind); - assertEquals("\n ", comment.getImage()); - comment = comment.getPreviousComment(); - assertEquals("Formal comment", JavaTokenKinds.FORMAL_COMMENT, comment.kind); - - FormalComment commentNode = new FormalComment(comment); - - Assert.assertEquals(2, commentNode.getNumChildren()); - - JavadocElement paramTag = (JavadocElement) commentNode.getChild(0); - Assert.assertEquals("param", paramTag.tag().label); - - JavadocElement returnTag = (JavadocElement) commentNode.getChild(1); - Assert.assertEquals("return", returnTag.tag().label); - } -} From 189c962d3e2da0e154e55de143d844863536cbb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 16 Nov 2020 20:56:13 +0100 Subject: [PATCH 10/20] Test removeSuffix --- .../sourceforge/pmd/util/document/CharsTest.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/util/document/CharsTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/util/document/CharsTest.java index 60f0a82586..d8587f9d7b 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/util/document/CharsTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/util/document/CharsTest.java @@ -149,6 +149,22 @@ public class CharsTest { } + @Test + public void removeSuffix() { + Chars bc = Chars.wrap("abcdb").slice(1, 2); + // -- + + Assert.assertEquals("bc", bc.toString()); + Assert.assertEquals("b", bc.removeSuffix("c").toString()); + Assert.assertEquals("", bc.removeSuffix("bc").toString()); + + bc = Chars.wrap("aaaaaaa").slice(2, 3); + // --- + + Assert.assertEquals("", bc.removeSuffix("aaa").toString()); + Assert.assertEquals("aaa", bc.removeSuffix("aaaa").toString()); + } + @Test public void trimNoop() { Chars bc = Chars.wrap("abcdb").slice(1, 2); From 0c19203b6248fb327a76ce9a7e3ba34f8915da7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 17 Nov 2020 00:19:40 +0100 Subject: [PATCH 11/20] Make the comment rules use rulechain --- .../documentation/CommentContentRule.java | 50 ++++++++--------- .../rule/documentation/CommentSizeRule.java | 53 ++++++++++--------- 2 files changed, 53 insertions(+), 50 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java index 0d71c34804..4c9fb12ca2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java @@ -14,7 +14,7 @@ import java.util.regex.Pattern; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.Comment; -import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; import net.sourceforge.pmd.properties.PropertyDescriptor; import net.sourceforge.pmd.util.document.Chars; @@ -25,12 +25,12 @@ import net.sourceforge.pmd.util.document.Chars; * * @author Brian Remedios */ -public class CommentContentRule extends AbstractJavaRule { +public class CommentContentRule extends AbstractJavaRulechainRule { private static final Pattern WHITESPACE = Pattern.compile("\\s+"); - // ignored when property above == True - public static final PropertyDescriptor CASE_SENSITIVE_DESCRIPTOR = booleanProperty("caseSensitive").defaultValue(false).desc("Case sensitive").build(); + public static final PropertyDescriptor CASE_SENSITIVE_DESCRIPTOR = + booleanProperty("caseSensitive").defaultValue(false).desc("Whether the words are case sensitive").build(); public static final PropertyDescriptor> DISSALLOWED_TERMS_DESCRIPTOR = stringListProperty("disallowedTerms") @@ -38,10 +38,30 @@ public class CommentContentRule extends AbstractJavaRule { .defaultValues("idiot", "jerk").build(); // TODO make blank property? or add more defaults? public CommentContentRule() { + super(ASTCompilationUnit.class); definePropertyDescriptor(CASE_SENSITIVE_DESCRIPTOR); definePropertyDescriptor(DISSALLOWED_TERMS_DESCRIPTOR); } + + @Override + public Object visit(ASTCompilationUnit cUnit, Object data) { + + List currentBadWords = getProperty(DISSALLOWED_TERMS_DESCRIPTOR); + boolean caseSensitive = getProperty(CASE_SENSITIVE_DESCRIPTOR); + + for (Comment comment : cUnit.getComments()) { + List badWords = illegalTermsIn(comment, currentBadWords, caseSensitive); + if (badWords.isEmpty()) { + continue; + } + + addViolationWithMessage(data, cUnit, errorMsgFor(badWords), comment.getBeginLine(), comment.getEndLine()); + } + + return super.visit(cUnit, data); + } + private List illegalTermsIn(Comment comment, List badWords, boolean caseSensitive) { if (badWords.isEmpty()) { @@ -79,28 +99,8 @@ public class CommentContentRule extends AbstractJavaRule { return msg.toString(); } - @Override - public Object visit(ASTCompilationUnit cUnit, Object data) { - - // NPE patch: Eclipse plugin doesn't call start() at onset? - List currentBadWords = getProperty(DISSALLOWED_TERMS_DESCRIPTOR); - boolean caseSensitive = getProperty(CASE_SENSITIVE_DESCRIPTOR); - - for (Comment comment : cUnit.getComments()) { - List badWords = illegalTermsIn(comment, currentBadWords, caseSensitive); - if (badWords.isEmpty()) { - continue; - } - - addViolationWithMessage(data, cUnit, errorMsgFor(badWords), comment.getBeginLine(), comment.getEndLine()); - } - - return super.visit(cUnit, data); - } - private boolean hasDisallowedTerms() { - List terms = getProperty(DISSALLOWED_TERMS_DESCRIPTOR); - return !terms.isEmpty(); + return !getProperty(DISSALLOWED_TERMS_DESCRIPTOR).isEmpty(); } @Override diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java index 2d6cc62577..b83e54ec48 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java @@ -15,7 +15,7 @@ import org.apache.commons.lang3.StringUtils; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.Comment; -import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; import net.sourceforge.pmd.properties.PropertyDescriptor; import net.sourceforge.pmd.properties.PropertyFactory; import net.sourceforge.pmd.util.document.Chars; @@ -25,7 +25,7 @@ import net.sourceforge.pmd.util.document.Chars; * * @author Brian Remedios */ -public class CommentSizeRule extends AbstractJavaRule { +public class CommentSizeRule extends AbstractJavaRulechainRule { public static final PropertyDescriptor MAX_LINES = PropertyFactory.intProperty("maxLines") @@ -44,10 +44,36 @@ public class CommentSizeRule extends AbstractJavaRule { Chars.wrap("*/")); public CommentSizeRule() { + super(ASTCompilationUnit.class); definePropertyDescriptor(MAX_LINES); definePropertyDescriptor(MAX_LINE_LENGTH); } + + @Override + public Object visit(ASTCompilationUnit cUnit, Object data) { + + for (Comment comment : cUnit.getComments()) { + if (hasTooManyLines(comment)) { + addViolationWithMessage(data, cUnit, this.getMessage() + ": Too many lines", comment.getBeginLine(), + comment.getEndLine()); + } + + List lineNumbers = overLengthLineIndicesIn(comment); + if (lineNumbers.isEmpty()) { + continue; + } + + int offset = comment.getBeginLine(); + for (int lineNum : lineNumbers) { + lineNum += offset; + addViolationWithMessage(data, cUnit, this.getMessage() + ": Line too long", lineNum, lineNum); + } + } + + return super.visit(cUnit, data); + } + private static boolean hasRealText(Chars line) { if (StringUtils.isBlank(line)) { @@ -92,27 +118,4 @@ public class CommentSizeRule extends AbstractJavaRule { return indices; } - @Override - public Object visit(ASTCompilationUnit cUnit, Object data) { - - for (Comment comment : cUnit.getComments()) { - if (hasTooManyLines(comment)) { - addViolationWithMessage(data, cUnit, this.getMessage() + ": Too many lines", comment.getBeginLine(), - comment.getEndLine()); - } - - List lineNumbers = overLengthLineIndicesIn(comment); - if (lineNumbers.isEmpty()) { - continue; - } - - int offset = comment.getBeginLine(); - for (int lineNum : lineNumbers) { - lineNum += offset; - addViolationWithMessage(data, cUnit, this.getMessage() + ": Line too long", lineNum, lineNum); - } - } - - return super.visit(cUnit, data); - } } From e014b03e8f662fc23d0757d788f7729727be04e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 17 Nov 2020 00:33:25 +0100 Subject: [PATCH 12/20] Make CommentContent use a regex --- .../pmd/lang/java/ast/Comment.java | 22 ++++-- .../documentation/CommentContentRule.java | 77 +++++-------------- .../rule/documentation/CommentSizeRule.java | 2 +- 3 files changed, 36 insertions(+), 65 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java index 525dfcd849..5d4cadc292 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java @@ -73,13 +73,21 @@ public class Comment implements Reportable { * @return List of lines of the comments */ public Iterable filteredLines() { - return () -> IteratorUtil.mapNotNull( - getText().lines().iterator(), - line -> { - line = removeCommentMarkup(line); - return line.isEmpty() ? null : line; - } - ); + return filteredLines(false); + } + + public Iterable filteredLines(boolean preserveEmptyLines) { + if (preserveEmptyLines) { + return () -> IteratorUtil.map(getText().lines().iterator(), Comment::removeCommentMarkup); + } else { + return () -> IteratorUtil.mapNotNull( + getText().lines().iterator(), + line -> { + line = removeCommentMarkup(line); + return line.isEmpty() ? null : line; + } + ); + } } /** diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java index 4c9fb12ca2..3be1079cc9 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java @@ -4,11 +4,9 @@ package net.sourceforge.pmd.lang.java.rule.documentation; -import static net.sourceforge.pmd.properties.PropertyFactory.booleanProperty; -import static net.sourceforge.pmd.properties.PropertyFactory.stringListProperty; +import static net.sourceforge.pmd.properties.PropertyFactory.regexProperty; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.regex.Pattern; @@ -27,19 +25,13 @@ import net.sourceforge.pmd.util.document.Chars; */ public class CommentContentRule extends AbstractJavaRulechainRule { - private static final Pattern WHITESPACE = Pattern.compile("\\s+"); - - public static final PropertyDescriptor CASE_SENSITIVE_DESCRIPTOR = - booleanProperty("caseSensitive").defaultValue(false).desc("Whether the words are case sensitive").build(); - - public static final PropertyDescriptor> DISSALLOWED_TERMS_DESCRIPTOR = - stringListProperty("disallowedTerms") - .desc("Illegal terms or phrases") - .defaultValues("idiot", "jerk").build(); // TODO make blank property? or add more defaults? + private static final PropertyDescriptor DISSALLOWED_TERMS_DESCRIPTOR = + regexProperty("forbiddenRegex") + .desc("Illegal terms or phrases") + .defaultValue("idiot|jerk").build(); public CommentContentRule() { super(ASTCompilationUnit.class); - definePropertyDescriptor(CASE_SENSITIVE_DESCRIPTOR); definePropertyDescriptor(DISSALLOWED_TERMS_DESCRIPTOR); } @@ -47,64 +39,35 @@ public class CommentContentRule extends AbstractJavaRulechainRule { @Override public Object visit(ASTCompilationUnit cUnit, Object data) { - List currentBadWords = getProperty(DISSALLOWED_TERMS_DESCRIPTOR); - boolean caseSensitive = getProperty(CASE_SENSITIVE_DESCRIPTOR); + Pattern pattern = getProperty(DISSALLOWED_TERMS_DESCRIPTOR); for (Comment comment : cUnit.getComments()) { - List badWords = illegalTermsIn(comment, currentBadWords, caseSensitive); - if (badWords.isEmpty()) { + List lineNumbers = illegalTermsIn(comment, pattern); + if (lineNumbers.isEmpty()) { continue; } - addViolationWithMessage(data, cUnit, errorMsgFor(badWords), comment.getBeginLine(), comment.getEndLine()); + int offset = comment.getBeginLine(); + for (int lineNum : lineNumbers) { + lineNum += offset; + addViolationWithMessage(data, cUnit, "Line matches forbidden content regex (" + pattern.pattern() + ")", lineNum, lineNum); + } } return super.visit(cUnit, data); } - private List illegalTermsIn(Comment comment, List badWords, boolean caseSensitive) { + private List illegalTermsIn(Comment comment, Pattern violationRegex) { - if (badWords.isEmpty()) { - return Collections.emptyList(); - } - - List foundWords = new ArrayList<>(); - for (Chars word : comment.getText().splits(WHITESPACE)) { - if (Comment.isMarkupWord(word)) { - continue; - } - - for (String badWord : badWords) { - if (word.contentEquals(badWord, !caseSensitive)) { - foundWords.add(badWord); - } + List lines = new ArrayList<>(); + int i = 0; + for (Chars line : comment.filteredLines(true)) { + if (violationRegex.matcher(line).find()) { + lines.add(i); } } - return foundWords; + return lines; } - private String errorMsgFor(List badWords) { - StringBuilder msg = new StringBuilder(this.getMessage()).append(": "); - if (badWords.size() == 1) { - msg.append("Invalid term: '").append(badWords.get(0)).append('\''); - } else { - msg.append("Invalid terms: '"); - msg.append(badWords.get(0)); - for (int i = 1; i < badWords.size(); i++) { - msg.append("', '").append(badWords.get(i)); - } - msg.append('\''); - } - return msg.toString(); - } - - private boolean hasDisallowedTerms() { - return !getProperty(DISSALLOWED_TERMS_DESCRIPTOR).isEmpty(); - } - - @Override - public String dysfunctionReason() { - return hasDisallowedTerms() ? null : "No disallowed terms specified"; - } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java index b83e54ec48..068670e574 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java @@ -110,7 +110,7 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { List indices = new ArrayList<>(); int i = 0; - for (Chars line : comment.filteredLines()) { + for (Chars line : comment.filteredLines(true)) { if (line.length() > maxLength) { indices.add(i); } From 1e5c2a9918eaa20cfd80d84a7cd57e041a00e04f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 9 Jul 2021 17:04:17 +0200 Subject: [PATCH 13/20] Fix checkstyle --- .../pmd/lang/java/ast/Comment.java | 13 +++++----- .../documentation/CommentContentRule.java | 13 ++++++---- .../rule/documentation/CommentSizeRule.java | 25 ++++++++++++------- .../lang/java/rule/internal/JavaRuleUtil.java | 4 +-- .../rule/documentation/xml/CommentContent.xml | 2 +- 5 files changed, 34 insertions(+), 23 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java index eb387e82ba..5349bb0fbe 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java @@ -98,13 +98,14 @@ public class Comment implements Reportable { * True if this is a comment delimiter or an asterisk. This * tests the whole parameter and not a prefix/suffix. */ + @SuppressWarnings("PMD.LiteralsFirstInComparisons") // a fp public static boolean isMarkupWord(Chars word) { - return word.length() <= 3 && - (word.contentEquals("*") - || word.contentEquals("//") - || word.contentEquals("/*") - || word.contentEquals("*/") - || word.contentEquals("/**")); + return word.length() <= 3 + && (word.contentEquals("*") + || word.contentEquals("//") + || word.contentEquals("/*") + || word.contentEquals("*/") + || word.contentEquals("/**")); } /** diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java index 1ae69e7476..13d0bac489 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java @@ -4,7 +4,6 @@ package net.sourceforge.pmd.lang.java.rule.documentation; -import static net.sourceforge.pmd.properties.PropertyFactory.booleanProperty; import static net.sourceforge.pmd.properties.PropertyFactory.regexProperty; import java.util.ArrayList; @@ -20,8 +19,6 @@ import net.sourceforge.pmd.util.document.Chars; /** * A rule that checks for illegal words in the comment text. * - * TODO implement regex option - * * @author Brian Remedios */ public class CommentContentRule extends AbstractJavaRulechainRule { @@ -50,8 +47,14 @@ public class CommentContentRule extends AbstractJavaRulechainRule { int offset = comment.getBeginLine(); for (int lineNum : lineNumbers) { - lineNum += offset; - addViolationWithMessage(data, cUnit, "Line matches forbidden content regex (" + pattern.pattern() + ")", lineNum, lineNum); + int lineNumWithOff = lineNum + offset; + addViolationWithMessage( + data, + cUnit, + "Line matches forbidden content regex (" + pattern.pattern() + ")", + lineNumWithOff, + lineNumWithOff + ); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java index 2d0f176025..f22aeba17c 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java @@ -55,8 +55,8 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { for (Comment comment : cUnit.getComments()) { if (hasTooManyLines(comment)) { - addViolationWithMessage(data, cUnit, this.getMessage() + ": Too many lines", comment.getBeginLine(), - comment.getEndLine()); + addViolationWithMessage(data, cUnit, this.getMessage() + + ": Too many lines", comment.getBeginLine(), comment.getEndLine()); } List lineNumbers = overLengthLineIndicesIn(comment); @@ -66,8 +66,14 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { int offset = comment.getBeginLine(); for (int lineNum : lineNumbers) { - lineNum += offset; - addViolationWithMessage(data, cUnit, this.getMessage() + ": Line too long", lineNum, lineNum); + int lineNumWithOff = lineNum + offset; + addViolationWithMessage( + data, + cUnit, + this.getMessage() + ": Line too long", + lineNumWithOff, + lineNum + ); } } @@ -86,8 +92,9 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { private boolean hasTooManyLines(Comment comment) { int firstLineWithText = -1; - int lastLineWithText = 0; + int lastLineWithText; int i = 0; + int maxLines = getProperty(MAX_LINES); for (Chars line : comment.getText().lines()) { boolean real = hasRealText(line); if (real) { @@ -95,13 +102,13 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { if (firstLineWithText == -1) { firstLineWithText = i; } + if (lastLineWithText - firstLineWithText + 1 > maxLines) { + return true; + } } i++; } - - int lineCount = lastLineWithText - firstLineWithText + 1; - - return lineCount > getProperty(MAX_LINES); + return false; } private List overLengthLineIndicesIn(Comment comment) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/JavaRuleUtil.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/JavaRuleUtil.java index acbed37a2e..4d802eba02 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/JavaRuleUtil.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/JavaRuleUtil.java @@ -622,8 +622,8 @@ public final class JavaRuleUtil { // Since type and variable names obscure one another, // it's ok to use a single renaming function. - Iterator thisIt = GenericToken.range(node.getFirstToken(), node.getLastToken()); - Iterator thatIt = GenericToken.range(other.getFirstToken(), other.getLastToken()); + Iterator thisIt = GenericToken.range(node.getFirstToken(), node.getLastToken()).iterator(); + Iterator thatIt = GenericToken.range(other.getFirstToken(), other.getLastToken()).iterator(); int lastKind = 0; while (thisIt.hasNext()) { if (!thatIt.hasNext()) { diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/documentation/xml/CommentContent.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/documentation/xml/CommentContent.xml index 099966e01d..a1f66e71fb 100755 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/documentation/xml/CommentContent.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/documentation/xml/CommentContent.xml @@ -6,7 +6,7 @@ Includes bad words - idiot|jerk + idiot|jerk 2 Date: Wed, 22 Dec 2021 20:04:03 +0100 Subject: [PATCH 14/20] Cleanups, rename FormalComment to JavadocComment --- pmd-java/etc/grammar/Java.jjt | 2 +- .../net/sourceforge/pmd/lang/java/ast/Comment.java | 3 +++ .../pmd/lang/java/ast/CommentAssignmentPass.java | 11 ++++++----- .../ast/{FormalComment.java => JavadocComment.java} | 10 +++++++--- .../pmd/lang/java/ast/JavadocCommentOwner.java | 2 +- .../java/rule/codestyle/UnnecessaryImportRule.java | 4 ++-- .../pmd/lang/java/ast/CommentAssignmentTest.java | 2 +- .../pmd/lang/java/ast/ParserCornersTest.java | 2 +- 8 files changed, 22 insertions(+), 14 deletions(-) rename pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/{FormalComment.java => JavadocComment.java} (70%) diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index 605e9528fe..81427c1028 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/pmd-java/etc/grammar/Java.jjt @@ -602,7 +602,7 @@ MORE : SPECIAL_TOKEN : { - { comments.add(new FormalComment(matchedToken)); } : DEFAULT + { comments.add(new JavadocComment(matchedToken)); } : DEFAULT } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java index 5349bb0fbe..50ce08960f 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java @@ -18,6 +18,9 @@ import net.sourceforge.pmd.util.document.Reportable; * Wraps a comment token to provide some utilities. * This is not a node, it's not part of the tree anywhere, * just convenient. + * + *

This class represents any kind of comment. A specialized subclass + * provides more API for Javadoc comments, see {@link JavadocComment}. */ public class Comment implements Reportable { //TODO maybe move part of this into pmd core diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentPass.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentPass.java index 3457334db5..13b3d50c19 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentPass.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentPass.java @@ -19,18 +19,19 @@ import net.sourceforge.pmd.util.document.FileLocation; final class CommentAssignmentPass { - private static final SimpleDataKey FORMAL_COMMENT_KEY = DataMap.simpleDataKey("java.comment"); + private static final SimpleDataKey FORMAL_COMMENT_KEY = DataMap.simpleDataKey("java.comment"); private CommentAssignmentPass() { // utility class } - static @Nullable FormalComment getComment(JavadocCommentOwner commentOwner) { + static @Nullable JavadocComment getComment(JavadocCommentOwner commentOwner) { return commentOwner.getUserMap().get(CommentAssignmentPass.FORMAL_COMMENT_KEY); } - private static void setComment(JavadocCommentOwner commentableNode, FormalComment comment) { + private static void setComment(JavadocCommentOwner commentableNode, JavadocComment comment) { commentableNode.getUserMap().set(FORMAL_COMMENT_KEY, comment); + comment.setOwner(commentableNode); } public static void assignCommentsToDeclarations(ASTCompilationUnit root) { @@ -45,11 +46,11 @@ final class CommentAssignmentPass { for (JavaccToken maybeComment : GenericToken.previousSpecials(firstToken)) { if (maybeComment.kind == JavaTokenKinds.FORMAL_COMMENT) { - FormalComment comment = new FormalComment(maybeComment); + JavadocComment comment = new JavadocComment(maybeComment); // deduplicate the comment int idx = Collections.binarySearch(comments, comment, Comparator.comparing(Comment::getReportLocation, FileLocation.COORDS_COMPARATOR)); assert idx >= 0 : "Formal comment not found? " + comment; - comment = (FormalComment) comments.get(idx); + comment = (JavadocComment) comments.get(idx); setComment(commentableNode, comment); continue outer; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/FormalComment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavadocComment.java similarity index 70% rename from pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/FormalComment.java rename to pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavadocComment.java index 77d9afb4dc..99b4378c38 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/FormalComment.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavadocComment.java @@ -9,13 +9,13 @@ import org.checkerframework.checker.nullness.qual.Nullable; import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken; /** - * A wrapper for Javadoc {@link Comment}s. + * A {@link Comment} that has Javadoc content. */ -public class FormalComment extends Comment { +public final class JavadocComment extends Comment { private JavadocCommentOwner owner; - public FormalComment(JavaccToken t) { + JavadocComment(JavaccToken t) { super(t); assert t.kind == JavaTokenKinds.FORMAL_COMMENT; } @@ -24,6 +24,10 @@ public class FormalComment extends Comment { this.owner = owner; } + /** + * Returns the owner of this comment. Null if this comment is + * misplaced. + */ public @Nullable JavadocCommentOwner getOwner() { return owner; } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavadocCommentOwner.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavadocCommentOwner.java index 32d32af77e..b6cee1615c 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavadocCommentOwner.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavadocCommentOwner.java @@ -16,7 +16,7 @@ public interface JavadocCommentOwner extends JavaNode { * Returns the javadoc comment that applies to this declaration. If * there is none, returns null. */ - default @Nullable FormalComment getJavadocComment() { + default @Nullable JavadocComment getJavadocComment() { return CommentAssignmentPass.getComment(this); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java index d1a7294ec5..2de392a30c 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java @@ -23,8 +23,8 @@ import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; import net.sourceforge.pmd.lang.java.ast.Comment; -import net.sourceforge.pmd.lang.java.ast.FormalComment; import net.sourceforge.pmd.lang.java.ast.JavaNode; +import net.sourceforge.pmd.lang.java.ast.JavadocComment; import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.java.ast.internal.ImportWrapper; import net.sourceforge.pmd.lang.java.ast.internal.PrettyPrintingUtil; @@ -97,7 +97,7 @@ public class UnnecessaryImportRule extends AbstractJavaRule { return; } for (Comment comment : node.getComments()) { - if (!(comment instanceof FormalComment)) { + if (!(comment instanceof JavadocComment)) { continue; } for (Pattern p : PATTERNS) { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java index 1e3bb694aa..bc4849d02d 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java @@ -39,7 +39,7 @@ public class CommentAssignmentTest extends BaseNonParserTest { comment = node.getComments().get(1); assertFalse(comment.isSingleLine()); assertTrue(comment.hasJavadocContent()); - assertThat(comment, instanceOf(FormalComment.class)); + assertThat(comment, instanceOf(JavadocComment.class)); assertEquals("a formal comment with blank lines", StringUtils.join(comment.filteredLines(), ' ')); } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ParserCornersTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ParserCornersTest.java index 939ae31248..6430c1c8c1 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ParserCornersTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ParserCornersTest.java @@ -40,7 +40,7 @@ public class ParserCornersTest extends BaseJavaTreeDumpTest { @Test public void testInvalidUnicodeEscape() { expect.expect(MalformedSourceException.class); // previously Error - expect.expectMessage("Source format error in file x/filename.java at line 1, column 1: Invalid unicode escape"); + expect.expectMessage("Source format error in file 'x/filename.java' at line 1, column 1: Invalid unicode escape"); java.parse("\\u00k0", null, "x/filename.java"); } From 1ecbbca27c4ebae68411e92c477dad45b899e366 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 7 Mar 2022 21:27:08 +0100 Subject: [PATCH 15/20] Rename to JavaComment --- .../java/net/sourceforge/pmd/lang/java/ast/ASTBlock.java | 2 +- .../sourceforge/pmd/lang/java/ast/ASTCompilationUnit.java | 6 +++--- .../pmd/lang/java/ast/CommentAssignmentPass.java | 4 ++-- .../pmd/lang/java/ast/{Comment.java => JavaComment.java} | 8 ++++---- .../net/sourceforge/pmd/lang/java/ast/JavadocComment.java | 4 ++-- .../rule/codestyle/CommentDefaultAccessModifierRule.java | 8 ++++---- .../lang/java/rule/codestyle/UnnecessaryImportRule.java | 4 ++-- .../lang/java/rule/documentation/CommentContentRule.java | 8 ++++---- .../pmd/lang/java/rule/documentation/CommentSizeRule.java | 8 ++++---- .../java/rule/xpath/internal/GetCommentOnFunction.java | 6 +++--- .../pmd/lang/java/ast/CommentAssignmentTest.java | 2 +- .../net/sourceforge/pmd/lang/java/ast/CommentTest.java | 2 +- 12 files changed, 31 insertions(+), 31 deletions(-) rename pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/{Comment.java => JavaComment.java} (95%) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTBlock.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTBlock.java index 828e862088..0086e2d86d 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTBlock.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTBlock.java @@ -33,7 +33,7 @@ public final class ASTBlock extends ASTMaybeEmptyListOf implements public boolean containsComment() { JavaccToken t = getLastToken().getPreviousComment(); while (t != null) { - if (Comment.isComment(t)) { + if (JavaComment.isComment(t)) { return true; } t = t.getPreviousComment(); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCompilationUnit.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCompilationUnit.java index 1b6ba66f57..13d0bd79c2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCompilationUnit.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCompilationUnit.java @@ -23,14 +23,14 @@ import net.sourceforge.pmd.lang.java.types.ast.LazyTypeResolver; public final class ASTCompilationUnit extends AbstractJavaTypeNode implements JavaNode, GenericNode, RootNode { private LazyTypeResolver lazyTypeResolver; - private List comments; + private List comments; private AstInfo astInfo; ASTCompilationUnit(int id) { super(id); } - public List getComments() { + public List getComments() { return comments; } @@ -43,7 +43,7 @@ public final class ASTCompilationUnit extends AbstractJavaTypeNode implements Ja return astInfo; } - void setComments(List comments) { + void setComments(List comments) { this.comments = comments; } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentPass.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentPass.java index 86f2c3e6f5..7c63aaf924 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentPass.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentPass.java @@ -35,7 +35,7 @@ final class CommentAssignmentPass { } public static void assignCommentsToDeclarations(ASTCompilationUnit root) { - final List comments = root.getComments(); + final List comments = root.getComments(); if (comments.isEmpty()) { return; } @@ -48,7 +48,7 @@ final class CommentAssignmentPass { if (maybeComment.kind == JavaTokenKinds.FORMAL_COMMENT) { JavadocComment comment = new JavadocComment(maybeComment); // deduplicate the comment - int idx = Collections.binarySearch(comments, comment, Comparator.comparing(Comment::getReportLocation, FileLocation.COORDS_COMPARATOR)); + int idx = Collections.binarySearch(comments, comment, Comparator.comparing(JavaComment::getReportLocation, FileLocation.COORDS_COMPARATOR)); assert idx >= 0 : "Formal comment not found? " + comment; comment = (JavadocComment) comments.get(idx); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaComment.java similarity index 95% rename from pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java rename to pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaComment.java index d528c217e2..aa280f1c01 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/Comment.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaComment.java @@ -22,12 +22,12 @@ import net.sourceforge.pmd.reporting.Reportable; *

This class represents any kind of comment. A specialized subclass * provides more API for Javadoc comments, see {@link JavadocComment}. */ -public class Comment implements Reportable { +public class JavaComment implements Reportable { //TODO maybe move part of this into pmd core private final JavaccToken token; - Comment(JavaccToken t) { + JavaComment(JavaccToken t) { this.token = t; } @@ -85,7 +85,7 @@ public class Comment implements Reportable { public Iterable filteredLines(boolean preserveEmptyLines) { if (preserveEmptyLines) { - return () -> IteratorUtil.map(getText().lines().iterator(), Comment::removeCommentMarkup); + return () -> IteratorUtil.map(getText().lines().iterator(), JavaComment::removeCommentMarkup); } else { return () -> IteratorUtil.mapNotNull( getText().lines().iterator(), @@ -140,6 +140,6 @@ public class Comment implements Reportable { if (node instanceof AccessNode) { node = ((AccessNode) node).getModifiers(); } - return getSpecialCommentsIn(node).filter(Comment::isComment); + return getSpecialCommentsIn(node).filter(JavaComment::isComment); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavadocComment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavadocComment.java index 99b4378c38..620407edda 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavadocComment.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavadocComment.java @@ -9,9 +9,9 @@ import org.checkerframework.checker.nullness.qual.Nullable; import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken; /** - * A {@link Comment} that has Javadoc content. + * A {@link JavaComment} that has Javadoc content. */ -public final class JavadocComment extends Comment { +public final class JavadocComment extends JavaComment { private JavadocCommentOwner owner; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java index 36bc715955..e6aa0be032 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java @@ -20,7 +20,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; import net.sourceforge.pmd.lang.java.ast.AccessNode; import net.sourceforge.pmd.lang.java.ast.AccessNode.Visibility; -import net.sourceforge.pmd.lang.java.ast.Comment; +import net.sourceforge.pmd.lang.java.ast.JavaComment; import net.sourceforge.pmd.lang.java.ast.internal.PrettyPrintingUtil; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; import net.sourceforge.pmd.lang.java.rule.internal.JavaPropertyUtil; @@ -62,7 +62,7 @@ public class CommentDefaultAccessModifierRule extends AbstractJavaRulechainRule @Override public Object visit(final ASTCompilationUnit node, final Object data) { interestingLineNumberComments.clear(); - for (final Comment comment : node.getComments()) { + for (final JavaComment comment : node.getComments()) { if (getProperty(REGEX_DESCRIPTOR).matcher(comment.getText()).matches()) { interestingLineNumberComments.add(comment.getBeginLine()); } @@ -150,8 +150,8 @@ public class CommentDefaultAccessModifierRule extends AbstractJavaRulechainRule private boolean hasOkComment(AccessNode node) { Pattern regex = getProperty(REGEX_DESCRIPTOR); - return Comment.getLeadingComments(node) - .anyMatch(it -> regex.matcher(it.getImageCs()).matches()); + return JavaComment.getLeadingComments(node) + .anyMatch(it -> regex.matcher(it.getImageCs()).matches()); } private boolean shouldReportTypeDeclaration(ASTAnyTypeDeclaration decl) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java index 47bd2cacd8..0f2260e581 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java @@ -20,7 +20,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTMethodCall; import net.sourceforge.pmd.lang.java.ast.ASTSwitchLabel; import net.sourceforge.pmd.lang.java.ast.ASTSwitchLike; import net.sourceforge.pmd.lang.java.ast.ASTVariableAccess; -import net.sourceforge.pmd.lang.java.ast.Comment; +import net.sourceforge.pmd.lang.java.ast.JavaComment; import net.sourceforge.pmd.lang.java.ast.JavadocComment; import net.sourceforge.pmd.lang.java.ast.internal.PrettyPrintingUtil; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; @@ -162,7 +162,7 @@ public class UnnecessaryImportRule extends AbstractJavaRule { private void visitComments(ASTCompilationUnit node) { // todo improve that when we have a javadoc parser - for (Comment comment : node.getComments()) { + for (JavaComment comment : node.getComments()) { if (!(comment instanceof JavadocComment)) { continue; } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java index 13d0bac489..ab13e52d96 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java @@ -10,11 +10,11 @@ import java.util.ArrayList; import java.util.List; import java.util.regex.Pattern; +import net.sourceforge.pmd.lang.document.Chars; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; -import net.sourceforge.pmd.lang.java.ast.Comment; +import net.sourceforge.pmd.lang.java.ast.JavaComment; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; import net.sourceforge.pmd.properties.PropertyDescriptor; -import net.sourceforge.pmd.util.document.Chars; /** * A rule that checks for illegal words in the comment text. @@ -39,7 +39,7 @@ public class CommentContentRule extends AbstractJavaRulechainRule { Pattern pattern = getProperty(DISSALLOWED_TERMS_DESCRIPTOR); - for (Comment comment : cUnit.getComments()) { + for (JavaComment comment : cUnit.getComments()) { List lineNumbers = illegalTermsIn(comment, pattern); if (lineNumbers.isEmpty()) { continue; @@ -61,7 +61,7 @@ public class CommentContentRule extends AbstractJavaRulechainRule { return null; } - private List illegalTermsIn(Comment comment, Pattern violationRegex) { + private List illegalTermsIn(JavaComment comment, Pattern violationRegex) { List lines = new ArrayList<>(); int i = 0; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java index 70cc9989dc..37a63c6dc3 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java @@ -15,7 +15,7 @@ import org.apache.commons.lang3.StringUtils; import net.sourceforge.pmd.lang.document.Chars; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; -import net.sourceforge.pmd.lang.java.ast.Comment; +import net.sourceforge.pmd.lang.java.ast.JavaComment; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; import net.sourceforge.pmd.properties.PropertyDescriptor; import net.sourceforge.pmd.properties.PropertyFactory; @@ -53,7 +53,7 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { @Override public Object visit(ASTCompilationUnit cUnit, Object data) { - for (Comment comment : cUnit.getComments()) { + for (JavaComment comment : cUnit.getComments()) { if (hasTooManyLines(comment)) { addViolationWithMessage(data, cUnit, this.getMessage() + ": Too many lines", comment.getBeginLine(), comment.getEndLine()); @@ -84,7 +84,7 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { return !StringUtils.isBlank(line) && !IGNORED_LINES.contains(line.trim()); } - private boolean hasTooManyLines(Comment comment) { + private boolean hasTooManyLines(JavaComment comment) { int firstLineWithText = -1; int lastLineWithText; @@ -106,7 +106,7 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { return false; } - private List overLengthLineIndicesIn(Comment comment) { + private List overLengthLineIndicesIn(JavaComment comment) { int maxLength = getProperty(MAX_LINE_LENGTH); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/xpath/internal/GetCommentOnFunction.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/xpath/internal/GetCommentOnFunction.java index feab32a8e7..8014821ecd 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/xpath/internal/GetCommentOnFunction.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/xpath/internal/GetCommentOnFunction.java @@ -8,7 +8,7 @@ import java.util.List; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; -import net.sourceforge.pmd.lang.java.ast.Comment; +import net.sourceforge.pmd.lang.java.ast.JavaComment; import net.sourceforge.pmd.lang.rule.xpath.internal.AstElementNode; import net.sf.saxon.expr.XPathContext; @@ -63,8 +63,8 @@ public class GetCommentOnFunction extends BaseJavaXPathFunction { int codeBeginLine = contextNode.getBeginLine(); int codeEndLine = contextNode.getEndLine(); - List commentList = contextNode.getFirstParentOfType(ASTCompilationUnit.class).getComments(); - for (Comment comment : commentList) { + List commentList = contextNode.getFirstParentOfType(ASTCompilationUnit.class).getComments(); + for (JavaComment comment : commentList) { if (comment.getBeginLine() == codeBeginLine || comment.getEndLine() == codeEndLine) { return new StringValue(comment.getText()); } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java index bbdd9f46d8..74ef770fe0 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java @@ -30,7 +30,7 @@ public class CommentAssignmentTest extends BaseParserTest { + " /** a formal comment with blank lines\n\n\n */" + "}"); - Comment comment = node.getComments().get(0); + JavaComment comment = node.getComments().get(0); assertFalse(comment.isSingleLine()); assertFalse(comment.hasJavadocContent()); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentTest.java index ee5f2fc606..735eb6ce2a 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentTest.java @@ -97,7 +97,7 @@ public class CommentTest extends BaseParserTest { } private String filter(String comment) { - Comment firstComment = java.parse(comment).getComments().get(0); + JavaComment firstComment = java.parse(comment).getComments().get(0); return StringUtils.join(firstComment.filteredLines(), '\n'); } From 7be498ae324a56114d8447ff43498563d2eb8500 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 7 Mar 2022 22:00:14 +0100 Subject: [PATCH 16/20] Fix rename --- pmd-java/etc/grammar/Java.jjt | 10 +++++----- .../java/net/sourceforge/pmd/cpd/JavaTokenizer.java | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index 5d9641db03..5e00b77f4c 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/pmd-java/etc/grammar/Java.jjt @@ -561,7 +561,7 @@ PARSER_END(JavaParserImpl) TOKEN_MGR_DECLS : { - protected List comments = new ArrayList(); + protected List comments = new ArrayList(); } /* WHITE SPACE */ @@ -587,7 +587,7 @@ SPECIAL_TOKEN : if (startOfNOPMD != -1) { suppressMap.put(matchedToken.getBeginLine(), matchedToken.getImage().substring(startOfNOPMD + suppressMarker.length())); } - comments.add(new Comment(matchedToken)); + comments.add(new JavaComment(matchedToken)); } } @@ -608,7 +608,7 @@ SPECIAL_TOKEN : SPECIAL_TOKEN : { - { comments.add(new Comment(matchedToken)); } : DEFAULT + { comments.add(new JavaComment(matchedToken)); } : DEFAULT } @@ -2612,14 +2612,14 @@ void AssertStatement() : void RUNSIGNEDSHIFT() #void: {} { - LOOKAHEAD({ JavaTokenDocument.getRealKind(getToken(1)) == RUNSIGNEDSHIFT}) + LOOKAHEAD({ JavaTokenDocumentBehavior.getRealKind(getToken(1)) == RUNSIGNEDSHIFT}) ">" ">" ">" } void RSIGNEDSHIFT() #void: {} { - LOOKAHEAD({ JavaTokenDocument.getRealKind(getToken(1)) == RSIGNEDSHIFT}) + LOOKAHEAD({ JavaTokenDocumentBehavior.getRealKind(getToken(1)) == RSIGNEDSHIFT}) ">" ">" } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/cpd/JavaTokenizer.java b/pmd-java/src/main/java/net/sourceforge/pmd/cpd/JavaTokenizer.java index a14d75b928..5d769e12c8 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/cpd/JavaTokenizer.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/cpd/JavaTokenizer.java @@ -43,7 +43,7 @@ public class JavaTokenizer extends JavaCCTokenizer { } @Override - protected JavaccTokenDocument.TokenDocumentBehavior tokenBehavior() throws IOException { + protected JavaccTokenDocument.TokenDocumentBehavior tokenBehavior() { return InternalApiBridge.javaTokenDoc(); } From 20d3d90fb0779b8c71bc945eca94b246d6ef55fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sat, 26 Nov 2022 16:27:56 +0100 Subject: [PATCH 17/20] Add some tests --- .../pmd/lang/java/ast/ASTBlock.java | 1 - .../pmd/lang/java/ast/JavaComment.java | 43 +++++++++-- .../CommentDefaultAccessModifierRule.java | 5 +- .../documentation/CommentContentRule.java | 2 +- .../rule/documentation/CommentSizeRule.java | 2 +- .../lang/java/ast/CommentAssignmentTest.java | 4 +- .../pmd/lang/java/ast/CommentTest.java | 2 +- .../pmd/lang/java/ast/JavaCommentTest.java | 76 +++++++++++++++++++ 8 files changed, 119 insertions(+), 16 deletions(-) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/JavaCommentTest.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTBlock.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTBlock.java index 6de8df112e..0461ca3a3d 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTBlock.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTBlock.java @@ -7,7 +7,6 @@ package net.sourceforge.pmd.lang.java.ast; import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken; import net.sourceforge.pmd.lang.java.ast.ASTList.ASTMaybeEmptyListOf; import net.sourceforge.pmd.lang.java.ast.InternalInterfaces.AllChildrenAreOfType; -import net.sourceforge.pmd.lang.java.ast.internal.JavaAstUtils; /** * A block of code. This is a {@linkplain ASTStatement statement} that diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaComment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaComment.java index aa280f1c01..8fe146edfc 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaComment.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaComment.java @@ -12,6 +12,7 @@ import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken; import net.sourceforge.pmd.lang.ast.impl.javacc.JjtreeNode; import net.sourceforge.pmd.lang.document.Chars; import net.sourceforge.pmd.lang.document.FileLocation; +import net.sourceforge.pmd.lang.java.ast.internal.JavaAstUtils; import net.sourceforge.pmd.reporting.Reportable; /** @@ -67,7 +68,7 @@ public class JavaComment implements Reportable { * of a comment token (there are three such kinds). */ public static boolean isComment(JavaccToken token) { - return JavaTokenDocumentBehavior.isComment(token); + return JavaAstUtils.isComment(token); } /** @@ -79,11 +80,11 @@ public class JavaComment implements Reportable { * * @return List of lines of the comments */ - public Iterable filteredLines() { - return filteredLines(false); + public Iterable getFilteredLines() { + return getFilteredLines(false); } - public Iterable filteredLines(boolean preserveEmptyLines) { + public Iterable getFilteredLines(boolean preserveEmptyLines) { if (preserveEmptyLines) { return () -> IteratorUtil.map(getText().lines().iterator(), JavaComment::removeCommentMarkup); } else { @@ -136,10 +137,40 @@ public class JavaComment implements Reportable { .flatMap(it -> IteratorUtil.toStream(GenericToken.previousSpecials(it).iterator())); } - public static Stream getLeadingComments(JavaNode node) { + public static Stream getLeadingComments(JavaNode node) { if (node instanceof AccessNode) { node = ((AccessNode) node).getModifiers(); } - return getSpecialCommentsIn(node).filter(JavaComment::isComment); + return getSpecialCommentsIn(node).filter(JavaComment::isComment) + .map(JavaComment::toComment); + } + + private static JavaComment toComment(JavaccToken tok) { + switch (tok.kind) { + case JavaTokenKinds.FORMAL_COMMENT: + return new JavadocComment(tok); + case JavaTokenKinds.MULTI_LINE_COMMENT: + case JavaTokenKinds.SINGLE_LINE_COMMENT: + return new JavaComment(tok); + default: + throw new IllegalArgumentException("Token is not a comment: " + tok); + } + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof JavaComment)) { + return false; + } + JavaComment that = (JavaComment) o; + return token.equals(that.token); + } + + @Override + public int hashCode() { + return token.hashCode(); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java index 4fc9edfaa2..5a2672a518 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java @@ -4,16 +4,13 @@ package net.sourceforge.pmd.lang.java.rule.codestyle; -import java.util.HashSet; import java.util.List; -import java.util.Set; import java.util.regex.Pattern; import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; @@ -161,7 +158,7 @@ public class CommentDefaultAccessModifierRule extends AbstractJavaRulechainRule private boolean hasOkComment(AccessNode node) { Pattern regex = getProperty(REGEX_DESCRIPTOR); return JavaComment.getLeadingComments(node) - .anyMatch(it -> regex.matcher(it.getImageCs()).matches()); + .anyMatch(it -> regex.matcher(it.getText()).matches()); } private boolean shouldReportTypeDeclaration(ASTAnyTypeDeclaration decl) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java index ab13e52d96..af50a90348 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java @@ -65,7 +65,7 @@ public class CommentContentRule extends AbstractJavaRulechainRule { List lines = new ArrayList<>(); int i = 0; - for (Chars line : comment.filteredLines(true)) { + for (Chars line : comment.getFilteredLines(true)) { if (violationRegex.matcher(line).find()) { lines.add(i); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java index 37a63c6dc3..6a409f3c76 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentSizeRule.java @@ -112,7 +112,7 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { List indices = new ArrayList<>(); int i = 0; - for (Chars line : comment.filteredLines(true)) { + for (Chars line : comment.getFilteredLines(true)) { if (line.length() > maxLength) { indices.add(i); } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java index ec6da75ce1..32f51278d8 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentAssignmentTest.java @@ -35,13 +35,13 @@ class CommentAssignmentTest extends BaseParserTest { assertFalse(comment.isSingleLine()); assertFalse(comment.hasJavadocContent()); - assertEquals("multi line comment with blank lines", StringUtils.join(comment.filteredLines(), ' ')); + assertEquals("multi line comment with blank lines", StringUtils.join(comment.getFilteredLines(), ' ')); comment = node.getComments().get(1); assertFalse(comment.isSingleLine()); assertTrue(comment.hasJavadocContent()); assertThat(comment, instanceOf(JavadocComment.class)); - assertEquals("a formal comment with blank lines", StringUtils.join(comment.filteredLines(), ' ')); + assertEquals("a formal comment with blank lines", StringUtils.join(comment.getFilteredLines(), ' ')); } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentTest.java index f87b05e5c8..29530d227d 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/CommentTest.java @@ -99,7 +99,7 @@ class CommentTest extends BaseParserTest { private String filter(String comment) { JavaComment firstComment = java.parse(comment).getComments().get(0); - return StringUtils.join(firstComment.filteredLines(), '\n'); + return StringUtils.join(firstComment.getFilteredLines(), '\n'); } private int lineCount(String filtered) { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/JavaCommentTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/JavaCommentTest.java new file mode 100644 index 0000000000..faee16b9c6 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/JavaCommentTest.java @@ -0,0 +1,76 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.ast; + + +import static net.sourceforge.pmd.util.CollectionUtil.listOf; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.List; +import java.util.stream.Collectors; + +import org.junit.jupiter.api.Test; + +import net.sourceforge.pmd.lang.document.Chars; +import net.sourceforge.pmd.lang.java.BaseParserTest; + +/** + * @author Clément Fournier + */ +public class JavaCommentTest extends BaseParserTest { + + @Test + public void testFilteredLines() { + JavaComment comment = parseComment( + "/**\n" + + " * @author Clément Fournier\n" + + " *" + + " */\n" + ); + + assertThat(comment.getFilteredLines(), + contains(Chars.wrap("@author Clément Fournier"))); + } + + @Test + public void testFilteredLinesKeepBlankLines() { + JavaComment comment = parseComment( + "/**\n" + + " * @author Clément Fournier\n" + + " *" + + " */\n" + ); + + assertThat(comment.getFilteredLines(true), + contains(Chars.wrap(""), Chars.wrap("@author Clément Fournier"), Chars.wrap(""))); + } + + JavaComment parseComment(String text) { + ASTCompilationUnit parsed = java.parse(text); + return JavaComment.getLeadingComments(parsed).findFirst().get(); + } + + + @Test + public void getLeadingComments() { + ASTCompilationUnit parsed = java.parse("/** a */ class Fooo { /** b */ int field; }"); + List docCommentOwners = parsed.descendants(JavadocCommentOwner.class).toList(); + + checkCommentMatches(docCommentOwners.get(0), "/** a */"); + checkCommentMatches(docCommentOwners.get(1), "/** b */"); + } + + private static void checkCommentMatches(JavadocCommentOwner commentOwner, String expectedText) { + // this is preassigned by the comment assignment pass + JavadocComment comment = commentOwner.getJavadocComment(); + assertEquals(expectedText, comment.getText().toString()); + + // this is fetched adhoc + List collected = JavaComment.getLeadingComments(commentOwner).collect(Collectors.toList()); + assertEquals(listOf(comment), collected); + } +} From 49c7f421f3fdba602594961923e266a54f810a1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sat, 26 Nov 2022 17:53:11 +0100 Subject: [PATCH 18/20] cleanups --- .../net/sourceforge/pmd/lang/document/Chars.java | 2 ++ .../sourceforge/pmd/lang/java/ast/TokenUtils.java | 13 ------------- .../codestyle/CommentDefaultAccessModifierRule.java | 3 +-- .../src/main/resources/category/java/codestyle.xml | 2 +- .../codestyle/xml/CommentDefaultAccessModifier.xml | 8 ++++---- 5 files changed, 8 insertions(+), 20 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/document/Chars.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/document/Chars.java index 425cc3bfee..d8b3b38214 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/document/Chars.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/document/Chars.java @@ -66,6 +66,8 @@ public final class Chars implements CharSequence { return this.start + off; } + + /** Whether this slice is the empty string. */ public boolean isEmpty() { return len == 0; } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TokenUtils.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TokenUtils.java index 8a9ccf1635..3a347e8390 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TokenUtils.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TokenUtils.java @@ -24,19 +24,6 @@ final class TokenUtils { } - public static > int compare(GenericToken t1, GenericToken t2) { - return t1.getRegion().compareTo(t2.getRegion()); - } - - public static > boolean isBefore(GenericToken t1, GenericToken t2) { - return t1.getRegion().compareTo(t2.getRegion()) < 0; - } - - public static > boolean isAfter(GenericToken t1, GenericToken t2) { - return t1.getRegion().compareTo(t2.getRegion()) > 0; - } - - public static > T nthFollower(T token, int n) { if (n < 0) { throw new IllegalArgumentException("Negative index?"); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java index 5a2672a518..00f34aa1cd 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java @@ -62,7 +62,6 @@ public class CommentDefaultAccessModifierRule extends AbstractJavaRulechainRule "org.junit.jupiter.api.AfterAll" ); - private static final String MESSAGE = "To avoid mistakes add a comment at the beginning of the {0} {1} if you want a default access modifier"; public CommentDefaultAccessModifierRule() { super(ASTMethodDeclaration.class, ASTAnyTypeDeclaration.class, @@ -131,7 +130,7 @@ public class CommentDefaultAccessModifierRule extends AbstractJavaRulechainRule private void report(RuleContext ctx, AccessNode decl, String kind, String signature) { - ctx.addViolationWithMessage(decl, MESSAGE, kind, signature); + ctx.addViolation(decl, kind, signature); } private boolean shouldReportNonTopLevel(final AccessNode decl) { diff --git a/pmd-java/src/main/resources/category/java/codestyle.xml b/pmd-java/src/main/resources/category/java/codestyle.xml index 28baa27c88..0fc9c2a18e 100644 --- a/pmd-java/src/main/resources/category/java/codestyle.xml +++ b/pmd-java/src/main/resources/category/java/codestyle.xml @@ -292,7 +292,7 @@ public class Éléphant {} language="java" since="5.4.0" class="net.sourceforge.pmd.lang.java.rule.codestyle.CommentDefaultAccessModifierRule" - message="To avoid mistakes, add a comment at the beginning of the {1} ''{0}'' if you want a default access modifier" + message="Missing commented default access modifier on {0} ''{1}''" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_codestyle.html#commentdefaultaccessmodifier"> To avoid mistakes if we want that an Annotation, Class, Enum, Method, Constructor or Field have a default access modifier diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/CommentDefaultAccessModifier.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/CommentDefaultAccessModifier.xml index 9b756d5e92..545fa870f3 100755 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/CommentDefaultAccessModifier.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/CommentDefaultAccessModifier.xml @@ -247,7 +247,7 @@ public enum TestEnum { @@ -383,7 +383,7 @@ public interface A { 1 6 - To avoid mistakes add a comment at the beginning of the method method() if you want a default access modifier + Missing commented default access modifier on method 'method()' 2 2,4 - To avoid mistakes add a comment at the beginning of the constructor Foo() if you want a default access modifier - To avoid mistakes add a comment at the beginning of the constructor Foo(String) if you want a default access modifier + Missing commented default access modifier on constructor 'Foo()' + Missing commented default access modifier on constructor 'Foo(String)' From e70ce39b62365960ee95604afd1a546f3534eb1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sat, 26 Nov 2022 18:32:09 +0100 Subject: [PATCH 19/20] Make LawOfDemeter not use the rulechain --- .../java/rule/design/LawOfDemeterRule.java | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/LawOfDemeterRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/LawOfDemeterRule.java index d6aa57e247..505b3f2753 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/LawOfDemeterRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/LawOfDemeterRule.java @@ -22,9 +22,11 @@ import java.util.stream.Stream; import org.checkerframework.checker.nullness.qual.Nullable; import net.sourceforge.pmd.RuleContext; +import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.ast.ASTArgumentList; import net.sourceforge.pmd.lang.java.ast.ASTArrayAccess; import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr; +import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.ASTConstructorCall; import net.sourceforge.pmd.lang.java.ast.ASTExpression; import net.sourceforge.pmd.lang.java.ast.ASTExpressionStatement; @@ -39,7 +41,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator; import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId; import net.sourceforge.pmd.lang.java.ast.QualifiableExpression; import net.sourceforge.pmd.lang.java.ast.internal.PrettyPrintingUtil; -import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass; import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass.AssignmentEntry; import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass.DataflowResult; @@ -73,7 +75,7 @@ import net.sourceforge.pmd.properties.PropertyFactory; * @since 5.0 * */ -public class LawOfDemeterRule extends AbstractJavaRulechainRule { +public class LawOfDemeterRule extends AbstractJavaRule { private static final PropertyDescriptor TRUST_RADIUS = @@ -86,7 +88,6 @@ public class LawOfDemeterRule extends AbstractJavaRulechainRule { private static final String METHOD_CALL_ON_FOREIGN_VALUE = "Call to `{0}` on foreign value `{1}` (degree {2})"; public LawOfDemeterRule() { - super(ASTMethodCall.class, ASTFieldAccess.class); definePropertyDescriptor(TRUST_RADIUS); } @@ -100,7 +101,19 @@ public class LawOfDemeterRule extends AbstractJavaRulechainRule { private final Map degreeCache = new LinkedHashMap<>(); @Override - public void end(RuleContext ctx) { + public void apply(Node target, RuleContext ctx) { + degreeCache.clear(); + // reimplement our own traversal instead of using the rulechain, + // so that we have a stable traversal order. + ((ASTCompilationUnit) target) + .descendants().crossFindBoundaries() + .forEach(it -> { + if (it instanceof ASTMethodCall) { + this.visit((ASTMethodCall) it, ctx); + } else if (it instanceof ASTFieldAccess) { + this.visit((ASTFieldAccess) it, ctx); + } + }); degreeCache.clear(); // avoid memory leak } From 5b6b7a2222d3f32ce7b7375c76523044f3ff5958 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 1 Dec 2022 16:25:51 +0100 Subject: [PATCH 20/20] [doc] Update release notes (#4237) --- docs/pages/7_0_0_release_notes.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/pages/7_0_0_release_notes.md b/docs/pages/7_0_0_release_notes.md index 2827008896..5ff9fc58d1 100644 --- a/docs/pages/7_0_0_release_notes.md +++ b/docs/pages/7_0_0_release_notes.md @@ -188,6 +188,9 @@ conversions that may be made implicit. * {% rule "java/design/UseUtilityClass" %}: The property `ignoredAnnotations` has been removed. * {% rule "java/design/LawOfDemeter" %}: the rule has a new property `trustRadius`. This defines the maximum degree of trusted data. The default of 1 is the most restrictive. +* {% rule "java/documentation/CommentContent" %}: The properties `caseSensitive` and `disallowedTerms` are removed. The + new property `fobiddenRegex` can be used now to define the disallowed terms with a single regular + expression. #### Deprecated Rules