From 4c0bd26091caff07d372c072248a918f6b194a11 Mon Sep 17 00:00:00 2001 From: oowekyala Date: Mon, 24 Jul 2017 16:13:56 +0200 Subject: [PATCH 1/9] Implemented hashCode and equals in metric keys factory methods --- .../lang/java/metrics/api/JavaClassMetricKey.java | 13 ++++++++++++- .../java/metrics/api/JavaOperationMetricKey.java | 13 +++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaClassMetricKey.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaClassMetricKey.java index 2254228eca..233071791b 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaClassMetricKey.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaClassMetricKey.java @@ -101,7 +101,18 @@ public enum JavaClassMetricKey implements MetricKey { public boolean supports(ASTAnyTypeDeclaration node) { return metric.supports(node); } + + + @Override + public boolean equals(Object obj) { + return obj == this; + } + + + @Override + public int hashCode() { + return metric.hashCode() * 31 + name.hashCode(); + } }; } - } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaOperationMetricKey.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaOperationMetricKey.java index 45679d760e..04111cc994 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaOperationMetricKey.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaOperationMetricKey.java @@ -93,6 +93,19 @@ public enum JavaOperationMetricKey implements MetricKey Date: Wed, 26 Jul 2017 17:49:58 +0200 Subject: [PATCH 2/9] First iteration of npath --- .../metrics/api/JavaOperationMetricKey.java | 14 +- .../lang/java/metrics/impl/CycloMetric.java | 42 +++ .../lang/java/metrics/impl/NpathMetric.java | 27 ++ .../impl/visitors/DefaultNpathVisitor.java | 143 +++++++++ .../impl/visitors/StandardCycloVisitor.java | 12 +- .../metrics/rule/NPathComplexityRule.java | 52 ++++ .../main/resources/rulesets/java/metrics.xml | 47 +++ .../java/metrics/impl/AllMetricsTest.java | 9 +- .../lang/java/metrics/impl/NPathTestRule.java | 25 ++ .../java/rule/metrics/MetricsRulesTest.java | 5 +- .../lang/java/metrics/impl/xml/NPathTest.xml | 276 ++++++++++++++++++ .../java/rule/metrics/xml/NPathComplexity.xml | 100 +++++++ .../resources/rulesets/java/metrics_test.xml | 6 + 13 files changed, 745 insertions(+), 13 deletions(-) create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/NpathMetric.java create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/DefaultNpathVisitor.java create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/rule/NPathComplexityRule.java create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/impl/NPathTestRule.java create mode 100644 pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/metrics/impl/xml/NPathTest.xml create mode 100644 pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/metrics/xml/NPathComplexity.xml diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaOperationMetricKey.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaOperationMetricKey.java index ef7120b0f9..f4c1680696 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaOperationMetricKey.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaOperationMetricKey.java @@ -9,6 +9,7 @@ import net.sourceforge.pmd.lang.java.metrics.impl.AtfdMetric.AtfdOperationMetric import net.sourceforge.pmd.lang.java.metrics.impl.CycloMetric.CycloOperationMetric; import net.sourceforge.pmd.lang.java.metrics.impl.LocMetric.LocOperationMetric; import net.sourceforge.pmd.lang.java.metrics.impl.NcssMetric.NcssOperationMetric; +import net.sourceforge.pmd.lang.java.metrics.impl.NpathMetric; import net.sourceforge.pmd.lang.metrics.api.Metric; import net.sourceforge.pmd.lang.metrics.api.MetricKey; @@ -43,7 +44,18 @@ public enum JavaOperationMetricKey implements MetricKeyReferences: + * *
    *
  • [1] Lanza, Object-Oriented Metrics in Practice, 2005. *
  • [2] McCabe, A Complexity Measure, in Proceedings of the 2nd ICSE (1976). @@ -49,8 +55,43 @@ import net.sourceforge.pmd.lang.metrics.api.ResultOption; */ public final class CycloMetric { + private CycloMetric() { + + } + // TODO:cf Cyclo should develop factorized boolean operators to count them + + /** + * Evaluates the number of paths through a boolean expression. This is the total number of {@code &&} and {@code ||} + * operators appearing in the expression. This is used in the calculation of cyclomatic and n-path complexity. + * + * @param expr Expression to analyse + * + * @return The number of paths through the expression + */ + public static int booleanExpressionComplexity(ASTExpression expr) { + if (expr == null) { + return 0; + } + + List andNodes = expr.findDescendantsOfType(ASTConditionalAndExpression.class); + List orNodes = expr.findDescendantsOfType(ASTConditionalOrExpression.class); + + int complexity = 0; + + for (ASTConditionalOrExpression element : orNodes) { + complexity += element.jjtGetNumChildren() - 1; + } + + for (ASTConditionalAndExpression element : andNodes) { + complexity += element.jjtGetNumChildren() - 1; + } + + return complexity; + } + + /** Variants of CYCLO. */ public enum CycloVersion implements MetricVersion { /** Do not count the paths in boolean expressions as decision points. */ @@ -78,4 +119,5 @@ public final class CycloMetric { return 1 + JavaMetrics.get(JavaOperationMetricKey.CYCLO, node, version, ResultOption.AVERAGE); } } + } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/NpathMetric.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/NpathMetric.java new file mode 100644 index 0000000000..a0ff672b16 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/NpathMetric.java @@ -0,0 +1,27 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.metrics.impl; + +import static net.sourceforge.pmd.lang.java.metrics.impl.visitors.DefaultNpathVisitor.NpathDataNode; + +import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; +import net.sourceforge.pmd.lang.java.metrics.impl.visitors.DefaultNpathVisitor; +import net.sourceforge.pmd.lang.metrics.api.MetricVersion; + +/** + * NPath complexity is a measurement of the acyclic execution paths through a function. See Nejmeh, Communications of + * the ACM Feb 1988 pp 188-200. + * + * @author Clément Fournier + */ +public class NpathMetric extends AbstractJavaOperationMetric { + + + @Override + public double computeFor(ASTMethodOrConstructorDeclaration node, MetricVersion version) { + NpathDataNode root = (NpathDataNode) node.jjtAccept(new DefaultNpathVisitor(), new NpathDataNode()); + return root.getComplexity(); + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/DefaultNpathVisitor.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/DefaultNpathVisitor.java new file mode 100644 index 0000000000..1a02c826de --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/DefaultNpathVisitor.java @@ -0,0 +1,143 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.metrics.impl.visitors; + +import java.util.ArrayList; +import java.util.List; + +import net.sourceforge.pmd.lang.java.ast.ASTConditionalExpression; +import net.sourceforge.pmd.lang.java.ast.ASTDoStatement; +import net.sourceforge.pmd.lang.java.ast.ASTExpression; +import net.sourceforge.pmd.lang.java.ast.ASTForStatement; +import net.sourceforge.pmd.lang.java.ast.ASTIfStatement; +import net.sourceforge.pmd.lang.java.ast.ASTSwitchStatement; +import net.sourceforge.pmd.lang.java.ast.ASTTryStatement; +import net.sourceforge.pmd.lang.java.ast.ASTWhileStatement; +import net.sourceforge.pmd.lang.java.ast.JavaParserVisitorAdapter; +import net.sourceforge.pmd.lang.java.metrics.impl.CycloMetric; + +/** + * Visitor for the default n-path complexity version. It takes a root {@link NpathDataNode} as data. + * + * @author Clément Fournier + */ +public class DefaultNpathVisitor extends JavaParserVisitorAdapter { + + @Override + public Object visit(ASTIfStatement node, Object data) { + int boolComplexity = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + + NpathDataNode n = ((NpathDataNode) data).addAndGetChild(boolComplexity); + super.visit(node, n); + return n.parent; + } + + + @Override + public Object visit(ASTWhileStatement node, Object data) { + int boolComplexity = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + + NpathDataNode n = ((NpathDataNode) data).addAndGetChild(boolComplexity); + super.visit(node, n); + return n.parent; + } + + + @Override + public Object visit(ASTForStatement node, Object data) { + int boolComplexity = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + + NpathDataNode n = ((NpathDataNode) data).addAndGetChild(boolComplexity); + super.visit(node, n); + return n.parent; + } + + + @Override + public Object visit(ASTDoStatement node, Object data) { + int boolComplexity = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + + NpathDataNode n = ((NpathDataNode) data).addAndGetChild(boolComplexity); + super.visit(node, n); + return n.parent; + } + + + @Override + public Object visit(ASTConditionalExpression node, Object data) { + int boolComplexity = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + + NpathDataNode n = ((NpathDataNode) data).addAndGetChild(boolComplexity); + super.visit(node, n); + return n.parent; + } + + + @Override + public Object visit(ASTTryStatement node, Object data) { + int boolComplexity = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + + NpathDataNode n = ((NpathDataNode) data).addAndGetChild(boolComplexity); + super.visit(node, n); + return n.parent; + } + + + @Override + public Object visit(ASTSwitchStatement node, Object data) { + int boolComplexity = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + + NpathDataNode n = ((NpathDataNode) data).addAndGetChild(boolComplexity); + super.visit(node, n); + return n.parent; + } + + + /** + * Heap structure used as the data of the visitor. Every node corresponds to a control flow statement. A control + * flow statement nested inside another is represented by a node that's a children of the other. + * + *

    The complexity of a node is given by multiplying the complexities of its children plus 1. The complexity of a + * leaf is the boolean complexity of the guard condition of the statement. The root node bears the complexity of the + * method. + */ + public static class NpathDataNode { + + private List children = new ArrayList<>(); + private NpathDataNode parent; + + private int booleanComplexity; + + + /** Creates a root node. */ + public NpathDataNode() { + + } + + + private NpathDataNode(int booleanComplexity, NpathDataNode parent) { + this.booleanComplexity = booleanComplexity; + this.parent = parent; + } + + + NpathDataNode addAndGetChild(int booleanComplexity) { + NpathDataNode newChild = new NpathDataNode(booleanComplexity, this); + children.add(newChild); + return newChild; + } + + + /** Gets the complexity of this node. */ + public int getComplexity() { + int complexity = 1 + booleanComplexity; + for (NpathDataNode child : children) { + complexity *= child.getComplexity(); + } + + return complexity; + } + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/StandardCycloVisitor.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/StandardCycloVisitor.java index fdf8432652..6631aba1a3 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/StandardCycloVisitor.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/StandardCycloVisitor.java @@ -15,6 +15,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTSwitchLabel; import net.sourceforge.pmd.lang.java.ast.ASTSwitchStatement; import net.sourceforge.pmd.lang.java.ast.ASTWhileStatement; import net.sourceforge.pmd.lang.java.ast.JavaNode; +import net.sourceforge.pmd.lang.java.metrics.impl.CycloMetric; import net.sourceforge.pmd.lang.java.rule.codesize.NPathComplexityRule; /** @@ -29,7 +30,7 @@ public class StandardCycloVisitor extends CycloPathUnawareOperationVisitor { public Object visit(ASTIfStatement node, Object data) { super.visit(node, data); - int boolCompIf = NPathComplexityRule.sumExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + int boolCompIf = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); ((MutableInt) data).add(boolCompIf); return data; } @@ -39,8 +40,7 @@ public class StandardCycloVisitor extends CycloPathUnawareOperationVisitor { public Object visit(ASTForStatement node, Object data) { super.visit(node, data); - int boolCompFor = NPathComplexityRule - .sumExpressionComplexity(node.getFirstDescendantOfType(ASTExpression.class)); + int boolCompFor = CycloMetric.booleanExpressionComplexity(node.getFirstDescendantOfType(ASTExpression.class)); ((MutableInt) data).add(boolCompFor); return data; } @@ -50,7 +50,7 @@ public class StandardCycloVisitor extends CycloPathUnawareOperationVisitor { public Object visit(ASTDoStatement node, Object data) { super.visit(node, data); - int boolCompDo = NPathComplexityRule.sumExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + int boolCompDo = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); ((MutableInt) data).add(boolCompDo); return data; } @@ -60,7 +60,7 @@ public class StandardCycloVisitor extends CycloPathUnawareOperationVisitor { public Object visit(ASTSwitchStatement node, Object data) { super.visit((JavaNode) node, data); // skip the superclass' treatment - int boolCompSwitch = NPathComplexityRule.sumExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + int boolCompSwitch = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); ((MutableInt) data).add(boolCompSwitch); return data; } @@ -80,7 +80,7 @@ public class StandardCycloVisitor extends CycloPathUnawareOperationVisitor { public Object visit(ASTWhileStatement node, Object data) { super.visit(node, data); - int boolCompWhile = NPathComplexityRule.sumExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + int boolCompWhile = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); ((MutableInt) data).add(boolCompWhile); return data; } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/rule/NPathComplexityRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/rule/NPathComplexityRule.java new file mode 100644 index 0000000000..e322fc7b49 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/rule/NPathComplexityRule.java @@ -0,0 +1,52 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.metrics.rule; + +import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; +import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; +import net.sourceforge.pmd.lang.java.metrics.JavaMetrics; +import net.sourceforge.pmd.lang.java.metrics.api.JavaOperationMetricKey; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaMetricsRule; +import net.sourceforge.pmd.lang.rule.properties.IntegerProperty; + +/** + * @author Clément Fournier + */ +public class NPathComplexityRule extends AbstractJavaMetricsRule { + + private static final IntegerProperty REPORT_LEVEL_DESCRIPTOR = new IntegerProperty( + "reportLevel", "N-Path Complexity reporting threshold", 1, 30, 200, 1.0f); + + + private static int reportLevel = 200; + + + public NPathComplexityRule() { + definePropertyDescriptor(REPORT_LEVEL_DESCRIPTOR); + } + + + @Override + public Object visit(ASTCompilationUnit node, Object data) { + reportLevel = getProperty(REPORT_LEVEL_DESCRIPTOR); + + super.visit(node, data); + return data; + } + + + @Override + public final Object visit(ASTMethodOrConstructorDeclaration node, Object data) { + int npath = (int) JavaMetrics.get(JavaOperationMetricKey.NPATH, node); + if (npath >= reportLevel) { + addViolation(data, node, new String[] {node instanceof ASTMethodDeclaration ? "method" : "constructor", + node.getQualifiedName().getOperation(), "" + npath,}); + } + + return data; + } + +} diff --git a/pmd-java/src/main/resources/rulesets/java/metrics.xml b/pmd-java/src/main/resources/rulesets/java/metrics.xml index a6220ba16a..e3c2e40ac2 100644 --- a/pmd-java/src/main/resources/rulesets/java/metrics.xml +++ b/pmd-java/src/main/resources/rulesets/java/metrics.xml @@ -97,4 +97,51 @@ public class Foo extends Bar { + + + The NPath complexity of a method is the number of acyclic execution paths through that method. + A threshold of 200 is generally considered the point where measures should be taken to reduce + complexity and increase readability. + + 3 + + r) { + doSomething(); + while (f < 5 ) { + anotherThing(); + f -= 27; + } + } else { + tryThis(); + } + } + } + if ( r - n > 45) { + while (doMagic()) { + findRabbits(); + } + } + try { + doSomethingDangerous(); + } catch (Exception ex) { + makeAmends(); + } finally { + dontDoItAgain(); + } + } +} + + ]]> + + + diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/impl/AllMetricsTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/impl/AllMetricsTest.java index 85605df75c..d926f8a43e 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/impl/AllMetricsTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/impl/AllMetricsTest.java @@ -26,10 +26,11 @@ public class AllMetricsTest extends SimpleAggregatorTst { @Override public void setUp() { - addRule(RULESET, "CycloTest"); - addRule(RULESET, "NcssTest"); - addRule(RULESET, "WmcTest"); - addRule(RULESET, "LocTest"); + // addRule(RULESET, "CycloTest"); + // addRule(RULESET, "NcssTest"); + // addRule(RULESET, "WmcTest"); + // addRule(RULESET, "LocTest"); + addRule(RULESET, "NPathTest"); } } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/impl/NPathTestRule.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/impl/NPathTestRule.java new file mode 100644 index 0000000000..f41e222558 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/impl/NPathTestRule.java @@ -0,0 +1,25 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.metrics.impl; + +import net.sourceforge.pmd.lang.java.metrics.api.JavaClassMetricKey; +import net.sourceforge.pmd.lang.java.metrics.api.JavaOperationMetricKey; + +/** + * @author Clément Fournier + */ +public class NPathTestRule extends AbstractMetricTestRule { + + @Override + protected JavaClassMetricKey getClassKey() { + return null; + } + + + @Override + protected JavaOperationMetricKey getOpKey() { + return JavaOperationMetricKey.NPATH; + } +} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/metrics/MetricsRulesTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/metrics/MetricsRulesTest.java index b2e806ed98..ea7452204b 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/metrics/MetricsRulesTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/metrics/MetricsRulesTest.java @@ -26,7 +26,8 @@ public class MetricsRulesTest extends SimpleAggregatorTst { @Override public void setUp() { - addRule(RULESET, "CyclomaticComplexity"); - addRule(RULESET, "NcssCount"); +// addRule(RULESET, "CyclomaticComplexity"); +// addRule(RULESET, "NcssCount"); + addRule(RULESET, "NPathComplexity"); } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/metrics/impl/xml/NPathTest.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/metrics/impl/xml/NPathTest.xml new file mode 100644 index 0000000000..bea6af0791 --- /dev/null +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/metrics/impl/xml/NPathTest.xml @@ -0,0 +1,276 @@ + + + + + + + + Full example + 1 + + '.Foo#bar()' has value 10. + + + + + + If with no else + 1 + + '.Foo#bar()' has value 2. + + + + + + Nested if with no else + 1 + + '.Foo#bar()' has value 3. + + + + + + Nested if with else + 1 + + '.Foo#bar()' has value 4. + + + + + + Two ifs + 1 + + '.Foo#bar()' has value 4. + + + + + + Two ifs one else + 1 + + '.Foo#bar()' has value 4. + + + + + + Two ifs and nested + 1 + + '.Foo#bar()' has value 6. + + + + + + + + ok + 0 + + + + + fail, with minimum + 1 + 1 + + The method 'bar()' has value 2 + + + + + + + + + + test case for bug 3484404 (Invalid NPath calculation in return statement) + 0 + + + + + test case for bug 3484404 (Invalid NPath calculation in return statement) with minimum 25 + + 2 + + The method x() has an NPath complexity of 25 + The method y() has an NPath complexity of 25 + + + + + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/metrics/xml/NPathComplexity.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/metrics/xml/NPathComplexity.xml new file mode 100644 index 0000000000..186c1522cc --- /dev/null +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/metrics/xml/NPathComplexity.xml @@ -0,0 +1,100 @@ + + + + + ok + 0 + + + + + fail, with minimum + 1 + 1 + + The method bar() has an NPath complexity of 2 + + + + + + Failure case + 1 + + + + + + + test case for bug 3484404 (Invalid NPath calculation in return statement) + 0 + + + + + test case for bug 3484404 (Invalid NPath calculation in return statement) with minimum 25 + 25.0 + 2 + + The method x() has an NPath complexity of 25 + The method y() has an NPath complexity of 25 + + + + + diff --git a/pmd-java/src/test/resources/rulesets/java/metrics_test.xml b/pmd-java/src/test/resources/rulesets/java/metrics_test.xml index 9f38135fa2..d3b6ecd4c7 100644 --- a/pmd-java/src/test/resources/rulesets/java/metrics_test.xml +++ b/pmd-java/src/test/resources/rulesets/java/metrics_test.xml @@ -33,4 +33,10 @@ metrics="true"> + + + From bef9e64021091eefe994bcb7d9375bdb59090a10 Mon Sep 17 00:00:00 2001 From: oowekyala Date: Wed, 26 Jul 2017 19:49:41 +0200 Subject: [PATCH 3/9] Reused existing npath implementation --- .../lang/java/metrics/impl/NpathMetric.java | 4 +- .../impl/visitors/DefaultNpathVisitor.java | 205 +++++++++++------- .../lang/java/metrics/impl/xml/NPathTest.xml | 193 ++++++++--------- 3 files changed, 216 insertions(+), 186 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/NpathMetric.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/NpathMetric.java index a0ff672b16..ce64226330 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/NpathMetric.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/NpathMetric.java @@ -4,7 +4,6 @@ package net.sourceforge.pmd.lang.java.metrics.impl; -import static net.sourceforge.pmd.lang.java.metrics.impl.visitors.DefaultNpathVisitor.NpathDataNode; import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; import net.sourceforge.pmd.lang.java.metrics.impl.visitors.DefaultNpathVisitor; @@ -21,7 +20,6 @@ public class NpathMetric extends AbstractJavaOperationMetric { @Override public double computeFor(ASTMethodOrConstructorDeclaration node, MetricVersion version) { - NpathDataNode root = (NpathDataNode) node.jjtAccept(new DefaultNpathVisitor(), new NpathDataNode()); - return root.getComplexity(); + return (Integer) node.jjtAccept(new DefaultNpathVisitor(), null); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/DefaultNpathVisitor.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/DefaultNpathVisitor.java index 1a02c826de..f7b2568c74 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/DefaultNpathVisitor.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/DefaultNpathVisitor.java @@ -12,132 +12,183 @@ import net.sourceforge.pmd.lang.java.ast.ASTDoStatement; import net.sourceforge.pmd.lang.java.ast.ASTExpression; import net.sourceforge.pmd.lang.java.ast.ASTForStatement; 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.ASTStatement; +import net.sourceforge.pmd.lang.java.ast.ASTSwitchLabel; import net.sourceforge.pmd.lang.java.ast.ASTSwitchStatement; import net.sourceforge.pmd.lang.java.ast.ASTTryStatement; import net.sourceforge.pmd.lang.java.ast.ASTWhileStatement; +import net.sourceforge.pmd.lang.java.ast.JavaNode; import net.sourceforge.pmd.lang.java.ast.JavaParserVisitorAdapter; import net.sourceforge.pmd.lang.java.metrics.impl.CycloMetric; /** - * Visitor for the default n-path complexity version. It takes a root {@link NpathDataNode} as data. + * Visitor for the default n-path complexity version. * * @author Clément Fournier + * @author Jason Bennett */ public class DefaultNpathVisitor extends JavaParserVisitorAdapter { + /* Multiplies the complexity of the children of this node. */ + private int multiplyChildrenComplexities(JavaNode node, Object data) { + int product = 1; + + for (int i = 0; i < node.jjtGetNumChildren(); i++) { + JavaNode n = (JavaNode) node.jjtGetChild(i); + product *= (Integer) n.jjtAccept(this, data); + } + + return product; + } + + + /* Sums the complexity of the children of the node. */ + private int sumChildrenComplexities(JavaNode node, Object data) { + int sum = 0; + + for (int i = 0; i < node.jjtGetNumChildren(); i++) { + JavaNode n = (JavaNode) node.jjtGetChild(i); + sum += (Integer) n.jjtAccept(this, data); + } + + return sum; + } + + + @Override + public Object visit(ASTMethodDeclaration node, Object data) { + return multiplyChildrenComplexities(node, data); + } + + + @Override + public Object visit(JavaNode node, Object data) { + return multiplyChildrenComplexities(node, data); + } + + @Override public Object visit(ASTIfStatement node, Object data) { - int boolComplexity = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + // (npath of if + npath of else (or 1) + bool_comp of if) * npath of next - NpathDataNode n = ((NpathDataNode) data).addAndGetChild(boolComplexity); - super.visit(node, n); - return n.parent; + List statementChildren = new ArrayList<>(); + for (int i = 0; i < node.jjtGetNumChildren(); i++) { + if (node.jjtGetChild(i) instanceof ASTStatement) { + statementChildren.add((JavaNode) node.jjtGetChild(i)); + } + } + + // add path for not taking if + int complexity = node.hasElse() ? 0 : 1; + + for (JavaNode element : statementChildren) { + complexity += (Integer) element.jjtAccept(this, data); + } + + int boolCompIf = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + return boolCompIf + complexity; } @Override public Object visit(ASTWhileStatement node, Object data) { - int boolComplexity = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + // (npath of while + bool_comp of while + 1) * npath of next - NpathDataNode n = ((NpathDataNode) data).addAndGetChild(boolComplexity); - super.visit(node, n); - return n.parent; - } + int boolCompWhile = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + int nPathWhile = (Integer) node.getFirstChildOfType(ASTStatement.class).jjtAccept(this, data); - @Override - public Object visit(ASTForStatement node, Object data) { - int boolComplexity = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); - - NpathDataNode n = ((NpathDataNode) data).addAndGetChild(boolComplexity); - super.visit(node, n); - return n.parent; + return boolCompWhile + nPathWhile + 1; } @Override public Object visit(ASTDoStatement node, Object data) { - int boolComplexity = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + // (npath of do + bool_comp of do + 1) * npath of next - NpathDataNode n = ((NpathDataNode) data).addAndGetChild(boolComplexity); - super.visit(node, n); - return n.parent; + int boolCompDo = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + + int nPathDo = (Integer) node.getFirstChildOfType(ASTStatement.class).jjtAccept(this, data); + + return boolCompDo + nPathDo + 1; } @Override - public Object visit(ASTConditionalExpression node, Object data) { - int boolComplexity = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + public Object visit(ASTForStatement node, Object data) { + // (npath of for + bool_comp of for + 1) * npath of next - NpathDataNode n = ((NpathDataNode) data).addAndGetChild(boolComplexity); - super.visit(node, n); - return n.parent; + int boolCompFor = CycloMetric.booleanExpressionComplexity(node.getFirstDescendantOfType(ASTExpression.class)); + + int nPathFor = (Integer) node.getFirstChildOfType(ASTStatement.class).jjtAccept(this, data); + + return boolCompFor + nPathFor + 1; } @Override - public Object visit(ASTTryStatement node, Object data) { - int boolComplexity = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + public Object visit(ASTReturnStatement node, Object data) { + // return statements are valued at 1, or the value of the boolean expression - NpathDataNode n = ((NpathDataNode) data).addAndGetChild(boolComplexity); - super.visit(node, n); - return n.parent; + ASTExpression expr = node.getFirstChildOfType(ASTExpression.class); + + if (expr == null) { + return 1; + } + + int boolCompReturn = CycloMetric.booleanExpressionComplexity(expr); + int conditionalExpressionComplexity = multiplyChildrenComplexities(expr, data); + + if (conditionalExpressionComplexity > 1) { + boolCompReturn += conditionalExpressionComplexity; + } + + return (boolCompReturn > 0) ? boolCompReturn : 1; } @Override public Object visit(ASTSwitchStatement node, Object data) { - int boolComplexity = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + // bool_comp of switch + sum(npath(case_range)) - NpathDataNode n = ((NpathDataNode) data).addAndGetChild(boolComplexity); - super.visit(node, n); - return n.parent; + int boolCompSwitch = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); + + int npath = 0; + int caseRange = 0; + for (int i = 0; i < node.jjtGetNumChildren(); i++) { + JavaNode n = (JavaNode) node.jjtGetChild(i); + + // Fall-through labels count as 1 for complexity + if (n instanceof ASTSwitchLabel) { + npath += caseRange; + caseRange = 1; + } else { + Integer complexity = (Integer) n.jjtAccept(this, data); + caseRange *= complexity; + } + } + // add in npath of last label + npath += caseRange; + return boolCompSwitch + npath; } - /** - * Heap structure used as the data of the visitor. Every node corresponds to a control flow statement. A control - * flow statement nested inside another is represented by a node that's a children of the other. - * - *

    The complexity of a node is given by multiplying the complexities of its children plus 1. The complexity of a - * leaf is the boolean complexity of the guard condition of the statement. The root node bears the complexity of the - * method. - */ - public static class NpathDataNode { - - private List children = new ArrayList<>(); - private NpathDataNode parent; - - private int booleanComplexity; + @Override + public Object visit(ASTConditionalExpression node, Object data) { + return node.isTernary() ? sumChildrenComplexities(node, data) + 2 : 1; + } - /** Creates a root node. */ - public NpathDataNode() { - - } - - - private NpathDataNode(int booleanComplexity, NpathDataNode parent) { - this.booleanComplexity = booleanComplexity; - this.parent = parent; - } - - - NpathDataNode addAndGetChild(int booleanComplexity) { - NpathDataNode newChild = new NpathDataNode(booleanComplexity, this); - children.add(newChild); - return newChild; - } - - - /** Gets the complexity of this node. */ - public int getComplexity() { - int complexity = 1 + booleanComplexity; - for (NpathDataNode child : children) { - complexity *= child.getComplexity(); - } - - return complexity; - } + @Override + public Object visit(ASTTryStatement node, Object data) { + /* + * This scenario was not addressed by the original paper. Based on the + * principles outlined in the paper, as well as the Checkstyle NPath + * implementation, this code will add the complexity of the try to the + * complexities of the catch and finally blocks. + */ + return sumChildrenComplexities(node, data); } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/metrics/impl/xml/NPathTest.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/metrics/impl/xml/NPathTest.xml index bea6af0791..71ca904ac6 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/metrics/impl/xml/NPathTest.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/metrics/impl/xml/NPathTest.xml @@ -3,40 +3,46 @@ Full example 1 - '.Foo#bar()' has value 10. + '.Foo#bar()' has value 2016. @@ -130,17 +136,11 @@ public class Foo { class Foo { void bar() { boolean a, b; - if (a) { + if (a) {} + else {} - } else { - - } - - if (b) { - - } else { - - } + if (b) {} + else {} } } ]]> @@ -156,15 +156,10 @@ public class Foo { class Foo { void bar() { boolean a, b; - if (a) { + if (a) {} + else {} - } else { - - } - - if (b) { - - } + if (b) {} } } ]]> @@ -180,95 +175,81 @@ public class Foo { class Foo { void bar() { boolean a, b; - if (a) { // 3 - if (b) { + if (a) { + if (b) {} + } else {} - } - } else { - - } - - if (b) { // times two - - } else { - - } + if (b) {} + else {} } } ]]> - - ok - 0 - - - - - fail, with minimum - 1 + Full switch 1 - The method 'bar()' has value 2 + '.Foo#bar()' has value 7. + class Foo { + void bar() { + boolean a, b; + int j = 23; + switch(j) { + case 1: + case 2: break; + case 3: j = 5; break; + case 4: if (b && a) { bar(); } break; + default: break; + } + } + } + ]]> + + Boolean operators + 1 + + '.Foo#bar()' has value 4. + + + test case for bug 3484404 (Invalid NPath calculation in return statement) - 0 - - - - - test case for bug 3484404 (Invalid NPath calculation in return statement) with minimum 25 - - 2 + 3 - The method x() has an NPath complexity of 25 - The method y() has an NPath complexity of 25 + '.Bar#x(boolean, boolean)' has value 25. + '.Bar#y(boolean, boolean)' has value 25. + '.Bar#z(int, int)' has value 1. From d06cb63d3897ffb11a568c758600366494ba22e1 Mon Sep 17 00:00:00 2001 From: oowekyala Date: Wed, 26 Jul 2017 23:50:08 +0200 Subject: [PATCH 4/9] Tests of the rule --- .../pmd/lang/java/metrics/impl/NpathMetric.java | 1 - .../metrics/impl/visitors/DefaultNpathVisitor.java | 8 ++++---- .../lang/java/metrics/rule/NPathComplexityRule.java | 2 +- .../pmd/lang/java/metrics/impl/AllMetricsTest.java | 10 ++++++---- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/NpathMetric.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/NpathMetric.java index ce64226330..d9e2a649af 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/NpathMetric.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/NpathMetric.java @@ -17,7 +17,6 @@ import net.sourceforge.pmd.lang.metrics.api.MetricVersion; */ public class NpathMetric extends AbstractJavaOperationMetric { - @Override public double computeFor(ASTMethodOrConstructorDeclaration node, MetricVersion version) { return (Integer) node.jjtAccept(new DefaultNpathVisitor(), null); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/DefaultNpathVisitor.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/DefaultNpathVisitor.java index f7b2568c74..962e6a89fa 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/DefaultNpathVisitor.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/DefaultNpathVisitor.java @@ -12,7 +12,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTDoStatement; import net.sourceforge.pmd.lang.java.ast.ASTExpression; import net.sourceforge.pmd.lang.java.ast.ASTForStatement; import net.sourceforge.pmd.lang.java.ast.ASTIfStatement; -import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement; import net.sourceforge.pmd.lang.java.ast.ASTStatement; import net.sourceforge.pmd.lang.java.ast.ASTSwitchLabel; @@ -20,7 +20,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTSwitchStatement; import net.sourceforge.pmd.lang.java.ast.ASTTryStatement; import net.sourceforge.pmd.lang.java.ast.ASTWhileStatement; import net.sourceforge.pmd.lang.java.ast.JavaNode; -import net.sourceforge.pmd.lang.java.ast.JavaParserVisitorAdapter; +import net.sourceforge.pmd.lang.java.ast.JavaParserVisitorReducedAdapter; import net.sourceforge.pmd.lang.java.metrics.impl.CycloMetric; /** @@ -29,7 +29,7 @@ import net.sourceforge.pmd.lang.java.metrics.impl.CycloMetric; * @author Clément Fournier * @author Jason Bennett */ -public class DefaultNpathVisitor extends JavaParserVisitorAdapter { +public class DefaultNpathVisitor extends JavaParserVisitorReducedAdapter { /* Multiplies the complexity of the children of this node. */ private int multiplyChildrenComplexities(JavaNode node, Object data) { @@ -58,7 +58,7 @@ public class DefaultNpathVisitor extends JavaParserVisitorAdapter { @Override - public Object visit(ASTMethodDeclaration node, Object data) { + public Object visit(ASTMethodOrConstructorDeclaration node, Object data) { return multiplyChildrenComplexities(node, data); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/rule/NPathComplexityRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/rule/NPathComplexityRule.java index e322fc7b49..76500357d4 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/rule/NPathComplexityRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/rule/NPathComplexityRule.java @@ -43,7 +43,7 @@ public class NPathComplexityRule extends AbstractJavaMetricsRule { int npath = (int) JavaMetrics.get(JavaOperationMetricKey.NPATH, node); if (npath >= reportLevel) { addViolation(data, node, new String[] {node instanceof ASTMethodDeclaration ? "method" : "constructor", - node.getQualifiedName().getOperation(), "" + npath,}); + node.getQualifiedName().getOperation(), "" + npath, }); } return data; diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/impl/AllMetricsTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/impl/AllMetricsTest.java index d926f8a43e..b09a8a4b67 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/impl/AllMetricsTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/impl/AllMetricsTest.java @@ -9,6 +9,8 @@ import net.sourceforge.pmd.lang.java.metrics.MetricsHook; import net.sourceforge.pmd.testframework.SimpleAggregatorTst; /** + * Executes the metrics testing rules. + * * @author Clément Fournier */ public class AllMetricsTest extends SimpleAggregatorTst { @@ -26,10 +28,10 @@ public class AllMetricsTest extends SimpleAggregatorTst { @Override public void setUp() { - // addRule(RULESET, "CycloTest"); - // addRule(RULESET, "NcssTest"); - // addRule(RULESET, "WmcTest"); - // addRule(RULESET, "LocTest"); + addRule(RULESET, "CycloTest"); + addRule(RULESET, "NcssTest"); + addRule(RULESET, "WmcTest"); + addRule(RULESET, "LocTest"); addRule(RULESET, "NPathTest"); } From 9eedf85395170edf0bc64b7c5cfd5cced77dc584 Mon Sep 17 00:00:00 2001 From: oowekyala Date: Thu, 27 Jul 2017 00:47:26 +0200 Subject: [PATCH 5/9] Finished the tests --- .../metrics/rule/NPathComplexityRule.java | 3 +- .../main/resources/rulesets/java/metrics.xml | 22 +- .../java/rule/metrics/MetricsRulesTest.java | 5 +- .../lang/java/metrics/impl/xml/NPathTest.xml | 52 +++- .../java/rule/metrics/xml/NPathComplexity.xml | 232 +++++++++++++----- 5 files changed, 227 insertions(+), 87 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/rule/NPathComplexityRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/rule/NPathComplexityRule.java index 76500357d4..a719a817e7 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/rule/NPathComplexityRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/rule/NPathComplexityRule.java @@ -13,6 +13,8 @@ import net.sourceforge.pmd.lang.java.rule.AbstractJavaMetricsRule; import net.sourceforge.pmd.lang.rule.properties.IntegerProperty; /** + * Simple n-path complexity rule. + * * @author Clément Fournier */ public class NPathComplexityRule extends AbstractJavaMetricsRule { @@ -48,5 +50,4 @@ public class NPathComplexityRule extends AbstractJavaMetricsRule { return data; } - } diff --git a/pmd-java/src/main/resources/rulesets/java/metrics.xml b/pmd-java/src/main/resources/rulesets/java/metrics.xml index e3c2e40ac2..befa73650f 100644 --- a/pmd-java/src/main/resources/rulesets/java/metrics.xml +++ b/pmd-java/src/main/resources/rulesets/java/metrics.xml @@ -66,7 +66,6 @@ public class Foo { // This has a Cyclomatic Complexity = 12 - @@ -99,7 +97,7 @@ public class Foo extends Bar { @@ -111,8 +109,8 @@ public class Foo extends Bar { 3 r) { doSomething(); @@ -134,9 +132,9 @@ void bar() { // this is something more complex than it needs to be, doSomethingDangerous(); } catch (Exception ex) { makeAmends(); - } finally { - dontDoItAgain(); - } + } finally { + dontDoItAgain(); + } } } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/metrics/MetricsRulesTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/metrics/MetricsRulesTest.java index ea7452204b..22148254aa 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/metrics/MetricsRulesTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/metrics/MetricsRulesTest.java @@ -11,7 +11,6 @@ import net.sourceforge.pmd.testframework.SimpleAggregatorTst; /** * @author Clément Fournier */ - public class MetricsRulesTest extends SimpleAggregatorTst { private static final String RULESET = "java-metrics"; @@ -26,8 +25,8 @@ public class MetricsRulesTest extends SimpleAggregatorTst { @Override public void setUp() { -// addRule(RULESET, "CyclomaticComplexity"); -// addRule(RULESET, "NcssCount"); + addRule(RULESET, "CyclomaticComplexity"); + addRule(RULESET, "NcssCount"); addRule(RULESET, "NPathComplexity"); } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/metrics/impl/xml/NPathTest.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/metrics/impl/xml/NPathTest.xml index 71ca904ac6..8c2387f79f 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/metrics/impl/xml/NPathTest.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/metrics/impl/xml/NPathTest.xml @@ -5,7 +5,7 @@ public class Foo { public static void bar() { boolean a, b = true; - try { // 2 * 2 + 2 + try { // total 6 if (true) { // 2 List buz = new ArrayList(); } @@ -210,6 +210,29 @@ public class Foo { ]]> + + Ensure switch has same complexity as equivalent ifs + 1 + + '.Foo#bar()' has value 7. + + + + + Boolean operators 1 @@ -227,8 +250,7 @@ public class Foo { + + + NPath supports constructors + 1 + + '.Foo#Foo()' has value 7. + + + + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/metrics/xml/NPathComplexity.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/metrics/xml/NPathComplexity.xml index 186c1522cc..491811515a 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/metrics/xml/NPathComplexity.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/metrics/xml/NPathComplexity.xml @@ -1,100 +1,196 @@ - - ok - 0 - - + ]]> + - fail, with minimum - 1 + Full example 1 - The method bar() has an NPath complexity of 2 + The method 'bar()' has an NPath complexity of 2016 + + + + + + + Test default report level - report 200 + 0 + 1 + + The method 'bar()' has an NPath complexity of 200 + public class Foo { + public static void bar() { + boolean a, b = true; + int j = 0; + + switch (j) { // 5 + case 0: + case 1: + case 3: if (a || b) {} break; + } + + switch (j) { // * 5 + case 0: + case 1: + case 3: if (a || b) {} break; + } + + if (true || a && b); // * 4 + while (j++ < 20); // * 2 + } + } + ]]> - Failure case - 1 + Test default report level - ignore 199 + 0 + class Foo { + void bar() { + boolean a, b; + try { + switch(j) { // 7 + case 1: + case 2: break; + case 3: j = 5; break; + case 4: if (b && a) { bar(); } break; + default: break; + } + + switch(j) { // * 7 + case 1: + case 2: break; + case 3: j = 5; break; + case 4: if (b && a) { bar(); } break; + default: break; + } + + if (true || a || b); // * 4 + + } catch (ScaryException e) { + if (true || a); // + 3 + } + } + } + ]]> + + Boolean operators + 2 + 1 + + The method 'bar()' has an NPath complexity of 4 + + + + + test case for bug 3484404 (Invalid NPath calculation in return statement) - 0 - - - - - test case for bug 3484404 (Invalid NPath calculation in return statement) with minimum 25 - 25.0 + 5 2 - The method x() has an NPath complexity of 25 - The method y() has an NPath complexity of 25 + The method 'x(boolean, boolean)' has an NPath complexity of 25 + The method 'y(boolean, boolean)' has an NPath complexity of 25 + + + NPath supports constructors + 6 + 1 + + The constructor 'Foo()' has an NPath complexity of 7 + + + From 1ee8cc4e8daf001172f2a3aa1a8c2a89706e988f Mon Sep 17 00:00:00 2001 From: oowekyala Date: Thu, 27 Jul 2017 01:18:31 +0200 Subject: [PATCH 6/9] Fix bug in key hashcode with null names or metric --- .../pmd/lang/java/metrics/api/JavaClassMetricKey.java | 2 +- .../pmd/lang/java/metrics/api/JavaOperationMetricKey.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaClassMetricKey.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaClassMetricKey.java index 501c1a05a3..38e2c9ef8f 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaClassMetricKey.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaClassMetricKey.java @@ -111,7 +111,7 @@ public enum JavaClassMetricKey implements MetricKey { @Override public int hashCode() { - return metric.hashCode() * 31 + name.hashCode(); + return (metric != null ? metric.hashCode() * 31 : 0) + (name != null ? name.hashCode() : 0); } }; } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaOperationMetricKey.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaOperationMetricKey.java index f4c1680696..3ab49b3c3f 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaOperationMetricKey.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaOperationMetricKey.java @@ -115,7 +115,7 @@ public enum JavaOperationMetricKey implements MetricKey Date: Thu, 27 Jul 2017 22:46:45 +0200 Subject: [PATCH 7/9] Corrections for PR #523 --- .../pmd/lang/java/metrics/api/JavaClassMetricKey.java | 6 +++++- .../metrics/impl/visitors/DefaultNpathVisitor.java | 10 ++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaClassMetricKey.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaClassMetricKey.java index 38e2c9ef8f..5752b54e2c 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaClassMetricKey.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaClassMetricKey.java @@ -4,6 +4,8 @@ package net.sourceforge.pmd.lang.java.metrics.api; +import java.util.Objects; + import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; import net.sourceforge.pmd.lang.java.metrics.impl.AtfdMetric.AtfdClassMetric; import net.sourceforge.pmd.lang.java.metrics.impl.CycloMetric.CycloClassMetric; @@ -105,7 +107,9 @@ public enum JavaClassMetricKey implements MetricKey { @Override public boolean equals(Object obj) { - return obj == this; + return obj != null && getClass() == obj.getClass() + && Objects.equals(name(), getClass().cast(obj).name()) + && Objects.equals(getCalculator(), getClass().cast(obj).getCalculator()); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/DefaultNpathVisitor.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/DefaultNpathVisitor.java index 962e6a89fa..bcbe3a3fec 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/DefaultNpathVisitor.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/DefaultNpathVisitor.java @@ -4,7 +4,6 @@ package net.sourceforge.pmd.lang.java.metrics.impl.visitors; -import java.util.ArrayList; import java.util.List; import net.sourceforge.pmd.lang.java.ast.ASTConditionalExpression; @@ -73,17 +72,12 @@ public class DefaultNpathVisitor extends JavaParserVisitorReducedAdapter { public Object visit(ASTIfStatement node, Object data) { // (npath of if + npath of else (or 1) + bool_comp of if) * npath of next - List statementChildren = new ArrayList<>(); - for (int i = 0; i < node.jjtGetNumChildren(); i++) { - if (node.jjtGetChild(i) instanceof ASTStatement) { - statementChildren.add((JavaNode) node.jjtGetChild(i)); - } - } + List statementChildren = node.findChildrenOfType(ASTStatement.class); // add path for not taking if int complexity = node.hasElse() ? 0 : 1; - for (JavaNode element : statementChildren) { + for (ASTStatement element : statementChildren) { complexity += (Integer) element.jjtAccept(this, data); } From 66df4948a0a2221889018c0284f5d6a8e469ed05 Mon Sep 17 00:00:00 2001 From: oowekyala Date: Fri, 28 Jul 2017 13:02:54 +0200 Subject: [PATCH 8/9] More corrections --- .../pmd/lang/java/metrics/api/JavaOperationMetricKey.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaOperationMetricKey.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaOperationMetricKey.java index 3ab49b3c3f..5d439a411a 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaOperationMetricKey.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaOperationMetricKey.java @@ -4,6 +4,8 @@ package net.sourceforge.pmd.lang.java.metrics.api; +import java.util.Objects; + import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; import net.sourceforge.pmd.lang.java.metrics.impl.AtfdMetric.AtfdOperationMetric; import net.sourceforge.pmd.lang.java.metrics.impl.CycloMetric.CycloOperationMetric; @@ -109,7 +111,9 @@ public enum JavaOperationMetricKey implements MetricKey Date: Fri, 28 Jul 2017 14:45:38 +0200 Subject: [PATCH 9/9] Update changelog, refs #523 --- src/site/markdown/overview/changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index 1a271ad3e6..366cd7a7bc 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -103,4 +103,5 @@ Based on those metrics, rules like "GodClass" detection can be implemented more * [#512](https://github.com/pmd/pmd/pull/512): \[java] Add incorporation to type inference - [Bendegúz Nagy](https://github.com/WinterGrascph) * [#513](https://github.com/pmd/pmd/pull/513): \[java] Fix for maximally specific method selection - [Bendegúz Nagy](https://github.com/WinterGrascph) * [#514](https://github.com/pmd/pmd/pull/514): \[java] Add static method type resolution - [Bendegúz Nagy](https://github.com/WinterGrascph) +* [#523](https://github.com/pmd/pmd/pull/523): \[java] Npath complexity metric and rule - [Clément Fournier](https://github.com/oowekyala)