From eb5c35a92908067f97cde3b8fc33ad8f729c76e7 Mon Sep 17 00:00:00 2001 From: Sven Jacob Date: Mon, 20 Nov 2006 11:32:15 +0000 Subject: [PATCH] fixed some lockups and added label-statements git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@4818 51baf565-9d33-0410-a72c-fc3788e3496d --- .../net/sourceforge/pmd/dfa/DataFlowNode.java | 2 + pmd/src/net/sourceforge/pmd/dfa/Linker.java | 72 ++++++++++++++----- pmd/src/net/sourceforge/pmd/dfa/NodeType.java | 4 ++ .../sourceforge/pmd/dfa/SequenceChecker.java | 7 ++ .../pmd/dfa/StatementAndBraceFinder.java | 12 +++- .../pmd/dfa/pathfinder/CurrentPath.java | 1 + .../pmd/dfa/pathfinder/DAAPathFinder.java | 30 +++++--- 7 files changed, 97 insertions(+), 31 deletions(-) diff --git a/pmd/src/net/sourceforge/pmd/dfa/DataFlowNode.java b/pmd/src/net/sourceforge/pmd/dfa/DataFlowNode.java index 2195e43249..332b18c848 100644 --- a/pmd/src/net/sourceforge/pmd/dfa/DataFlowNode.java +++ b/pmd/src/net/sourceforge/pmd/dfa/DataFlowNode.java @@ -162,6 +162,8 @@ public class DataFlowNode implements IDataFlowNode { typeMap.put(new Integer(NodeType.RETURN_STATEMENT), "RETURN_STATEMENT"); typeMap.put(new Integer(NodeType.BREAK_STATEMENT), "BREAK_STATEMENT"); typeMap.put(new Integer(NodeType.CONTINUE_STATEMENT), "CONTINUE_STATEMENT"); + typeMap.put(new Integer(NodeType.LABEL_STATEMENT), "LABEL_STATEMENT"); + typeMap.put(new Integer(NodeType.LABEL_LAST_STATEMENT), "LABEL_END"); } if (!typeMap.containsKey(new Integer(intype))) { throw new RuntimeException("Couldn't find type id " + intype); diff --git a/pmd/src/net/sourceforge/pmd/dfa/Linker.java b/pmd/src/net/sourceforge/pmd/dfa/Linker.java index a5a2b93290..42642ae4b5 100644 --- a/pmd/src/net/sourceforge/pmd/dfa/Linker.java +++ b/pmd/src/net/sourceforge/pmd/dfa/Linker.java @@ -5,6 +5,9 @@ package net.sourceforge.pmd.dfa; import java.util.List; +import net.sourceforge.pmd.ast.ASTLabeledStatement; +import net.sourceforge.pmd.ast.SimpleNode; + /** * @author raik * Links data flow nodes to each other. @@ -63,7 +66,7 @@ public class Linker { case NodeType.DO_BEFORE_FIRST_STATEMENT: this.computeDo(sc.getFirstIndex(), sc.getLastIndex()); break; - + default: } @@ -85,22 +88,10 @@ public class Linker { continueBreakReturnStack.remove(0); break; case NodeType.BREAK_STATEMENT: - // What about breaks to labels above if statements? - List bList = node.getFlow(); - for (int i = bList.indexOf(node); i < bList.size(); i++) { - IDataFlowNode n = (IDataFlowNode) bList.get(i); - if (n.isType(NodeType.WHILE_LAST_STATEMENT) || - n.isType(NodeType.SWITCH_END) || - n.isType(NodeType.FOR_END) || - n.isType(NodeType.IF_LAST_STATEMENT_WITHOUT_ELSE) || - n.isType(NodeType.DO_EXPR)) { - node.removePathToChild((IDataFlowNode) node.getChildren().get(0)); - IDataFlowNode last = (IDataFlowNode) bList.get(i + 1); - node.addPathToChild(last); - continueBreakReturnStack.remove(0); - break; - } - } + IDataFlowNode last = getNodeToBreakStatement(node); + node.removePathToChild((IDataFlowNode) node.getChildren().get(0)); + node.addPathToChild(last); + continueBreakReturnStack.remove(0); break; case NodeType.CONTINUE_STATEMENT: @@ -170,10 +161,55 @@ public class Linker { } } + private IDataFlowNode getNodeToBreakStatement(IDataFlowNode node) { + // What about breaks to labels above if statements? + List bList = node.getFlow(); + int findEnds = 1; // ignore ends of other for's while's etc. + + + // find out index of the node where the path goes to after the break + int index = bList.indexOf(node); + for (; index < bList.size()-2; index++) { + IDataFlowNode n = (IDataFlowNode) bList.get(index); + if (n.isType(NodeType.DO_EXPR) || + n.isType(NodeType.FOR_INIT) || + n.isType(NodeType.WHILE_EXPR) || + n.isType(NodeType.SWITCH_START)) { + findEnds++; + } + if (n.isType(NodeType.WHILE_LAST_STATEMENT) || + n.isType(NodeType.SWITCH_END) || + n.isType(NodeType.FOR_END) || + n.isType(NodeType.DO_EXPR)) { + if (findEnds > 1) { + // thats not the right node + findEnds--; + } else { + break; + } + } + + if (n.isType(NodeType.LABEL_LAST_STATEMENT)) { + SimpleNode parentNode = (SimpleNode) n.getSimpleNode().getFirstParentOfType(ASTLabeledStatement.class); + if (parentNode == null) { + break; + } else { + String label = node.getSimpleNode().getImage(); + if (label == null || label.equals(parentNode.getImage())) { + node.removePathToChild((IDataFlowNode) node.getChildren().get(0)); + IDataFlowNode last = (IDataFlowNode) bList.get(index + 1); + node.addPathToChild(last); + break; + } + } + } + } + return (IDataFlowNode)node.getFlow().get(index+1); + } + private void computeDo(int first, int last) { IDataFlowNode doSt = ((StackObject) this.braceStack.get(first)).getDataFlowNode(); IDataFlowNode doExpr = ((StackObject) this.braceStack.get(last)).getDataFlowNode(); - //IDataFlowNode doFirst = (IDataFlowNode)doSt.getChildren().get(0); IDataFlowNode doFirst = (IDataFlowNode) doSt.getFlow().get(doSt.getIndex() + 1); if (doFirst.getIndex() != doExpr.getIndex()) { doExpr.addPathToChild(doFirst); diff --git a/pmd/src/net/sourceforge/pmd/dfa/NodeType.java b/pmd/src/net/sourceforge/pmd/dfa/NodeType.java index 8206ff7a5b..28c90fde61 100644 --- a/pmd/src/net/sourceforge/pmd/dfa/NodeType.java +++ b/pmd/src/net/sourceforge/pmd/dfa/NodeType.java @@ -30,6 +30,10 @@ public interface NodeType { int RETURN_STATEMENT = 50; int BREAK_STATEMENT = 51; int CONTINUE_STATEMENT = 52; + + int LABEL_STATEMENT = 60; + int LABEL_LAST_STATEMENT = 61; + // TODO - throw statements? } diff --git a/pmd/src/net/sourceforge/pmd/dfa/SequenceChecker.java b/pmd/src/net/sourceforge/pmd/dfa/SequenceChecker.java index e273e78379..a705f90ef9 100644 --- a/pmd/src/net/sourceforge/pmd/dfa/SequenceChecker.java +++ b/pmd/src/net/sourceforge/pmd/dfa/SequenceChecker.java @@ -88,6 +88,9 @@ public class SequenceChecker { Status doSt = new Status(NodeType.DO_BEFORE_FIRST_STATEMENT); Status doExpr = new Status(NodeType.DO_EXPR, true); + Status labelNode = new Status(NodeType.LABEL_STATEMENT); + Status labelEnd = new Status(NodeType.LABEL_LAST_STATEMENT, true); + root.addStep(ifNode); root.addStep(whileNode); root.addStep(switchNode); @@ -96,6 +99,7 @@ public class SequenceChecker { root.addStep(forUpdate); root.addStep(forSt); root.addStep(doSt); + root.addStep(labelNode); ifNode.addStep(ifSt); ifNode.addStep(ifStWithoutElse); @@ -103,6 +107,9 @@ public class SequenceChecker { ifStWithoutElse.addStep(root); elseSt.addStep(root); + labelNode.addStep(labelEnd); + labelEnd.addStep(root); + whileNode.addStep(whileSt); whileSt.addStep(root); diff --git a/pmd/src/net/sourceforge/pmd/dfa/StatementAndBraceFinder.java b/pmd/src/net/sourceforge/pmd/dfa/StatementAndBraceFinder.java index a0e3a2bc70..8c3f19a566 100644 --- a/pmd/src/net/sourceforge/pmd/dfa/StatementAndBraceFinder.java +++ b/pmd/src/net/sourceforge/pmd/dfa/StatementAndBraceFinder.java @@ -79,7 +79,7 @@ public class StatementAndBraceFinder extends JavaParserVisitorAdapter { } else if (node.jjtGetParent() instanceof ASTDoStatement) { dataFlow.createNewNode(node); // DO EXPR dataFlow.pushOnStack(NodeType.DO_EXPR, dataFlow.getLast()); - } + } return super.visit(node, data); } @@ -95,6 +95,12 @@ public class StatementAndBraceFinder extends JavaParserVisitorAdapter { return data; } + public Object visit(ASTLabeledStatement node, Object data) { + dataFlow.createNewNode(node); + dataFlow.pushOnStack(NodeType.LABEL_STATEMENT, dataFlow.getLast()); + return super.visit(node, data); + } + public Object visit(ASTForUpdate node, Object data) { if (!(data instanceof Structure)) { return data; @@ -121,7 +127,7 @@ public class StatementAndBraceFinder extends JavaParserVisitorAdapter { } else if (node.jjtGetParent() instanceof ASTDoStatement) { dataFlow.pushOnStack(NodeType.DO_BEFORE_FIRST_STATEMENT, dataFlow.getLast()); dataFlow.createNewNode((SimpleNode) node.jjtGetParent()); - } + } super.visit(node, data); @@ -138,6 +144,8 @@ public class StatementAndBraceFinder extends JavaParserVisitorAdapter { dataFlow.pushOnStack(NodeType.WHILE_LAST_STATEMENT, dataFlow.getLast()); } else if (node.jjtGetParent() instanceof ASTForStatement) { dataFlow.pushOnStack(NodeType.FOR_END, dataFlow.getLast()); + } else if (node.jjtGetParent() instanceof ASTLabeledStatement) { + dataFlow.pushOnStack(NodeType.LABEL_LAST_STATEMENT, dataFlow.getLast()); } return data; } diff --git a/pmd/src/net/sourceforge/pmd/dfa/pathfinder/CurrentPath.java b/pmd/src/net/sourceforge/pmd/dfa/pathfinder/CurrentPath.java index aa190008e5..0847928874 100644 --- a/pmd/src/net/sourceforge/pmd/dfa/pathfinder/CurrentPath.java +++ b/pmd/src/net/sourceforge/pmd/dfa/pathfinder/CurrentPath.java @@ -36,6 +36,7 @@ public class CurrentPath { public void addLast(IDataFlowNode n) { list.addLast(n); + //System.out.println("adding: " + n); } public boolean isDoBranchNode() { diff --git a/pmd/src/net/sourceforge/pmd/dfa/pathfinder/DAAPathFinder.java b/pmd/src/net/sourceforge/pmd/dfa/pathfinder/DAAPathFinder.java index ea96eca1ea..a47efbeae2 100644 --- a/pmd/src/net/sourceforge/pmd/dfa/pathfinder/DAAPathFinder.java +++ b/pmd/src/net/sourceforge/pmd/dfa/pathfinder/DAAPathFinder.java @@ -16,7 +16,6 @@ import javax.swing.tree.DefaultMutableTreeNode; */ public class DAAPathFinder { private static final int MAX_PATHS = 5000; - private static final int MAX_PATH_LENGTH = 1000; private IDataFlowNode rootNode; private Executable shim; @@ -49,19 +48,18 @@ public class DAAPathFinder { boolean flag = true; do { i++; +// System.out.println("Building path from " + currentPath.getLast()); phase2(flag); shim.execute(currentPath); flag = false; } while (i < maxPaths && phase3()); - //System.out.println("found: " + i + " path(s)"); } /* * Builds up the path. * */ private void phase2(boolean flag) { - while (!currentPath.isEndNode() - && currentPath.getLength() < MAX_PATH_LENGTH) { // lockup in special cases, see bug 1461873 + while (!currentPath.isEndNode()) { if (currentPath.isBranch() || currentPath.isFirstDoStatement()) { if (flag) { addNodeToTree(); @@ -71,7 +69,8 @@ public class DAAPathFinder { addCurrentChild(); continue; } else { - addSecondChild(); + // jump out of that loop + addLastChild(); continue; } } else { @@ -111,11 +110,17 @@ public class DAAPathFinder { return e.currentChild + 1 < e.node.getChildren().size(); } - private void addSecondChild() { + private void addLastChild() { PathElement e = (PathElement) stack.getLastLeaf().getUserObject(); - currentPath.addLast((IDataFlowNode) e.node.getChildren().get(e.currentChild == 1 ? 0 : 1)); + for (int i=e.node.getChildren().size()-1; i >= 0; i--) { + if (i != e.currentChild) { + currentPath.addLast((IDataFlowNode) e.node.getChildren().get(i)); + break; + } + } } + private void addCurrentChild() { if (currentPath.isBranch()) { // TODO WHY???? PathElement last = (PathElement) stack.getLastLeaf().getUserObject(); @@ -146,8 +151,8 @@ public class DAAPathFinder { while (true) { if (level.getChildCount() != 0) { - PathElement ref; - if ((ref = this.isNodeInLevel(level)) != null) { + PathElement ref = this.isNodeInLevel(level); + if (ref != null) { this.addRefPseudoPathElement(level, ref); break; } else { @@ -167,9 +172,12 @@ public class DAAPathFinder { if (currentPath.isDoBranchNode()) { while (!this.equalsPseudoPathElementWithDoBranchNodeInLevel(level)) { level = this.getLastChildNode(level); + if (level.getChildCount() == 0) { + break; + } } - PathElement ref; - if ((ref = this.getDoBranchNodeInLevel(level)) != null) { + PathElement ref = this.getDoBranchNodeInLevel(level); + if (ref != null) { addNode(level, ref); } else { this.addNewPathElement(level);