diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/UnusedAssignmentRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/UnusedAssignmentRule.java index 7f1f021d60..24782384da 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/UnusedAssignmentRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/UnusedAssignmentRule.java @@ -5,9 +5,7 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; -import java.util.ArrayDeque; import java.util.Collections; -import java.util.Deque; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -16,7 +14,11 @@ import java.util.Set; import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator; import net.sourceforge.pmd.lang.java.ast.ASTBreakStatement; +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.ASTIfStatement; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTName; @@ -24,6 +26,7 @@ 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.ASTReturnStatement; +import net.sourceforge.pmd.lang.java.ast.ASTStatement; import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression; import net.sourceforge.pmd.lang.java.ast.ASTThrowStatement; import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator; @@ -79,13 +82,8 @@ public class UnusedAssignmentRule extends AbstractJavaRule { static class LivenessVisitor extends JavaParserVisitorAdapter { - - private final Deque breakAddresses = new ArrayDeque<>(); - private final Map namedBreaks = new HashMap<>(); - // following deals with control flow - @Override public Object visit(JavaNode node, Object data) { @@ -99,29 +97,73 @@ public class UnusedAssignmentRule extends AbstractJavaRule { @Override public Object visit(ASTIfStatement node, Object data) { - ScopeData before = (ScopeData) node.getCondition().jjtAccept(this, data); + ScopeData before = acceptOpt(node.getCondition(), (ScopeData) data); - ScopeData thenData = before.fork(); - thenData = (ScopeData) node.getThenBranch().jjtAccept(this, thenData); - if (node.hasElse()) { - ScopeData elseData = (ScopeData) node.getElseBranch().jjtAccept(this, before.fork()); - return thenData.join(elseData); - } else { - return before.join(thenData); - } + ScopeData thenData = acceptOpt(node.getThenBranch(), before.fork()); + ScopeData elseData = acceptOpt(node.getElseBranch(), before.fork()); + + return thenData.join(elseData); } @Override public Object visit(ASTWhileStatement node, Object data) { - ScopeData before = (ScopeData) node.getCondition().jjtAccept(this, data); + // perform a few "iterations", to make sure that assignments in + // the body can affect themselves in the next iteration, and + // that they affect the condition + ScopeData before = acceptOpt(node.getCondition(), (ScopeData) data); - ScopeData iter = (ScopeData) node.getBody().jjtAccept(this, before.fork()); - iter = (ScopeData) node.getCondition().jjtAccept(this, iter); - iter = (ScopeData) node.getBody().jjtAccept(this, iter); + ScopeData iter = acceptOpt(node.getBody(), before.fork()); + iter = acceptOpt(node.getCondition(), iter); + iter = acceptOpt(node.getBody(), iter); return before.join(iter); } + @Override + public Object visit(ASTDoStatement node, Object data) { + // same as while but don't check the condition first + ScopeData before = (ScopeData) data; + + ScopeData iter = acceptOpt(node.getBody(), before.fork()); + iter = acceptOpt(node.getCondition(), iter); + iter = acceptOpt(node.getBody(), iter); + + return before.join(iter); + } + + @Override + public Object visit(ASTForStatement node, Object data) { + ASTStatement body = node.getBody(); + if (node.isForeach()) { + // the iterable expression + ScopeData before = (ScopeData) node.getChild(1).jjtAccept(this, data); + + ScopeData iter = acceptOpt(body, before.fork()); + iter = acceptOpt(body, iter); // the body must be able to affect itself + + return before.join(iter); + } else { + ASTForInit init = node.getFirstChildOfType(ASTForInit.class); + ASTExpression cond = node.getCondition(); + ASTForUpdate update = node.getFirstChildOfType(ASTForUpdate.class); + + ScopeData before = (ScopeData) data; + before = acceptOpt(init, before); + before = acceptOpt(cond, before); + + ScopeData iter = acceptOpt(body, before.fork()); + iter = acceptOpt(update, iter); + iter = acceptOpt(cond, iter); + iter = acceptOpt(body, iter); // the body must be able to affect itself + + return before.join(iter); + } + } + + private ScopeData acceptOpt(JavaNode node, ScopeData before) { + return node == null ? before : (ScopeData) node.jjtAccept(this, before); + } + @Override public Object visit(ASTThrowStatement node, Object data) { data = super.visit(node, data); @@ -216,14 +258,14 @@ public class UnusedAssignmentRule extends AbstractJavaRule { if (suffix.isArguments() || suffix.isArrayDereference()) { return null; } - return findVar(primary.getScope(), true, suffix.getImage()); + return findVar(primary.getScope(), true, substringBeforeFirst(suffix.getImage(), '.')); } else { if (inLhs && primary.getNumChildren() > 1) { return null; } if (prefix.getChild(0) instanceof ASTName) { - return findVar(prefix.getScope(), false, prefix.getChild(0).getImage()); + return findVar(prefix.getScope(), false, substringBeforeFirst(prefix.getChild(0).getImage(), '.')); } } } @@ -231,6 +273,11 @@ public class UnusedAssignmentRule extends AbstractJavaRule { return null; } + private static String substringBeforeFirst(String str, char delim) { + int i = str.indexOf(delim); + return i < 0 ? str : str.substring(0, i); + } + private VariableNameDeclaration findVar(Scope scope, boolean isThis, String name) { if (name == null) { return null; diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UnusedAssignment.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UnusedAssignment.xml index 1bc4f5eec3..5c598f01d7 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UnusedAssignment.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UnusedAssignment.xml @@ -179,8 +179,12 @@ public class Foo { #1393 PMD hanging during DataflowAnomalyAnalysis - - 3 + 2 + 10,19 + + The value assigned to variable 'fail' is never used + The value assigned to variable 'fail' is never used + - #1905 [java] DataflowAnomalyAnalysis Rule in right order : Case 2. DU-Anomaly(a) - 1 - 5 + For loop + 0 + + + + + For loop 2 + 2 + 3,5 The value assigned to variable 'a' is never used + The value assigned to variable 'a' is never used + + + + For loop 3 + 0 + + + + + For loop 4 + 0 + - #1905 [java] DataflowAnomalyAnalysis Rule in right order : Case 5. DU-Anomaly(a) - 1 - 6 - - Found 'DU'-anomaly for variable 'a' (lines '6'-'9'). - + Do while 0 + 0 + + Do while 1 + 1 + 3 + + The value assigned to variable 'a' is never used + + + + + + Do while with break + 2 + 7,8 + + The value assigned to variable 'i' is never used + The value assigned to variable 'a' is never used + + = 20) { + i = 4; + a *= 5; + break; + } + + a = i + 3; + i += 3; + } while (i < 30); + } +} + ]]> + + #1905 [java] DataflowAnomalyAnalysis Rule in right order : Case 6. DU-Anomaly(a) 4 @@ -482,11 +578,11 @@ class Test{ - #1905 [java] DataflowAnomalyAnalysis Rule in right order : Case 9. DU-Anomaly(t1) + Usage as LHS of method 1 5 - Found 'DU'-anomaly for variable 't1' (lines '5'-'6'). + The value assigned to variable 't1' is never used - #1905 [java] DataflowAnomalyAnalysis Rule in right order : Case 12. DU-Anomaly(t1) + Assignment in operand 1 6 - Found 'DU'-anomaly for variable 't1' (lines '6'-'7'). + The value assigned to variable 't1' is never used - #1905 [java] DataflowAnomalyAnalysis Rule in right order : Case 13 + Assignment in operand 2 + 1 + 7 + + The value assigned to variable 't1' is never used + + + + + + Assignment in operand 3 0 - #1905 [java] DataflowAnomalyAnalysis Rule in right order : Case 14. DU-Anomaly(t1, t2) + Assignment in operand 4 2 4,6 - Found 'DU'-anomaly for variable 't2' (lines '4'-'7'). - Found 'DU'-anomaly for variable 't1' (lines '6'-'7'). + The value assigned to variable 't2' is never used + The value assigned to variable 't1' is never used 1 4 - Found 'DU'-anomaly for variable 'a' (lines '4'-'5'). + The value assigned to variable 'a' is never used + + + Compound assignment + 1 + 4 + + The value assigned to variable 'a' is never used + + + + + Another case + 1 + 3,5 + + The value assigned to variable 'iter' is never used + The value assigned to variable 'iter' is never used + +