CONTINUE_TARGET_MAPPER =
NodeStream.asInstanceOf(ASTLoopStatement.class);
ASTContinueStatement(int id) {
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTDoStatement.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTDoStatement.java
index 4837ee446e..0b89f8b660 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTDoStatement.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTDoStatement.java
@@ -25,6 +25,7 @@ public final class ASTDoStatement extends AbstractStatement implements ASTLoopSt
* Returns the node that represents the guard of this loop.
* This may be any expression of type boolean.
*/
+ @Override
public ASTExpression getCondition() {
return (ASTExpression) getChild(1);
}
@@ -34,6 +35,7 @@ public final class ASTDoStatement extends AbstractStatement implements ASTLoopSt
* Returns the statement that will be run while the guard
* evaluates to true.
*/
+ @Override
public ASTStatement getBody() {
return (ASTStatement) getChild(0);
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTForStatement.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTForStatement.java
index cc76696e40..79a0c21c63 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTForStatement.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTForStatement.java
@@ -29,13 +29,7 @@ public final class ASTForStatement extends AbstractStatement implements ASTLoopS
}
- /**
- * Returns the node that represents the condition of this loop.
- * This may be any expression of type boolean.
- *
- * If there is no specified guard, then returns null.
- */
- @Nullable
+ @Override
public ASTExpression getCondition() {
return getFirstChildOfType(ASTExpression.class);
}
@@ -58,9 +52,5 @@ public final class ASTForStatement extends AbstractStatement implements ASTLoopS
return update == null ? null : update.getExprList();
}
- /** Returns the statement that represents the body of this loop. */
- public ASTStatement getBody() {
- return (ASTStatement) getChild(getNumChildren() - 1);
- }
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTForeachStatement.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTForeachStatement.java
index 9adb51dbb9..3455ba3c78 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTForeachStatement.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTForeachStatement.java
@@ -43,12 +43,5 @@ public final class ASTForeachStatement extends AbstractStatement implements Inte
return getFirstChildOfType(ASTExpression.class);
}
- /**
- * Returns the statement that represents the body of this
- * loop.
- */
- public ASTStatement getBody() {
- return (ASTStatement) getChild(getNumChildren() - 1);
- }
}
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 16f4a56272..2c3589697f 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
@@ -114,6 +114,19 @@ public abstract class ASTList extends AbstractJavaNode imple
return list == null ? 0 : list.size();
}
+
+ /**
+ * Returns the element if there is exactly one, otherwise returns null.
+ *
+ * @param list List node
+ * @param Type of elements
+ *
+ * @return An element, or null.
+ */
+ public static @Nullable N singleOrNull(@Nullable ASTList list) {
+ return list == null || list.size() != 1 ? null : list.get(0);
+ }
+
/**
* Super type for *nonempty* lists that *only* have nodes of type {@code }
* as a child.
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTLoopStatement.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTLoopStatement.java
index bab6252aae..1097a4770b 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTLoopStatement.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTLoopStatement.java
@@ -4,6 +4,8 @@
package net.sourceforge.pmd.lang.java.ast;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
/**
* A loop statement.
*
@@ -19,4 +21,23 @@ package net.sourceforge.pmd.lang.java.ast;
*
*/
public interface ASTLoopStatement extends ASTStatement {
+
+
+ /** Returns the statement that represents the body of this loop. */
+ default ASTStatement getBody() {
+ return (ASTStatement) getLastChild();
+ }
+
+
+ /**
+ * Returns the node that represents the condition of this loop.
+ * This may be any expression of type boolean.
+ *
+ * If there is no specified guard, then returns null (in particular,
+ * returns null if this is a foreach loop).
+ */
+ default @Nullable ASTExpression getCondition() {
+ return null;
+ }
+
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodCall.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodCall.java
index e27673e5ff..bd06c74068 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodCall.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodCall.java
@@ -22,7 +22,8 @@ import org.checkerframework.checker.nullness.qual.Nullable;
public final class ASTMethodCall extends AbstractInvocationExpr
implements ASTPrimaryExpression,
QualifiableExpression,
- InvocationNode {
+ InvocationNode,
+ MethodUsage {
ASTMethodCall(int id) {
super(id);
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodDeclaration.java
index 7dcccf0756..7ccde2be00 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodDeclaration.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodDeclaration.java
@@ -50,7 +50,6 @@ public final class ASTMethodDeclaration extends AbstractMethodOrConstructorDecla
return visitor.visit(this, data);
}
-
/**
* Returns true if this method is overridden.
* TODO for now, this just checks for an @Override annotation,
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodReference.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodReference.java
index 79d97e486a..01bc091769 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodReference.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodReference.java
@@ -22,7 +22,11 @@ import net.sourceforge.pmd.lang.java.types.TypeSystem;
*
*
*/
-public final class ASTMethodReference extends AbstractJavaExpr implements ASTPrimaryExpression, QualifiableExpression, LeftRecursiveNode {
+public final class ASTMethodReference extends AbstractJavaExpr
+ implements ASTPrimaryExpression,
+ QualifiableExpression,
+ LeftRecursiveNode,
+ MethodUsage {
private JMethodSig functionalMethod;
private JMethodSig compileTimeDecl;
@@ -90,6 +94,7 @@ public final class ASTMethodReference extends AbstractJavaExpr implements ASTPri
* Returns the method name, or an {@link JConstructorSymbol#CTOR_NAME}
* if this is a {@linkplain #isConstructorReference() constructor reference}.
*/
+ @Override
public @NonNull String getMethodName() {
return super.getImage();
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorId.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorId.java
index d42a01fbf7..41659a2f0e 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorId.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorId.java
@@ -4,6 +4,8 @@
package net.sourceforge.pmd.lang.java.ast;
+import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
import org.checkerframework.checker.nullness.qual.NonNull;
@@ -11,6 +13,7 @@ import org.checkerframework.checker.nullness.qual.Nullable;
import net.sourceforge.pmd.annotation.InternalApi;
import net.sourceforge.pmd.lang.ast.Node;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
import net.sourceforge.pmd.lang.java.symbols.JVariableSymbol;
import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
import net.sourceforge.pmd.lang.rule.xpath.DeprecatedAttribute;
@@ -50,6 +53,8 @@ public final class ASTVariableDeclaratorId extends AbstractTypedSymbolDeclarator
private VariableNameDeclaration nameDeclaration;
+ private List usages = Collections.emptyList();
+
ASTVariableDeclaratorId(int id) {
super(id);
}
@@ -72,10 +77,33 @@ public final class ASTVariableDeclaratorId extends AbstractTypedSymbolDeclarator
nameDeclaration = decl;
}
+ /**
+ * @deprecated transitional, use {@link #getLocalUsages()}
+ */
+ @Deprecated
public List getUsages() {
return getScope().getDeclarations(VariableNameDeclaration.class).get(nameDeclaration);
}
+ /**
+ * Returns an unmodifiable list of the usages of this variable that
+ * are made in this file. Note that for a record component, this returns
+ * usages both for the formal parameter symbol and its field counterpart.
+ *
+ * Note that a variable initializer is not part of the usages
+ * (though this should be evident from the return type).
+ */
+ public List getLocalUsages() {
+ return usages;
+ }
+
+ void addUsage(ASTNamedReferenceExpr usage) {
+ if (usages.isEmpty()) {
+ usages = new ArrayList<>(4); //make modifiable
+ }
+ usages.add(usage);
+ }
+
/**
* Returns the extra array dimensions associated with this variable.
* For example in the declaration {@code int a[]}, {@link #getTypeNode()}
@@ -94,13 +122,13 @@ public final class ASTVariableDeclaratorId extends AbstractTypedSymbolDeclarator
return getModifierOwnerParent().getModifiers();
}
-
@Override
public Visibility getVisibility() {
return isPatternBinding() ? Visibility.V_LOCAL
: getModifierOwnerParent().getVisibility();
}
+
private AccessNode getModifierOwnerParent() {
JavaNode parent = getParent();
if (parent instanceof ASTVariableDeclarator) {
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTWhileStatement.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTWhileStatement.java
index 28ab3950fa..ebc39d8d0a 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTWhileStatement.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTWhileStatement.java
@@ -24,19 +24,12 @@ public final class ASTWhileStatement extends AbstractStatement implements ASTLoo
* Returns the node that represents the guard of this loop.
* This may be any expression of type boolean.
*/
+ @Override
public ASTExpression getCondition() {
return (ASTExpression) getChild(0);
}
- /**
- * Returns the statement that will be run while the guard
- * evaluates to true.
- */
- public ASTStatement getBody() {
- return (ASTStatement) getChild(1);
- }
-
@Override
protected R acceptVisitor(JavaVisitor visitor, P data) {
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/BinaryOp.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/BinaryOp.java
index 62081fc7e7..08739cd071 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/BinaryOp.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/BinaryOp.java
@@ -161,4 +161,28 @@ public enum BinaryOp implements InternalInterfaces.OperatorLike {
return -1;
}
}
+
+
+ /**
+ * Complement, for boolean operators. Eg for {@code ==}, return {@code !=},
+ * for {@code <=}, returns {@code >}. Returns null if this is another kind
+ * of operator.
+ */
+ public BinaryOp getComplement() {
+ switch (this) {
+ case CONDITIONAL_OR: return CONDITIONAL_AND;
+ case CONDITIONAL_AND: return CONDITIONAL_OR;
+ case OR: return AND;
+ case AND: return OR;
+
+ case EQ: return NE;
+ case NE: return EQ;
+ case LE: return GT;
+ case GE: return LT;
+ case GT: return LE;
+ case LT: return GE;
+
+ default: return null;
+ }
+ }
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/InternalApiBridge.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/InternalApiBridge.java
index 756f81a3cf..3f50e561bc 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/InternalApiBridge.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/InternalApiBridge.java
@@ -10,6 +10,7 @@ import net.sourceforge.pmd.annotation.InternalApi;
import net.sourceforge.pmd.lang.ast.NodeStream;
import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken;
import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccTokenDocument;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
import net.sourceforge.pmd.lang.java.internal.JavaAstProcessor;
import net.sourceforge.pmd.lang.java.symbols.JClassSymbol;
import net.sourceforge.pmd.lang.java.symbols.JConstructorSymbol;
@@ -122,6 +123,20 @@ public final class InternalApiBridge {
AstDisambiguationPass.disambigWithCtx(nodes, ctx);
}
+ public static void usageResolution(JavaAstProcessor processor, ASTCompilationUnit root) {
+ root.descendants(ASTNamedReferenceExpr.class)
+ .crossFindBoundaries()
+ .forEach(node -> {
+ JVariableSymbol sym = node.getReferencedSym();
+ if (sym != null) {
+ ASTVariableDeclaratorId reffed = sym.tryGetNode();
+ if (reffed != null) { // declared in this file
+ reffed.addUsage(node);
+ }
+ }
+ });
+ }
+
public static @Nullable JTypeMirror getTypeMirrorInternal(TypeNode node) {
return ((AbstractJavaTypeNode) node).getTypeMirrorInternal();
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/InvocationNode.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/InvocationNode.java
index 1c8e94f9ab..80563b665b 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/InvocationNode.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/InvocationNode.java
@@ -4,10 +4,8 @@
package net.sourceforge.pmd.lang.java.ast;
-import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
-import net.sourceforge.pmd.lang.java.symbols.JConstructorSymbol;
import net.sourceforge.pmd.lang.java.types.JMethodSig;
import net.sourceforge.pmd.lang.java.types.OverloadSelectionResult;
@@ -24,7 +22,7 @@ import net.sourceforge.pmd.lang.java.types.OverloadSelectionResult;
* of the {@linkplain #getMethodType() compile-time declaration}
* of this node.
*/
-public interface InvocationNode extends TypeNode {
+public interface InvocationNode extends TypeNode, MethodUsage {
/**
* Returns the node representing the list of arguments
@@ -57,12 +55,5 @@ public interface InvocationNode extends TypeNode {
*/
OverloadSelectionResult getOverloadSelectionInfo();
- /**
- * Returns the name of the called method. If this is a constructor
- * call, returns {@link JConstructorSymbol#CTOR_NAME}.
- */
- default @NonNull String getMethodName() {
- return JConstructorSymbol.CTOR_NAME;
- }
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaVisitorBase.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaVisitorBase.java
index f984b710b0..ef18296cef 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaVisitorBase.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaVisitorBase.java
@@ -5,6 +5,7 @@
package net.sourceforge.pmd.lang.java.ast;
import net.sourceforge.pmd.lang.ast.AstVisitorBase;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
/**
* Base implementation of {@link JavaVisitor}. This adds delegation logic
@@ -185,11 +186,6 @@ public class JavaVisitorBase extends AstVisitorBase implements JavaV
return visitPrimaryExpr(node, data);
}
- @Override
- public R visit(ASTFieldAccess node, P data) {
- return visitPrimaryExpr(node, data);
- }
-
@Override
public R visit(ASTConstructorCall node, P data) {
return visitPrimaryExpr(node, data);
@@ -207,10 +203,18 @@ public class JavaVisitorBase extends AstVisitorBase implements JavaV
return visitPrimaryExpr(node, data);
}
+ public R visitNamedExpr(ASTNamedReferenceExpr node, P data) {
+ return visitPrimaryExpr(node, data);
+ }
@Override
public R visit(ASTVariableAccess node, P data) {
- return visitPrimaryExpr(node, data);
+ return visitNamedExpr(node, data);
+ }
+
+ @Override
+ public R visit(ASTFieldAccess node, P data) {
+ return visitNamedExpr(node, data);
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/MethodUsage.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/MethodUsage.java
new file mode 100644
index 0000000000..d85602411c
--- /dev/null
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/MethodUsage.java
@@ -0,0 +1,27 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.ast;
+
+import org.checkerframework.checker.nullness.qual.NonNull;
+
+import net.sourceforge.pmd.lang.java.symbols.JConstructorSymbol;
+
+/**
+ * A node that uses another method or constructor. Those are
+ * {@link InvocationNode#getMethodType() InvocationNode}s and
+ * {@link ASTMethodReference#getReferencedMethod() MethodReference}.
+ *
+ * TODO should these method be named the same, and added to this interface?
+ */
+public interface MethodUsage extends JavaNode {
+
+ /**
+ * Returns the name of the called method. If this is a constructor
+ * call, returns {@link JConstructorSymbol#CTOR_NAME}.
+ */
+ default @NonNull String getMethodName() {
+ return JConstructorSymbol.CTOR_NAME;
+ }
+}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/internal/JavaAstProcessor.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/internal/JavaAstProcessor.java
index 2ef32105b4..f43dbd57ed 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/internal/JavaAstProcessor.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/internal/JavaAstProcessor.java
@@ -145,6 +145,7 @@ public final class JavaAstProcessor {
bench("2. Symbol table resolution", () -> SymbolTableResolver.traverse(this, acu));
bench("3. AST disambiguation", () -> InternalApiBridge.disambigWithCtx(NodeStream.of(acu), ReferenceCtx.root(this, acu)));
bench("4. Comment assignment", () -> InternalApiBridge.assignComments(acu));
+ bench("5. Usage resolution", () -> InternalApiBridge.usageResolution(this, acu));
}
public TypeSystem getTypeSystem() {
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractClassWithoutAbstractMethodRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractClassWithoutAbstractMethodRule.java
index e6f5f12208..0d0d8c33e4 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractClassWithoutAbstractMethodRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractClassWithoutAbstractMethodRule.java
@@ -5,45 +5,33 @@
package net.sourceforge.pmd.lang.java.rule.bestpractices;
-import org.checkerframework.checker.nullness.qual.NonNull;
-
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTExtendsList;
-import net.sourceforge.pmd.lang.java.ast.ASTImplementsList;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
-import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
-import net.sourceforge.pmd.lang.rule.RuleTargetSelector;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
-public class AbstractClassWithoutAbstractMethodRule extends AbstractJavaRule {
+public class AbstractClassWithoutAbstractMethodRule extends AbstractJavaRulechainRule {
- @Override
- protected @NonNull RuleTargetSelector buildTargetSelector() {
- return RuleTargetSelector.forTypes(ASTClassOrInterfaceDeclaration.class);
+ public AbstractClassWithoutAbstractMethodRule() {
+ super(ASTClassOrInterfaceDeclaration.class);
}
@Override
public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
- if (!node.isAbstract() || doesExtend(node) || doesImplement(node)) {
+ if (node.isInterface() || !node.isAbstract() || doesExtend(node) || doesImplement(node)) {
return data;
}
- int countOfAbstractMethods = 0;
- for (ASTMethodDeclaration methodDecl : node.descendants(ASTMethodDeclaration.class)) {
- if (methodDecl.isAbstract()) {
- countOfAbstractMethods++;
- }
- }
- if (countOfAbstractMethods == 0) {
+ if (node.getDeclarations(ASTMethodDeclaration.class).none(ASTMethodDeclaration::isAbstract)) {
addViolation(data, node);
}
return data;
}
private boolean doesExtend(ASTClassOrInterfaceDeclaration node) {
- return node.getFirstChildOfType(ASTExtendsList.class) != null;
+ return node.getSuperClassTypeNode() != null;
}
private boolean doesImplement(ASTClassOrInterfaceDeclaration node) {
- return node.getFirstChildOfType(ASTImplementsList.class) != null;
+ return !node.getSuperInterfaceTypeNodes().isEmpty();
}
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningCatchVariablesRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningCatchVariablesRule.java
index 108f038fac..874c9ae288 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningCatchVariablesRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningCatchVariablesRule.java
@@ -4,48 +4,31 @@
package net.sourceforge.pmd.lang.java.rule.bestpractices;
-import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator;
-import net.sourceforge.pmd.lang.java.ast.ASTCatchClause;
-import net.sourceforge.pmd.lang.java.ast.ASTName;
-import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
+import org.checkerframework.checker.nullness.qual.NonNull;
+
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.AccessType;
+import net.sourceforge.pmd.lang.java.ast.ASTCatchParameter;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
-import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
-import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
+import net.sourceforge.pmd.lang.rule.RuleTargetSelector;
public class AvoidReassigningCatchVariablesRule extends AbstractJavaRule {
- public AvoidReassigningCatchVariablesRule() {
- addRuleChainVisit(ASTCatchClause.class);
+ @Override
+ protected @NonNull RuleTargetSelector buildTargetSelector() {
+ return RuleTargetSelector.forTypes(ASTCatchParameter.class);
}
@Override
- public Object visit(ASTCatchClause catchStatement, Object data) {
- ASTVariableDeclaratorId caughtExceptionId = catchStatement.getParameter().getVarId();
- String caughtExceptionVar = caughtExceptionId.getName();
- for (NameOccurrence usage : caughtExceptionId.getUsages()) {
- JavaNode operation = getOperationOfUsage(usage);
- if (isAssignment(operation)) {
- String assignedVar = getAssignedVariableName(operation);
- if (caughtExceptionVar.equals(assignedVar)) {
- addViolation(data, operation, caughtExceptionVar);
- }
+ public Object visit(ASTCatchParameter catchParam, Object data) {
+ ASTVariableDeclaratorId caughtExceptionId = catchParam.getVarId();
+ for (ASTNamedReferenceExpr usage : caughtExceptionId.getLocalUsages()) {
+ if (usage.getAccessType() == AccessType.WRITE) {
+ addViolation(data, usage, caughtExceptionId.getName());
}
+
}
return data;
}
-
- private JavaNode getOperationOfUsage(NameOccurrence usage) {
- return usage.getLocation()
- .getFirstParentOfType(ASTPrimaryExpression.class)
- .getParent();
- }
-
- private boolean isAssignment(JavaNode operation) {
- return operation.hasDescendantOfType(ASTAssignmentOperator.class);
- }
-
- private String getAssignedVariableName(JavaNode operation) {
- return operation.getFirstDescendantOfType(ASTName.class).getImage();
- }
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningLoopVariablesRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningLoopVariablesRule.java
index 939f4c73b4..138edf64b5 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningLoopVariablesRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningLoopVariablesRule.java
@@ -8,275 +8,212 @@ import static java.util.Arrays.asList;
import static net.sourceforge.pmd.properties.PropertyFactory.enumProperty;
import static net.sourceforge.pmd.util.CollectionUtil.associateBy;
-import java.util.HashSet;
-import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.stream.Collectors;
-import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator;
+import net.sourceforge.pmd.RuleContext;
+import net.sourceforge.pmd.lang.ast.NodeStream;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.AccessType;
import net.sourceforge.pmd.lang.java.ast.ASTBlock;
-import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement;
+import net.sourceforge.pmd.lang.java.ast.ASTBreakStatement;
import net.sourceforge.pmd.lang.java.ast.ASTContinueStatement;
-import net.sourceforge.pmd.lang.java.ast.ASTDoStatement;
import net.sourceforge.pmd.lang.java.ast.ASTExpression;
-import net.sourceforge.pmd.lang.java.ast.ASTForInit;
import net.sourceforge.pmd.lang.java.ast.ASTForStatement;
import net.sourceforge.pmd.lang.java.ast.ASTForUpdate;
+import net.sourceforge.pmd.lang.java.ast.ASTForeachStatement;
import net.sourceforge.pmd.lang.java.ast.ASTIfStatement;
-import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTName;
-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.ASTLocalClassStatement;
+import net.sourceforge.pmd.lang.java.ast.ASTLoopStatement;
+import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement;
import net.sourceforge.pmd.lang.java.ast.ASTStatement;
import net.sourceforge.pmd.lang.java.ast.ASTSwitchStatement;
-import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTThrowStatement;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
-import net.sourceforge.pmd.lang.java.ast.ASTWhileStatement;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
-import net.sourceforge.pmd.lang.java.rule.performance.AbstractOptimizationRule;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
+import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
import net.sourceforge.pmd.properties.PropertyDescriptor;
import net.sourceforge.pmd.util.StringUtil.CaseConvention;
-public class AvoidReassigningLoopVariablesRule extends AbstractOptimizationRule {
+public class AvoidReassigningLoopVariablesRule extends AbstractJavaRulechainRule {
private static final Map FOREACH_REASSIGN_VALUES =
associateBy(asList(ForeachReassignOption.values()), ForeachReassignOption::getDisplayName);
private static final PropertyDescriptor FOREACH_REASSIGN
- = enumProperty("foreachReassign", FOREACH_REASSIGN_VALUES)
- .defaultValue(ForeachReassignOption.DENY)
- .desc("how/if foreach control variables may be reassigned")
- .build();
+ = enumProperty("foreachReassign", FOREACH_REASSIGN_VALUES)
+ .defaultValue(ForeachReassignOption.DENY)
+ .desc("how/if foreach control variables may be reassigned")
+ .build();
private static final Map FOR_REASSIGN_VALUES =
associateBy(asList(ForReassignOption.values()), ForReassignOption::getDisplayName);
private static final PropertyDescriptor FOR_REASSIGN
- = enumProperty("forReassign", FOR_REASSIGN_VALUES)
- .defaultValue(ForReassignOption.DENY)
- .desc("how/if for control variables may be reassigned")
- .build();
+ = enumProperty("forReassign", FOR_REASSIGN_VALUES)
+ .defaultValue(ForReassignOption.DENY)
+ .desc("how/if for control variables may be reassigned")
+ .build();
public AvoidReassigningLoopVariablesRule() {
+ super(ASTForStatement.class, ASTForeachStatement.class);
definePropertyDescriptor(FOREACH_REASSIGN);
definePropertyDescriptor(FOR_REASSIGN);
- addRuleChainVisit(ASTLocalVariableDeclaration.class);
}
@Override
- public Object visit(ASTLocalVariableDeclaration node, Object data) {
- final Set loopVariables = new HashSet<>();
- for (ASTVariableDeclaratorId declaratorId : node.findDescendantsOfType(ASTVariableDeclaratorId.class)) {
- loopVariables.add(declaratorId.getImage());
+ public Object visit(ASTForeachStatement loopStmt, Object data) {
+ ForeachReassignOption behavior = getProperty(FOREACH_REASSIGN);
+ if (behavior == ForeachReassignOption.ALLOW) {
+ return data;
}
+ ASTVariableDeclaratorId loopVar = loopStmt.getVarId();
+ boolean ignoreNext = behavior == ForeachReassignOption.FIRST_ONLY;
+ for (ASTNamedReferenceExpr usage : loopVar.getLocalUsages()) {
+ if (usage.getAccessType() == AccessType.WRITE) {
+ if (ignoreNext) {
+ ignoreNext = false;
+ continue;
+ }
+ addViolation(data, usage, loopVar.getName());
+ } else {
+ ignoreNext = false;
+ }
+ }
+ return null;
+ }
- if (node.getParent() instanceof ASTForInit) {
- // regular for loop: LocalVariableDeclaration -> ForInit -> ForStatement
- final ASTStatement loopBody = node.getParent().getParent().getFirstChildOfType(ASTStatement.class);
- final ForReassignOption forReassign = getProperty(FOR_REASSIGN);
-
- if (forReassign != ForReassignOption.ALLOW) {
- // check assignments
- checkAssignExceptIncrement(data, loopVariables, loopBody);
-
- if (forReassign == ForReassignOption.SKIP) {
- // skipping allowed -> only check non-conditional increments
- checkIncrementAndDecrement(data, loopVariables, loopBody, IgnoreFlags.IGNORE_CONDITIONAL);
- } else {
- // skipping not allowed -> check all increments
- checkIncrementAndDecrement(data, loopVariables, loopBody);
+ @Override
+ public Object visit(ASTForStatement loopStmt, Object data) {
+ ForReassignOption behavior = getProperty(FOR_REASSIGN);
+ if (behavior == ForReassignOption.ALLOW) {
+ return data;
+ }
+ ASTForUpdate update = loopStmt.getFirstChildOfType(ASTForUpdate.class);
+ NodeStream loopVars = JavaRuleUtil.getLoopVariables(loopStmt);
+ if (behavior == ForReassignOption.DENY) {
+ for (ASTVariableDeclaratorId loopVar : loopVars) {
+ for (ASTNamedReferenceExpr usage : loopVar.getLocalUsages()) {
+ if (usage.getAccessType() == AccessType.WRITE) {
+ if (update != null && usage.ancestors(ASTForUpdate.class).first() == update) {
+ continue;
+ }
+ addViolation(data, usage, loopVar.getName());
+ }
}
}
+ } else {
+ Set loopVarNames = loopVars.collect(Collectors.mapping(ASTVariableDeclaratorId::getName, Collectors.toSet()));
+ Set labels = JavaRuleUtil.getStatementLabels(loopStmt);
+ new ControlFlowCtx(false, loopVarNames, (RuleContext) data, labels, false, false).roamStatementsForExit(loopStmt.getBody());
+ }
+ return null;
+ }
- } else if (node.getParent() instanceof ASTForStatement) {
- // for-each loop: LocalVariableDeclaration -> ForStatement
- final ASTStatement loopBody = node.getParent().getFirstChildOfType(ASTStatement.class);
- final ForeachReassignOption foreachReassign = getProperty(FOREACH_REASSIGN);
+ class ControlFlowCtx {
- if (foreachReassign == ForeachReassignOption.FIRST_ONLY) {
- checkAssignExceptIncrement(data, loopVariables, loopBody, IgnoreFlags.IGNORE_FIRST);
- checkIncrementAndDecrement(data, loopVariables, loopBody, IgnoreFlags.IGNORE_FIRST);
+ private final boolean guarded;
+ private boolean mayExit;
+ private final Set loopVarNames;
+ private final RuleContext ruleCtx;
- } else if (foreachReassign == ForeachReassignOption.DENY) {
- checkAssignExceptIncrement(data, loopVariables, loopBody);
- checkIncrementAndDecrement(data, loopVariables, loopBody);
- }
+ private final Set outerLoopNames;
+ private final boolean breakHidden;
+ private final boolean continueHidden;
+
+ ControlFlowCtx(boolean guarded, Set loopVarNames, RuleContext ctx, Set outerLoopNames, boolean breakHidden, boolean continueHidden) {
+ this.guarded = guarded;
+ this.loopVarNames = loopVarNames;
+ this.ruleCtx = ctx;
+ this.outerLoopNames = outerLoopNames;
+ this.breakHidden = breakHidden;
+ this.continueHidden = continueHidden;
}
- return data;
- }
+ ControlFlowCtx withGuard(boolean isGuarded) {
+ return copy(isGuarded, breakHidden, continueHidden);
+ }
- /**
- * Report usages of assignments except '+=' and '-='.
- *
- * @param ignoreFlags which statements should be ignored
- */
- private void checkAssignExceptIncrement(Object data, Set loopVariables, ASTStatement loopBody, IgnoreFlags... ignoreFlags) {
- checkAssignments(data, loopVariables, loopBody, false, ignoreFlags);
- }
+ ControlFlowCtx copy(boolean isGuarded, boolean breakHidden, boolean continueHidden) {
+ return new ControlFlowCtx(isGuarded, loopVarNames, ruleCtx, outerLoopNames, breakHidden, continueHidden);
+ }
- /**
- * Report usages of increments ('++', '--', '+=', '-=').
- *
- * @param ignoreFlags which statements should be ignored
- */
- private void checkIncrementAndDecrement(Object data, Set loopVariables, ASTStatement loopBody, IgnoreFlags... ignoreFlags) {
- for (ASTUnaryExpression expression : loopBody.findDescendantsOfType(ASTUnaryExpression.class)) {
- if (expression.getOperator().isPure() || ignoreNode(expression, loopBody, ignoreFlags)) {
- continue;
+ private boolean roamStatementsForExit(JavaNode node) {
+ if (node == null) {
+ return false;
}
- checkVariable(data, loopVariables, singleVariableName(expression.getFirstDescendantOfType(ASTPrimaryExpression.class)));
+ NodeStream unwrappedBlock =
+ node instanceof ASTBlock
+ ? ((ASTBlock) node).toStream()
+ : NodeStream.of(node);
+
+ return roamStatementsForExit(unwrappedBlock);
}
- // foo += x and foo -= x
- checkAssignments(data, loopVariables, loopBody, true, ignoreFlags);
- }
-
- /**
- * Report usages of assignments.
- *
- * @param checkIncrement true: check only '+=' and '-=',
- * false: check all other assignments
- * @param ignoreFlags which statements should be ignored
- */
- private void checkAssignments(Object data, Set loopVariables, ASTStatement loopBody, boolean checkIncrement, IgnoreFlags... ignoreFlags) {
- for (ASTAssignmentOperator operator : loopBody.findDescendantsOfType(ASTAssignmentOperator.class)) {
- // check if the current operator is an assign-increment or assign-decrement operator
- final String operatorImage = operator.getImage();
- final boolean isIncrement = "+=".equals(operatorImage) || "-=".equals(operatorImage);
-
- if (isIncrement != checkIncrement) {
- // wrong type of operator
- continue;
- }
-
- if (ignoreNode(operator, loopBody, ignoreFlags)) {
- continue;
- }
-
- final ASTPrimaryExpression primaryExpression = operator.getParent().getFirstChildOfType(ASTPrimaryExpression.class);
- checkVariable(data, loopVariables, singleVariableName(primaryExpression));
- }
- }
-
- /**
- * Check if the node should be ignored, depending on the given flags and the context.
- */
- private boolean ignoreNode(Node node, ASTStatement loopBody, IgnoreFlags... ignoreFlags) {
- if (ignoreFlags.length == 0) {
- return false;
- }
- final List ignoreFlagsList = asList(ignoreFlags);
-
- // ignore the first statement
- final boolean ignoredFirstStatement = ignoreFlagsList.contains(IgnoreFlags.IGNORE_FIRST) && isFirstStatementInBlock(node, loopBody);
-
- // ignore conditionally executed statement
- final boolean ignoredConditional = ignoreFlagsList.contains(IgnoreFlags.IGNORE_CONDITIONAL) && isConditionallyExecuted(node, loopBody);
-
- return ignoredFirstStatement || ignoredConditional;
- }
-
- /**
- * Extracts the variable name by traversing PrimaryExpression -> PrimaryPrefix -> Name.
- * Also check if there is a PrimaryExpression -> PrimarySuffix which indicates a field or array access.
- *
- * @returns the Name or null if the PrimaryPrefix is "this" or "super"
- */
- private ASTName singleVariableName(ASTPrimaryExpression primaryExpression) {
- final ASTPrimaryPrefix primaryPrefix = primaryExpression.getFirstChildOfType(ASTPrimaryPrefix.class);
- final ASTPrimarySuffix primarySuffix = primaryExpression.getFirstChildOfType(ASTPrimarySuffix.class);
-
- if (primarySuffix != null || primaryPrefix == null) {
- return null;
- }
-
- return primaryPrefix.getFirstChildOfType(ASTName.class);
- }
-
- /**
- * Check if the given node is the first statement in the block.
- */
- private boolean isFirstStatementInBlock(Node node, ASTStatement loopBody) {
- // find the statement of the operation and the loop body block statement
- final ASTBlockStatement statement = node.getFirstParentOfType(ASTBlockStatement.class);
- final ASTBlock block = loopBody.getFirstDescendantOfType(ASTBlock.class);
-
- if (statement == null || block == null) {
- return false;
- }
-
- // is the first statement in the loop body?
- return block.equals(statement.getParent()) && statement.getIndexInParent() == 0;
- }
-
- /**
- * Check if the node will only be executed conditionally by checking,
- * if the node is inside any kind of control flow statement or
- * if any prior statement contains a {@code continue} statement.
- *
- * This doesn't check
- */
- private boolean isConditionallyExecuted(Node node, ASTStatement loopBody) {
- // starting at the assignment/increment node, traverse the tree up to
- // check if we're inside the conditionally executed block of a control flow statement
-
- Node checkNode = node;
- while (checkNode.getParent() != null && !checkNode.getParent().equals(loopBody)) {
- final Node parent = checkNode.getParent();
-
- // if/switch/while-statement, excluding the expression
- if (parent instanceof ASTIfStatement || parent instanceof ASTSwitchStatement || parent instanceof ASTWhileStatement || parent instanceof ASTDoStatement) {
- return !(checkNode instanceof ASTExpression);
- }
-
- // for-statement, excluding the initializer, expression and update
- if (parent instanceof ASTForStatement) {
- return !(checkNode instanceof ASTForInit || checkNode instanceof ASTExpression || checkNode instanceof ASTForUpdate);
- }
- checkNode = parent;
- }
-
- // iterating the statements of the loop body, check if there is a
- // continue statement before the increment statement
- final ASTBlock block = loopBody.getFirstDescendantOfType(ASTBlock.class);
- if (block != null) {
- for (int i = 0; i < block.getNumChildren(); i++) {
- final Node statement = block.getChild(i);
-
- if (statement.hasDescendantOfType(ASTContinueStatement.class)) {
+ // return true if any statement may exit the outer loop abruptly
+ // This way increments of variables are allowed if they are guarded by a conditional
+ private boolean roamStatementsForExit(NodeStream stmts) {
+ for (JavaNode stmt : stmts) {
+ if (stmt instanceof ASTThrowStatement
+ || stmt instanceof ASTReturnStatement) {
return true;
+ } else if (stmt instanceof ASTBreakStatement) {
+ String label = ((ASTBreakStatement) stmt).getLabel();
+ return label != null && outerLoopNames.contains(label) || !breakHidden;
+ } else if (stmt instanceof ASTContinueStatement) {
+ String label = ((ASTContinueStatement) stmt).getLabel();
+ return label != null && outerLoopNames.contains(label) || !continueHidden;
}
- if (isParent(statement, node)) {
- return false;
+
+ // note that we mean to use |= and not shortcut evaluation
+
+ if (stmt instanceof ASTLoopStatement) {
+
+ ASTStatement body = ((ASTLoopStatement) stmt).getBody();
+ for (JavaNode child : stmt.children()) {
+ if (child != body) { // NOPMD
+ checkVorViolations(child);
+ }
+ }
+
+ mayExit |= copy(true, true, true).roamStatementsForExit(body);
+
+ } else if (stmt instanceof ASTSwitchStatement) {
+
+ ASTSwitchStatement switchStmt = (ASTSwitchStatement) stmt;
+ checkVorViolations(switchStmt.getTestedExpression());
+
+ mayExit |= copy(true, true, false).roamStatementsForExit(switchStmt.getBranches());
+
+ } else if (stmt instanceof ASTIfStatement) {
+
+ ASTIfStatement ifStmt = (ASTIfStatement) stmt;
+ checkVorViolations(ifStmt.getCondition());
+ mayExit |= withGuard(true).roamStatementsForExit(ifStmt.getThenBranch());
+ mayExit |= withGuard(this.guarded).roamStatementsForExit(ifStmt.getElseBranch());
+ } else if (stmt instanceof ASTExpression) { // these two catch-all clauses implement other statements & eg switch branches
+ checkVorViolations(stmt);
+ } else if (!(stmt instanceof ASTLocalClassStatement)) {
+ mayExit |= roamStatementsForExit(stmt.children());
}
}
+ return mayExit;
}
- return false;
- }
-
- private boolean isParent(Node possibleParent, Node node) {
- Node checkNode = node;
- while (checkNode.getParent() != null) {
- if (checkNode.getParent().equals(possibleParent)) {
- return true;
+ private void checkVorViolations(JavaNode node) {
+ if (node == null) {
+ return;
}
- checkNode = checkNode.getParent();
- }
- return false;
- }
-
- /**
- * Add a violation, if the node image is one of the loop variables.
- */
- private void checkVariable(Object data, Set loopVariables, JavaNode node) {
- if (node != null && loopVariables.contains(node.getImage())) {
- addViolation(data, node, node.getImage());
+ final boolean onlyConsiderWrite = guarded || mayExit;
+ node.descendants(ASTNamedReferenceExpr.class)
+ .filter(it -> loopVarNames.contains(it.getName()))
+ .filter(it -> onlyConsiderWrite ? JavaRuleUtil.isVarAccessStrictlyWrite(it)
+ : JavaRuleUtil.isVarAccessReadAndWrite(it))
+ .forEach(it -> addViolation(ruleCtx, it, it.getName()));
}
}
@@ -342,11 +279,4 @@ public class AvoidReassigningLoopVariablesRule extends AbstractOptimizationRule
}
}
- private enum IgnoreFlags {
-
- IGNORE_FIRST,
-
- IGNORE_CONDITIONAL
-
- }
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersRule.java
index ec06c65580..6cfe5cbdf7 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersRule.java
@@ -4,55 +4,43 @@
package net.sourceforge.pmd.lang.java.rule.bestpractices;
-import java.util.List;
-import java.util.Map;
-
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.AccessType;
import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclarator;
-import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
-import net.sourceforge.pmd.lang.java.symboltable.JavaNameOccurrence;
-import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
-import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
+import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter;
+import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
+import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration;
+import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
-public class AvoidReassigningParametersRule extends AbstractJavaRule {
+public class AvoidReassigningParametersRule extends AbstractJavaRulechainRule {
- @Override
- public Object visit(ASTMethodDeclarator node, Object data) {
- Map> params = node.getScope()
- .getDeclarations(VariableNameDeclaration.class);
- this.lookForViolation(params, data);
- return super.visit(node, data);
+ public AvoidReassigningParametersRule() {
+ super(ASTMethodDeclaration.class, ASTConstructorDeclaration.class);
}
- private void lookForViolation(Map> params, Object data) {
- for (Map.Entry> entry : params.entrySet()) {
- VariableNameDeclaration decl = entry.getKey();
- List usages = entry.getValue();
+ @Override
+ public Object visit(ASTMethodDeclaration node, Object data) {
+ lookForViolations(node, data);
+ return data;
+ }
- // Only look for formal parameters
- if (!decl.getDeclaratorId().isFormalParameter()) {
- continue;
- }
- for (NameOccurrence occ : usages) {
- JavaNameOccurrence jocc = (JavaNameOccurrence) occ;
- if ((jocc.isOnLeftHandSide() || jocc.isSelfAssignment())
- && jocc.getNameForWhichThisIsAQualifier() == null && !jocc.useThisOrSuper() && !decl.isVarargs()
- && (!decl.isArray()
- || jocc.getLocation().getParent().getParent().getNumChildren() == 1)) {
- // not an array or no primary suffix to access the array
- // values
- addViolation(data, decl.getNode(), decl.getImage());
+ @Override
+ public Object visit(ASTConstructorDeclaration node, Object data) {
+ lookForViolations(node, data);
+ return data;
+ }
+
+ private void lookForViolations(ASTMethodOrConstructorDeclaration node, Object data) {
+ for (ASTFormalParameter formal : node.getFormalParameters()) {
+ ASTVariableDeclaratorId varId = formal.getVarId();
+ for (ASTNamedReferenceExpr usage : varId.getLocalUsages()) {
+ if (usage.getAccessType() == AccessType.WRITE) {
+ addViolation(data, usage, varId.getName());
}
}
}
}
- @Override
- public Object visit(ASTConstructorDeclaration node, Object data) {
- Map> params = node.getScope()
- .getDeclarations(VariableNameDeclaration.class);
- this.lookForViolation(params, data);
- return super.visit(node, data);
- }
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidUsingHardCodedIPRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidUsingHardCodedIPRule.java
index d09dc1a2c5..85f0b1e430 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidUsingHardCodedIPRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidUsingHardCodedIPRule.java
@@ -1,101 +1,83 @@
-/**
+/*
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.bestpractices;
-import java.util.Collections;
-import java.util.HashMap;
+import static java.util.Arrays.asList;
+
+import java.util.EnumSet;
import java.util.List;
-import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.apache.commons.lang3.StringUtils;
-import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
-import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
-import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
+import net.sourceforge.pmd.RuleContext;
+import net.sourceforge.pmd.lang.java.ast.ASTStringLiteral;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.properties.PropertyDescriptor;
import net.sourceforge.pmd.properties.PropertyFactory;
-public class AvoidUsingHardCodedIPRule extends AbstractJavaRule {
+public class AvoidUsingHardCodedIPRule extends AbstractJavaRulechainRule {
- // why is everything public?
+ private enum AddressKinds {
+ IPV4("IPv4"),
+ IPV6("IPv6"),
+ IPV4_MAPPED_IPV6("IPv4 mapped IPv6");
- public static final String IPV4 = "IPv4";
- public static final String IPV6 = "IPv6";
- public static final String IPV4_MAPPED_IPV6 = "IPv4 mapped IPv6";
+ private final String label;
- private static final Map ADDRESSES_TO_CHECK;
-
- static {
- Map tmp = new HashMap<>();
- tmp.put(IPV4, IPV4);
- tmp.put(IPV6, IPV6);
- tmp.put(IPV4_MAPPED_IPV6, IPV4_MAPPED_IPV6);
- ADDRESSES_TO_CHECK = Collections.unmodifiableMap(tmp);
+ AddressKinds(String label) {
+ this.label = label;
+ }
}
- public static final PropertyDescriptor> CHECK_ADDRESS_TYPES_DESCRIPTOR =
- PropertyFactory.enumListProperty("checkAddressTypes", ADDRESSES_TO_CHECK)
- .desc("Check for IP address types.")
- .defaultValue(ADDRESSES_TO_CHECK.keySet()).build();
+ private static final PropertyDescriptor> CHECK_ADDRESS_TYPES_DESCRIPTOR =
+ PropertyFactory.enumListProperty("checkAddressTypes", AddressKinds.class, k -> k.label)
+ .desc("Check for IP address types.")
+ .defaultValue(asList(AddressKinds.values()))
+ .build();
// Provides 4 capture groups that can be used for additional validation
- protected static final String IPV4_REGEXP = "([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})";
+ private static final String IPV4_REGEXP = "([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})";
// Uses IPv4 pattern, but changes the groups to be non-capture
- protected static final String IPV6_REGEXP = "(?:(?:[0-9a-fA-F]{1,4})?\\:)+(?:[0-9a-fA-F]{1,4}|"
- + IPV4_REGEXP.replace("(", "(?:") + ")?";
+ private static final String IPV6_REGEXP = "(?:(?:[0-9a-fA-F]{1,4})?\\:)+(?:[0-9a-fA-F]{1,4}|"
+ + IPV4_REGEXP.replace("(", "(?:") + ")?";
- protected static final Pattern IPV4_PATTERN = Pattern.compile("^" + IPV4_REGEXP + "$");
- protected static final Pattern IPV6_PATTERN = Pattern.compile("^" + IPV6_REGEXP + "$");
+ private static final Pattern IPV4_PATTERN = Pattern.compile("^" + IPV4_REGEXP + "$");
+ private static final Pattern IPV6_PATTERN = Pattern.compile("^" + IPV6_REGEXP + "$");
- protected boolean checkIPv4;
- protected boolean checkIPv6;
- protected boolean checkIPv4MappedIPv6;
+ private final EnumSet kindsToCheck = EnumSet.noneOf(AddressKinds.class);
public AvoidUsingHardCodedIPRule() {
+ super(ASTStringLiteral.class);
definePropertyDescriptor(CHECK_ADDRESS_TYPES_DESCRIPTOR);
-
- addRuleChainVisit(ASTCompilationUnit.class);
- addRuleChainVisit(ASTLiteral.class);
}
@Override
- public Object visit(ASTCompilationUnit node, Object data) {
- checkIPv4 = false;
- checkIPv6 = false;
- checkIPv4MappedIPv6 = false;
- for (Object addressType : getProperty(CHECK_ADDRESS_TYPES_DESCRIPTOR)) {
- if (IPV4.equals(addressType)) {
- checkIPv4 = true;
- } else if (IPV6.equals(addressType)) {
- checkIPv6 = true;
- } else if (IPV4_MAPPED_IPV6.equals(addressType)) {
- checkIPv4MappedIPv6 = true;
- }
- }
- return data;
+ public void start(RuleContext ctx) {
+ kindsToCheck.clear();
+ kindsToCheck.addAll(getProperty(CHECK_ADDRESS_TYPES_DESCRIPTOR));
}
@Override
- public Object visit(ASTLiteral node, Object data) {
- if (!node.isStringLiteral()) {
- return data;
- }
-
- // Remove the quotes
- final String image = node.getImage().substring(1, node.getImage().length() - 1);
+ public Object visit(ASTStringLiteral node, Object data) {
+ final String image = node.getConstValue();
// Note: We used to check the addresses using
// InetAddress.getByName(String), but that's extremely slow,
// so we created more robust checking methods.
if (image.length() > 0) {
final char firstChar = Character.toUpperCase(image.charAt(0));
+
+ boolean checkIPv4 = kindsToCheck.contains(AddressKinds.IPV4);
+ boolean checkIPv6 = kindsToCheck.contains(AddressKinds.IPV6);
+ boolean checkIPv4MappedIPv6 = kindsToCheck.contains(AddressKinds.IPV4_MAPPED_IPV6);
+
if (checkIPv4 && isIPv4(firstChar, image) || isIPv6(firstChar, image, checkIPv6, checkIPv4MappedIPv6)) {
addViolation(data, node);
}
@@ -216,13 +198,8 @@ public class AvoidUsingHardCodedIPRule extends AbstractJavaRule {
}
}
- public boolean hasChosenAddressTypes() {
- return getProperty(CHECK_ADDRESS_TYPES_DESCRIPTOR).size() > 0;
- }
-
-
@Override
public String dysfunctionReason() {
- return hasChosenAddressTypes() ? null : "No address types specified";
+ return !getProperty(CHECK_ADDRESS_TYPES_DESCRIPTOR).isEmpty() ? null : "No address types specified";
}
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/CheckResultSetRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/CheckResultSetRule.java
index fdc79937e0..9cb1e5ab74 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/CheckResultSetRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/CheckResultSetRule.java
@@ -4,90 +4,51 @@
package net.sourceforge.pmd.lang.java.rule.bestpractices;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Map;
+import static net.sourceforge.pmd.util.CollectionUtil.setOf;
+
+import java.sql.ResultSet;
import java.util.Set;
-import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
import net.sourceforge.pmd.lang.java.ast.ASTIfStatement;
-import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTName;
+import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement;
-import net.sourceforge.pmd.lang.java.ast.ASTType;
-import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
-import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.ast.ASTWhileStatement;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
+import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
/**
* Rule that verifies, that the return values of next(), first(), last(), etc.
* calls to a java.sql.ResultSet are actually verified.
- *
*/
public class CheckResultSetRule extends AbstractJavaRule {
- private Map resultSetVariables = new HashMap<>();
+ private static final Set METHODS = setOf("next", "previous", "last", "first");
- private static Set methods = new HashSet<>();
-
- static {
- methods.add(".next");
- methods.add(".previous");
- methods.add(".last");
- methods.add(".first");
+ @Override
+ public Object visit(ASTWhileStatement node, Object data) {
+ return data;
}
@Override
- public Object visit(ASTMethodDeclaration node, Object data) {
- resultSetVariables.clear();
- return super.visit(node, data);
+ public Object visit(ASTReturnStatement node, Object data) {
+ return data;
}
@Override
- public Object visit(ASTLocalVariableDeclaration node, Object data) {
- ASTClassOrInterfaceType type = null;
- if (!node.isTypeInferred()) {
- type = node.getFirstChildOfType(ASTType.class).getFirstDescendantOfType(ASTClassOrInterfaceType.class);
- }
- if (type != null && (type.getType() != null && "java.sql.ResultSet".equals(type.getType().getName())
- || "ResultSet".equals(type.getImage()))) {
- ASTVariableDeclarator declarator = node.getFirstChildOfType(ASTVariableDeclarator.class);
- if (declarator != null) {
- ASTName name = declarator.getFirstDescendantOfType(ASTName.class);
- if (type.getType() != null || name != null && name.getImage().endsWith("executeQuery")) {
- ASTVariableDeclaratorId id = declarator.getFirstChildOfType(ASTVariableDeclaratorId.class);
- resultSetVariables.put(id.getImage(), node);
- }
- }
+ public Object visit(ASTIfStatement node, Object data) {
+ return data;
+ }
+
+ @Override
+ public Object visit(ASTMethodCall node, Object data) {
+ if (isResultSetMethod(node)) {
+ addViolation(data, node);
}
return super.visit(node, data);
}
- @Override
- public Object visit(ASTName node, Object data) {
- String image = node.getImage();
- String var = getResultSetVariableName(image);
- if (var != null && resultSetVariables.containsKey(var)
- && node.getFirstParentOfType(ASTIfStatement.class) == null
- && node.getFirstParentOfType(ASTWhileStatement.class) == null
- && node.getFirstParentOfType(ASTReturnStatement.class) == null) {
-
- addViolation(data, resultSetVariables.get(var));
- }
- return super.visit(node, data);
- }
-
- private String getResultSetVariableName(String image) {
- if (image.contains(".")) {
- for (String method : methods) {
- if (image.endsWith(method)) {
- return image.substring(0, image.lastIndexOf(method));
- }
- }
- }
- return null;
+ private boolean isResultSetMethod(ASTMethodCall node) {
+ return METHODS.contains(node.getMethodName())
+ && TypeTestUtil.isDeclaredInClass(ResultSet.class, node.getMethodType());
}
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementRule.java
index 4ea5b89cc4..3b1f1df0a5 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementRule.java
@@ -11,21 +11,22 @@ import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
+import java.util.logging.Level;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
import net.sourceforge.pmd.Rule;
-import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.java.ast.ASTAdditiveExpression;
-import net.sourceforge.pmd.lang.java.ast.ASTArgumentList;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
import net.sourceforge.pmd.lang.java.ast.ASTExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTExpressionStatement;
import net.sourceforge.pmd.lang.java.ast.ASTIfStatement;
-import net.sourceforge.pmd.lang.java.ast.ASTName;
-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.ASTStatementExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTLambdaExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
+import net.sourceforge.pmd.lang.java.ast.ASTStringLiteral;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
+import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
import net.sourceforge.pmd.properties.PropertyDescriptor;
/**
@@ -101,120 +102,47 @@ public class GuardLogStatementRule extends AbstractJavaRule implements Rule {
}
@Override
- public Object visit(ASTStatementExpression node, Object data) {
- if (node.getNumChildren() < 1 || !(node.getChild(0) instanceof ASTPrimaryExpression)) {
- // only consider primary expressions
- return node;
+ public Object visit(ASTExpressionStatement node, Object data) {
+ ASTExpression expr = node.getExpr();
+ if (!(expr instanceof ASTMethodCall)) {
+ return null;
}
- ASTPrimaryExpression primary = (ASTPrimaryExpression) node.getChild(0);
- if (primary.getNumChildren() >= 2 && primary.getChild(0) instanceof ASTPrimaryPrefix) {
- ASTPrimaryPrefix prefix = (ASTPrimaryPrefix) primary.getChild(0);
- String methodCall = getMethodCallName(prefix);
- String logLevel = getLogLevelName(primary, methodCall);
-
- if (guardStmtByLogLevel.containsKey(methodCall) && logLevel != null
- && primary.getChild(1) instanceof ASTPrimarySuffix
- && primary.getChild(1).hasDescendantOfType(ASTAdditiveExpression.class)) {
-
- if (!hasGuard(primary, methodCall, logLevel)) {
- super.addViolation(data, node);
- }
+ ASTMethodCall methodCall = (ASTMethodCall) expr;
+ String logLevel = getLogLevelName(methodCall);
+ if (logLevel != null && guardStmtByLogLevel.containsKey(logLevel)) {
+ if (!hasGuard(methodCall, logLevel)) {
+ addViolation(data, node);
}
}
- return super.visit(node, data);
+ return null;
}
- private boolean hasGuard(ASTPrimaryExpression node, String methodCall, String logLevel) {
- ASTIfStatement ifStatement = node.getFirstParentOfType(ASTIfStatement.class);
+ private boolean hasGuard(ASTMethodCall node, String logLevel) {
+ ASTIfStatement ifStatement = node.ancestors(ASTIfStatement.class).first();
if (ifStatement == null) {
return false;
}
- // an if statement always has an expression
- ASTExpression expr = ifStatement.getFirstChildOfType(ASTExpression.class);
- List guardCalls = expr.findDescendantsOfType(ASTPrimaryPrefix.class);
- if (guardCalls.isEmpty()) {
- return false;
- }
+ for (ASTMethodCall maybeAGuardCall : ifStatement.getCondition().descendantsOrSelf().filterIs(ASTMethodCall.class)) {
+ String guardMethodName = maybeAGuardCall.getMethodName();
+ // the guard is adapted to the actual log statement
- boolean foundGuard = false;
- // check all conditions in the if expression
- for (ASTPrimaryPrefix guardCall : guardCalls) {
- if (guardCall.getNumChildren() < 1
- || guardCall.getChild(0).getImage() == null) {
+ if (!guardStmtByLogLevel.get(logLevel).contains(guardMethodName)) {
continue;
}
- String guardMethodCall = getLastPartOfName(guardCall.getChild(0));
- boolean guardMethodCallMatches = guardStmtByLogLevel.get(methodCall).contains(guardMethodCall);
- boolean hasArguments = guardCall.getParent().hasDescendantOfType(ASTArgumentList.class);
-
- if (guardMethodCallMatches && !JAVA_UTIL_LOG_GUARD_METHOD.equals(guardMethodCall)) {
- // simple case: guard method without the need to check arguments found
- foundGuard = true;
- } else if (guardMethodCallMatches && hasArguments) {
+ if (JAVA_UTIL_LOG_GUARD_METHOD.equals(guardMethodName)) {
// java.util.logging: guard method with argument. Verify the log level
- String guardArgLogLevel = getLogLevelName(guardCall.getParent(), guardMethodCall);
- foundGuard = logLevel.equals(guardArgLogLevel);
- }
-
- if (foundGuard) {
- break;
- }
- }
-
- return foundGuard;
- }
-
- /**
- * Extracts the method name of the method call.
- * @param prefix the method call
- * @return the name of the called method
- */
- private String getMethodCallName(ASTPrimaryPrefix prefix) {
- String result = "";
- if (prefix.getNumChildren() == 1 && prefix.getChild(0) instanceof ASTName) {
- result = getLastPartOfName(prefix.getChild(0));
- }
- return result;
- }
-
- private String getLastPartOfName(Node name) {
- String result = "";
- if (name != null) {
- result = name.getImage();
- }
- int dotIndex = result.lastIndexOf('.');
- if (dotIndex > -1 && result.length() > dotIndex + 1) {
- result = result.substring(dotIndex + 1);
- }
- return result;
- }
-
- /**
- * Gets the first child, first grand child, ... of the given types.
- * The children must follow the given order of types
- *
- * @param root the node from where to start the search
- * @param childrenTypes the list of types
- * @param should match the last type of childrenType, otherwise you'll get a ClassCastException
- * @return the found child node or null
- */
- @SafeVarargs
- private static N getFirstChild(Node root, Class ... childrenTypes) {
- Node current = root;
- for (Class clazz : childrenTypes) {
- Node child = current.getFirstChildOfType(clazz);
- if (child != null) {
- current = child;
+ if (logLevel.equals(getJutilLogLevelInFirstArg(maybeAGuardCall))) {
+ return true;
+ }
} else {
- return null;
+ return true;
}
+
}
- @SuppressWarnings("unchecked")
- N result = (N) current;
- return result;
+ return false;
}
/**
@@ -222,32 +150,41 @@ public class GuardLogStatementRule extends AbstractJavaRule implements Rule {
* itself or - in case java util logging is used, then it is the first argument of
* the method call (if it exists).
*
- * @param node the method call
- * @param methodCallName the called method name previously determined
+ * @param methodCall the method call
+ *
* @return the log level or null if it could not be determined
*/
- private String getLogLevelName(Node node, String methodCallName) {
- if (!JAVA_UTIL_LOG_METHOD.equals(methodCallName) && !JAVA_UTIL_LOG_GUARD_METHOD.equals(methodCallName)) {
- return methodCallName;
- }
-
- String logLevel = null;
- ASTPrimarySuffix suffix = node.getFirstDescendantOfType(ASTPrimarySuffix.class);
- if (suffix != null) {
- ASTArgumentList argumentList = suffix.getFirstDescendantOfType(ASTArgumentList.class);
- if (argumentList != null && argumentList.getNumChildren() > 0) {
- // at least one argument - the log level. If the method call is "log", then a message might follow
- ASTName name = GuardLogStatementRule.getFirstChild(argumentList.getChild(0),
- ASTPrimaryExpression.class, ASTPrimaryPrefix.class, ASTName.class);
- String lastPart = getLastPartOfName(name);
- lastPart = lastPart.toLowerCase(Locale.ROOT);
- if (!lastPart.isEmpty()) {
- logLevel = lastPart;
- }
+ private @Nullable String getLogLevelName(ASTMethodCall methodCall) {
+ String methodName = methodCall.getMethodName();
+ if (!JAVA_UTIL_LOG_METHOD.equals(methodName) && !JAVA_UTIL_LOG_GUARD_METHOD.equals(methodName)) {
+ if (isUnguardedAccessOk(methodCall, 0)) {
+ return null;
}
+ return methodName; // probably logger.warn(...)
}
- return logLevel;
+ // else it's java.util.logging, eg
+ // LOGGER.log(Level.FINE, "m")
+ if (isUnguardedAccessOk(methodCall, 1)) {
+ return null;
+ }
+
+ return getJutilLogLevelInFirstArg(methodCall);
+ }
+
+ private @Nullable String getJutilLogLevelInFirstArg(ASTMethodCall methodCall) {
+ ASTExpression firstArg = methodCall.getArguments().toStream().get(0);
+ if (TypeTestUtil.isA(Level.class, firstArg) && firstArg instanceof ASTNamedReferenceExpr) {
+ return ((ASTNamedReferenceExpr) firstArg).getName().toLowerCase(Locale.ROOT);
+ }
+ return null;
+ }
+
+ private boolean isUnguardedAccessOk(ASTMethodCall call, int messageArgIndex) {
+ // return true if the statement has limited overhead even if unguarded,
+ // so that we can ignore it
+ ASTExpression messageArg = call.getArguments().toStream().get(messageArgIndex);
+ return messageArg instanceof ASTStringLiteral || messageArg instanceof ASTLambdaExpression;
}
private void extractProperties() {
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedFormalParameterRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedFormalParameterRule.java
index 5398229061..64f2895624 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedFormalParameterRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedFormalParameterRule.java
@@ -6,29 +6,14 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;
import static net.sourceforge.pmd.properties.PropertyFactory.booleanProperty;
-import java.io.InvalidObjectException;
-import java.io.ObjectInputStream;
-import java.util.List;
-import java.util.Map;
-
-import org.checkerframework.checker.nullness.qual.Nullable;
-
-import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclarator;
-import net.sourceforge.pmd.lang.java.ast.ASTThrowsList;
-import net.sourceforge.pmd.lang.java.ast.ASTType;
+import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
-import net.sourceforge.pmd.lang.java.ast.JavaNode;
+import net.sourceforge.pmd.lang.java.ast.AccessNode.Visibility;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
-import net.sourceforge.pmd.lang.java.symboltable.JavaNameOccurrence;
-import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
-import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
import net.sourceforge.pmd.properties.PropertyDescriptor;
@@ -48,83 +33,24 @@ public class UnusedFormalParameterRule extends AbstractJavaRule {
@Override
public Object visit(ASTMethodDeclaration node, Object data) {
- if (!node.isPrivate() && !getProperty(CHECKALL_DESCRIPTOR)) {
+ if (node.getVisibility() != Visibility.V_PRIVATE && !getProperty(CHECKALL_DESCRIPTOR)) {
return data;
}
- if (!node.isNative() && !node.isAbstract() && !isSerializationMethod(node) && !hasOverrideAnnotation(node)) {
+ if (node.getBody() != null && !JavaRuleUtil.isSerializationReadObject(node) && !node.isOverridden()) {
check(node, data);
}
return data;
}
- private boolean isSerializationMethod(ASTMethodDeclaration node) {
- ASTMethodDeclarator declarator = node.getFirstDescendantOfType(ASTMethodDeclarator.class);
- List parameters = declarator.findDescendantsOfType(ASTFormalParameter.class);
- if (node.isPrivate() && "readObject".equals(node.getName()) && parameters.size() == 1
- && throwsOneException(node, InvalidObjectException.class)) {
- ASTType type = parameters.get(0).getTypeNode();
- if (type.getType() == ObjectInputStream.class
- || ObjectInputStream.class.getSimpleName().equals(type.getTypeImage())
- || ObjectInputStream.class.getName().equals(type.getTypeImage())) {
- return true;
- }
- }
- return false;
- }
-
- private boolean throwsOneException(ASTMethodDeclaration node, Class exception) {
- @Nullable ASTThrowsList throwsList = node.getThrowsList();
- if (throwsList != null && throwsList.getNumChildren() == 1) {
- ASTClassOrInterfaceType n = throwsList.getChild(0);
- if (n.getType() == exception || exception.getSimpleName().equals(n.getImage())
- || exception.getName().equals(n.getImage())) {
- return true;
- }
- }
- return false;
- }
-
- private void check(Node node, Object data) {
- Node parent = node.getParent().getParent().getParent();
- if (parent instanceof ASTClassOrInterfaceDeclaration
- && !((ASTClassOrInterfaceDeclaration) parent).isInterface()) {
- Map> vars = ((JavaNode) node).getScope()
- .getDeclarations(VariableNameDeclaration.class);
- for (Map.Entry> entry : vars.entrySet()) {
- VariableNameDeclaration nameDecl = entry.getKey();
-
- ASTVariableDeclaratorId declNode = nameDecl.getDeclaratorId();
- if (!declNode.isFormalParameter()) {
- continue;
+ private void check(ASTMethodOrConstructorDeclaration node, Object data) {
+ if (!node.getEnclosingType().isInterface()) {
+ for (ASTFormalParameter formal : node.getFormalParameters()) {
+ ASTVariableDeclaratorId varId = formal.getVarId();
+ if (JavaRuleUtil.isNeverUsed(varId) && !JavaRuleUtil.isExplicitUnusedVarName(varId.getName())) {
+ addViolation(data, varId, new Object[] {node instanceof ASTMethodDeclaration ? "method" : "constructor", varId.getName(), });
}
-
- if (actuallyUsed(nameDecl, entry.getValue())
- || JavaRuleUtil.isExplicitUnusedVarName(nameDecl.getName())) {
- continue;
- }
- addViolation(data, nameDecl.getNode(), new Object[] {
- node instanceof ASTMethodDeclaration ? "method" : "constructor", nameDecl.getImage(), });
}
}
}
- private boolean actuallyUsed(VariableNameDeclaration nameDecl, List usages) {
- for (NameOccurrence occ : usages) {
- JavaNameOccurrence jocc = (JavaNameOccurrence) occ;
- if (jocc.isOnLeftHandSide()) {
- if (nameDecl.isArray() && jocc.getLocation().getParent().getParent().getNumChildren() > 1) {
- // array element access
- return true;
- }
- continue;
- } else {
- return true;
- }
- }
- return false;
- }
-
- private boolean hasOverrideAnnotation(ASTMethodDeclaration node) {
- return node.isAnnotationPresent(Override.class);
- }
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedLocalVariableRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedLocalVariableRule.java
index 3dbe508338..85934e797d 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedLocalVariableRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedLocalVariableRule.java
@@ -4,49 +4,30 @@
package net.sourceforge.pmd.lang.java.rule.bestpractices;
-import java.util.List;
+import org.checkerframework.checker.nullness.qual.NonNull;
import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
-import net.sourceforge.pmd.lang.java.symboltable.JavaNameOccurrence;
-import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
+import net.sourceforge.pmd.lang.rule.RuleTargetSelector;
public class UnusedLocalVariableRule extends AbstractJavaRule {
- public UnusedLocalVariableRule() {
- addRuleChainVisit(ASTLocalVariableDeclaration.class);
+ @Override
+ protected @NonNull RuleTargetSelector buildTargetSelector() {
+ return RuleTargetSelector.forTypes(ASTLocalVariableDeclaration.class);
}
@Override
public Object visit(ASTLocalVariableDeclaration decl, Object data) {
- for (int i = 0; i < decl.getNumChildren(); i++) {
- if (!(decl.getChild(i) instanceof ASTVariableDeclarator)) {
- continue;
- }
- ASTVariableDeclaratorId node = (ASTVariableDeclaratorId) decl.getChild(i).getChild(0);
- // TODO this isArray() check misses some cases
- // need to add DFAish code to determine if an array
- // is initialized locally or gotten from somewhere else
- if (!node.getNameDeclaration().isArray()
- && !actuallyUsed(node.getUsages())
- && !JavaRuleUtil.isExplicitUnusedVarName(node.getName())) {
- addViolation(data, node, node.getNameDeclaration().getImage());
+ for (ASTVariableDeclaratorId varId : decl.getVarIds()) {
+ if (JavaRuleUtil.isNeverUsed(varId)
+ && !JavaRuleUtil.isExplicitUnusedVarName(varId.getName())) {
+ addViolation(data, varId, varId.getName());
}
}
return data;
}
- private boolean actuallyUsed(List usages) {
- for (NameOccurrence occ : usages) {
- JavaNameOccurrence jocc = (JavaNameOccurrence) occ;
- if (!jocc.isOnLeftHandSide()) {
- return true;
- }
- }
- return false;
- }
-
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateFieldRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateFieldRule.java
index cd639b7599..446b71a197 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateFieldRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateFieldRule.java
@@ -6,28 +6,25 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.List;
-import java.util.Map;
-import net.sourceforge.pmd.lang.java.ast.ASTBodyDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBody;
-import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTEnumBody;
-import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTName;
-import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
-import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix;
-import net.sourceforge.pmd.lang.java.ast.ASTTypeBody;
-import net.sourceforge.pmd.lang.java.ast.AccessNode;
-import net.sourceforge.pmd.lang.java.ast.Annotatable;
+import org.checkerframework.checker.nullness.qual.NonNull;
+
+import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
+import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
+import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
+import net.sourceforge.pmd.lang.java.ast.AccessNode.Visibility;
+import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.rule.AbstractLombokAwareRule;
-import net.sourceforge.pmd.lang.java.symboltable.JavaNameOccurrence;
-import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
-import net.sourceforge.pmd.lang.symboltable.NameDeclaration;
-import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
+import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
+import net.sourceforge.pmd.lang.rule.RuleTargetSelector;
public class UnusedPrivateFieldRule extends AbstractLombokAwareRule {
+ @Override
+ protected @NonNull RuleTargetSelector buildTargetSelector() {
+ return RuleTargetSelector.forTypes(ASTAnyTypeDeclaration.class);
+ }
+
@Override
protected Collection defaultSuppressionAnnotations() {
Collection defaultValues = new ArrayList<>(super.defaultSuppressionAnnotations());
@@ -39,91 +36,27 @@ public class UnusedPrivateFieldRule extends AbstractLombokAwareRule {
}
@Override
- public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
- if (hasIgnoredAnnotation(node)) {
- return super.visit(node, data);
- }
-
- Map> vars = node.getScope()
- .getDeclarations(VariableNameDeclaration.class);
- for (Map.Entry> entry : vars.entrySet()) {
- VariableNameDeclaration decl = entry.getKey();
- AccessNode accessNodeParent = decl.getAccessNodeParent();
- if (!accessNodeParent.isPrivate()
- || isOK(decl.getImage())
- || hasIgnoredAnnotation((Annotatable) accessNodeParent)) {
- continue;
- }
- if (!actuallyUsed(entry.getValue())) {
- if (!usedInOuterClass(node, decl) && !usedInOuterEnum(node, decl)) {
- addViolation(data, decl.getNode(), decl.getImage());
- }
- }
- }
- return super.visit(node, data);
- }
-
- private boolean usedInOuterEnum(ASTClassOrInterfaceDeclaration node, NameDeclaration decl) {
- List outerEnums = node.getParentsOfType(ASTEnumDeclaration.class);
- for (ASTEnumDeclaration outerEnum : outerEnums) {
- ASTEnumBody enumBody = outerEnum.getFirstChildOfType(ASTEnumBody.class);
- if (usedInOuter(decl, enumBody)) {
- return true;
- }
- }
- return false;
- }
-
- /**
- * Find out whether the variable is used in an outer class
- */
- private boolean usedInOuterClass(ASTClassOrInterfaceDeclaration node, NameDeclaration decl) {
- List outerClasses = node.getParentsOfType(ASTClassOrInterfaceDeclaration.class);
- for (ASTClassOrInterfaceDeclaration outerClass : outerClasses) {
- ASTClassOrInterfaceBody classOrInterfaceBody = outerClass
- .getFirstChildOfType(ASTClassOrInterfaceBody.class);
- if (usedInOuter(decl, classOrInterfaceBody)) {
- return true;
- }
- }
- return false;
- }
-
- private boolean usedInOuter(NameDeclaration decl, ASTTypeBody body) {
- for (ASTBodyDeclaration node : body.toStream()) {
- for (ASTPrimarySuffix primarySuffix : node.findDescendantsOfType(ASTPrimarySuffix.class, true)) {
- if (decl.getImage().equals(primarySuffix.getImage())) {
- return true; // No violation
- }
+ public Object visitJavaNode(JavaNode node, Object data) {
+ if (node instanceof ASTAnyTypeDeclaration) {
+ ASTAnyTypeDeclaration type = (ASTAnyTypeDeclaration) node;
+ if (hasIgnoredAnnotation(type)) {
+ return null;
}
- for (ASTPrimaryPrefix primaryPrefix : node.findDescendantsOfType(ASTPrimaryPrefix.class, true)) {
- ASTName name = primaryPrefix.getFirstDescendantOfType(ASTName.class);
-
- if (name != null) {
- for (String id : name.getImage().split("\\.")) {
- if (id.equals(decl.getImage())) {
- return true; // No violation
+ for (ASTFieldDeclaration field : type.getDeclarations()
+ .filterIs(ASTFieldDeclaration.class)) {
+ if (field.getVisibility() == Visibility.V_PRIVATE
+ && !JavaRuleUtil.isSerialPersistentFields(field)
+ && !JavaRuleUtil.isSerialVersionUID(field)
+ && !hasIgnoredAnnotation(field)) {
+ for (ASTVariableDeclaratorId varId : field.getVarIds()) {
+ if (JavaRuleUtil.isNeverUsed(varId)) {
+ addViolation(data, varId, varId.getName());
}
}
}
}
}
- return false;
- }
-
- private boolean actuallyUsed(List usages) {
- for (NameOccurrence nameOccurrence : usages) {
- JavaNameOccurrence jNameOccurrence = (JavaNameOccurrence) nameOccurrence;
- if (!jNameOccurrence.isOnLeftHandSide()) {
- return true;
- }
- }
-
- return false;
- }
-
- private boolean isOK(String image) {
- return "serialVersionUID".equals(image) || "serialPersistentFields".equals(image) || "IDENT".equals(image);
+ return null;
}
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodRule.java
index f44819c8d1..de67db1867 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodRule.java
@@ -4,123 +4,103 @@
package net.sourceforge.pmd.lang.java.rule.bestpractices;
-import java.util.Arrays;
+import static net.sourceforge.pmd.util.CollectionUtil.setOf;
+
import java.util.Collection;
import java.util.Collections;
-import java.util.HashSet;
-import java.util.List;
+import java.util.HashMap;
import java.util.Map;
import java.util.Set;
+import java.util.stream.Collectors;
-import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTInitializer;
+import net.sourceforge.pmd.lang.ast.NodeStream;
+import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
+import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
-import net.sourceforge.pmd.lang.java.ast.Annotatable;
+import net.sourceforge.pmd.lang.java.ast.ASTMethodReference;
+import net.sourceforge.pmd.lang.java.ast.AccessNode.Visibility;
+import net.sourceforge.pmd.lang.java.ast.JavaNode;
+import net.sourceforge.pmd.lang.java.ast.MethodUsage;
+import net.sourceforge.pmd.lang.java.ast.internal.PrettyPrintingUtil;
import net.sourceforge.pmd.lang.java.rule.AbstractIgnoredAnnotationRule;
-import net.sourceforge.pmd.lang.java.symboltable.ClassScope;
-import net.sourceforge.pmd.lang.java.symboltable.MethodNameDeclaration;
-import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
+import net.sourceforge.pmd.lang.java.symbols.JExecutableSymbol;
+import net.sourceforge.pmd.util.CollectionUtil;
/**
* This rule detects private methods, that are not used and can therefore be
* deleted.
*/
public class UnusedPrivateMethodRule extends AbstractIgnoredAnnotationRule {
- private static final Set SERIALIZATION_METHODS = new HashSet<>(Arrays.asList(
- "readObject", "writeObject", "readResolve", "writeReplace"));
+
+ private static final Set SERIALIZATION_METHODS =
+ setOf("readObject", "writeObject", "readResolve", "writeReplace");
@Override
protected Collection defaultSuppressionAnnotations() {
return Collections.singletonList("java.lang.Deprecated");
}
- /**
- * Visit each method declaration.
- *
- * @param node
- * the method declaration
- * @param data
- * data - rule context
- * @return data
- */
@Override
- public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
- if (node.isInterface()) {
- return data;
- }
+ public Object visit(ASTCompilationUnit file, Object param) {
+ // We do just two traversals:
+ // - one to find the "interesting methods", ie those that may be violations
+ // - another to find the possible usages. We only try to resolve
+ // method calls/method refs that may refer to a method in the
+ // first set, ie, not every call in the file.
- Map> methods = node.getScope().getEnclosingScope(ClassScope.class)
- .getMethodDeclarations();
- for (MethodNameDeclaration mnd : findUnique(methods)) {
- List occs = methods.get(mnd);
- if (!privateAndNotExcluded(mnd) || hasIgnoredAnnotation((Annotatable) mnd.getNode().getParent())) {
- continue;
- }
- if (occs.isEmpty()) {
- addViolation(data, mnd.getNode(), mnd.getImage() + mnd.getParameterDisplaySignature());
- } else {
- if (isMethodNotCalledFromOtherMethods(mnd, occs)) {
- addViolation(data, mnd.getNode(), mnd.getImage() + mnd.getParameterDisplaySignature());
+ Map> consideredNames =
+ file.descendants(ASTMethodDeclaration.class)
+ .crossFindBoundaries()
+ // get methods whose usages are all in this file
+ // TODO we could use getEffectiveVisibility here, but we need to consider overrides then.
+ .filter(it -> it.getVisibility() == Visibility.V_PRIVATE)
+ .filter(it -> !hasIgnoredAnnotation(it) && !hasExcludedName(it) && !it.isAnnotationPresent(Override.class))
+ .toStream()
+ .collect(Collectors.groupingBy(ASTMethodDeclaration::getName, HashMap::new, CollectionUtil.toMutableSet()));
+
+
+ file.descendants()
+ .crossFindBoundaries()
+ .map(NodeStream.asInstanceOf(ASTMethodCall.class, ASTMethodReference.class))
+ .forEach(ref -> {
+ String methodName = ref.getMethodName();
+ // the considered names might be mutated during the traversal
+ if (!consideredNames.containsKey(methodName)) {
+ return;
+ }
+ JExecutableSymbol sym;
+ if (ref instanceof ASTMethodCall) {
+ sym = ((ASTMethodCall) ref).getMethodType().getSymbol();
+ } else if (ref instanceof ASTMethodReference) {
+ sym = ((ASTMethodReference) ref).getReferencedMethod().getSymbol();
+ } else {
+ return;
}
+ JavaNode reffed = sym.tryGetNode();
+ if (reffed instanceof ASTMethodDeclaration
+ && ref.ancestors(ASTMethodDeclaration.class).first() != reffed) {
+ // remove from set, but only if it is called outside of itself
+ Set remainingUnused = consideredNames.get(methodName);
+ if (remainingUnused != null
+ && remainingUnused.remove(reffed) // note: side-effect
+ && remainingUnused.isEmpty()) {
+ consideredNames.remove(methodName); // clear this name
+ }
+ }
+ });
+
+ // those that remain are unused
+ consideredNames.forEach((name, unused) -> {
+ for (ASTMethodDeclaration m : unused) {
+ addViolation(param, m, PrettyPrintingUtil.displaySignature(m));
}
- }
- return data;
+ });
+
+ return null;
}
- private Set findUnique(Map> methods) {
- // some rather hideous hackery here
- // to work around the fact that PMD does not yet do full type analysis
- // when it does, delete this
- Set unique = new HashSet<>();
- Set sigs = new HashSet<>();
- for (MethodNameDeclaration mnd : methods.keySet()) {
- String sig = mnd.getImage() + mnd.getParameterCount() + mnd.isVarargs();
- if (!sigs.contains(sig)) {
- unique.add(mnd);
- }
- sigs.add(sig);
- }
- return unique;
- }
-
- /**
- * Checks, whether the given method {@code mnd} is called from other methods or constructors.
- *
- * @param mnd the private method, that is checked
- * @param occs the usages of the private method
- * @return true if the method is not used (except maybe from itself), false
- * if the method is called by other methods.
- */
- private boolean isMethodNotCalledFromOtherMethods(MethodNameDeclaration mnd, List occs) {
- int callsFromOutsideMethod = 0;
- for (NameOccurrence occ : occs) {
- Node occNode = occ.getLocation();
- ASTConstructorDeclaration enclosingConstructor = occNode
- .getFirstParentOfType(ASTConstructorDeclaration.class);
- if (enclosingConstructor != null) {
- callsFromOutsideMethod++;
- break; // Do we miss unused private constructors here?
- }
- ASTInitializer enclosingInitializer = occNode.getFirstParentOfType(ASTInitializer.class);
- if (enclosingInitializer != null) {
- callsFromOutsideMethod++;
- break;
- }
-
- ASTMethodDeclaration enclosingMethod = occNode.getFirstParentOfType(ASTMethodDeclaration.class);
- if (enclosingMethod == null || !mnd.getNode().getParent().equals(enclosingMethod)) {
- callsFromOutsideMethod++;
- break;
- }
- }
- return callsFromOutsideMethod == 0;
- }
-
- private boolean privateAndNotExcluded(MethodNameDeclaration mnd) {
- ASTMethodDeclaration node = mnd.getDeclarator();
- return node.isPrivate() && !SERIALIZATION_METHODS.contains(node.getName());
+ private boolean hasExcludedName(ASTMethodDeclaration node) {
+ return SERIALIZATION_METHODS.contains(node.getName());
}
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesRule.java
index a86a433dd0..413f9b2915 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesRule.java
@@ -7,16 +7,12 @@ package net.sourceforge.pmd.lang.java.rule.codestyle;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
-import java.util.Objects;
import java.util.Set;
-import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTCatchClause;
-import net.sourceforge.pmd.lang.java.ast.ASTName;
-import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
-import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix;
import net.sourceforge.pmd.lang.java.ast.ASTTryStatement;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
+import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
/**
@@ -29,7 +25,10 @@ public class IdenticalCatchBranchesRule extends AbstractJavaRule {
private boolean areEquivalent(ASTCatchClause st1, ASTCatchClause st2) {
- return hasSameSubTree(st1.getBody(), st2.getBody(), st1.getParameter().getName(), st2.getParameter().getName());
+ String e1Name = st1.getParameter().getName();
+ String e2Name = st2.getParameter().getName();
+
+ return JavaRuleUtil.tokenEquals(st1.getBody(), st2.getBody(), name -> name.equals(e1Name) ? e2Name : name);
}
@@ -96,79 +95,4 @@ public class IdenticalCatchBranchesRule extends AbstractJavaRule {
}
- /**
- * Checks whether two nodes define the same subtree,
- * up to the renaming of one local variable.
- *
- * @param node1 the first node to check
- * @param node2 the second node to check
- * @param exceptionName1 the first exception variable name
- * @param exceptionName2 the second exception variable name
- */
- private boolean hasSameSubTree(Node node1, Node node2, String exceptionName1, String exceptionName2) {
- if (node1 == null && node2 == null) {
- return true;
- } else if (node1 == null || node2 == null) {
- return false;
- }
-
- //numbers of child node are different
- if (node1.getNumChildren() != node2.getNumChildren()) {
- return false;
- }
-
- for (int num = 0; num < node1.getNumChildren(); num++) {
-
- if (!basicEquivalence(node1.getChild(num), node2.getChild(num), exceptionName1, exceptionName2)) {
- return false;
- }
-
- //subtree of nodes are different
- if (!hasSameSubTree(node1.getChild(num), node2.getChild(num),
- exceptionName1, exceptionName2)) {
- return false;
- }
- }
- return true;
- }
-
-
- // no subtree comparison
- private boolean basicEquivalence(Node node1, Node node2, String varName1, String varName2) {
- // Nodes must have the same type
- if (node1.getClass() != node2.getClass()) {
- return false;
- }
-
- String image1 = node1.getImage();
- String image2 = node2.getImage();
-
- // image of nodes must be the same
- return Objects.equals(image1, image2)
- // or must be references to the variable we allow to interchange
- || Objects.equals(image1, varName1) && Objects.equals(image2, varName2)
- // which means we must filter out method references.
- && isNoMethodName(node1) && isNoMethodName(node2);
-
- }
-
-
- private boolean isNoMethodName(Node name) {
-
- if (name instanceof ASTName
- && (name.getParent() instanceof ASTPrimaryPrefix || name.getParent() instanceof ASTPrimarySuffix)) {
-
- Node prefixOrSuffix = name.getParent();
-
- if (prefixOrSuffix.getParent().getNumChildren() > 1 + prefixOrSuffix.getIndexInParent()) {
- // there's one next sibling
-
- Node next = prefixOrSuffix.getParent().getChild(prefixOrSuffix.getIndexInParent() + 1);
- if (next instanceof ASTPrimarySuffix) {
- return !((ASTPrimarySuffix) next).isArguments();
- }
- }
- }
- return true;
- }
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/LocalVariableCouldBeFinalRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/LocalVariableCouldBeFinalRule.java
index b7ce0cab4b..efb2b384c9 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/LocalVariableCouldBeFinalRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/LocalVariableCouldBeFinalRule.java
@@ -6,45 +6,31 @@ package net.sourceforge.pmd.lang.java.rule.codestyle;
import static net.sourceforge.pmd.properties.PropertyFactory.booleanProperty;
-import java.util.List;
-import java.util.Map;
-
-import net.sourceforge.pmd.lang.java.ast.ASTForStatement;
+import net.sourceforge.pmd.RuleContext;
+import net.sourceforge.pmd.lang.java.ast.ASTForeachStatement;
import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
-import net.sourceforge.pmd.lang.java.rule.performance.AbstractOptimizationRule;
-import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
-import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
-import net.sourceforge.pmd.lang.symboltable.Scope;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.properties.PropertyDescriptor;
-public class LocalVariableCouldBeFinalRule extends AbstractOptimizationRule {
+public class LocalVariableCouldBeFinalRule extends AbstractJavaRulechainRule {
private static final PropertyDescriptor IGNORE_FOR_EACH =
- booleanProperty("ignoreForEachDecl").defaultValue(false).desc("Ignore non-final loop variables in a for-each statement.").build();
+ booleanProperty("ignoreForEachDecl").defaultValue(false).desc("Ignore non-final loop variables in a for-each statement.").build();
public LocalVariableCouldBeFinalRule() {
+ super(ASTLocalVariableDeclaration.class);
definePropertyDescriptor(IGNORE_FOR_EACH);
}
@Override
public Object visit(ASTLocalVariableDeclaration node, Object data) {
- if (node.isFinal()) {
+ if (node.isFinal()) { // also for implicit finals, like resources
return data;
}
- if (getProperty(IGNORE_FOR_EACH) && node.getParent() instanceof ASTForStatement) {
+ if (getProperty(IGNORE_FOR_EACH) && node.getParent() instanceof ASTForeachStatement) {
return data;
}
- Scope s = node.getScope();
- Map> decls = s.getDeclarations(VariableNameDeclaration.class);
- for (Map.Entry> entry : decls.entrySet()) {
- VariableNameDeclaration var = entry.getKey();
- if (var.getAccessNodeParent() != node) {
- continue;
- }
- if (!assigned(entry.getValue())) {
- addViolation(data, var.getAccessNodeParent(), var.getImage());
- }
- }
+ MethodArgumentCouldBeFinalRule.checkForFinal((RuleContext) data, this, node.getVarIds());
return data;
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/MethodArgumentCouldBeFinalRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/MethodArgumentCouldBeFinalRule.java
index 0d11dcf72d..bd95e874e8 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/MethodArgumentCouldBeFinalRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/MethodArgumentCouldBeFinalRule.java
@@ -4,44 +4,60 @@
package net.sourceforge.pmd.lang.java.rule.codestyle;
-import java.util.List;
-import java.util.Map;
-
+import net.sourceforge.pmd.RuleContext;
+import net.sourceforge.pmd.lang.ast.NodeStream;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.AccessType;
import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
-import net.sourceforge.pmd.lang.java.ast.AccessNode;
-import net.sourceforge.pmd.lang.java.rule.performance.AbstractOptimizationRule;
-import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
-import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
-import net.sourceforge.pmd.lang.symboltable.Scope;
+import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration;
+import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
+import net.sourceforge.pmd.lang.rule.AbstractRule;
-public class MethodArgumentCouldBeFinalRule extends AbstractOptimizationRule {
+public class MethodArgumentCouldBeFinalRule extends AbstractJavaRulechainRule {
+
+ public MethodArgumentCouldBeFinalRule() {
+ super(ASTMethodOrConstructorDeclaration.class);
+ }
@Override
public Object visit(ASTMethodDeclaration meth, Object data) {
- if (meth.isNative() || meth.isAbstract()) {
+ if (meth.getBody() == null) {
return data;
}
- this.lookForViolation(meth.getScope(), data);
- return super.visit(meth, data);
- }
-
- private void lookForViolation(Scope scope, Object data) {
- Map> decls = scope.getDeclarations(VariableNameDeclaration.class);
- for (Map.Entry> entry : decls.entrySet()) {
- VariableNameDeclaration var = entry.getKey();
- AccessNode node = var.getAccessNodeParent();
- if (!node.isFinal() && node instanceof ASTFormalParameter && !assigned(entry.getValue())) {
- addViolation(data, node, var.getImage());
- }
- }
+ lookForViolation(meth, data);
+ return data;
}
@Override
public Object visit(ASTConstructorDeclaration constructor, Object data) {
- this.lookForViolation(constructor.getScope(), data);
- return super.visit(constructor, data);
+ lookForViolation(constructor, data);
+ return data;
+ }
+
+ private void lookForViolation(ASTMethodOrConstructorDeclaration node, Object data) {
+ checkForFinal((RuleContext) data, this, node.getFormalParameters().toStream().map(ASTFormalParameter::getVarId));
+ }
+
+ static void checkForFinal(RuleContext ruleContext, AbstractRule rule, NodeStream variables) {
+ outer:
+ for (ASTVariableDeclaratorId var : variables) {
+ if (var.isFinal()) {
+ continue;
+ }
+ boolean used = false;
+ for (ASTNamedReferenceExpr usage : var.getLocalUsages()) {
+ used = true;
+ if (usage.getAccessType() == AccessType.WRITE) {
+ continue outer;
+ }
+ }
+ if (used) {
+ rule.addViolation(ruleContext, var, var.getName());
+ }
+ }
}
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java
index e9f43f3400..ce35ada222 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java
@@ -6,173 +6,51 @@ package net.sourceforge.pmd.lang.java.rule.codestyle;
import static net.sourceforge.pmd.properties.PropertyFactory.booleanProperty;
-import java.util.List;
-import java.util.Map;
-
-import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.java.ast.ASTAnnotation;
-import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement;
-import net.sourceforge.pmd.lang.java.ast.ASTExpression;
-import net.sourceforge.pmd.lang.java.ast.ASTMemberSelector;
-import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTName;
-import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
-import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix;
+import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement;
-import net.sourceforge.pmd.lang.java.ast.ASTVariableInitializer;
-import net.sourceforge.pmd.lang.java.ast.AccessNode;
-import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
-import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
-import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
-import net.sourceforge.pmd.lang.symboltable.Scope;
+import net.sourceforge.pmd.lang.java.ast.ASTVariableAccess;
+import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
+import net.sourceforge.pmd.lang.java.symbols.JVariableSymbol;
import net.sourceforge.pmd.properties.PropertyDescriptor;
-public class UnnecessaryLocalBeforeReturnRule extends AbstractJavaRule {
+public class UnnecessaryLocalBeforeReturnRule extends AbstractJavaRulechainRule {
private static final PropertyDescriptor STATEMENT_ORDER_MATTERS = booleanProperty("statementOrderMatters").defaultValue(true).desc("If set to false this rule no longer requires the variable declaration and return statement to be on consecutive lines. Any variable that is used solely in a return statement will be reported.").build();
public UnnecessaryLocalBeforeReturnRule() {
+ super(ASTReturnStatement.class);
definePropertyDescriptor(STATEMENT_ORDER_MATTERS);
}
- @Override
- public Object visit(ASTMethodDeclaration meth, Object data) {
- // skip void/abstract/native method
- if (meth.isVoid() || meth.isAbstract() || meth.isNative()) {
- return data;
- }
- return super.visit(meth, data);
- }
@Override
- public Object visit(ASTReturnStatement rtn, Object data) {
- // skip returns of literals
- ASTName name = rtn.getFirstDescendantOfType(ASTName.class);
- if (name == null) {
- return data;
+ public Object visit(ASTReturnStatement returnStmt, Object data) {
+ if (!(returnStmt.getExpr() instanceof ASTVariableAccess)) {
+ return null;
+ }
+ ASTVariableAccess varExpr = (ASTVariableAccess) returnStmt.getExpr();
+ JVariableSymbol sym = varExpr.getReferencedSym();
+ if (sym == null) {
+ return null;
}
- // skip 'complicated' expressions
- if (rtn.findDescendantsOfType(ASTExpression.class).size() > 1
- || rtn.getFirstDescendantOfType(ASTMemberSelector.class) != null
- || rtn.findDescendantsOfType(ASTPrimaryExpression.class).size() > 1 || isMethodCall(rtn)) {
- return data;
+ ASTVariableDeclaratorId varDecl = sym.tryGetNode();
+ if (varDecl == null || !varDecl.isLocalVariable() || varDecl.getDeclaredAnnotations().nonEmpty()) {
+ return null;
}
- Map> vars = name.getScope()
- .getDeclarations(VariableNameDeclaration.class);
- for (Map.Entry> entry : vars.entrySet()) {
- VariableNameDeclaration variableDeclaration = entry.getKey();
- if (variableDeclaration.getDeclaratorId().isFormalParameter()) {
- continue;
- }
-
- List usages = entry.getValue();
-
- if (usages.size() == 1) { // If there is more than 1 usage, then it's not only returned
- NameOccurrence occ = usages.get(0);
-
- if (occ.getLocation().equals(name) && isNotAnnotated(variableDeclaration)) {
- String var = name.getImage();
- if (var.indexOf('.') != -1) {
- var = var.substring(0, var.indexOf('.'));
- }
-
- // Is the variable initialized with another member that is later used?
- if (!isInitDataModifiedAfterInit(variableDeclaration, rtn)
- && !statementsBeforeReturn(variableDeclaration, rtn)) {
- addViolation(data, rtn, var);
- }
- }
- }
+ if (varDecl.getLocalUsages().size() != 1) {
+ return null;
}
- return data;
- }
+ // then this is the only usage
- private boolean statementsBeforeReturn(VariableNameDeclaration variableDeclaration, ASTReturnStatement returnStatement) {
- if (!getProperty(STATEMENT_ORDER_MATTERS)) {
- return false;
+ if (!getProperty(STATEMENT_ORDER_MATTERS)
+ || varDecl.ancestors(ASTLocalVariableDeclaration.class).firstOrThrow().getNextSibling() == returnStmt) {
+ addViolation(data, varDecl, varDecl.getName());
}
-
- ASTBlockStatement declarationStatement = variableDeclaration.getAccessNodeParent().getFirstParentOfType(ASTBlockStatement.class);
- ASTBlockStatement returnBlockStatement = returnStatement.getFirstParentOfType(ASTBlockStatement.class);
-
- // double check: we should now be at the same level in the AST - both block statements are children of the same parent
- if (declarationStatement.getParent() == returnBlockStatement.getParent()) {
- return returnBlockStatement.getIndexInParent() - declarationStatement.getIndexInParent() > 1;
- }
- return false;
- }
-
- // TODO : should node define isAfter / isBefore helper methods for Nodes?
- private static boolean isAfter(Node n1, Node n2) {
- return n1.getBeginLine() > n2.getBeginLine()
- || n1.getBeginLine() == n2.getBeginLine() && n1.getBeginColumn() >= n2.getEndColumn();
- }
-
- private boolean isInitDataModifiedAfterInit(final VariableNameDeclaration variableDeclaration,
- final ASTReturnStatement rtn) {
- final ASTVariableInitializer initializer = variableDeclaration.getAccessNodeParent()
- .getFirstDescendantOfType(ASTVariableInitializer.class);
-
- if (initializer != null) {
- // Get the block statements for each, so we can compare apples to apples
- final ASTBlockStatement initializerStmt = variableDeclaration.getAccessNodeParent()
- .getFirstParentOfType(ASTBlockStatement.class);
- final ASTBlockStatement rtnStmt = rtn.getFirstParentOfType(ASTBlockStatement.class);
-
- final List referencedNames = initializer.findDescendantsOfType(ASTName.class);
- for (final ASTName refName : referencedNames) {
- // TODO : Shouldn't the scope allow us to search for a var name occurrences directly, moving up through parent scopes?
- Scope scope = refName.getScope();
- do {
- final Map> declarations = scope
- .getDeclarations(VariableNameDeclaration.class);
- for (final Map.Entry> entry : declarations
- .entrySet()) {
- if (entry.getKey().getName().equals(refName.getImage())) {
- // Variable found! Check usage locations
- for (final NameOccurrence occ : entry.getValue()) {
- final ASTBlockStatement location = occ.getLocation().getFirstParentOfType(ASTBlockStatement.class);
-
- // Is it used after initializing our "unnecessary" local but before the return statement?
- if (location != null && isAfter(location, initializerStmt) && isAfter(rtnStmt, location)) {
- return true;
- }
- }
-
- return false;
- }
- }
- scope = scope.getParent();
- } while (scope != null);
- }
- }
-
- return false;
- }
-
- private boolean isNotAnnotated(VariableNameDeclaration variableDeclaration) {
- AccessNode accessNodeParent = variableDeclaration.getAccessNodeParent();
- return !accessNodeParent.hasDescendantOfType(ASTAnnotation.class);
- }
-
- /**
- * Determine if the given return statement has any embedded method calls.
- *
- * @param rtn
- * return statement to analyze
- * @return true if any method calls are made within the given return
- */
- private boolean isMethodCall(ASTReturnStatement rtn) {
- List suffix = rtn.findDescendantsOfType(ASTPrimarySuffix.class);
- for (ASTPrimarySuffix element : suffix) {
- if (element.isArguments()) {
- return true;
- }
- }
- return false;
+ return null;
}
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyBooleanReturnsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyBooleanReturnsRule.java
index 624608b721..6fa8e0d37a 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyBooleanReturnsRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyBooleanReturnsRule.java
@@ -4,258 +4,80 @@
package net.sourceforge.pmd.lang.java.rule.design;
-import net.sourceforge.pmd.lang.ast.Node;
+import static net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil.areComplements;
+import static net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil.isBooleanLiteral;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
import net.sourceforge.pmd.lang.java.ast.ASTBlock;
-import net.sourceforge.pmd.lang.java.ast.ASTBooleanLiteral;
+import net.sourceforge.pmd.lang.java.ast.ASTExpression;
import net.sourceforge.pmd.lang.java.ast.ASTIfStatement;
-import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement;
-import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpressionNotPlusMinus;
-import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
+import net.sourceforge.pmd.lang.java.ast.JavaNode;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.lang.java.types.JPrimitiveType.PrimitiveTypeKind;
-public class SimplifyBooleanReturnsRule extends AbstractJavaRule {
+public class SimplifyBooleanReturnsRule extends AbstractJavaRulechainRule {
+
+ public SimplifyBooleanReturnsRule() {
+ super(ASTReturnStatement.class);
+ }
@Override
- public Object visit(ASTMethodDeclaration node, Object data) {
- // only boolean methods should be inspected
- if (node.getResultType().getTypeMirror().isPrimitive(PrimitiveTypeKind.BOOLEAN)) {
- return super.visit(node, data);
+ public Object visit(ASTReturnStatement node, Object data) {
+ ASTExpression expr = node.getExpr();
+ if (expr == null
+ || !expr.getTypeMirror().isPrimitive(PrimitiveTypeKind.BOOLEAN)
+ || !isThenBranchOfSomeIf(node)) {
+ return null;
+ }
+ return checkIf(node.ancestors(ASTIfStatement.class).firstOrThrow(), data, expr);
+ }
+
+ // Only explore the then branch. If we explore the else, then we'll report twice.
+ // In the case if .. else, both branches need to be symmetric anyway.
+ private boolean isThenBranchOfSomeIf(ASTReturnStatement node) {
+ if (node.getParent() instanceof ASTIfStatement) {
+ return node.getIndexInParent() == 1;
+ }
+ if (node.getParent() instanceof ASTBlock
+ && ((ASTBlock) node.getParent()).size() == 1
+ && node.getParent().getParent() instanceof ASTIfStatement) {
+ return node.getParent().getIndexInParent() == 1;
+ }
+ return false;
+ }
+
+ private Object checkIf(ASTIfStatement node, Object data, ASTExpression thenExpr) {
+ // that's the case: if..then..return; return;
+ ASTExpression elseExpr = getElseExpr(node);
+ if (elseExpr == null) {
+ return data;
+ }
+
+ if (isBooleanLiteral(thenExpr) || isBooleanLiteral(elseExpr)) {
+ addViolation(data, node);
+ } else if (areComplements(thenExpr, elseExpr)) {
+ // if (foo) return !a;
+ // else return a;
+ addViolation(data, node);
}
- // skip method
return data;
}
- @Override
- public Object visit(ASTIfStatement node, Object data) {
- // that's the case: if..then..return; return;
- if (!node.hasElse() && isIfJustReturnsBoolean(node) && isJustReturnsBooleanAfter(node)) {
- addViolation(data, node);
- return super.visit(node, data);
+ private @Nullable ASTExpression getReturnExpr(JavaNode node) {
+ if (node instanceof ASTReturnStatement) {
+ return ((ASTReturnStatement) node).getExpr();
+ } else if (node instanceof ASTBlock && ((ASTBlock) node).size() == 1) {
+ return getReturnExpr(((ASTBlock) node).get(0));
}
-
- // only deal with if..then..else stmts
- if (node.getNumChildren() != 3) {
- return super.visit(node, data);
- }
-
- // don't bother if either the if or the else block is empty
- if (node.getChild(1).getNumChildren() == 0 || node.getChild(2).getNumChildren() == 0) {
- return super.visit(node, data);
- }
-
- Node returnStatement1 = node.getChild(1).getChild(0);
- Node returnStatement2 = node.getChild(2).getChild(0);
-
- if (returnStatement1 instanceof ASTReturnStatement && returnStatement2 instanceof ASTReturnStatement) {
- Node expression1 = returnStatement1.getChild(0).getChild(0);
- Node expression2 = returnStatement2.getChild(0).getChild(0);
- if (terminatesInBooleanLiteral(returnStatement1) && terminatesInBooleanLiteral(returnStatement2)) {
- addViolation(data, node);
- } else if (expression1 instanceof ASTUnaryExpressionNotPlusMinus
- ^ expression2 instanceof ASTUnaryExpressionNotPlusMinus) {
- // We get the nodes under the '!' operator
- // If they are the same => error
- if (isNodesEqualWithUnaryExpression(expression1, expression2)) {
- // second case:
- // If
- // Expr
- // Statement
- // ReturnStatement
- // UnaryExpressionNotPlusMinus '!'
- // Expression E
- // Statement
- // ReturnStatement
- // Expression E
- // i.e.,
- // if (foo)
- // return !a;
- // else
- // return a;
- addViolation(data, node);
- }
- }
- } else if (hasOneBlockStmt(node.getChild(1)) && hasOneBlockStmt(node.getChild(2))) {
- // We have blocks so we must go down three levels (BlockStatement,
- // Statement, ReturnStatement)
- returnStatement1 = returnStatement1.getChild(0).getChild(0).getChild(0);
- returnStatement2 = returnStatement2.getChild(0).getChild(0).getChild(0);
-
- // if we have 2 return;
- if (isSimpleReturn(returnStatement1) && isSimpleReturn(returnStatement2)) {
- // third case
- // If
- // Expr
- // Statement
- // Block
- // BlockStatement
- // Statement
- // ReturnStatement
- // Statement
- // Block
- // BlockStatement
- // Statement
- // ReturnStatement
- // i.e.,
- // if (foo) {
- // return true;
- // } else {
- // return false;
- // }
- addViolation(data, node);
- } else {
- Node expression1 = getDescendant(returnStatement1, 4);
- Node expression2 = getDescendant(returnStatement2, 4);
- if (terminatesInBooleanLiteral(node.getChild(1).getChild(0))
- && terminatesInBooleanLiteral(node.getChild(2).getChild(0))) {
- addViolation(data, node);
- } else if (expression1 instanceof ASTUnaryExpressionNotPlusMinus
- ^ expression2 instanceof ASTUnaryExpressionNotPlusMinus) {
- // We get the nodes under the '!' operator
- // If they are the same => error
- if (isNodesEqualWithUnaryExpression(expression1, expression2)) {
- // forth case
- // If
- // Expr
- // Statement
- // Block
- // BlockStatement
- // Statement
- // ReturnStatement
- // UnaryExpressionNotPlusMinus '!'
- // Expression E
- // Statement
- // Block
- // BlockStatement
- // Statement
- // ReturnStatement
- // Expression E
- // i.e.,
- // if (foo) {
- // return !a;
- // } else {
- // return a;
- // }
- addViolation(data, node);
- }
- }
- }
- }
- return super.visit(node, data);
+ return null;
}
- /**
- * Checks, whether there is a statement after the given if statement, and if
- * so, whether this is just a return boolean statement.
- *
- * @param ifNode
- * the if statement
- * @return
- */
- private boolean isJustReturnsBooleanAfter(ASTIfStatement ifNode) {
- Node blockStatement = ifNode.getParent().getParent();
- Node block = blockStatement.getParent();
- if (block.getNumChildren() != blockStatement.getIndexInParent() + 1 + 1) {
- return false;
- }
-
- Node nextBlockStatement = block.getChild(blockStatement.getIndexInParent() + 1);
- return terminatesInBooleanLiteral(nextBlockStatement);
+ private @Nullable ASTExpression getElseExpr(ASTIfStatement node) {
+ return node.hasElse() ? getReturnExpr(node.getElseBranch())
+ : getReturnExpr(node.getNextSibling()); // may be followed immediately by return
}
- /**
- * Checks whether the given ifstatement just returns a boolean in the if
- * clause.
- *
- * @param ifNode
- * the if statement
- * @return
- */
- private boolean isIfJustReturnsBoolean(ASTIfStatement ifNode) {
- Node node = ifNode.getChild(1);
- return node.getNumChildren() == 1
- && (hasOneBlockStmt(node) || terminatesInBooleanLiteral(node.getChild(0)));
- }
-
- private boolean hasOneBlockStmt(Node node) {
- return node.getChild(0) instanceof ASTBlock && node.getChild(0).getNumChildren() == 1
- && terminatesInBooleanLiteral(node.getChild(0).getChild(0));
- }
-
- /**
- * Returns the first child node going down 'level' levels or null if level
- * is invalid
- */
- private Node getDescendant(Node node, int level) {
- Node n = node;
- for (int i = 0; i < level; i++) {
- if (n.getNumChildren() == 0) {
- return null;
- }
- n = n.getChild(0);
- }
- return n;
- }
-
- private boolean terminatesInBooleanLiteral(Node node) {
- return eachNodeHasOneChild(node) && getLastChild(node) instanceof ASTBooleanLiteral;
- }
-
- private boolean eachNodeHasOneChild(Node node) {
- if (node.getNumChildren() > 1) {
- return false;
- }
- if (node.getNumChildren() == 0) {
- return true;
- }
- return eachNodeHasOneChild(node.getChild(0));
- }
-
- private Node getLastChild(Node node) {
- if (node.getNumChildren() == 0) {
- return node;
- }
- return getLastChild(node.getChild(0));
- }
-
- private boolean isNodesEqualWithUnaryExpression(Node n1, Node n2) {
- Node node1;
- Node node2;
- if (n1 instanceof ASTUnaryExpressionNotPlusMinus) {
- node1 = n1.getChild(0);
- } else {
- node1 = n1;
- }
- if (n2 instanceof ASTUnaryExpressionNotPlusMinus) {
- node2 = n2.getChild(0);
- } else {
- node2 = n2;
- }
- return isNodesEquals(node1, node2);
- }
-
- private boolean isNodesEquals(Node n1, Node n2) {
- int numberChild1 = n1.getNumChildren();
- int numberChild2 = n2.getNumChildren();
- if (numberChild1 != numberChild2) {
- return false;
- }
- if (!n1.getClass().equals(n2.getClass())) {
- return false;
- }
- if (!n1.toString().equals(n2.toString())) {
- return false;
- }
- for (int i = 0; i < numberChild1; i++) {
- if (!isNodesEquals(n1.getChild(i), n2.getChild(i))) {
- return false;
- }
- }
- return true;
- }
-
- private boolean isSimpleReturn(Node node) {
- return node instanceof ASTReturnStatement && node.getNumChildren() == 0;
- }
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentRequiredRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentRequiredRule.java
index 78cabc82e2..fcef9c76a5 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentRequiredRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentRequiredRule.java
@@ -5,7 +5,6 @@
package net.sourceforge.pmd.lang.java.rule.documentation;
-import java.io.ObjectStreamField;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
@@ -21,13 +20,11 @@ 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.ASTMethodOrConstructorDeclaration;
-import net.sourceforge.pmd.lang.java.ast.JModifier;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.ast.JavadocCommentOwner;
import net.sourceforge.pmd.lang.java.multifile.signature.JavaOperationSignature;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
-import net.sourceforge.pmd.lang.java.types.JPrimitiveType.PrimitiveTypeKind;
-import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
+import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
import net.sourceforge.pmd.properties.PropertyBuilder.GenericPropertyBuilder;
import net.sourceforge.pmd.properties.PropertyDescriptor;
import net.sourceforge.pmd.properties.PropertyFactory;
@@ -187,9 +184,9 @@ public class CommentRequiredRule extends AbstractJavaRule {
@Override
public Object visit(ASTFieldDeclaration decl, Object data) {
- if (isSerialVersionUID(decl)) {
+ if (JavaRuleUtil.isSerialVersionUID(decl)) {
checkCommentMeetsRequirement(data, decl, SERIAL_VERSION_UID_CMT_REQUIREMENT_DESCRIPTOR);
- } else if (isSerialPersistentFields(decl)) {
+ } else if (JavaRuleUtil.isSerialPersistentFields(decl)) {
checkCommentMeetsRequirement(data, decl, SERIAL_PERSISTENT_FIELDS_CMT_REQUIREMENT_DESCRIPTOR);
} else {
checkCommentMeetsRequirement(data, decl, FIELD_CMT_REQUIREMENT_DESCRIPTOR);
@@ -199,30 +196,6 @@ public class CommentRequiredRule extends AbstractJavaRule {
}
- @SuppressWarnings("PMD.UnusedFormalParameter")
- private boolean isSerialVersionUID(ASTFieldDeclaration field) {
- return field.getVarIds().any(it -> "serialVersionUID".equals(it.getName()))
- && field.hasModifiers(JModifier.FINAL, JModifier.STATIC)
- && field.getTypeNode().getTypeMirror().isPrimitive(PrimitiveTypeKind.LONG);
- }
-
- /**
- * Whether the given field is a serialPersistentFields variable.
- *
- * This field must be initialized with an array of ObjectStreamField objects.
- * The modifiers for the field are required to be private, static, and final.
- *
- * @param field the field, must not be null
- * @return true if the field is a serialPersistentFields variable, otherwise false
- * @see Oracle docs
- */
- @SuppressWarnings("PMD.UnusedFormalParameter")
- private boolean isSerialPersistentFields(final ASTFieldDeclaration field) {
- return field.getVarIds().any(it -> "serialPersistentFields".equals(it.getName()))
- && field.hasModifiers(JModifier.FINAL, JModifier.STATIC, JModifier.PRIVATE)
- && TypeTestUtil.isA(ObjectStreamField[].class, field.getTypeNode());
- }
-
@Override
public Object visit(ASTEnumDeclaration decl, Object data) {
checkCommentMeetsRequirement(data, decl, ENUM_CMT_REQUIREMENT_DESCRIPTOR);
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentInOperandRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentInOperandRule.java
index a36cf640d5..459eb99466 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentInOperandRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentInOperandRule.java
@@ -6,32 +6,34 @@ package net.sourceforge.pmd.lang.java.rule.errorprone;
import static net.sourceforge.pmd.properties.PropertyFactory.booleanProperty;
-import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator;
+import net.sourceforge.pmd.RuleContext;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignmentExpression;
import net.sourceforge.pmd.lang.java.ast.ASTExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTExpressionStatement;
import net.sourceforge.pmd.lang.java.ast.ASTForStatement;
import net.sourceforge.pmd.lang.java.ast.ASTIfStatement;
import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpression;
import net.sourceforge.pmd.lang.java.ast.ASTWhileStatement;
-import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
+import net.sourceforge.pmd.lang.java.ast.JavaNode;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
+import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
import net.sourceforge.pmd.properties.PropertyDescriptor;
import net.sourceforge.pmd.properties.PropertySource;
/**
- *
*
*/
-public class AssignmentInOperandRule extends AbstractJavaRule {
+public class AssignmentInOperandRule extends AbstractJavaRulechainRule {
private static final PropertyDescriptor ALLOW_IF_DESCRIPTOR =
- booleanProperty("allowIf")
- .desc("Allow assignment within the conditional expression of an if statement")
- .defaultValue(false).build();
+ booleanProperty("allowIf")
+ .desc("Allow assignment within the conditional expression of an if statement")
+ .defaultValue(false).build();
private static final PropertyDescriptor ALLOW_FOR_DESCRIPTOR =
- booleanProperty("allowFor")
- .desc("Allow assignment within the conditional expression of a for statement")
- .defaultValue(false).build();
+ booleanProperty("allowFor")
+ .desc("Allow assignment within the conditional expression of a for statement")
+ .defaultValue(false).build();
private static final PropertyDescriptor ALLOW_WHILE_DESCRIPTOR =
booleanProperty("allowWhile")
@@ -45,6 +47,7 @@ public class AssignmentInOperandRule extends AbstractJavaRule {
public AssignmentInOperandRule() {
+ super(ASTAssignmentExpression.class, ASTUnaryExpression.class);
definePropertyDescriptor(ALLOW_IF_DESCRIPTOR);
definePropertyDescriptor(ALLOW_FOR_DESCRIPTOR);
definePropertyDescriptor(ALLOW_WHILE_DESCRIPTOR);
@@ -52,24 +55,32 @@ public class AssignmentInOperandRule extends AbstractJavaRule {
}
@Override
- public Object visit(ASTExpression node, Object data) {
- Node parent = node.getParent();
- if ((parent instanceof ASTIfStatement && !getProperty(ALLOW_IF_DESCRIPTOR)
- || parent instanceof ASTWhileStatement && !getProperty(ALLOW_WHILE_DESCRIPTOR)
- || parent instanceof ASTForStatement && parent.getChild(1) == node
- && !getProperty(ALLOW_FOR_DESCRIPTOR))
- && (node.hasDescendantOfType(ASTAssignmentOperator.class)
- || !getProperty(ALLOW_INCREMENT_DECREMENT_DESCRIPTOR) && hasIncrement(node))) {
-
- addViolation(data, node);
- return data;
- }
- return super.visit(node, data);
+ public Object visit(ASTAssignmentExpression node, Object data) {
+ checkAssignment(node, (RuleContext) data);
+ return null;
}
- private boolean hasIncrement(ASTExpression node) {
- // todo use node streams
- return node.findDescendantsOfType(ASTUnaryExpression.class).stream().anyMatch(it -> !it.getOperator().isPure());
+ @Override
+ public Object visit(ASTUnaryExpression node, Object data) {
+ if (!getProperty(ALLOW_INCREMENT_DECREMENT_DESCRIPTOR) && !node.getOperator().isPure()) {
+ checkAssignment(node, (RuleContext) data);
+ }
+ return null;
+ }
+
+ private void checkAssignment(ASTExpression impureExpr, RuleContext ctx) {
+ ASTExpression toplevel = JavaRuleUtil.getTopLevelExpr(impureExpr);
+ JavaNode parent = toplevel.getParent();
+ if (parent instanceof ASTExpressionStatement) {
+ // that's ok
+ return;
+ }
+ if (parent instanceof ASTIfStatement && !getProperty(ALLOW_IF_DESCRIPTOR)
+ || parent instanceof ASTWhileStatement && !getProperty(ALLOW_WHILE_DESCRIPTOR)
+ || parent instanceof ASTForStatement && ((ASTForStatement) parent).getCondition() == toplevel && !getProperty(ALLOW_FOR_DESCRIPTOR)) {
+
+ addViolation(ctx, impureExpr);
+ }
}
public boolean allowsAllAssignments() {
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 ddc6fdc2ed..b6d553544d 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
@@ -4,21 +4,64 @@
package net.sourceforge.pmd.lang.java.rule.internal;
+import static net.sourceforge.pmd.lang.java.types.JPrimitiveType.PrimitiveTypeKind.LONG;
+
+import java.io.InvalidObjectException;
+import java.io.ObjectInputStream;
+import java.io.ObjectStreamField;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import net.sourceforge.pmd.lang.ast.GenericToken;
+import net.sourceforge.pmd.lang.ast.NodeStream;
+import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken;
import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
+import net.sourceforge.pmd.lang.java.ast.ASTArgumentList;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.AccessType;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignmentExpression;
import net.sourceforge.pmd.lang.java.ast.ASTBodyDeclaration;
+import net.sourceforge.pmd.lang.java.ast.ASTBooleanLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
+import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
+import net.sourceforge.pmd.lang.java.ast.ASTConstructorCall;
import net.sourceforge.pmd.lang.java.ast.ASTExpression;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
+import net.sourceforge.pmd.lang.java.ast.ASTForStatement;
+import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter;
+import net.sourceforge.pmd.lang.java.ast.ASTFormalParameters;
import net.sourceforge.pmd.lang.java.ast.ASTInfixExpression;
import net.sourceforge.pmd.lang.java.ast.ASTInitializer;
+import net.sourceforge.pmd.lang.java.ast.ASTLabeledStatement;
+import net.sourceforge.pmd.lang.java.ast.ASTList;
+import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
+import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
+import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration;
+import net.sourceforge.pmd.lang.java.ast.ASTNullLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTNumericLiteral;
+import net.sourceforge.pmd.lang.java.ast.ASTStatement;
+import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.ast.AccessNode;
import net.sourceforge.pmd.lang.java.ast.AccessNode.Visibility;
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.JavaTokenKinds;
+import net.sourceforge.pmd.lang.java.ast.TypeNode;
+import net.sourceforge.pmd.lang.java.ast.UnaryOp;
+import net.sourceforge.pmd.lang.java.types.JPrimitiveType.PrimitiveTypeKind;
+import net.sourceforge.pmd.lang.java.types.JTypeMirror;
import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
+import net.sourceforge.pmd.util.CollectionUtil;
/**
* Utilities shared between rules.
@@ -80,6 +123,47 @@ public final class JavaRuleUtil {
return false;
}
+ public static boolean isStringConcatExpr(@Nullable JavaNode e) {
+ if (e instanceof ASTInfixExpression) {
+ ASTInfixExpression infix = (ASTInfixExpression) e;
+ return infix.getOperator() == BinaryOp.ADD && TypeTestUtil.isA(String.class, infix);
+ }
+ return false;
+ }
+
+ /**
+ * If the parameter is an operand of a binary infix expression,
+ * returns the other operand. Otherwise returns null.
+ */
+ public static @Nullable ASTExpression getOtherOperandIfInInfixExpr(@Nullable JavaNode e) {
+ if (e != null && e.getParent() instanceof ASTInfixExpression) {
+ return (ASTExpression) e.getParent().getChild(1 - e.getIndexInParent());
+ }
+ return null;
+ }
+
+ /**
+ * Returns true if the expression is a stringbuilder (or stringbuffer)
+ * append call, or a constructor call for one of these classes.
+ */
+ public static boolean isStringBuilderCtorOrAppend(@Nullable ASTExpression e) {
+ if (e instanceof ASTMethodCall) {
+ ASTMethodCall call = (ASTMethodCall) e;
+ if ("append".equals(call.getMethodName())) {
+ ASTExpression qual = ((ASTMethodCall) e).getQualifier();
+ return qual != null && isStringBufferOrBuilder(qual);
+ }
+ } else if (e instanceof ASTConstructorCall) {
+ return isStringBufferOrBuilder(((ASTConstructorCall) e).getTypeNode());
+ }
+ return false;
+ }
+
+ private static boolean isStringBufferOrBuilder(TypeNode node) {
+ return TypeTestUtil.isExactlyA(StringBuilder.class, node)
+ || TypeTestUtil.isExactlyA(StringBuffer.class, node);
+ }
+
/**
* Returns true if the node is a {@link ASTMethodDeclaration} that
* is a main method.
@@ -149,4 +233,284 @@ public final class JavaRuleUtil {
|| name.startsWith("unused")
|| "_".equals(name); // before java 9 it's ok
}
+
+
+ /**
+ * Returns true if the formal parameters of the method or constructor
+ * match the given types exactly. Note that for varargs methods, the
+ * last param must have an array type (but it is not checked to be varargs).
+ * This will return false if we're not sure.
+ *
+ * @param node Method or ctor
+ * @param types List of types to match (may be empty)
+ *
+ * @throws NullPointerException If any of the classes is null, or the node is null
+ * @see TypeTestUtil#isExactlyA(Class, TypeNode)
+ */
+ public static boolean hasParameters(ASTMethodOrConstructorDeclaration node, Class... types) {
+ ASTFormalParameters formals = node.getFormalParameters();
+ if (formals.size() != types.length) {
+ return false;
+ }
+ for (int i = 0; i < formals.size(); i++) {
+ ASTFormalParameter fi = formals.get(i);
+ if (!TypeTestUtil.isExactlyA(types[i], fi)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Returns true if the {@code throws} declaration of the method or constructor
+ * matches the given types exactly.
+ *
+ * @param node Method or ctor
+ * @param types List of exception types to match (may be empty)
+ *
+ * @throws NullPointerException If any of the classes is null, or the node is null
+ * @see TypeTestUtil#isExactlyA(Class, TypeNode)
+ */
+ @SafeVarargs
+ public static boolean hasExceptionList(ASTMethodOrConstructorDeclaration node, Class... types) {
+ @NonNull List formals = ASTList.orEmpty(node.getThrowsList());
+ if (formals.size() != types.length) {
+ return false;
+ }
+ for (int i = 0; i < formals.size(); i++) {
+ ASTClassOrInterfaceType fi = formals.get(i);
+ if (!TypeTestUtil.isExactlyA(types[i], fi)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * True if the variable is never used. Note that the visibility of
+ * the variable must be less than {@link Visibility#V_PRIVATE} for
+ * us to be sure of it.
+ */
+ public static boolean isNeverUsed(ASTVariableDeclaratorId varId) {
+ return CollectionUtil.none(varId.getLocalUsages(), JavaRuleUtil::isReadUsage);
+ }
+
+ private static boolean isReadUsage(ASTNamedReferenceExpr expr) {
+ return expr.getAccessType() == AccessType.READ
+ // foo(x++)
+ || expr.getParent() instanceof ASTUnaryExpression
+ && expr.getParent().getParent() instanceof ASTArgumentList;
+ }
+
+ /**
+ * True if the variable is incremented or decremented via a compound
+ * assignment operator, or a unary increment/decrement expression.
+ */
+ public static boolean isVarAccessReadAndWrite(ASTNamedReferenceExpr expr) {
+ return expr.getAccessType() == AccessType.WRITE
+ && (!(expr.getParent() instanceof ASTAssignmentExpression)
+ || ((ASTAssignmentExpression) expr.getParent()).getOperator().isCompound());
+ }
+
+ /**
+ * True if the variable access is a non-compound assignment.
+ */
+ public static boolean isVarAccessStrictlyWrite(ASTNamedReferenceExpr expr) {
+ return expr.getParent() instanceof ASTAssignmentExpression
+ && expr.getIndexInParent() == 0
+ && !((ASTAssignmentExpression) expr.getParent()).getOperator().isCompound();
+ }
+
+ /**
+ * Returns the set of labels on this statement.
+ */
+ public static Set getStatementLabels(ASTStatement node) {
+ if (!(node.getParent() instanceof ASTLabeledStatement)) {
+ return Collections.emptySet();
+ }
+
+ return node.ancestors().takeWhile(it -> it instanceof ASTLabeledStatement)
+ .toStream()
+ .map(it -> ((ASTLabeledStatement) it).getLabel())
+ .collect(Collectors.toSet());
+ }
+
+ /**
+ * Will cut through argument lists, except those of enum constants
+ * and explicit invocation nodes.
+ */
+ public static @NonNull ASTExpression getTopLevelExpr(ASTExpression expr) {
+ return (ASTExpression) expr.ancestorsOrSelf()
+ .takeWhile(it -> it instanceof ASTExpression
+ || it instanceof ASTArgumentList && it.getParent() instanceof ASTExpression)
+ .last();
+ }
+
+ /**
+ * Returns the variable IDS corresponding to variables declared in
+ * the init clause of the loop.
+ */
+ public static NodeStream getLoopVariables(ASTForStatement loop) {
+ return NodeStream.of(loop.getInit())
+ .filterIs(ASTLocalVariableDeclaration.class)
+ .flatMap(ASTLocalVariableDeclaration::getVarIds);
+ }
+
+ // TODO at least UnusedPrivateMethod has some serialization-related logic.
+
+ /**
+ * Whether some variable declared by the given node is a serialPersistentFields
+ * (serialization-specific field).
+ */
+ public static boolean isSerialPersistentFields(final ASTFieldDeclaration field) {
+ return field.hasModifiers(JModifier.FINAL, JModifier.STATIC, JModifier.PRIVATE)
+ && field.getVarIds().any(it -> "serialPersistentFields".equals(it.getName()) && TypeTestUtil.isA(ObjectStreamField[].class, it));
+ }
+
+ /**
+ * Whether some variable declared by the given node is a serialVersionUID
+ * (serialization-specific field).
+ */
+ public static boolean isSerialVersionUID(ASTFieldDeclaration field) {
+ return field.hasModifiers(JModifier.FINAL, JModifier.STATIC)
+ && field.getVarIds().any(it -> "serialVersionUID".equals(it.getName()) && it.getTypeMirror().isPrimitive(LONG));
+ }
+
+ /**
+ * True if the method is a {@code readObject} method defined for serialization.
+ */
+ public static boolean isSerializationReadObject(ASTMethodDeclaration node) {
+ return node.getVisibility() == Visibility.V_PRIVATE
+ && "readObject".equals(node.getName())
+ && hasExceptionList(node, InvalidObjectException.class)
+ && hasParameters(node, ObjectInputStream.class);
+ }
+
+ /**
+ * Whether one expression is the boolean negation of the other. Many
+ * forms are not yet supported. This method is symmetric so only needs
+ * to be called once.
+ */
+ public static boolean areComplements(ASTExpression e1, ASTExpression e2) {
+ if (isBooleanNegation(e1)) {
+ return areEqual(unaryOperand(e1), e2);
+ } else if (isBooleanNegation(e2)) {
+ return areEqual(e1, unaryOperand(e2));
+ } else if (e1 instanceof ASTInfixExpression && e2 instanceof ASTInfixExpression) {
+ ASTInfixExpression ifx1 = (ASTInfixExpression) e1;
+ ASTInfixExpression ifx2 = (ASTInfixExpression) e2;
+ if (ifx1.getOperator().getComplement() != ifx2.getOperator()) {
+ return false;
+ }
+ if (ifx1.getOperator().hasSamePrecedenceAs(BinaryOp.EQ)) {
+ // NOT(a == b, a != b)
+ // NOT(a == b, b != a)
+ return areEqual(ifx1.getLeftOperand(), ifx2.getLeftOperand())
+ && areEqual(ifx1.getRightOperand(), ifx2.getRightOperand())
+ || areEqual(ifx2.getLeftOperand(), ifx1.getLeftOperand())
+ && areEqual(ifx2.getRightOperand(), ifx1.getRightOperand());
+ }
+ // todo we could continue with de Morgan and such
+ }
+ return false;
+ }
+
+ private static boolean areEqual(ASTExpression e1, ASTExpression e2) {
+ return tokenEquals(e1, e2);
+ }
+
+ /**
+ * Returns true if both nodes have exactly the same tokens.
+ *
+ * @param node First node
+ * @param that Other node
+ */
+ public static boolean tokenEquals(JavaNode node, JavaNode that) {
+ return tokenEquals(node, that, null);
+ }
+
+ /**
+ * Returns true if both nodes have the same tokens, modulo some renaming
+ * function. The renaming function maps unqualified variables and type
+ * identifiers of the first node to the other. This should be used
+ * in nodes living in the same lexical scope, so that unqualified
+ * names mean the same thing.
+ *
+ * @param node First node
+ * @param other Other node
+ * @param varRenamer A renaming function. If null, no renaming is applied.
+ * Must not return null, if no renaming occurs, returns its argument.
+ */
+ public static boolean tokenEquals(@NonNull JavaNode node,
+ @NonNull JavaNode other,
+ @Nullable Function varRenamer) {
+ // 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());
+ int lastKind = 0;
+ while (thisIt.hasNext()) {
+ if (!thatIt.hasNext()) {
+ return false;
+ }
+ JavaccToken o1 = thisIt.next();
+ JavaccToken o2 = thatIt.next();
+ if (o1.kind != o2.kind) {
+ return false;
+ }
+
+ String mappedImage = o1.getImage();
+ if (varRenamer != null
+ && o1.kind == JavaTokenKinds.IDENTIFIER
+ && lastKind != JavaTokenKinds.DOT
+ && lastKind != JavaTokenKinds.METHOD_REF
+ //method name
+ && o1.getNext() != null && o1.getNext().kind != JavaTokenKinds.LPAREN) {
+ mappedImage = varRenamer.apply(mappedImage);
+ }
+
+ if (!o2.getImage().equals(mappedImage)) {
+ return false;
+ }
+
+ lastKind = o1.kind;
+ }
+ return !thatIt.hasNext();
+ }
+
+ public static boolean isBooleanLiteral(ASTExpression e) {
+ return e instanceof ASTBooleanLiteral;
+ }
+
+ public static boolean isBooleanNegation(ASTExpression e) {
+ return e instanceof ASTUnaryExpression && ((ASTUnaryExpression) e).getOperator() == UnaryOp.NEGATION;
+ }
+
+ /**
+ * If the argument is a unary expression, returns its operand, otherwise
+ * returns null.
+ */
+ public static @Nullable ASTExpression unaryOperand(ASTExpression e) {
+ return e instanceof ASTUnaryExpression ? ((ASTUnaryExpression) e).getOperand()
+ : null;
+ }
+
+ /**
+ * Returns true if the expression is the default field value for
+ * the given type.
+ */
+ public static boolean isDefaultValue(JTypeMirror type, ASTExpression expr) {
+ if (type.isPrimitive()) {
+ if (type.isPrimitive(PrimitiveTypeKind.BOOLEAN)) {
+ return expr instanceof ASTBooleanLiteral && !((ASTBooleanLiteral) expr).isTrue();
+ } else {
+ Object constValue = expr.getConstValue();
+ return constValue instanceof Number && ((Number) constValue).doubleValue() == 0d
+ || constValue instanceof Character && constValue.equals('\u0000');
+ }
+ } else {
+ return expr instanceof ASTNullLiteral;
+ }
+ }
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AbstractOptimizationRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AbstractOptimizationRule.java
deleted file mode 100644
index f882fc860d..0000000000
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AbstractOptimizationRule.java
+++ /dev/null
@@ -1,44 +0,0 @@
-/*
- * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
- */
-
-package net.sourceforge.pmd.lang.java.rule.performance;
-
-import java.util.List;
-
-import net.sourceforge.pmd.annotation.InternalApi;
-import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
-import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
-import net.sourceforge.pmd.lang.java.symboltable.JavaNameOccurrence;
-import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
-
-/**
- * Base class with utility methods for optimization rules
- *
- * @author mgriffa
- * @since Created on Jan 11, 2005
- * @deprecated Internal API
- */
-@Deprecated
-@InternalApi
-public class AbstractOptimizationRule extends AbstractJavaRule {
-
- @Override
- public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
- if (node.isInterface()) {
- return data;
- }
- return super.visit(node, data);
- }
-
- protected boolean assigned(List usages) {
- for (NameOccurrence occ : usages) {
- JavaNameOccurrence jocc = (JavaNameOccurrence) occ;
- if (jocc.isOnLeftHandSide() || jocc.isSelfAssignment()) {
- return true;
- }
- }
- return false;
- }
-
-}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/ConsecutiveAppendsShouldReuseRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/ConsecutiveAppendsShouldReuseRule.java
index 26172c63a8..00812b3de6 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/ConsecutiveAppendsShouldReuseRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/ConsecutiveAppendsShouldReuseRule.java
@@ -4,138 +4,113 @@
package net.sourceforge.pmd.lang.java.rule.performance;
-import java.util.List;
-import java.util.Map;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignmentExpression;
import net.sourceforge.pmd.lang.java.ast.ASTExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTExpressionStatement;
import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTName;
-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.ASTStatement;
-import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
-import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
-import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
-import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
+import net.sourceforge.pmd.lang.java.symbols.JExecutableSymbol;
+import net.sourceforge.pmd.lang.java.symbols.JVariableSymbol;
+import net.sourceforge.pmd.lang.java.types.OverloadSelectionResult;
+import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
+import net.sourceforge.pmd.lang.rule.RuleTargetSelector;
public class ConsecutiveAppendsShouldReuseRule extends AbstractJavaRule {
@Override
- public Object visit(ASTBlockStatement node, Object data) {
- String variable = getVariableAppended(node);
- if (variable != null) {
- ASTBlockStatement nextSibling = getNextBlockStatementSibling(node);
- if (nextSibling != null) {
- String nextVariable = getVariableAppended(nextSibling);
+ protected @NonNull RuleTargetSelector buildTargetSelector() {
+ return RuleTargetSelector.forTypes(ASTExpressionStatement.class, ASTLocalVariableDeclaration.class);
+ }
+
+ @Override
+ public Object visit(ASTExpressionStatement node, Object data) {
+ Node nextSibling = node.asStream().followingSiblings().first();
+ if (nextSibling instanceof ASTExpressionStatement) {
+ @Nullable JVariableSymbol variable = getVariableAppended(node);
+ if (variable != null) {
+ @Nullable JVariableSymbol nextVariable = getVariableAppended((ASTExpressionStatement) nextSibling);
if (nextVariable != null && nextVariable.equals(variable)) {
addViolation(data, node);
}
}
}
- return super.visit(node, data);
+ return data;
}
- private ASTBlockStatement getNextBlockStatementSibling(Node node) {
- Node parent = node.getParent();
- int childIndex = -1;
- for (int i = 0; i < parent.getNumChildren(); i++) {
- if (parent.getChild(i) == node) {
- childIndex = i;
- break;
+ @Override
+ public Object visit(ASTLocalVariableDeclaration node, Object data) {
+ Node nextSibling = node.asStream().followingSiblings().first();
+ if (nextSibling instanceof ASTExpressionStatement) {
+ @Nullable JVariableSymbol nextVariable = getVariableAppended((ASTExpressionStatement) nextSibling);
+ if (nextVariable != null) {
+ ASTVariableDeclaratorId varDecl = nextVariable.tryGetNode();
+ if (varDecl != null && node.getVarIds().any(it -> it == varDecl)
+ && isStringBuilderAppend(varDecl.getInitializer())) {
+ addViolation(data, node);
+ }
}
}
- if (childIndex + 1 < parent.getNumChildren()) {
- Node nextSibling = parent.getChild(childIndex + 1);
- if (nextSibling instanceof ASTBlockStatement) {
- return (ASTBlockStatement) nextSibling;
- }
+ return data;
+ }
+
+
+ private @Nullable JVariableSymbol getVariableAppended(ASTExpressionStatement node) {
+ ASTExpression expr = node.getExpr();
+ if (expr instanceof ASTMethodCall) {
+ return getAsVarAccess(getAppendChainQualifier(expr));
+ } else if (expr instanceof ASTAssignmentExpression) {
+ ASTExpression rhs = ((ASTAssignmentExpression) expr).getRightOperand();
+ return getAppendChainQualifier(rhs) != null ? getAssignmentLhsAsVar(expr) : null;
}
return null;
}
- private String getVariableAppended(ASTBlockStatement node) {
- if (isFirstChild(node, ASTStatement.class)) {
- ASTStatement statement = (ASTStatement) node.getChild(0);
- if (isFirstChild(statement, ASTStatementExpression.class)) {
- ASTStatementExpression stmtExp = (ASTStatementExpression) statement.getChild(0);
- if (stmtExp.getNumChildren() == 1) {
- ASTPrimaryPrefix primaryPrefix = stmtExp.getFirstDescendantOfType(ASTPrimaryPrefix.class);
- if (primaryPrefix != null) {
- ASTName name = primaryPrefix.getFirstChildOfType(ASTName.class);
- if (name != null) {
- String image = name.getImage();
- if (image.endsWith(".append")) {
- String variable = image.substring(0, image.indexOf('.'));
- if (isAStringBuilderBuffer(primaryPrefix, variable)) {
- return variable;
- }
- }
- }
- }
- } else {
- final ASTExpression exp = stmtExp.getFirstDescendantOfType(ASTExpression.class);
- if (isFirstChild(exp, ASTPrimaryExpression.class)) {
- final ASTPrimarySuffix primarySuffix = ((ASTPrimaryExpression) exp.getChild(0))
- .getFirstDescendantOfType(ASTPrimarySuffix.class);
- if (primarySuffix != null) {
- final String name = primarySuffix.getImage();
- if ("append".equals(name)) {
- final ASTPrimaryExpression pExp = stmtExp
- .getFirstDescendantOfType(ASTPrimaryExpression.class);
- if (pExp != null) {
- final ASTName astName = stmtExp.getFirstDescendantOfType(ASTName.class);
- if (astName != null) {
- final String variable = astName.getImage();
- if (isAStringBuilderBuffer(primarySuffix, variable)) {
- return variable;
- }
- }
- }
- }
- }
- }
- }
- }
- } else if (isFirstChild(node, ASTLocalVariableDeclaration.class)) {
- ASTLocalVariableDeclaration lvd = (ASTLocalVariableDeclaration) node.getChild(0);
-
- ASTVariableDeclaratorId vdId = lvd.getFirstDescendantOfType(ASTVariableDeclaratorId.class);
- ASTExpression exp = lvd.getFirstDescendantOfType(ASTExpression.class);
-
- if (exp != null) {
- ASTPrimarySuffix primarySuffix = exp.getFirstDescendantOfType(ASTPrimarySuffix.class);
- if (primarySuffix != null) {
- final String name = primarySuffix.getImage();
- if ("append".equals(name)) {
- String variable = vdId.getImage();
- if (isAStringBuilderBuffer(primarySuffix, variable)) {
- return variable;
- }
- }
- }
- }
+ private @Nullable ASTExpression getAppendChainQualifier(final ASTExpression base) {
+ ASTExpression expr = base;
+ while (expr instanceof ASTMethodCall && isStringBuilderAppend(expr)) {
+ expr = ((ASTMethodCall) expr).getQualifier();
}
+ return base == expr ? null : expr; // NOPMD
+ }
+ private @Nullable JVariableSymbol getAssignmentLhsAsVar(@Nullable ASTExpression expr) {
+ if (expr instanceof ASTAssignmentExpression) {
+ return getAsVarAccess(((ASTAssignmentExpression) expr).getLeftOperand());
+ }
return null;
}
- private boolean isAStringBuilderBuffer(JavaNode node, String name) {
- Map> declarations = node.getScope()
- .getDeclarations(VariableNameDeclaration.class);
- for (VariableNameDeclaration decl : declarations.keySet()) {
- if (decl.getName().equals(name) && ConsecutiveLiteralAppendsRule.isStringBuilderOrBuffer(decl.getDeclaratorId())) {
- return true;
- }
+ private @Nullable JVariableSymbol getAsVarAccess(@Nullable ASTExpression expr) {
+ if (expr instanceof ASTNamedReferenceExpr) {
+ return ((ASTNamedReferenceExpr) expr).getReferencedSym();
+ }
+ return null;
+ }
+
+ private boolean isStringBuilderAppend(@Nullable ASTExpression e) {
+ if (e instanceof ASTMethodCall) {
+ ASTMethodCall call = (ASTMethodCall) e;
+ return "append".equals(call.getMethodName())
+ && isStringBuilderAppend(call.getOverloadSelectionInfo());
}
return false;
}
- private boolean isFirstChild(Node node, Class clazz) {
- return node.getNumChildren() == 1 && clazz.isAssignableFrom(node.getChild(0).getClass());
+ private boolean isStringBuilderAppend(OverloadSelectionResult result) {
+ if (result.isFailed()) {
+ return false;
+ }
+
+ JExecutableSymbol symbol = result.getMethodType().getSymbol();
+ return TypeTestUtil.isExactlyA(StringBuffer.class, symbol.getEnclosingClass())
+ || TypeTestUtil.isExactlyA(StringBuilder.class, symbol.getEnclosingClass());
}
+
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/InefficientStringBufferingRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/InefficientStringBufferingRule.java
index 9cf762af10..b85b553c62 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/InefficientStringBufferingRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/InefficientStringBufferingRule.java
@@ -4,31 +4,15 @@
package net.sourceforge.pmd.lang.java.rule.performance;
-import java.util.ArrayList;
-import java.util.Iterator;
-import java.util.List;
-
+import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.java.ast.ASTAdditiveExpression;
-import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression;
import net.sourceforge.pmd.lang.java.ast.ASTArgumentList;
-import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement;
-import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
-import net.sourceforge.pmd.lang.java.ast.ASTConditionalExpression;
-import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter;
-import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
-import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTName;
-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.ASTPrimitiveType;
-import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression;
-import net.sourceforge.pmd.lang.java.ast.ASTType;
-import net.sourceforge.pmd.lang.java.ast.AccessNode;
-import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
-import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
-import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
+import net.sourceforge.pmd.lang.java.ast.ASTConstructorCall;
+import net.sourceforge.pmd.lang.java.ast.ASTExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTList;
+import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
+import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
/**
* How this rule works: find additive expressions: + check that the addition is
@@ -37,240 +21,46 @@ import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
*
* @author mgriffa
*/
-public class InefficientStringBufferingRule extends AbstractJavaRule {
+public class InefficientStringBufferingRule extends AbstractJavaRulechainRule {
public InefficientStringBufferingRule() {
- addRuleChainVisit(ASTAdditiveExpression.class);
+ super(ASTConstructorCall.class, ASTMethodCall.class);
}
@Override
- public Object visit(ASTAdditiveExpression node, Object data) {
- if (node.getParent() instanceof ASTConditionalExpression || node.getNthParent(2) instanceof ASTConditionalExpression) {
- // ignore concats in ternary expressions
- return data;
+ public Object visit(ASTMethodCall node, Object data) {
+ if (JavaRuleUtil.isStringBuilderCtorOrAppend(node)) {
+ checkArgument(node.getArguments(), (RuleContext) data);
}
-
- ASTBlockStatement bs = node.getFirstParentOfType(ASTBlockStatement.class);
- if (bs == null) {
- return data;
- }
-
- int immediateLiterals = 0;
- int immediateStringLiterals = 0;
- List nodes = node.findDescendantsOfType(ASTLiteral.class);
- for (ASTLiteral literal : nodes) {
- if (literal.getNthParent(3) instanceof ASTAdditiveExpression) {
- immediateLiterals++;
- if (literal.isStringLiteral()) {
- immediateStringLiterals++;
- }
- }
- if (literal.isIntLiteral() || literal.isFloatLiteral() || literal.isDoubleLiteral()
- || literal.isLongLiteral()) {
- return data;
- }
- }
-
- boolean onlyStringLiterals = immediateLiterals == immediateStringLiterals
- && immediateLiterals == node.getNumChildren();
- if (onlyStringLiterals) {
- return data;
- }
-
- // if literal + final, return
- List nameNodes = node.findDescendantsOfType(ASTName.class);
- for (ASTName name : nameNodes) {
- if (name.getNameDeclaration() != null && name.getNameDeclaration() instanceof VariableNameDeclaration) {
- VariableNameDeclaration vnd = (VariableNameDeclaration) name.getNameDeclaration();
- AccessNode accessNodeParent = vnd.getAccessNodeParent();
- if (accessNodeParent.isFinal()) {
- return data;
- }
- }
- }
-
- // if literal primitive type and not strings variables, then return
- boolean stringFound = false;
- for (ASTName name : nameNodes) {
- if (!isPrimitiveType(name) && isStringType(name)) {
- stringFound = true;
- break;
- }
- }
- if (!stringFound && immediateStringLiterals == 0) {
- return data;
- }
-
- if (bs.isAllocation()) {
- for (Iterator iterator = nameNodes.iterator(); iterator.hasNext();) {
- ASTName name = iterator.next();
- if (!name.getImage().endsWith("length")) {
- break;
- } else if (!iterator.hasNext()) {
- return data; // All names end with length
- }
- }
-
- if (isAllocatedStringBuffer(node)) {
- addViolation(data, node);
- } else if (isInStringBufferOperationChain(node, "append")) {
- addViolation(data, node);
- }
- } else if (isInStringBufferOperationChain(node, "append")) {
- addViolation(data, node);
- }
- return data;
+ return null;
}
- private boolean isStringType(ASTName name) {
- ASTType type = getTypeNode(name);
- if (type != null) {
- List types = type.findDescendantsOfType(ASTClassOrInterfaceType.class);
- if (!types.isEmpty()) {
- return TypeTestUtil.isA(String.class, types.get(0));
- }
+ @Override
+ public Object visit(ASTConstructorCall node, Object data) {
+ if (JavaRuleUtil.isStringBuilderCtorOrAppend(node)) {
+ checkArgument(node.getArguments(), (RuleContext) data);
}
- return false;
+ return null;
}
- private boolean isPrimitiveType(ASTName name) {
- ASTType type = getTypeNode(name);
- return type != null && !type.findChildrenOfType(ASTPrimitiveType.class).isEmpty();
+ private void checkArgument(ASTArgumentList argList, RuleContext ctx) {
+ ASTExpression arg = ASTList.singleOrNull(argList);
+
+ if (JavaRuleUtil.isStringConcatExpr(arg)
+ // ignore concatenations that produce constants
+ && !arg.isCompileTimeConstant()) {
+ addViolation(ctx, arg);
+ }
}
- private ASTType getTypeNode(ASTName name) {
- ASTType result = null;
- if (name.getNameDeclaration() instanceof VariableNameDeclaration) {
- VariableNameDeclaration vnd = (VariableNameDeclaration) name.getNameDeclaration();
- if (vnd.getAccessNodeParent() instanceof ASTLocalVariableDeclaration) {
- ASTLocalVariableDeclaration l = (ASTLocalVariableDeclaration) vnd.getAccessNodeParent();
- result = l.getTypeNode();
- } else if (vnd.getAccessNodeParent() instanceof ASTFormalParameter) {
- ASTFormalParameter p = (ASTFormalParameter) vnd.getAccessNodeParent();
- result = p.getTypeNode();
- }
- }
- return result;
- }
-
- static boolean isInStringBufferOperationChain(Node node, String methodName) {
- ASTPrimaryExpression expr = node.getFirstParentOfType(ASTPrimaryExpression.class);
- MethodCallChain methodCalls = MethodCallChain.wrap(expr);
- while (expr != null && methodCalls == null) {
- expr = expr.getFirstParentOfType(ASTPrimaryExpression.class);
- methodCalls = MethodCallChain.wrap(expr);
- }
-
- if (methodCalls != null && !methodCalls.isExactlyOfAnyType(StringBuffer.class, StringBuilder.class)) {
- methodCalls = null;
- }
-
- return methodCalls != null && methodCalls.getMethodNames().contains(methodName);
- }
-
- /**
- * @deprecated will be removed with PMD 7
- */
- @Deprecated
- protected static boolean isInStringBufferOperation(Node node, int length, String methodName) {
- if (!(node.getNthParent(length) instanceof ASTStatementExpression)) {
- return false;
- }
- ASTStatementExpression s = node.getFirstParentOfType(ASTStatementExpression.class);
- if (s == null) {
- return false;
- }
- ASTName n = s.getFirstDescendantOfType(ASTName.class);
- if (n == null || !n.getImage().contains(methodName)
- || !(n.getNameDeclaration() instanceof VariableNameDeclaration)) {
+ public static boolean isInStringBufferOperationChain(Node node, String append) {
+ // todo this was replaced by something that doesn't really work
+ if (!(node instanceof ASTExpression)) {
return false;
}
+ Node parent = node.getParent();
- // TODO having to hand-code this kind of dredging around is ridiculous
- // we need something to support this in the framework
- // but, "for now" (tm):
- // if more than one arg to append(), skip it
- ASTArgumentList argList = s.getFirstDescendantOfType(ASTArgumentList.class);
- if (argList == null || argList.getNumChildren() > 1) {
- return false;
- }
- return ConsecutiveLiteralAppendsRule.isStringBuilderOrBuffer(((VariableNameDeclaration) n.getNameDeclaration()).getDeclaratorId());
- }
-
- private boolean isAllocatedStringBuffer(ASTAdditiveExpression node) {
- ASTAllocationExpression ao = node.getFirstParentOfType(ASTAllocationExpression.class);
- if (ao == null) {
- return false;
- }
- // note that the child can be an ArrayDimsAndInits, for example, from
- // java.lang.FloatingDecimal: t = new int[ nWords+wordcount+1 ];
- ASTClassOrInterfaceType an = ao.getFirstChildOfType(ASTClassOrInterfaceType.class);
- return ConsecutiveLiteralAppendsRule.isStringBuilderOrBuffer(an);
- }
-
- private static class MethodCallChain {
- private final ASTPrimaryExpression primary;
-
- private MethodCallChain(ASTPrimaryExpression primary) {
- this.primary = primary;
- }
-
- // Note: The impl here is technically not correct: The type of a method call
- // chain is the result of the last method called, not the type of the
- // first receiver object (== PrimaryPrefix).
- boolean isExactlyOfAnyType(Class clazz, Class ... clazzes) {
- ASTPrimaryPrefix typeNode = getTypeNode();
-
- if (TypeTestUtil.isExactlyA(clazz, typeNode)) {
- return true;
- }
- if (clazzes != null) {
- for (Class c : clazzes) {
- if (TypeTestUtil.isExactlyA(c, typeNode)) {
- return true;
- }
- }
- }
- return false;
- }
-
- ASTPrimaryPrefix getTypeNode() {
- return primary.getFirstChildOfType(ASTPrimaryPrefix.class);
- }
-
- List getMethodNames() {
- List methodNames = new ArrayList<>();
-
- ASTPrimaryPrefix prefix = getTypeNode();
- ASTName name = prefix.getFirstChildOfType(ASTName.class);
- if (name != null) {
- String firstMethod = name.getImage();
- int dot = firstMethod.lastIndexOf('.');
- if (dot != -1) {
- firstMethod = firstMethod.substring(dot + 1);
- }
- methodNames.add(firstMethod);
- }
-
- for (ASTPrimarySuffix suffix : primary.findChildrenOfType(ASTPrimarySuffix.class)) {
- if (suffix.getImage() != null) {
- methodNames.add(suffix.getImage());
- }
- }
- return methodNames;
- }
-
- static MethodCallChain wrap(ASTPrimaryExpression primary) {
- if (primary != null && isMethodCall(primary)) {
- return new MethodCallChain(primary);
- }
- return null;
- }
-
- private static boolean isMethodCall(ASTPrimaryExpression primary) {
- return primary.getNumChildren() >= 2
- && primary.getChild(0) instanceof ASTPrimaryPrefix
- && primary.getChild(1) instanceof ASTPrimarySuffix;
- }
+ return parent instanceof ASTMethodCall
+ && JavaRuleUtil.isStringBuilderCtorOrAppend((ASTMethodCall) parent);
}
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/RedundantFieldInitializerRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/RedundantFieldInitializerRule.java
index 801e1f4ab0..9970dd31ea 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/RedundantFieldInitializerRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/RedundantFieldInitializerRule.java
@@ -4,17 +4,14 @@
package net.sourceforge.pmd.lang.java.rule.performance;
-import net.sourceforge.pmd.lang.java.ast.ASTBooleanLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTExpression;
import net.sourceforge.pmd.lang.java.ast.ASTFieldAccess;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTNullLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTVariableAccess;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.ast.JModifier;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
-import net.sourceforge.pmd.lang.java.types.JPrimitiveType.PrimitiveTypeKind;
-import net.sourceforge.pmd.lang.java.types.JTypeMirror;
+import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
/**
* Detects redundant field initializers, i.e. the field initializer expressions
@@ -35,7 +32,7 @@ public class RedundantFieldInitializerRule extends AbstractJavaRulechainRule {
for (ASTVariableDeclaratorId varId : fieldDeclaration.getVarIds()) {
ASTExpression init = varId.getInitializer();
if (init != null) {
- if (isDefaultValue(varId.getTypeMirror(), init)) {
+ if (!isWhitelisted(init) && JavaRuleUtil.isDefaultValue(varId.getTypeMirror(), init)) {
addViolation(data, varId);
}
}
@@ -44,25 +41,8 @@ public class RedundantFieldInitializerRule extends AbstractJavaRulechainRule {
return data;
}
- private boolean isDefaultValue(JTypeMirror type, ASTExpression expr) {
- if (type.isPrimitive()) {
- if (type.isPrimitive(PrimitiveTypeKind.BOOLEAN)) {
- return expr instanceof ASTBooleanLiteral && !((ASTBooleanLiteral) expr).isTrue();
- } else {
- if (!isOkExpr(expr)) {
- // whitelist named constants or calculations involving them
- return false;
- }
- Object constValue = expr.getConstValue();
- return constValue instanceof Number && ((Number) constValue).doubleValue() == 0d
- || constValue instanceof Character && constValue.equals('\u0000');
- }
- } else {
- return expr instanceof ASTNullLiteral;
- }
- }
-
- private static boolean isOkExpr(ASTExpression e) {
- return e.descendantsOrSelf().none(it -> it instanceof ASTVariableAccess || it instanceof ASTFieldAccess);
+ // whitelist if there are named variables in there
+ private static boolean isWhitelisted(ASTExpression e) {
+ return e.descendantsOrSelf().any(it -> it instanceof ASTVariableAccess || it instanceof ASTFieldAccess);
}
}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UselessStringValueOfRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UselessStringValueOfRule.java
index 64a2a8c1c7..0720956404 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UselessStringValueOfRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UselessStringValueOfRule.java
@@ -8,10 +8,9 @@ import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
import net.sourceforge.pmd.lang.java.ast.ASTExpression;
-import net.sourceforge.pmd.lang.java.ast.ASTInfixExpression;
import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
-import net.sourceforge.pmd.lang.java.ast.BinaryOp;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
+import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
import net.sourceforge.pmd.lang.rule.RuleTargetSelector;
@@ -25,8 +24,7 @@ public class UselessStringValueOfRule extends AbstractJavaRule {
@Override
public Object visit(ASTMethodCall node, Object data) {
- if (node.getParent() instanceof ASTInfixExpression
- && ((ASTInfixExpression) node.getParent()).getOperator() == BinaryOp.ADD) {
+ if (JavaRuleUtil.isStringConcatExpr(node.getParent())) {
ASTExpression valueOfArg = getValueOfArg(node);
if (valueOfArg == null) {
return data; //not a valueOf call
@@ -35,7 +33,7 @@ public class UselessStringValueOfRule extends AbstractJavaRule {
return data;
}
- ASTExpression sibling = (ASTExpression) node.getParent().getChild(1 - node.getIndexInParent());
+ ASTExpression sibling = JavaRuleUtil.getOtherOperandIfInInfixExpr(node);
if (TypeTestUtil.isExactlyA(String.class, sibling)
&& !valueOfArg.getTypeMirror().isArray()
// In `String.valueOf(a) + String.valueOf(b)`,
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/ImplicitMemberSymbols.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/ImplicitMemberSymbols.java
index 24584575b8..9cc2c0caae 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/ImplicitMemberSymbols.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/ImplicitMemberSymbols.java
@@ -13,7 +13,9 @@ import java.util.function.BiFunction;
import java.util.function.Function;
import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
+import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.symbols.JClassSymbol;
import net.sourceforge.pmd.lang.java.symbols.JConstructorSymbol;
import net.sourceforge.pmd.lang.java.symbols.JExecutableSymbol;
@@ -115,7 +117,7 @@ public final class ImplicitMemberSymbols {
modifiers,
CollectionUtil.map(
recordComponents,
- f -> c -> new FakeFormalParamSym(c, f.getSimpleName(), (ts, sym) -> f.getTypeMirror(Substitution.EMPTY))
+ f -> c -> new FakeFormalParamSym(c, f.getSimpleName(), f.tryGetNode(), (ts, sym) -> f.getTypeMirror(Substitution.EMPTY))
)
);
}
@@ -279,14 +281,25 @@ public final class ImplicitMemberSymbols {
private final JExecutableSymbol owner;
private final String name;
+ private final ASTVariableDeclaratorId node;
private final BiFunction type;
private FakeFormalParamSym(JExecutableSymbol owner, String name, BiFunction type) {
+ this(owner, name, null, type);
+ }
+
+ private FakeFormalParamSym(JExecutableSymbol owner, String name, @Nullable ASTVariableDeclaratorId node, BiFunction type) {
this.owner = owner;
this.name = name;
+ this.node = node;
this.type = type;
}
+ @Override
+ public @Nullable ASTVariableDeclaratorId tryGetNode() {
+ return node;
+ }
+
@Override
public TypeSystem getTypeSystem() {
return owner.getTypeSystem();
diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml
index 178cbb9a2f..bf5753c6b4 100644
--- a/pmd-java/src/main/resources/category/java/bestpractices.xml
+++ b/pmd-java/src/main/resources/category/java/bestpractices.xml
@@ -1202,7 +1202,7 @@ public void bar() {
@@ -1215,11 +1215,9 @@ will (and by priority) and avoid clogging the Standard out log.
@@ -1801,7 +1799,7 @@ a block `{}` is sufficient.
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractClassWithoutAbstractMethodTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractClassWithoutAbstractMethodTest.java
index 16409a73f1..a7ff179f29 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractClassWithoutAbstractMethodTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractClassWithoutAbstractMethodTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;
import net.sourceforge.pmd.testframework.PmdRuleTst;
-@org.junit.Ignore("Rule has not been updated yet")
public class AbstractClassWithoutAbstractMethodTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningCatchVariablesTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningCatchVariablesTest.java
index 9088347cc0..5afdbca56b 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningCatchVariablesTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningCatchVariablesTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;
import net.sourceforge.pmd.testframework.PmdRuleTst;
-@org.junit.Ignore("Rule has not been updated yet")
public class AvoidReassigningCatchVariablesTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningLoopVariablesTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningLoopVariablesTest.java
index 322cc16603..3157601173 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningLoopVariablesTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningLoopVariablesTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;
import net.sourceforge.pmd.testframework.PmdRuleTst;
-@org.junit.Ignore("Rule has not been updated yet")
public class AvoidReassigningLoopVariablesTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersTest.java
index 35143dabda..906b82d966 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;
import net.sourceforge.pmd.testframework.PmdRuleTst;
-@org.junit.Ignore("Rule has not been updated yet")
public class AvoidReassigningParametersTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidUsingHardCodedIPTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidUsingHardCodedIPTest.java
index 8b97968659..63e4523ca2 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidUsingHardCodedIPTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidUsingHardCodedIPTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;
import net.sourceforge.pmd.testframework.PmdRuleTst;
-@org.junit.Ignore("Rule has not been updated yet")
public class AvoidUsingHardCodedIPTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/CheckResultSetTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/CheckResultSetTest.java
index 0b4109f74f..effe3630b2 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/CheckResultSetTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/CheckResultSetTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;
import net.sourceforge.pmd.testframework.PmdRuleTst;
-@org.junit.Ignore("Rule has not been updated yet")
public class CheckResultSetTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementTest.java
index c4ade9053a..02f1628b38 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;
import net.sourceforge.pmd.testframework.PmdRuleTst;
-@org.junit.Ignore("Rule has not been updated yet")
public class GuardLogStatementTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/SystemPrintlnTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/SystemPrintlnTest.java
index aebe3bb168..e17f5ee7be 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/SystemPrintlnTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/SystemPrintlnTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;
import net.sourceforge.pmd.testframework.PmdRuleTst;
-@org.junit.Ignore("Rule has not been updated yet")
public class SystemPrintlnTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedFormalParameterTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedFormalParameterTest.java
index f4e6c18889..154952c1f5 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedFormalParameterTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedFormalParameterTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;
import net.sourceforge.pmd.testframework.PmdRuleTst;
-@org.junit.Ignore("Rule has not been updated yet")
public class UnusedFormalParameterTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedLocalVariableTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedLocalVariableTest.java
index 3f3885a95a..aad6d106c4 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedLocalVariableTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedLocalVariableTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;
import net.sourceforge.pmd.testframework.PmdRuleTst;
-@org.junit.Ignore("Rule has not been updated yet")
public class UnusedLocalVariableTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateFieldTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateFieldTest.java
index 16da41a239..09e8be52e3 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateFieldTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateFieldTest.java
@@ -4,27 +4,8 @@
package net.sourceforge.pmd.lang.java.rule.bestpractices;
-import org.junit.Assert;
-import org.junit.Test;
-
import net.sourceforge.pmd.testframework.PmdRuleTst;
-@org.junit.Ignore("Rule has not been updated yet")
public class UnusedPrivateFieldTest extends PmdRuleTst {
- /**
- * This test will fail, as soon Lombok classes are on the test classpath.
- * The test classpath is used as auxclasspath during unit tests.
- * If lombok is present, then the test case for #1952 will never fail
- * and won't reproduce the false-negative case anymore.
- */
- @Test
- public void makeSureLombokIsNotOnClasspath() {
- try {
- Class.forName("lombok.Value");
- Assert.fail();
- } catch (ClassNotFoundException e) {
- // this is ok
- }
- }
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodTest.java
index 85c62ecfca..5cc101a04f 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;
import net.sourceforge.pmd.testframework.PmdRuleTst;
-@org.junit.Ignore("Rule has not been updated yet")
public class UnusedPrivateMethodTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesTest.java
index f9f9bdcd15..da7244d121 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesTest.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 IdenticalCatchBranchesTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/LocalVariableCouldBeFinalTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/LocalVariableCouldBeFinalTest.java
index a903c67f45..d342b03ba9 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/LocalVariableCouldBeFinalTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/LocalVariableCouldBeFinalTest.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 LocalVariableCouldBeFinalTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/MethodArgumentCouldBeFinalTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/MethodArgumentCouldBeFinalTest.java
index 7adb238896..66b362def9 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/MethodArgumentCouldBeFinalTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/MethodArgumentCouldBeFinalTest.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 MethodArgumentCouldBeFinalTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnTest.java
index b127254902..73dc6cdac3 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnTest.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 UnnecessaryLocalBeforeReturnTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyBooleanReturnsTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyBooleanReturnsTest.java
index 17f5f12d74..487cc554e4 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyBooleanReturnsTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyBooleanReturnsTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.design;
import net.sourceforge.pmd.testframework.PmdRuleTst;
-@org.junit.Ignore("Rule has not been updated yet")
public class SimplifyBooleanReturnsTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentInOperandTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentInOperandTest.java
index e083a2d0fe..3432ef4411 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentInOperandTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentInOperandTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone;
import net.sourceforge.pmd.testframework.PmdRuleTst;
-@org.junit.Ignore("Rule has not been updated yet")
public class AssignmentInOperandTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/ConsecutiveAppendsShouldReuseTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/ConsecutiveAppendsShouldReuseTest.java
index 7a4f7f4181..e7b68474ad 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/ConsecutiveAppendsShouldReuseTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/ConsecutiveAppendsShouldReuseTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.performance;
import net.sourceforge.pmd.testframework.PmdRuleTst;
-@org.junit.Ignore("Rule has not been updated yet")
public class ConsecutiveAppendsShouldReuseTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/InefficientStringBufferingTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/InefficientStringBufferingTest.java
index 7ab7078a81..a7c07cecfa 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/InefficientStringBufferingTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/InefficientStringBufferingTest.java
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.performance;
import net.sourceforge.pmd.testframework.PmdRuleTst;
-@org.junit.Ignore("Rule has not been updated yet")
public class InefficientStringBufferingTest extends PmdRuleTst {
// no additional unit tests
}
diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/UsageResolutionTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/UsageResolutionTest.kt
new file mode 100644
index 0000000000..381d95f0d9
--- /dev/null
+++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/UsageResolutionTest.kt
@@ -0,0 +1,79 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+package net.sourceforge.pmd.lang.java.ast
+
+import io.kotest.matchers.collections.shouldBeEmpty
+import io.kotest.matchers.collections.shouldBeSingleton
+import io.kotest.matchers.collections.shouldContainExactly
+import io.kotest.matchers.collections.shouldHaveSize
+import io.kotest.matchers.shouldBe
+import net.sourceforge.pmd.lang.ast.test.shouldBe
+import net.sourceforge.pmd.lang.ast.test.shouldBeA
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.AccessType.WRITE
+import net.sourceforge.pmd.lang.java.symbols.JFieldSymbol
+import net.sourceforge.pmd.lang.java.symbols.JFormalParamSymbol
+
+class UsageResolutionTest : ProcessorTestSpec({
+
+ parserTest("Test usage resolution") {
+ val acu = parser.parse("""
+ class Bar {
+ int f1;
+ {
+ this.f1 = 4;
+ }
+ }
+ class Foo extends Bar {
+ int f1; // hides Bar#f1
+ int f2;
+ {
+ int f2 = 0;
+ super.f1 = 0;
+ f1 = 0;
+ this.f1 = 0;
+ }
+ {
+ int f2 = 0;
+ f2 = this.f2;
+ }
+ }
+ """)
+ val (barF1, fooF1, fooF2, localF2, localF22) = acu.descendants(ASTVariableDeclaratorId::class.java).toList()
+ barF1.localUsages.map { it.text.toString() }.shouldContainExactly("this.f1", "super.f1")
+ fooF1.localUsages.map { it.text.toString() }.shouldContainExactly("f1", "this.f1")
+ fooF2.localUsages.map { it.text.toString() }.shouldContainExactly("this.f2")
+ localF2.localUsages.shouldBeEmpty()
+ localF22.localUsages.shouldBeSingleton {
+ it.accessType shouldBe WRITE
+ }
+ }
+
+ parserTest("Test record components") {
+ val acu = parser.parse("""
+ record Foo(int p) {
+ Foo {
+ p = 10;
+ }
+
+ void pPlus1() { return p + 1; }
+ }
+ """)
+
+ val (p) = acu.descendants(ASTVariableDeclaratorId::class.java).toList()
+
+ p::isRecordComponent shouldBe true
+ p.localUsages.shouldHaveSize(2)
+ p.localUsages[0].shouldBeA {
+ it.referencedSym!!.shouldBeA {
+ it.tryGetNode() shouldBe p
+ }
+ }
+ p.localUsages[1].shouldBeA {
+ it.referencedSym!!.shouldBeA {
+ it.tryGetNode() shouldBe p
+ }
+ }
+ }
+
+})
diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/SpecialMethodsTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/SpecialMethodsTest.kt
index 400f8d192a..efecaa6052 100644
--- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/SpecialMethodsTest.kt
+++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/SpecialMethodsTest.kt
@@ -14,6 +14,7 @@ import net.sourceforge.pmd.lang.java.symbols.JFormalParamSymbol
import net.sourceforge.pmd.lang.java.types.captureMatcher
import net.sourceforge.pmd.lang.java.types.parseWithTypeInferenceSpy
import net.sourceforge.pmd.lang.java.types.typeDsl
+import net.sourceforge.pmd.lang.java.types.varId
import java.util.function.Supplier
/**
@@ -201,16 +202,17 @@ class SpecialMethodsTest : ProcessorTestSpec({
""".trimIndent())
val (compLhs, compRhs) = acu.descendants(ASTVariableAccess::class.java).toList()
+ val id = acu.varId("comp")
spy.shouldBeOk {
compLhs.referencedSym.shouldBeA {
- it.tryGetNode() shouldBe null // this could be controversial
+ it.tryGetNode() shouldBe id
it.declaringSymbol.shouldBeA()
}
// same spec
compRhs.referencedSym.shouldBeA {
- it.tryGetNode() shouldBe null
+ it.tryGetNode() shouldBe id
it.declaringSymbol.shouldBeA()
}
}
diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AbstractClassWithoutAbstractMethod.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AbstractClassWithoutAbstractMethod.xml
index 2efff27060..386251c7af 100644
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AbstractClassWithoutAbstractMethod.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AbstractClassWithoutAbstractMethod.xml
@@ -44,7 +44,7 @@ public abstract class Foo {
abstract class implements interface
0
4) {
+ break;
+ }
+ i++;
+ }
+ }
+ }
+ }
+ ]]>
+
+
violation: incremented 'for' loop variable inside nested for declaration
skip
@@ -577,7 +672,8 @@ public class Foo {
]]>
-
+
+
violation: various conditional reassignments of 'for' loop variable, skip allowed
skip
4
diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningParameters.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningParameters.xml
index c639198e7e..37f7407b24 100644
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningParameters.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningParameters.xml
@@ -162,7 +162,12 @@ public class Foo {
The rule should take into account uses of field names, inherited or not, matching the method parameter name.
0
The result set is appropriately tested before using it, no violation.
0
This most common violation case, not testing is done before a call to 'last()'.
1
This most common violation case, not testing is done before a call to 'first()'.
1
Using a 'while' instead of 'if' shouldn't result in a violation.
0
#942 CheckResultSet False Positive
1
#1135 CheckResultSet ignores results set declared outside of try/catch (good case)
0
#1135 CheckResultSet ignores results set declared outside of try/catch
1
#1135 CheckResultSet ignores results set declared outside of try/catch - prevent false positive
0
stringList = new ArrayList();
diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatement.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatement.xml
index 182d5d6d42..782c4ab244 100644
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatement.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/GuardLogStatement.xml
@@ -45,6 +45,7 @@ public class Test {
Guarded call - OK - java util
0
#370 rule not considering lambdas
0
one
1
- System.out.println is used
+ Usage of System.out/err
many
3
-
- System.out.println is used
- System.out.println is used
- System.out.println is used
-
#1217 SystemPrintln always says "System.out.print is used"
4
-
- System.out.print is used
- System.out.println is used
- System.err.print is used
- System.err.println is used
-
#1159 false positive UnusedFormalParameter readObject(ObjectInputStream) if not used
0
- a compound assignment operator doth a usage make
- 0
+ a compound assignment operator does not a usage make
+ 1
[] constructors = String.class.getConstructors();
+ }
+ }
+ ]]>
+
diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateField.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateField.xml
index 5d7bda8a23..b6042ad8d9 100644
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateField.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateField.xml
@@ -466,7 +466,7 @@ public class Foo {
#1420 UnusedPrivateField: Ignore fields if using lombok - 3
0
#1420 UnusedPrivateField: Ignore fields if using lombok - 7
0
1
6
- two private methods, both used, same name, same arg count, diff types
- 0
+ two private methods, only one used, same name, same arg count, diff types
+ 1
+ 7
+
+ Avoid unused private methods such as 'foo(List)'.
+
#1403 False positive UnusedPrivateMethod with JAVA8
0
actual
@@ -1658,4 +1665,101 @@ public class OOO {
}
]]>
+
+ UnusedPrivateMethod false positive #2890
+ 0
+ optionIndexes, MenuEntry[] entries, int index1, int index2) {
+ MenuEntry entry1 = entries[index1],
+ entry2 = entries[index2];
+
+ entries[index1] = entry2;
+ entries[index2] = entry1;
+
+ client.setMenuEntries(entries);
+
+ // Update optionIndexes
+ String option1 = Text.removeTags(entry1.getOption()).toLowerCase(),
+ option2 = Text.removeTags(entry2.getOption()).toLowerCase();
+
+ List list1 = optionIndexes.get(option1),
+ list2 = optionIndexes.get(option2);
+
+ // call remove(Object) instead of remove(int)
+ list1.remove((Integer) index1);
+ list2.remove((Integer) index2);
+
+ sortedInsert(list1, index2);
+ sortedInsert(list2, index1);
+ }
+
+ private static > void sortedInsert(List list, T value) // NOPMD: UnusedPrivateMethod: false positive
+ {
+ int idx = Collections.binarySearch(list, value);
+ list.add(idx < 0 ? -idx - 1 : idx, value);
+ }
+
+}
+ ]]>
+
+
+ [java] UnusedPrivateMethod false positive from inner class via external class #1189
+ 1
+ 4
+
do while false
1
- 3
+ 4
- TEST1
+ May be final
1
- TEST3
+ Unused, no violation
+ 0
+
- TEST4
+ Compound assignment
0
- TEST5
+
+ Blank local var
2
- TEST6
+ Prefix increment
0
- TEST8
+ Postfix increment
0
1
@@ -19,7 +21,10 @@ public class Foo {
2
diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml
index c1e5f21a5f..1afde48b6d 100644
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml
@@ -208,8 +208,8 @@ public class UnnecessaryLocalBeforeReturnFP {
#310 UnnecessaryLocalBeforeReturn statement order does not matter
false
- 1
- 5
+ 2
+ 3,10
No violation: Avoid concat in append method invocations
- 0
+ 3
+ 26,39,45