diff --git a/pmd/rulesets/optimizations.xml b/pmd/rulesets/optimizations.xml index 8ce78d2dc1..c1ffa9225b 100644 --- a/pmd/rulesets/optimizations.xml +++ b/pmd/rulesets/optimizations.xml @@ -10,7 +10,7 @@ These rules deal with different optimizations that generally apply to performanc diff --git a/pmd/src/net/sourceforge/pmd/rules/optimization/AbstractOptimizationRule.java b/pmd/src/net/sourceforge/pmd/rules/optimization/AbstractOptimizationRule.java index 6671c3e709..76901fb0e6 100644 --- a/pmd/src/net/sourceforge/pmd/rules/optimization/AbstractOptimizationRule.java +++ b/pmd/src/net/sourceforge/pmd/rules/optimization/AbstractOptimizationRule.java @@ -10,19 +10,8 @@ import java.util.List; import net.sourceforge.pmd.AbstractRule; import net.sourceforge.pmd.Rule; -import net.sourceforge.pmd.ast.ASTAssignmentOperator; -import net.sourceforge.pmd.ast.ASTExpression; -import net.sourceforge.pmd.ast.ASTLocalVariableDeclaration; -import net.sourceforge.pmd.ast.ASTMethodDeclaration; -import net.sourceforge.pmd.ast.ASTName; -import net.sourceforge.pmd.ast.ASTPostfixExpression; -import net.sourceforge.pmd.ast.ASTPreDecrementExpression; -import net.sourceforge.pmd.ast.ASTPreIncrementExpression; -import net.sourceforge.pmd.ast.ASTPrimaryExpression; -import net.sourceforge.pmd.ast.ASTPrimaryPrefix; -import net.sourceforge.pmd.ast.ASTVariableDeclaratorId; -import net.sourceforge.pmd.ast.Node; -import net.sourceforge.pmd.ast.SimpleNode; +import net.sourceforge.pmd.ast.ASTClassOrInterfaceDeclaration; +import net.sourceforge.pmd.symboltable.NameOccurrence; /** * Base class with utility methods for optimization rules @@ -31,109 +20,21 @@ import net.sourceforge.pmd.ast.SimpleNode; */ public class AbstractOptimizationRule extends AbstractRule implements Rule { - protected final boolean isVarWritterInMethod(String varName, ASTMethodDeclaration md) { - List assignments = md.findChildrenOfType(ASTAssignmentOperator.class); - return (variableAssigned(varName, assignments) || numericWithPrePost(md, varName)); + public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { + if (node.isInterface()) { + return data; + } + return super.visit(node, data); } - // TODO - symbol table? - protected final String getVarName(ASTLocalVariableDeclaration node) { - List l = node.findChildrenOfType(ASTVariableDeclaratorId.class); - if (!l.isEmpty()) { - ASTVariableDeclaratorId vd = (ASTVariableDeclaratorId) l.get(0); - return vd.getImage(); - } - return null; - } - - /** - * Check constructions like - * int i; - * ++i; - * --i; - * i++; - * i*=1; - * i+=1; - */ - private final boolean numericWithPrePost(ASTMethodDeclaration md, String varName) { - // ++i - List preinc = md.findChildrenOfType(ASTPreIncrementExpression.class); - if (preinc != null && !preinc.isEmpty()) { - for (Iterator it = preinc.iterator(); it.hasNext();) { - if (isName((Node)it.next(), varName)) { - return true; - } - } - } - - // --i - List predec = md.findChildrenOfType(ASTPreDecrementExpression.class); - if (predec != null && !predec.isEmpty()) { - for (Iterator it = predec.iterator(); it.hasNext();) { - if (isName((Node)it.next(), varName)) { - return true; - } - } - } - - List pf = md.findChildrenOfType(ASTPostfixExpression.class); - if (pf != null && !pf.isEmpty()) { - for (Iterator it = pf.iterator(); it.hasNext();) { - ASTPostfixExpression pe = (ASTPostfixExpression) it.next(); - if ((pe.hasImageEqualTo("++") || pe.hasImageEqualTo("--"))) { - if (isName(pe, varName)) { - return true; - } - } - } - } - return false; - } - - private final boolean isName(Node node, String varName) { - Node first = node.jjtGetChild(0); - Node second = first.jjtGetChild(0); - if (second.jjtGetNumChildren() == 0) { - return false; - } - - Node third = second.jjtGetChild(0); - if (!(third instanceof ASTName)) { - if (first instanceof ASTPrimaryExpression) { - // deals with extra parenthesis: - // "(varName)++" instead of "varName++" for instance - if (first.jjtGetNumChildren() != 1) { - return false; - } - if (second instanceof ASTPrimaryPrefix && second.jjtGetNumChildren() == 1 && - third instanceof ASTExpression && third.jjtGetNumChildren() == 1) { - return isName(third, varName); - } - } - return false; - } - ASTName name = (ASTName) third; - return name.hasImageEqualTo(varName); - } - - private final boolean variableAssigned(final String varName, final List assignments) { - if (assignments == null || assignments.isEmpty()) { - return false; - } - for (Iterator it = assignments.iterator(); it.hasNext();) { - final ASTAssignmentOperator a = (ASTAssignmentOperator) it.next(); - // if node is assigned return true - SimpleNode firstChild = (SimpleNode) a.jjtGetParent().jjtGetChild(0); - SimpleNode otherChild = (SimpleNode) firstChild.jjtGetChild(0); - if (otherChild.jjtGetNumChildren() == 0 || !(otherChild.jjtGetChild(0) instanceof ASTName)) { - continue; - } - ASTName n = (ASTName) otherChild.jjtGetChild(0); - if (n.hasImageEqualTo(varName)) { + protected boolean assigned(List usages) { + for (Iterator j = usages.iterator(); j.hasNext();) { + NameOccurrence occ = (NameOccurrence) j.next(); + if (occ.isOnLeftHandSide() || occ.isSelfAssignment()) { return true; } + continue; } - return false; } diff --git a/pmd/src/net/sourceforge/pmd/rules/optimization/AvoidInstantiatingObjectsInLoops.java b/pmd/src/net/sourceforge/pmd/rules/optimization/AvoidInstantiatingObjectsInLoops.java index 3d49731b12..c07487e4c9 100644 --- a/pmd/src/net/sourceforge/pmd/rules/optimization/AvoidInstantiatingObjectsInLoops.java +++ b/pmd/src/net/sourceforge/pmd/rules/optimization/AvoidInstantiatingObjectsInLoops.java @@ -4,7 +4,6 @@ package net.sourceforge.pmd.rules.optimization; import net.sourceforge.pmd.ast.ASTAllocationExpression; -import net.sourceforge.pmd.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.ast.ASTDoStatement; import net.sourceforge.pmd.ast.ASTForStatement; import net.sourceforge.pmd.ast.ASTReturnStatement; @@ -13,14 +12,6 @@ import net.sourceforge.pmd.ast.ASTWhileStatement; public class AvoidInstantiatingObjectsInLoops extends AbstractOptimizationRule { - public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { - if (node.isInterface()) { - return data; - } - return super.visit(node, data); - } - - public Object visit(ASTAllocationExpression node, Object data) { if (insideLoop(node) && fourthParentNotThrow(node) && fourthParentNotReturn(node)) { addViolation(data, node); diff --git a/pmd/src/net/sourceforge/pmd/rules/optimization/LocalVariableCouldBeFinal.java b/pmd/src/net/sourceforge/pmd/rules/optimization/LocalVariableCouldBeFinal.java index 8a38fdd8a3..0ead95c201 100644 --- a/pmd/src/net/sourceforge/pmd/rules/optimization/LocalVariableCouldBeFinal.java +++ b/pmd/src/net/sourceforge/pmd/rules/optimization/LocalVariableCouldBeFinal.java @@ -3,29 +3,30 @@ */ package net.sourceforge.pmd.rules.optimization; -import net.sourceforge.pmd.ast.ASTClassOrInterfaceDeclaration; +import java.util.Iterator; +import java.util.List; +import java.util.Map; + import net.sourceforge.pmd.ast.ASTLocalVariableDeclaration; -import net.sourceforge.pmd.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.symboltable.Scope; +import net.sourceforge.pmd.symboltable.VariableNameDeclaration; public class LocalVariableCouldBeFinal extends AbstractOptimizationRule { - public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { - if (node.isInterface()) { - return data; - } - return super.visit(node, data); - } - public Object visit(ASTLocalVariableDeclaration node, Object data) { if (node.isFinal()) { return data; } - final String varName = getVarName(node); - - ASTMethodDeclaration md = (ASTMethodDeclaration) node.getFirstParentOfType(ASTMethodDeclaration.class); - if (md != null) { - if (!isVarWritterInMethod(varName, md)) { - addViolation(data, node); + Scope s = node.getScope(); + Map decls = s.getVariableDeclarations(); + for (Iterator i = decls.entrySet().iterator(); i.hasNext();) { + Map.Entry entry = (Map.Entry) i.next(); + VariableNameDeclaration var = (VariableNameDeclaration) entry.getKey(); + if (var.getAccessNodeParent() != node) { + continue; + } + if (!assigned((List) entry.getValue())) { + addViolation(data, var.getAccessNodeParent(), var.getImage()); } } return data; diff --git a/pmd/src/net/sourceforge/pmd/rules/optimization/MethodArgumentCouldBeFinal.java b/pmd/src/net/sourceforge/pmd/rules/optimization/MethodArgumentCouldBeFinal.java index 181811bd98..1ff6795913 100644 --- a/pmd/src/net/sourceforge/pmd/rules/optimization/MethodArgumentCouldBeFinal.java +++ b/pmd/src/net/sourceforge/pmd/rules/optimization/MethodArgumentCouldBeFinal.java @@ -3,25 +3,17 @@ */ package net.sourceforge.pmd.rules.optimization; -import net.sourceforge.pmd.ast.ASTClassOrInterfaceDeclaration; -import net.sourceforge.pmd.ast.ASTFormalParameter; -import net.sourceforge.pmd.ast.ASTMethodDeclaration; -import net.sourceforge.pmd.symboltable.NameOccurrence; -import net.sourceforge.pmd.symboltable.Scope; -import net.sourceforge.pmd.symboltable.VariableNameDeclaration; - import java.util.Iterator; import java.util.List; import java.util.Map; -public class MethodArgumentCouldBeFinal extends AbstractOptimizationRule { +import net.sourceforge.pmd.ast.ASTFormalParameter; +import net.sourceforge.pmd.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.ast.AccessNode; +import net.sourceforge.pmd.symboltable.Scope; +import net.sourceforge.pmd.symboltable.VariableNameDeclaration; - public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { - if (node.isInterface()) { - return data; - } - return super.visit(node, data); - } +public class MethodArgumentCouldBeFinal extends AbstractOptimizationRule { public Object visit(ASTMethodDeclaration meth, Object data) { if (meth.isNative() || meth.isAbstract()) { @@ -32,21 +24,12 @@ public class MethodArgumentCouldBeFinal extends AbstractOptimizationRule { for (Iterator i = decls.entrySet().iterator(); i.hasNext();) { Map.Entry entry = (Map.Entry) i.next(); VariableNameDeclaration var = (VariableNameDeclaration) entry.getKey(); - if (!var.getAccessNodeParent().isFinal() && (var.getAccessNodeParent() instanceof ASTFormalParameter) && !assigned((List) entry.getValue())) { - addViolation(data, var.getAccessNodeParent(), var.getImage()); + AccessNode node = var.getAccessNodeParent(); + if (!node.isFinal() && (node instanceof ASTFormalParameter) && !assigned((List) entry.getValue())) { + addViolation(data, node, var.getImage()); } } return data; } - private boolean assigned(List usages) { - for (Iterator j = usages.iterator(); j.hasNext();) { - NameOccurrence occ = (NameOccurrence) j.next(); - if (occ.isOnLeftHandSide() || occ.isSelfAssignment()) { - return true; - } - continue; - } - return false; - } }