hiifong/pmd
1
0
Fork 0
forked from phoedos/pmd
Code Pull Requests Activity

- fixed dfa lockup

Browse Source 
- fixed variable access for definitions
- added DaaRule for controversial ruleset (DataflowAnomalyAnalysis)


git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@4797 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
Sven Jacob
2006-11-16 16:38:53 +00:00
parent e62434479b
commit bfca01c305
6 changed files with 182 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
pmd/rulesets/controversial.xml
View File

@ -381,8 +381,37 @@ public class Foo {
</example>
</rule>
<rule name="DataflowAnomalyAnalysis"
message="Found ''{0}''-anomaly for variable ''{1}'' (lines ''{2}''-''{3}'')."
class="net.sourceforge.pmd.dfa.DaaRule"
dfa="true">
<description>The dataflow analysis tracks local definitions, undefinitions and references to variables on different paths on the data flow.
From those informations there can be found various problems.
1. UR - Anomaly: There is a reference to a variable that was not defined before. This is a bug and leads to an error.
2. DU - Anomaly: A recently defined variable is undefined. These anomalies may appear in normal source text.
3. DD - Anomaly: A recently defined variable is redefined. This is ominous but don't have to be a bug.
</description>
<priority>5</priority>
<properties>
<property name="maxviolations" value="100"/>
<property name="maxpaths" value="1000"/>
</properties>
<example>
<![CDATA[
public class Foo {
public void foo() {
int buz = 5;
buz = 6; // redefinition of buz -> dd-anomaly
foo(buz);
buz = 2;
} // buz is undefined when leaving scope -> du-anomaly
}
]]>
</example>
</rule>
</ruleset>

100
pmd/src/net/sourceforge/pmd/dfa/DaaRule.java
View File

@ -5,6 +5,7 @@ package net.sourceforge.pmd.dfa;
import net.sourceforge.pmd.AbstractRule;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.ast.SimpleNode;
import net.sourceforge.pmd.dfa.pathfinder.CurrentPath;
@ -12,6 +13,7 @@ import net.sourceforge.pmd.dfa.pathfinder.DAAPathFinder;
import net.sourceforge.pmd.dfa.pathfinder.Executable;
import net.sourceforge.pmd.dfa.variableaccess.VariableAccess;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Hashtable;
import java.util.Iterator;
@ -23,19 +25,38 @@ import java.util.List;
* Starts path search for each method and runs code if found.
*/
public class DaaRule extends AbstractRule implements Executable {
private RuleContext rc;
private int counter;
private static final int MAX_PATHS = 5000;
private List daaRuleViolations;
private final static String PROPERTY_MAX_PATH = "maxpaths";
private final static String PROPERTY_MAX_VIOLATIONS = "maxviolations";
private final static int DEFAULT_MAX_VIOLATIONS = 1000;
private int maxRuleViolations;
private int currentRuleViolationCount;
public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
if (this.hasProperty(PROPERTY_MAX_VIOLATIONS)) {
this.maxRuleViolations = this.getIntProperty(PROPERTY_MAX_VIOLATIONS);
} else {
this.maxRuleViolations = DEFAULT_MAX_VIOLATIONS;
}
this.currentRuleViolationCount = 0;
return super.visit(node, data);
}
public Object visit(ASTMethodDeclaration node, Object data) {
this.rc = (RuleContext) data;
counter = 0;
this.daaRuleViolations = new ArrayList();
IDataFlowNode n = (IDataFlowNode) node.getDataFlowNode().getFlow().get(0);
System.out.println("In DaaRule, IDataFlowNode n = " + n);
DAAPathFinder a = new DAAPathFinder(n, this);
DAAPathFinder a;
if (this.hasProperty(PROPERTY_MAX_PATH)) {
a = new DAAPathFinder(n, this, this.getIntProperty(PROPERTY_MAX_PATH));
} else {
a = new DAAPathFinder(n, this);
}
a.run();
super.visit(node, data);
@ -43,12 +64,13 @@ public class DaaRule extends AbstractRule implements Executable {
}
public void execute(CurrentPath path) {
Hashtable hash = new Hashtable();
counter++;
if (counter == 5000) {
System.out.print("|");
counter = 0;
if (maxNumberOfViolationsReached()) {
// dont execute this path if the limit is already reached
return;
}
Hashtable hash = new Hashtable();
for (Iterator d = path.iterator(); d.hasNext();) {
IDataFlowNode inode = (IDataFlowNode) d.next();
if (inode.getVariableAccess() != null) {
@ -81,7 +103,7 @@ public class DaaRule extends AbstractRule implements Executable {
// undefinition outside, get the node of the definition
SimpleNode lastSimpleNode = (SimpleNode)array.get(1);
if (lastSimpleNode != null) {
addDaaViolation(rc, lastSimpleNode, "DU", va.getVariableName(), startLine, endLine);
addDaaViolation(rc, lastSimpleNode, "DU", va.getVariableName(), startLine, endLine);
}
}
}
@ -103,8 +125,50 @@ public class DaaRule extends AbstractRule implements Executable {
* @param node the node that produces the violation
* @param msg specific message to put in the report
*/
private final void addDaaViolation(Object data, SimpleNode node, String msg, String var, int startLine, int endLine) {
RuleContext ctx = (RuleContext) data;
ctx.getReport().addRuleViolation(new DaaRuleViolation(this, ctx, node, msg, var, startLine, endLine));
private final void addDaaViolation(Object data, SimpleNode node, String type, String var, int startLine, int endLine) {
if (!maxNumberOfViolationsReached()
&& !violationAlreadyExists(type, var, startLine, endLine)) {
RuleContext ctx = (RuleContext) data;
Object[] params = new Object[] { type, var, new Integer(startLine), new Integer(endLine) };
String msg = type;
if (getMessage() != null) {
msg = MessageFormat.format(getMessage(), params);
}
DaaRuleViolation violation = new DaaRuleViolation(this, ctx, node, type, msg, var, startLine, endLine);
ctx.getReport().addRuleViolation(violation);
this.daaRuleViolations.add(violation);
this.currentRuleViolationCount++;
}
}
/**
* Maximum number of violations was already reached?
* @return
*/
private boolean maxNumberOfViolationsReached() {
return this.currentRuleViolationCount >= this.maxRuleViolations;
}
/**
* Checks if a violation already exists.
* This is needed because on the different paths same anomalies can occur.
* @param type
* @param var
* @param startLine
* @param endLine
* @return true if the violation already was added to the report
*/
private boolean violationAlreadyExists(String type, String var, int startLine, int endLine) {
Iterator violationIterator = this.daaRuleViolations.iterator();
while (violationIterator.hasNext()) {
DaaRuleViolation violation = (DaaRuleViolation)violationIterator.next();
if ((violation.getBeginLine() == startLine)
&& (violation.getEndLine() == endLine)
&& violation.getType().equals(type)
&& violation.getVariableName().equals(var)) {
return true;
}
}
return false;
}
}

12
pmd/src/net/sourceforge/pmd/dfa/DaaRuleViolation.java
View File

@ -16,12 +16,14 @@ public class DaaRuleViolation extends RuleViolation {
private String variableName;
private int beginLine;
private int endLine;
private String type;
public DaaRuleViolation(Rule rule, RuleContext ctx, SimpleNode node, String specificMsg, String variableName, int beginLine, int endLine) {
super(rule, ctx, node, specificMsg);
this.variableName = variableName;
public DaaRuleViolation(Rule rule, RuleContext ctx, SimpleNode node, String type, String msg, String var, int beginLine, int endLine) {
super(rule, ctx, node, msg);
this.variableName = var;
this.beginLine = beginLine;
this.endLine = endLine;
this.type = type;
}
public String getVariableName() {
@ -35,4 +37,8 @@ public class DaaRuleViolation extends RuleViolation {
public int getEndLine() {
return endLine;
}
public String getType() {
return type;
}
}

4
pmd/src/net/sourceforge/pmd/dfa/pathfinder/CurrentPath.java
View File

@ -14,6 +14,10 @@ public class CurrentPath {
list = new LinkedList();
}
public int getLength() {
return list.size();
}
public Iterator iterator() {
return list.iterator();
}

14
pmd/src/net/sourceforge/pmd/dfa/pathfinder/DAAPathFinder.java
View File

@ -15,17 +15,25 @@ import javax.swing.tree.DefaultMutableTreeNode;
* 2 paths. This is special to the data flow anomaly analysis.
*/
public class DAAPathFinder {
private static final int MAX_PATHS = 5000;
private static final int MAX_PATH_LENGTH = 5000;
private IDataFlowNode rootNode;
private Executable shim;
private CurrentPath currentPath = new CurrentPath();
private DefaultMutableTreeNode stack = new DefaultMutableTreeNode();
private int maxPaths;
public DAAPathFinder(IDataFlowNode rootNode, Executable shim) {
this.rootNode = rootNode;
this.shim = shim;
this.maxPaths = MAX_PATHS;
}
public DAAPathFinder(IDataFlowNode rootNode, Executable shim, int maxPaths) {
this.rootNode = rootNode;
this.shim = shim;
this.maxPaths = maxPaths;
}
public void run() {
@ -44,7 +52,7 @@ public class DAAPathFinder {
phase2(flag);
shim.execute(currentPath);
flag = false;
} while (i < MAX_PATHS && phase3());
} while (i < maxPaths && phase3());
//System.out.println("found: " + i + " path(s)");
}
@ -52,7 +60,7 @@ public class DAAPathFinder {
* Builds up the path.
* */
private void phase2(boolean flag) {
while (!currentPath.isEndNode()) {
while (!currentPath.isEndNode() && currentPath.getLength() < MAX_PATH_LENGTH) {
if (currentPath.isBranch() || currentPath.isFirstDoStatement()) {
if (flag) {
addNodeToTree();

58
pmd/src/net/sourceforge/pmd/dfa/variableaccess/VariableAccessVisitor.java
View File

@ -5,7 +5,10 @@ package net.sourceforge.pmd.dfa.variableaccess;
import net.sourceforge.pmd.ast.ASTClassOrInterfaceBodyDeclaration;
import net.sourceforge.pmd.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.ast.ASTFormalParameter;
import net.sourceforge.pmd.ast.ASTFormalParameters;
import net.sourceforge.pmd.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.ast.ASTVariableInitializer;
import net.sourceforge.pmd.ast.JavaParserVisitorAdapter;
import net.sourceforge.pmd.ast.SimpleNode;
import net.sourceforge.pmd.dfa.IDataFlowNode;
@ -21,7 +24,7 @@ import java.util.Map;
import java.util.Set;
/**
* @author raik
* @author raik, Sven Jacob
* <p/>
* Searches for special nodes and computes based on the sequence, the type of
* access of a variable.
@ -43,10 +46,11 @@ public class VariableAccessVisitor extends JavaParserVisitorAdapter {
List undefinitions = markUsages(inode);
// is this necessary? Why does the first node need undefs?
// all variables are first in state undefinition
IDataFlowNode firstINode = (IDataFlowNode) inode.getFlow().get(0);
firstINode.setVariableAccess(undefinitions);
// all variables are getting undefined when leaving scope
IDataFlowNode lastINode = (IDataFlowNode) inode.getFlow().get(inode.getFlow().size() - 1);
lastINode.setVariableAccess(undefinitions);
}
@ -58,10 +62,24 @@ public class VariableAccessVisitor extends JavaParserVisitorAdapter {
for (Iterator i = variableDeclarations.iterator(); i.hasNext();) {
Map declarations = (Map) i.next();
for (Iterator j = declarations.entrySet().iterator(); j.hasNext();) {
Map.Entry entry = (Map.Entry) j.next();
Map.Entry entry = (Map.Entry) j.next();
VariableNameDeclaration vnd = (VariableNameDeclaration) entry.getKey();
addVariableAccess(vnd.getNode().getBeginLine(), new VariableAccess(VariableAccess.DEFINITION, vnd.getImage()), inode.getFlow());
if (vnd.getAccessNodeParent() instanceof ASTFormalParameter) {
// add definition for parameters
addVariableAccess(
(SimpleNode)vnd.getNode().getFirstParentOfType(ASTFormalParameters.class),
new VariableAccess(VariableAccess.DEFINITION, vnd.getImage()),
inode.getFlow());
} else if (vnd.getAccessNodeParent().getFirstChildOfType(ASTVariableInitializer.class) != null) {
// add definition for initialized variables
addVariableAccess(
vnd.getNode(),
new VariableAccess(VariableAccess.DEFINITION, vnd.getImage()),
inode.getFlow());
}
undefinitions.add(new VariableAccess(VariableAccess.UNDEFINITION, vnd.getImage()));
for (Iterator k = ((List) entry.getValue()).iterator(); k.hasNext();) {
addAccess(k, inode);
}
@ -89,19 +107,35 @@ public class VariableAccessVisitor extends JavaParserVisitorAdapter {
private void addAccess(Iterator k, IDataFlowNode inode) {
NameOccurrence occurrence = (NameOccurrence) k.next();
if (occurrence.isOnLeftHandSide()) {
this.addVariableAccess(occurrence.getLocation().getBeginLine(), new VariableAccess(VariableAccess.DEFINITION, occurrence.getImage()), inode.getFlow());
this.addVariableAccess(occurrence.getLocation(), new VariableAccess(VariableAccess.DEFINITION, occurrence.getImage()), inode.getFlow());
} else if (occurrence.isOnRightHandSide() || (!occurrence.isOnLeftHandSide() && !occurrence.isOnRightHandSide())) {
this.addVariableAccess(occurrence.getLocation().getBeginLine(), new VariableAccess(VariableAccess.REFERENCING, occurrence.getImage()), inode.getFlow());
this.addVariableAccess(occurrence.getLocation(), new VariableAccess(VariableAccess.REFERENCING, occurrence.getImage()), inode.getFlow());
}
}
private void addVariableAccess(int line, VariableAccess va, List flow) {
for (int i = 1; i < flow.size(); i++) {
/**
* Adds a VariableAccess to a dataflow node.
* @param node location of the access of a variable
* @param va variable access to add
* @param flow dataflownodes that can contain the node.
*/
private void addVariableAccess(SimpleNode node, VariableAccess va, List flow) {
// backwards to find the right inode (not a method declaration)
for (int i = flow.size()-1; i > 0; i--) {
IDataFlowNode inode = (IDataFlowNode) flow.get(i);
if (line == inode.getLine()) {
List v = new ArrayList();
v.add(va);
inode.setVariableAccess(v);
if (inode.getSimpleNode() == null) {
continue;
}
List children = inode.getSimpleNode().findChildrenOfType(node.getClass());
Iterator childrenIterator = children.iterator();
while (childrenIterator.hasNext()) {
if (node.equals(childrenIterator.next())) {
List v = new ArrayList();
v.add(va);
inode.setVariableAccess(v);
return;
}
}
}
}