From 056516bee4f02af66f4f74aa637c29547f3b42b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sat, 14 Jan 2023 22:33:40 +0100 Subject: [PATCH 1/5] Micro-opts --- .../src/main/java/net/sourceforge/pmd/RuleSet.java | 2 +- .../sourceforge/pmd/lang/ast/impl/AbstractNode.java | 12 +++++++----- .../pmd/lang/rule/xpath/internal/AstTreeInfo.java | 13 +++++++++++++ .../net/sourceforge/pmd/lang/ast/DummyNode.java | 10 ---------- .../pmd/lang/java/ast/ASTAnnotationMemberList.java | 5 ----- .../pmd/lang/java/ast/ASTIntersectionType.java | 6 ------ .../net/sourceforge/pmd/lang/java/ast/ASTList.java | 4 ---- .../pmd/lang/java/ast/ASTMemberValuePair.java | 6 ------ .../sourceforge/pmd/lang/java/ast/ASTUnionType.java | 4 ---- .../pmd/lang/java/ast/ASTWildcardType.java | 5 ----- .../pmd/lang/java/ast/InternalInterfaces.java | 11 ++++------- .../rule/design/UselessOverridingMethodRule.java | 3 ++- .../java/rule/documentation/CommentSizeRule.java | 7 +++---- .../AvoidInstantiatingObjectsInLoopsRule.java | 7 +------ .../sourceforge/pmd/lang/java/ast/NodeParsingCtx.kt | 2 +- 15 files changed, 32 insertions(+), 65 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSet.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSet.java index 3614388738..723f5216eb 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSet.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSet.java @@ -650,7 +650,7 @@ public class RuleSet implements ChecksumAware { public static boolean applies(Rule rule, LanguageVersion languageVersion) { final LanguageVersion min = rule.getMinimumLanguageVersion(); final LanguageVersion max = rule.getMaximumLanguageVersion(); - Objects.requireNonNull(rule.getLanguage(), "Rule has no language " + rule); + assert rule.getLanguage() != null : "Rule has no language " + rule; return rule.getLanguage().equals(languageVersion.getLanguage()) && (min == null || min.compareTo(languageVersion) <= 0) && (max == null || max.compareTo(languageVersion) >= 0); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/AbstractNode.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/AbstractNode.java index 9d3839d411..5481dc9f70 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/AbstractNode.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/AbstractNode.java @@ -24,7 +24,9 @@ import net.sourceforge.pmd.util.DataMap.DataKey; * @param Public interface for nodes of this language (eg JavaNode * in the java module). */ -public abstract class AbstractNode, N extends GenericNode> implements GenericNode { +public abstract class AbstractNode, + // node the Node as first bound here is to make casts from Node to N noops at runtime. + N extends Node & GenericNode> implements GenericNode { private static final Node[] EMPTY_ARRAY = new Node[0]; @@ -41,22 +43,22 @@ public abstract class AbstractNode, N extends Gener } @Override - public N getParent() { + public final N getParent() { return (N) parent; } @Override - public int getIndexInParent() { + public final int getIndexInParent() { return childIndex; } @Override - public N getChild(final int index) { + public final N getChild(final int index) { return (N) children[index]; } @Override - public int getNumChildren() { + public final int getNumChildren() { return children.length; } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/AstTreeInfo.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/AstTreeInfo.java index 386fa630b0..9469f6b8e7 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/AstTreeInfo.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/AstTreeInfo.java @@ -4,7 +4,10 @@ package net.sourceforge.pmd.lang.rule.xpath.internal; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; +import java.util.Map.Entry; import org.apache.commons.lang3.mutable.MutableInt; @@ -21,6 +24,12 @@ import net.sf.saxon.om.GenericTreeInfo; public final class AstTreeInfo extends GenericTreeInfo { private DeprecatedAttrLogger logger; + private final Map wrapperCache = new LinkedHashMap() { + @Override + protected boolean removeEldestEntry(Entry eldest) { + return size() > 128; + } + }; /** * Builds an AstDocument, with the given node as the root. @@ -37,6 +46,10 @@ public final class AstTreeInfo extends GenericTreeInfo { } public AstElementNode findWrapperFor(Node node) { + return wrapperCache.computeIfAbsent(node, this::findWrapperImpl); + } + + private AstElementNode findWrapperImpl(Node node) { // for the RootNode, this returns the document node List indices = node.ancestorsOrSelf().toList(Node::getIndexInParent); AstElementNode cur = getRootNode().getRootElement(); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNode.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNode.java index 4b7a88fb87..9c5b3e0bb7 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNode.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNode.java @@ -50,21 +50,11 @@ public class DummyNode extends AbstractNode { } } - @Override - public DummyNode getParent() { - return super.getParent(); - } - @Override public void addChild(DummyNode child, int index) { super.addChild(child, index); } - @Override - public DummyNode getChild(int index) { - return super.getChild(index); - } - @Override public void setParent(DummyNode node) { super.setParent(node); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotationMemberList.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotationMemberList.java index b9b6123601..d36eab477a 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotationMemberList.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotationMemberList.java @@ -24,11 +24,6 @@ public final class ASTAnnotationMemberList extends ASTMaybeEmptyListOf R acceptVisitor(JavaVisitor visitor, P data) { return visitor.visit(this, data); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTList.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTList.java index 5af84841a2..ff06abdb17 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTList.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTList.java @@ -167,9 +167,5 @@ public abstract class ASTList extends AbstractJavaNode imple return children(); } - @Override - public T getChild(int index) { - return (T) super.getChild(index); - } } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMemberValuePair.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMemberValuePair.java index 784dfea68d..02d0fcb2fe 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMemberValuePair.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMemberValuePair.java @@ -53,12 +53,6 @@ public final class ASTMemberValuePair extends AbstractJavaNode { } - @Override - public ASTAnnotationMemberList getParent() { - return (ASTAnnotationMemberList) super.getParent(); - } - - @Override protected R acceptVisitor(JavaVisitor visitor, P data) { return visitor.visit(this, data); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnionType.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnionType.java index d25de5e9f0..409e60f3bd 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnionType.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnionType.java @@ -47,8 +47,4 @@ public final class ASTUnionType extends AbstractJavaTypeNode return children(ASTClassOrInterfaceType.class).iterator(); } - @Override - public ASTClassOrInterfaceType getChild(int index) { - return (ASTClassOrInterfaceType) super.getChild(index); - } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTWildcardType.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTWildcardType.java index dc97754a65..09fa07f8cc 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTWildcardType.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTWildcardType.java @@ -28,11 +28,6 @@ public final class ASTWildcardType extends AbstractJavaTypeNode implements ASTRe isLowerBound = lowerBound; } - @Override - public ASTTypeArguments getParent() { - return (ASTTypeArguments) super.getParent(); - } - /** * Return true if this is an upper type bound, e.g. * {@code }, or the unbounded diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/InternalInterfaces.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/InternalInterfaces.java index d5660840b6..3b41ebb4e4 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/InternalInterfaces.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/InternalInterfaces.java @@ -84,16 +84,13 @@ final class InternalInterfaces { interface AllChildrenAreOfType extends JavaNode { - @Override - T getChild(int index); - @Override @Nullable default T getFirstChild() { if (getNumChildren() == 0) { return null; } - return getChild(0); + return (T) getChild(0); } @@ -103,7 +100,7 @@ final class InternalInterfaces { if (getNumChildren() == 0) { return null; } - return getChild(getNumChildren() - 1); + return (T) getChild(getNumChildren() - 1); } } @@ -118,7 +115,7 @@ final class InternalInterfaces { @NonNull default T getFirstChild() { assert getNumChildren() > 0 : "No children for node implementing AtLeastOneChild " + this; - return getChild(0); + return (T) getChild(0); } @@ -127,7 +124,7 @@ final class InternalInterfaces { @NonNull default T getLastChild() { assert getNumChildren() > 0 : "No children for node implementing AtLeastOneChild " + this; - return getChild(getNumChildren() - 1); + return (T) getChild(getNumChildren() - 1); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UselessOverridingMethodRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UselessOverridingMethodRule.java index cd764b2d86..fde6edc99c 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UselessOverridingMethodRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UselessOverridingMethodRule.java @@ -12,6 +12,7 @@ import static net.sourceforge.pmd.properties.PropertyFactory.booleanProperty; import java.lang.reflect.Modifier; import net.sourceforge.pmd.lang.java.ast.ASTArgumentList; +import net.sourceforge.pmd.lang.java.ast.ASTExpression; import net.sourceforge.pmd.lang.java.ast.ASTExpressionStatement; import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter; import net.sourceforge.pmd.lang.java.ast.ASTList; @@ -97,7 +98,7 @@ public class UselessOverridingMethodRule extends AbstractJavaRulechainRule { ASTArgumentList arg = methodCall.getArguments(); int i = 0; for (ASTFormalParameter formal : node.getFormalParameters()) { - if (!JavaAstUtils.isReferenceToVar(arg.getChild(i), formal.getVarId().getSymbol())) { + if (!JavaAstUtils.isReferenceToVar((ASTExpression) arg.getChild(i), formal.getVarId().getSymbol())) { return false; } 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 6a409f3c76..fa23667104 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 @@ -67,12 +67,11 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { int offset = comment.getBeginLine(); for (int lineNum : lineNumbers) { int lineNumWithOff = lineNum + offset; - addViolationWithMessage( - data, + asCtx(data).addViolationWithPosition( cUnit, - this.getMessage() + ": Line too long", lineNumWithOff, - lineNum + lineNumWithOff, + this.getMessage() + ": Line too long" ); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidInstantiatingObjectsInLoopsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidInstantiatingObjectsInLoopsRule.java index a491bb20f2..14ae136dc9 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidInstantiatingObjectsInLoopsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidInstantiatingObjectsInLoopsRule.java @@ -19,7 +19,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTForeachStatement; import net.sourceforge.pmd.lang.java.ast.ASTLoopStatement; import net.sourceforge.pmd.lang.java.ast.ASTMethodCall; import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement; -import net.sourceforge.pmd.lang.java.ast.ASTStatement; import net.sourceforge.pmd.lang.java.ast.ASTThrowStatement; import net.sourceforge.pmd.lang.java.ast.JavaNode; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; @@ -80,11 +79,7 @@ public class AvoidInstantiatingObjectsInLoopsRule extends AbstractJavaRulechainR private boolean notBreakFollowing(JavaNode node) { JavaNode statement = node.ancestors().filter(n -> n.getParent() instanceof ASTBlock).first(); if (statement != null) { - ASTBlock block = (ASTBlock) statement.getParent(); - if (block.getNumChildren() > statement.getIndexInParent() + 1) { - ASTStatement next = block.getChild(statement.getIndexInParent() + 1); - return !(next instanceof ASTBreakStatement); - } + return !(statement.getNextSibling() instanceof ASTBreakStatement); } return true; } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/NodeParsingCtx.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/NodeParsingCtx.kt index 4f1c72d546..b104247530 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/NodeParsingCtx.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/NodeParsingCtx.kt @@ -117,7 +117,7 @@ object StatementParsingCtx : NodeParsingCtx("statement") { override fun retrieveNode(acu: ASTCompilationUnit): ASTStatement = TypeBodyParsingCtx.retrieveNode(acu) .descendants(ASTBlock::class.java) - .firstOrThrow().getChild(0) + .firstOrThrow().firstChild as ASTStatement } object TypeBodyParsingCtx : NodeParsingCtx("body declaration") { From ea4c8271169b33e3e79f4b5003e2c8cb8bbff5eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 23 Jan 2023 17:33:33 +0100 Subject: [PATCH 2/5] Convert AddEmptyString to java rule --- .../rule/performance/AddEmptyStringRule.java | 49 +++++++++++++++++++ .../resources/category/java/performance.xml | 17 +------ 2 files changed, 50 insertions(+), 16 deletions(-) create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AddEmptyStringRule.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AddEmptyStringRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AddEmptyStringRule.java new file mode 100644 index 0000000000..5c4224fb2e --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AddEmptyStringRule.java @@ -0,0 +1,49 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.performance; + +import net.sourceforge.pmd.lang.java.ast.ASTAnnotation; +import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr; +import net.sourceforge.pmd.lang.java.ast.ASTStringLiteral; +import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator; +import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId; +import net.sourceforge.pmd.lang.java.ast.BinaryOp; +import net.sourceforge.pmd.lang.java.ast.JModifier; +import net.sourceforge.pmd.lang.java.ast.JavaNode; +import net.sourceforge.pmd.lang.java.ast.internal.JavaAstUtils; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; + + +public class AddEmptyStringRule extends AbstractJavaRulechainRule { + + public AddEmptyStringRule() { + super(ASTStringLiteral.class); + } + + @Override + public Object visit(ASTStringLiteral node, Object data) { + if (!node.isEmpty()) { + return null; + } + JavaNode parent = node.getParent(); + checkExpr(data, parent); + if (parent instanceof ASTVariableDeclarator) { + ASTVariableDeclaratorId varId = ((ASTVariableDeclarator) parent).getVarId(); + if (varId.hasModifiers(JModifier.FINAL)) { + for (ASTNamedReferenceExpr usage : varId.getLocalUsages()) { + checkExpr(data, usage.getParent()); + } + } + } + return null; + } + + private void checkExpr(Object data, JavaNode parent) { + if (JavaAstUtils.isInfixExprWithOperator(parent, BinaryOp.ADD) + && parent.ancestors(ASTAnnotation.class).isEmpty()) { + addViolation(data, parent); + } + } +} diff --git a/pmd-java/src/main/resources/category/java/performance.xml b/pmd-java/src/main/resources/category/java/performance.xml index d52a13006e..e586ada4cb 100644 --- a/pmd-java/src/main/resources/category/java/performance.xml +++ b/pmd-java/src/main/resources/category/java/performance.xml @@ -13,28 +13,13 @@ Rules that flag suboptimal code. language="java" since="4.0" message="Do not add empty strings" - class="net.sourceforge.pmd.lang.rule.XPathRule" + class="net.sourceforge.pmd.lang.java.rule.performance.AddEmptyStringRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_performance.html#addemptystring"> The conversion of literals to strings by concatenating them with empty strings is inefficient. It is much better to use one of the type-specific `toString()` methods instead or `String.valueOf()`. 3 - - - - - - - Date: Fri, 27 Jan 2023 15:50:45 +0100 Subject: [PATCH 3/5] Dogfood --- .../performance/AvoidInstantiatingObjectsInLoopsRule.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidInstantiatingObjectsInLoopsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidInstantiatingObjectsInLoopsRule.java index 14ae136dc9..b43c5be2f0 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidInstantiatingObjectsInLoopsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidInstantiatingObjectsInLoopsRule.java @@ -78,10 +78,7 @@ public class AvoidInstantiatingObjectsInLoopsRule extends AbstractJavaRulechainR private boolean notBreakFollowing(JavaNode node) { JavaNode statement = node.ancestors().filter(n -> n.getParent() instanceof ASTBlock).first(); - if (statement != null) { - return !(statement.getNextSibling() instanceof ASTBreakStatement); - } - return true; + return statement == null || !(statement.getNextSibling() instanceof ASTBreakStatement); } /** From 8834c6e981c9b1bcdee2a2cc39714a086181076b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 27 Jan 2023 16:09:46 +0100 Subject: [PATCH 4/5] Improve CommentSize --- .../pmd/lang/java/ast/JavaComment.java | 2 +- .../rule/documentation/CommentSizeRule.java | 46 +++++-------------- .../rule/documentation/xml/CommentSize.xml | 21 +++++++++ 3 files changed, 33 insertions(+), 36 deletions(-) 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 8fe146edfc..3e19a5923c 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 @@ -116,7 +116,7 @@ public class JavaComment implements Reportable { * Trim the start of the provided line to remove a comment * markup opener ({@code //, /*, /**, *}) or closer {@code * /}. */ - private static Chars removeCommentMarkup(Chars line) { + public static Chars removeCommentMarkup(Chars line) { line = line.trim().removeSuffix("*/"); int subseqFrom = 0; if (line.startsWith('/', 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 fa23667104..7c999ea643 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 @@ -5,14 +5,8 @@ package net.sourceforge.pmd.lang.java.rule.documentation; import static net.sourceforge.pmd.properties.constraints.NumericConstraints.positive; -import static net.sourceforge.pmd.util.CollectionUtil.setOf; - -import java.util.ArrayList; -import java.util.List; -import java.util.Set; - -import org.apache.commons.lang3.StringUtils; +import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.lang.document.Chars; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.JavaComment; @@ -37,12 +31,6 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { .desc("Maximum line length") .require(positive()).defaultValue(80).build(); - static final Set IGNORED_LINES = setOf(Chars.wrap("//"), - Chars.wrap("/*"), - Chars.wrap("/**"), - Chars.wrap("*"), - Chars.wrap("*/")); - public CommentSizeRule() { super(ASTCompilationUnit.class); definePropertyDescriptor(MAX_LINES); @@ -59,28 +47,14 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { + ": Too many lines", comment.getBeginLine(), comment.getEndLine()); } - List lineNumbers = overLengthLineIndicesIn(comment); - if (lineNumbers.isEmpty()) { - continue; - } - - int offset = comment.getBeginLine(); - for (int lineNum : lineNumbers) { - int lineNumWithOff = lineNum + offset; - asCtx(data).addViolationWithPosition( - cUnit, - lineNumWithOff, - lineNumWithOff, - this.getMessage() + ": Line too long" - ); - } + reportLinesTooLong(cUnit, asCtx(data), comment); } return null; } private static boolean hasRealText(Chars line) { - return !StringUtils.isBlank(line) && !IGNORED_LINES.contains(line.trim()); + return !JavaComment.removeCommentMarkup(line).isEmpty(); } private boolean hasTooManyLines(JavaComment comment) { @@ -90,8 +64,7 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { int i = 0; int maxLines = getProperty(MAX_LINES); for (Chars line : comment.getText().lines()) { - boolean real = hasRealText(line); - if (real) { + if (hasRealText(line)) { lastLineWithText = i; if (firstLineWithText == -1) { firstLineWithText = i; @@ -105,18 +78,21 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { return false; } - private List overLengthLineIndicesIn(JavaComment comment) { + private void reportLinesTooLong(ASTCompilationUnit acu, RuleContext ctx, JavaComment comment) { int maxLength = getProperty(MAX_LINE_LENGTH); - List indices = new ArrayList<>(); + int offset = comment.getReportLocation().getStartLine(); int i = 0; for (Chars line : comment.getFilteredLines(true)) { if (line.length() > maxLength) { - indices.add(i); + ctx.addViolationWithPosition(acu, + i + offset, + i + offset, + getMessage() + ": Line too long"); } + i++; } - return indices; } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/documentation/xml/CommentSize.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/documentation/xml/CommentSize.xml index 0469bd08f2..2df48596fd 100755 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/documentation/xml/CommentSize.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/documentation/xml/CommentSize.xml @@ -22,6 +22,27 @@ public class Foo { public Foo() { } + public void doNothing() { + } +} + ]]> + + + Line too long + 5 + 1 + 5 + Date: Fri, 27 Jan 2023 16:12:55 +0100 Subject: [PATCH 5/5] Revert changes to CommentSize --- .../rule/documentation/CommentSizeRule.java | 47 ++++++++++++++----- .../rule/documentation/xml/CommentSize.xml | 21 --------- 2 files changed, 36 insertions(+), 32 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 7c999ea643..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 @@ -5,8 +5,14 @@ package net.sourceforge.pmd.lang.java.rule.documentation; import static net.sourceforge.pmd.properties.constraints.NumericConstraints.positive; +import static net.sourceforge.pmd.util.CollectionUtil.setOf; + +import java.util.ArrayList; +import java.util.List; +import java.util.Set; + +import org.apache.commons.lang3.StringUtils; -import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.lang.document.Chars; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.JavaComment; @@ -31,6 +37,12 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { .desc("Maximum line length") .require(positive()).defaultValue(80).build(); + static final Set IGNORED_LINES = setOf(Chars.wrap("//"), + Chars.wrap("/*"), + Chars.wrap("/**"), + Chars.wrap("*"), + Chars.wrap("*/")); + public CommentSizeRule() { super(ASTCompilationUnit.class); definePropertyDescriptor(MAX_LINES); @@ -47,14 +59,29 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { + ": Too many lines", comment.getBeginLine(), comment.getEndLine()); } - reportLinesTooLong(cUnit, asCtx(data), comment); + List lineNumbers = overLengthLineIndicesIn(comment); + if (lineNumbers.isEmpty()) { + continue; + } + + int offset = comment.getBeginLine(); + for (int lineNum : lineNumbers) { + int lineNumWithOff = lineNum + offset; + addViolationWithMessage( + data, + cUnit, + this.getMessage() + ": Line too long", + lineNumWithOff, + lineNum + ); + } } return null; } private static boolean hasRealText(Chars line) { - return !JavaComment.removeCommentMarkup(line).isEmpty(); + return !StringUtils.isBlank(line) && !IGNORED_LINES.contains(line.trim()); } private boolean hasTooManyLines(JavaComment comment) { @@ -64,7 +91,8 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { int i = 0; int maxLines = getProperty(MAX_LINES); for (Chars line : comment.getText().lines()) { - if (hasRealText(line)) { + boolean real = hasRealText(line); + if (real) { lastLineWithText = i; if (firstLineWithText == -1) { firstLineWithText = i; @@ -78,21 +106,18 @@ public class CommentSizeRule extends AbstractJavaRulechainRule { return false; } - private void reportLinesTooLong(ASTCompilationUnit acu, RuleContext ctx, JavaComment comment) { + private List overLengthLineIndicesIn(JavaComment comment) { int maxLength = getProperty(MAX_LINE_LENGTH); - int offset = comment.getReportLocation().getStartLine(); + List indices = new ArrayList<>(); int i = 0; for (Chars line : comment.getFilteredLines(true)) { if (line.length() > maxLength) { - ctx.addViolationWithPosition(acu, - i + offset, - i + offset, - getMessage() + ": Line too long"); + indices.add(i); } - i++; } + return indices; } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/documentation/xml/CommentSize.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/documentation/xml/CommentSize.xml index 2df48596fd..0469bd08f2 100755 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/documentation/xml/CommentSize.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/documentation/xml/CommentSize.xml @@ -22,27 +22,6 @@ public class Foo { public Foo() { } - public void doNothing() { - } -} - ]]> - - - Line too long - 5 - 1 - 5 -