diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 8775473a776..17af49c021b 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -40,6 +40,7 @@ This is a minor release. * [#1216](https://github.com/pmd/pmd/issues/1216): \[java] UnnecessaryFullyQualifiedName false positive for the same name method * java-design * [#1217](https://github.com/pmd/pmd/issues/1217): \[java] CyclomaticComplexityRule counts ?-operator twice + * [#1226](https://github.com/pmd/pmd/issues/1226): \[java] NPath complexity false negative due to overflow * plsql * [#980](https://github.com/pmd/pmd/issues/980): \[plsql] ParseException for CREATE TABLE * [#981](https://github.com/pmd/pmd/issues/981): \[plsql] ParseException when parsing VIEW diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/NpathBaseVisitor.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/NpathBaseVisitor.java index ba88b112f6a..328d64b23b9 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/NpathBaseVisitor.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/NpathBaseVisitor.java @@ -39,7 +39,16 @@ private int multiplyChildrenComplexities(JavaNode node, Object data) { for (int i = 0; i < node.jjtGetNumChildren(); i++) { JavaNode n = (JavaNode) node.jjtGetChild(i); - product *= (Integer) n.jjtAccept(this, data); + int childComplexity = (int) n.jjtAccept(this, data); + + int newProduct = product * childComplexity; + if (newProduct >= product) { + product = newProduct; + } else { + // Overflow happened + product = Integer.MAX_VALUE; + break; + } } return product; @@ -52,7 +61,16 @@ private int sumChildrenComplexities(JavaNode node, Object data) { for (int i = 0; i < node.jjtGetNumChildren(); i++) { JavaNode n = (JavaNode) node.jjtGetChild(i); - sum += (Integer) n.jjtAccept(this, data); + int childComplexity = (int) n.jjtAccept(this, data); + + int newSum = sum + childComplexity; + if (newSum >= sum) { + sum = newSum; + } else { + // Overflow happened + sum = Integer.MAX_VALUE; + break; + } } return sum; @@ -81,7 +99,7 @@ public Object visit(ASTIfStatement node, Object data) { int complexity = node.hasElse() ? 0 : 1; for (ASTStatement element : statementChildren) { - complexity += (Integer) element.jjtAccept(this, data); + complexity += (int) element.jjtAccept(this, data); } int boolCompIf = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); @@ -95,7 +113,7 @@ public Object visit(ASTWhileStatement node, Object data) { int boolCompWhile = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); - int nPathWhile = (Integer) node.getFirstChildOfType(ASTStatement.class).jjtAccept(this, data); + int nPathWhile = (int) node.getFirstChildOfType(ASTStatement.class).jjtAccept(this, data); return boolCompWhile + nPathWhile + 1; } @@ -107,7 +125,7 @@ public Object visit(ASTDoStatement node, Object data) { int boolCompDo = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class)); - int nPathDo = (Integer) node.getFirstChildOfType(ASTStatement.class).jjtAccept(this, data); + int nPathDo = (int) node.getFirstChildOfType(ASTStatement.class).jjtAccept(this, data); return boolCompDo + nPathDo + 1; } @@ -119,7 +137,7 @@ public Object visit(ASTForStatement node, Object data) { int boolCompFor = CycloMetric.booleanExpressionComplexity(node.getFirstDescendantOfType(ASTExpression.class)); - int nPathFor = (Integer) node.getFirstChildOfType(ASTStatement.class).jjtAccept(this, data); + int nPathFor = (int) node.getFirstChildOfType(ASTStatement.class).jjtAccept(this, data); return boolCompFor + nPathFor + 1; } @@ -162,7 +180,7 @@ public Object visit(ASTSwitchStatement node, Object data) { npath += caseRange; caseRange = 1; } else { - Integer complexity = (Integer) n.jjtAccept(this, data); + int complexity = (int) n.jjtAccept(this, data); caseRange *= complexity; } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/NPathComplexityRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/NPathComplexityRule.java index f3957872945..59891ef4615 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/NPathComplexityRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/NPathComplexityRule.java @@ -9,6 +9,7 @@ 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.ast.MethodLikeNode; 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; @@ -68,7 +69,7 @@ private int getReportLevel() { @Override public final Object visit(ASTMethodOrConstructorDeclaration node, Object data) { - int npath = (int) JavaMetrics.get(JavaOperationMetricKey.NPATH, node); + int npath = (int) JavaMetrics.get(JavaOperationMetricKey.NPATH, (MethodLikeNode) node); if (npath >= reportLevel) { addViolation(data, node, new String[]{node instanceof ASTMethodDeclaration ? "method" : "constructor", node.getQualifiedName().getOperation(), "" + npath, }); 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 0180ebfb9ef..8a305fa9f5f 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 @@ -459,4 +459,51 @@ class Bar { } ]]> + + + #1226 [java] NPath complexity false negative + 0 + 1 + + 'NPathComplexityOverflow#complexMethod()' has value 2147483647. + + +