Merge pull request #4353 from oowekyala/pmd7.micro-opts

[core] Micro optimizations for Node API
This commit is contained in:
Juan Martín Sotuyo Dodero
2023-01-27 19:44:44 -03:00
committed by GitHub
16 changed files with 79 additions and 80 deletions

View File

@@ -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);

View File

@@ -24,7 +24,9 @@ import net.sourceforge.pmd.util.DataMap.DataKey;
* @param <N> Public interface for nodes of this language (eg JavaNode
* in the java module).
*/
public abstract class AbstractNode<B extends AbstractNode<B, N>, N extends GenericNode<N>> implements GenericNode<N> {
public abstract class AbstractNode<B extends AbstractNode<B, N>,
// node the Node as first bound here is to make casts from Node to N noops at runtime.
N extends Node & GenericNode<N>> implements GenericNode<N> {
private static final Node[] EMPTY_ARRAY = new Node[0];
@@ -41,22 +43,22 @@ public abstract class AbstractNode<B extends AbstractNode<B, N>, 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;
}

View File

@@ -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<Node, AstElementNode> wrapperCache = new LinkedHashMap<Node, AstElementNode>() {
@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<Integer> indices = node.ancestorsOrSelf().toList(Node::getIndexInParent);
AstElementNode cur = getRootNode().getRootElement();

View File

@@ -50,21 +50,11 @@ public class DummyNode extends AbstractNode<DummyNode, DummyNode> {
}
}
@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);

View File

@@ -24,11 +24,6 @@ public final class ASTAnnotationMemberList extends ASTMaybeEmptyListOf<ASTMember
super(id, ASTMemberValuePair.class);
}
@Override
public ASTAnnotation getParent() {
return (ASTAnnotation) super.getParent();
}
/**
* Returns the value of the attribute with the given name, returns
* null if no such attribute was mentioned.

View File

@@ -40,12 +40,6 @@ public final class ASTIntersectionType extends AbstractJavaTypeNode
return children(ASTClassOrInterfaceType.class);
}
@Override
public ASTType getChild(int index) {
return (ASTType) super.getChild(index);
}
@Override
protected <P, R> R acceptVisitor(JavaVisitor<? super P, ? extends R> visitor, P data) {
return visitor.visit(this, data);

View File

@@ -167,9 +167,5 @@ public abstract class ASTList<N extends JavaNode> extends AbstractJavaNode imple
return children();
}
@Override
public T getChild(int index) {
return (T) super.getChild(index);
}
}
}

View File

@@ -53,12 +53,6 @@ public final class ASTMemberValuePair extends AbstractJavaNode {
}
@Override
public ASTAnnotationMemberList getParent() {
return (ASTAnnotationMemberList) super.getParent();
}
@Override
protected <P, R> R acceptVisitor(JavaVisitor<? super P, ? extends R> visitor, P data) {
return visitor.visit(this, data);

View File

@@ -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);
}
}

View File

@@ -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 <? extends Integer>}, or the unbounded

View File

@@ -84,16 +84,13 @@ final class InternalInterfaces {
interface AllChildrenAreOfType<T extends JavaNode> 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);
}
}

View File

@@ -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++;

View File

@@ -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);
}
}
}

View File

@@ -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;
@@ -79,14 +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) {
ASTBlock block = (ASTBlock) statement.getParent();
if (block.getNumChildren() > statement.getIndexInParent() + 1) {
ASTStatement next = block.getChild(statement.getIndexInParent() + 1);
return !(next instanceof ASTBreakStatement);
}
}
return true;
return statement == null || !(statement.getNextSibling() instanceof ASTBreakStatement);
}
/**

View File

@@ -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">
<description>
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()`.
</description>
<priority>3</priority>
<properties>
<property name="xpath">
<value>
<![CDATA[
//InfixExpression[@Operator='+']/StringLiteral[@Empty=true()][not(ancestor::Annotation)]
|
//InfixExpression[@Operator='+']/VariableAccess
[@Name = (//FieldDeclaration[pmd-java:modifiers() = 'final']|ancestor::MethodDeclaration//LocalVariableDeclaration[pmd-java:modifiers() = 'final'])
/VariableDeclarator[@Initializer = true()]
[StringLiteral[@Empty=true()]]
/VariableDeclaratorId/@Name]
]]>
</value>
</property>
</properties>
<example>
<![CDATA[
String s = "" + 123; // inefficient

View File

@@ -117,7 +117,7 @@ object StatementParsingCtx : NodeParsingCtx<ASTStatement>("statement") {
override fun retrieveNode(acu: ASTCompilationUnit): ASTStatement =
TypeBodyParsingCtx.retrieveNode(acu)
.descendants(ASTBlock::class.java)
.firstOrThrow().getChild(0)
.firstOrThrow().firstChild as ASTStatement
}
object TypeBodyParsingCtx : NodeParsingCtx<ASTBodyDeclaration>("body declaration") {