Merge branch 'pr-555'

SumitXLR15 · Aug 15, 2017 · fa6b412 · fa6b412
2 parents f3aa28c + 16c55fc
commit fa6b412
Show file tree

Hide file tree

Showing 11 changed files with 293 additions and 179 deletions.
diff --git a/1 b/1
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
@@ -6,7 +6,6 @@
 
 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;
 import net.sourceforge.pmd.lang.java.metrics.impl.LocMetric.LocClassMetric;
 import net.sourceforge.pmd.lang.java.metrics.impl.NcssMetric.NcssClassMetric;
 import net.sourceforge.pmd.lang.java.metrics.impl.WmcMetric;
@@ -31,13 +30,6 @@ public enum JavaClassMetricKey implements MetricKey<ASTAnyTypeDeclaration> {
      */
     WMC(new WmcMetric()),
 
-    /**
-     * Cyclomatic complexity.
-     *
-     * @see net.sourceforge.pmd.lang.java.metrics.impl.CycloMetric
-     */
-    CYCLO(new CycloClassMetric()),
-
     /**
      * Non Commenting Source Statements.
      *

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
@@ -6,7 +6,7 @@
 
 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;
+import net.sourceforge.pmd.lang.java.metrics.impl.CycloMetric;
 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;
@@ -27,9 +27,9 @@ public enum JavaOperationMetricKey implements MetricKey<ASTMethodOrConstructorDe
     /**
      * Cyclomatic complexity.
      *
-     * @see net.sourceforge.pmd.lang.java.metrics.impl.CycloMetric
+     * @see CycloMetric
      */
-    CYCLO(new CycloOperationMetric()),
+    CYCLO(new CycloMetric()),
 
     /**
      * Non Commenting Source Statements.

diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/CycloMetric.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/CycloMetric.java
@@ -8,18 +8,15 @@
 
 import org.apache.commons.lang3.mutable.MutableInt;
 
-import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTConditionalAndExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTConditionalOrExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration;
 import net.sourceforge.pmd.lang.java.ast.JavaParserVisitor;
-import net.sourceforge.pmd.lang.java.metrics.JavaMetrics;
-import net.sourceforge.pmd.lang.java.metrics.api.JavaOperationMetricKey;
 import net.sourceforge.pmd.lang.java.metrics.impl.visitors.CycloPathUnawareOperationVisitor;
 import net.sourceforge.pmd.lang.java.metrics.impl.visitors.StandardCycloVisitor;
 import net.sourceforge.pmd.lang.metrics.MetricVersion;
-import net.sourceforge.pmd.lang.metrics.ResultOption;
+
 
 /**
  * McCabe's Cyclomatic Complexity. Number of independent paths through a block of code [1, 2]. Formally, given that the
@@ -53,15 +50,24 @@
  * @author Clément Fournier
  * @since June 2017
  */
-public final class CycloMetric {
-
-    private CycloMetric() {
+public final class CycloMetric extends AbstractJavaOperationMetric {
 
-    }
 
     // TODO:cf Cyclo should develop factorized boolean operators to count them
 
 
+    @Override
+    public double computeFor(ASTMethodOrConstructorDeclaration node, MetricVersion version) {
+
+        JavaParserVisitor visitor = (CycloVersion.IGNORE_BOOLEAN_PATHS == version)
+                                    ? new CycloPathUnawareOperationVisitor()
+                                    : new StandardCycloVisitor();
+
+        MutableInt cyclo = (MutableInt) node.jjtAccept(visitor, new MutableInt(1));
+        return (double) cyclo.getValue();
+    }
+
+
     /**
      * 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.
@@ -98,26 +104,4 @@ public enum CycloVersion implements MetricVersion {
         IGNORE_BOOLEAN_PATHS
     }
 
-    public static final class CycloOperationMetric extends AbstractJavaOperationMetric {
-
-        @Override
-        public double computeFor(ASTMethodOrConstructorDeclaration node, MetricVersion version) {
-
-            JavaParserVisitor visitor = (CycloVersion.IGNORE_BOOLEAN_PATHS == version)
-                                        ? new CycloPathUnawareOperationVisitor()
-                                        : new StandardCycloVisitor();
-
-            MutableInt cyclo = (MutableInt) node.jjtAccept(visitor, new MutableInt(1));
-            return (double) cyclo.getValue();
-        }
-    }
-
-    public static final class CycloClassMetric extends AbstractJavaClassMetric {
-
-        @Override
-        public double computeFor(ASTAnyTypeDeclaration node, MetricVersion version) {
-            return 1 + JavaMetrics.get(JavaOperationMetricKey.CYCLO, node, version, ResultOption.AVERAGE);
-        }
-    }
-
 }
diff --git a/...c/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/StandardCycloVisitor.java b/...c/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/StandardCycloVisitor.java
@@ -91,9 +91,8 @@ public Object visit(ASTConditionalExpression node, Object data) {
         super.visit(node, data);
 
         if (node.isTernary()) {
-            int boolCompTern = NPathComplexityRule
-                .sumExpressionComplexity(node.getFirstChildOfType(ASTExpression.class));
-            ((MutableInt) data).add(boolCompTern);
+            int boolCompTern = NPathComplexityRule.sumExpressionComplexity(node.getFirstChildOfType(ASTExpression.class));
+            ((MutableInt) data).add(1 + boolCompTern);
         }
         return data;
     }

diff --git a/...va/src/main/java/net/sourceforge/pmd/lang/java/metrics/rule/CyclomaticComplexityRule.java b/...va/src/main/java/net/sourceforge/pmd/lang/java/metrics/rule/CyclomaticComplexityRule.java
@@ -19,7 +19,6 @@
 import net.sourceforge.pmd.lang.metrics.Metric.Version;
 import net.sourceforge.pmd.lang.metrics.MetricVersion;
 import net.sourceforge.pmd.lang.metrics.ResultOption;
-import net.sourceforge.pmd.lang.rule.properties.BooleanProperty;
 import net.sourceforge.pmd.lang.rule.properties.EnumeratedProperty;
 import net.sourceforge.pmd.lang.rule.properties.IntegerProperty;
 
@@ -30,15 +29,11 @@
  */
 public class CyclomaticComplexityRule extends AbstractJavaMetricsRule {
 
-    private static final IntegerProperty REPORT_LEVEL_DESCRIPTOR = new IntegerProperty(
-        "reportLevel", "Cyclomatic Complexity reporting threshold", 1, 30, 10, 1.0f);
-
-    private static final BooleanProperty REPORT_CLASSES_DESCRIPTOR = new BooleanProperty(
-        "reportClasses", "Add class average violations to the report", true, 2.0f);
-
-    private static final BooleanProperty REPORT_METHODS_DESCRIPTOR = new BooleanProperty(
-        "reportMethods", "Add method average violations to the report", true, 3.0f);
+    private static final IntegerProperty CLASS_LEVEL_DESCRIPTOR = new IntegerProperty(
+        "classReportLevel", "Total class complexity reporting threshold", 1, 600, 80, 1.0f);
 
+    private static final IntegerProperty METHOD_LEVEL_DESCRIPTOR = new IntegerProperty(
+        "methodReportLevel", "Cyclomatic complexity reporting threshold", 1, 50, 10, 1.0f);
 
     private static final Map<String, MetricVersion> VERSION_MAP;
 
@@ -54,26 +49,23 @@ public class CyclomaticComplexityRule extends AbstractJavaMetricsRule {
         "cycloVersion", "Choose a variant of Cyclo or the standard",
         VERSION_MAP, Version.STANDARD, MetricVersion.class, 3.0f);
 
-    private int reportLevel;
-    private boolean reportClasses = true;
-    private boolean reportMethods = true;
+    private int methodReportLevel;
+    private int classReportLevel;
     private MetricVersion cycloVersion = Version.STANDARD;
 
 
     public CyclomaticComplexityRule() {
-        definePropertyDescriptor(REPORT_LEVEL_DESCRIPTOR);
-        definePropertyDescriptor(REPORT_CLASSES_DESCRIPTOR);
-        definePropertyDescriptor(REPORT_METHODS_DESCRIPTOR);
+        definePropertyDescriptor(CLASS_LEVEL_DESCRIPTOR);
+        definePropertyDescriptor(METHOD_LEVEL_DESCRIPTOR);
         definePropertyDescriptor(CYCLO_VERSION_DESCRIPTOR);
     }
 
 
     @Override
     public Object visit(ASTCompilationUnit node, Object data) {
-        reportLevel = getProperty(REPORT_LEVEL_DESCRIPTOR);
+        methodReportLevel = getProperty(METHOD_LEVEL_DESCRIPTOR);
+        classReportLevel = getProperty(CLASS_LEVEL_DESCRIPTOR);
         cycloVersion = getProperty(CYCLO_VERSION_DESCRIPTOR);
-        reportClasses = getProperty(REPORT_CLASSES_DESCRIPTOR);
-        reportMethods = getProperty(REPORT_METHODS_DESCRIPTOR);
 
         super.visit(node, data);
         return data;
@@ -85,14 +77,16 @@ public Object visit(ASTAnyTypeDeclaration node, Object data) {
 
         super.visit(node, data);
 
-        if (reportClasses && JavaClassMetricKey.CYCLO.supports(node)) {
-            int classCyclo = (int) JavaMetrics.get(JavaClassMetricKey.CYCLO, node, cycloVersion);
-            int classHighest = (int) JavaMetrics.get(JavaOperationMetricKey.CYCLO, node, cycloVersion, ResultOption.HIGHEST);
+        if (JavaClassMetricKey.WMC.supports(node)) {
+            int classWmc = (int) JavaMetrics.get(JavaClassMetricKey.WMC, node, cycloVersion);
+
+            if (classWmc >= classReportLevel) {
+                int classHighest = (int) JavaMetrics.get(JavaOperationMetricKey.CYCLO, node, cycloVersion, ResultOption.HIGHEST);
 
-            if (classCyclo >= reportLevel || classHighest >= reportLevel) {
                 String[] messageParams = {node.getTypeKind().name().toLowerCase(),
                                           node.getImage(),
-                                          classCyclo + " (Highest = " + classHighest + ")", };
+                                          " total",
+                                          classWmc + " (highest " + classHighest + ")", };
 
                 addViolation(data, node, messageParams);
             }
@@ -104,13 +98,14 @@ public Object visit(ASTAnyTypeDeclaration node, Object data) {
     @Override
     public final Object visit(ASTMethodOrConstructorDeclaration node, Object data) {
 
-        if (reportMethods) {
-            int cyclo = (int) JavaMetrics.get(JavaOperationMetricKey.CYCLO, node, cycloVersion);
-            if (cyclo >= reportLevel) {
-                addViolation(data, node, new String[] {node instanceof ASTMethodDeclaration ? "method" : "constructor",
-                                                       node.getQualifiedName().getOperation(), "" + cyclo, });
-            }
+        int cyclo = (int) JavaMetrics.get(JavaOperationMetricKey.CYCLO, node, cycloVersion);
+        if (cyclo >= methodReportLevel) {
+            addViolation(data, node, new String[] {node instanceof ASTMethodDeclaration ? "method" : "constructor",
+                                                   node.getQualifiedName().getOperation(),
+                                                   "",
+                                                   "" + cyclo, });
         }
+
         return data;
     }
 

diff --git a/pmd-java/src/main/resources/rulesets/java/metrics.xml b/pmd-java/src/main/resources/rulesets/java/metrics.xml
@@ -10,7 +10,7 @@
     </description>
 
     <rule name="CyclomaticComplexity"
-          message="The {0} ''{1}'' has a Cyclomatic Complexity of {2}."
+          message="The {0} ''{1}'' has a{2} cyclomatic complexity of {3}."
           since="1.03"
           class="net.sourceforge.pmd.lang.java.metrics.rule.CyclomaticComplexityRule"
           metrics="true"
@@ -25,7 +25,9 @@
             details on the calculation, see the documentation of the [Cyclo metric](/pmd_java_metrics_index.html#cyclomatic-complexity-cyclo).
 
             Generally, numbers ranging from 1-4 denote low complexity, 5-7 denote moderate complexity, 8-10 denote
-            high complexity, and 11+ is very high complexity.
+            high complexity, and 11+ is very high complexity. By default, this rule reports methods with a complexity >= 10.
+            Additionnally, classes with many methods of moderate complexity get reported as well once the total of their
+            methods' complexities reaches 80, even if none of the methods was directly reported.
 
             Reported methods should be broken down into several smaller methods. Reported classes should probably be broken down
             into subcomponents.]]>

diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/impl/CycloTestRule.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/impl/CycloTestRule.java
@@ -20,7 +20,7 @@ public class CycloTestRule extends AbstractMetricTestRule {
 
     @Override
     protected JavaClassMetricKey getClassKey() {
-        return JavaClassMetricKey.CYCLO;
+        return null;
     }
 
 

diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/metrics/impl/xml/CycloTest.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/metrics/impl/xml/CycloTest.xml
@@ -66,21 +66,19 @@
 
     <test-code>
         <description>Complicated method - Standard</description>
-        <expected-problems>3</expected-problems>
+        <expected-problems>2</expected-problems>
         <expected-messages>
-            <message>'.Complicated' has value 13 highest 21.</message>
             <message>'.Complicated#exception()' has value 4.</message>
-            <message>'.Complicated#example()' has value 21.</message>
+            <message>'.Complicated#example()' has value 23.</message>
         </expected-messages>
         <code-ref id="full-example"/>
     </test-code>
 
     <test-code>
         <description>Complicated method - Ignore boolean path version</description>
-        <expected-problems>3</expected-problems>
+        <expected-problems>2</expected-problems>
         <rule-property name="metricVersion">ignoreBooleanPaths</rule-property>
         <expected-messages>
-            <message>'.Complicated' has value 10 highest 14.</message>
             <message>'.Complicated#exception()' has value 4.</message>
             <message>'.Complicated#example()' has value 14.</message>
         </expected-messages>
@@ -103,21 +101,6 @@
         </code>
     </test-code>
 
-    <test-code>
-        <description>Empty class should count 0</description>
-        <expected-problems>1</expected-problems>
-        <expected-messages>
-            <message>'.Foo' has value 0 highest 0.</message>
-        </expected-messages>
-        <code>
-            <![CDATA[
-            class Foo {
-            }
-            ]]>
-        </code>
-    </test-code>
-
-
     <code-fragment id="constructor-violation">
         <![CDATA[
         public class Test {
@@ -135,8 +118,7 @@
     </code-fragment>
 
     <test-code>
-        <description>#984 Cyclomatic complexity should treat constructors like methods
-        </description>
+        <description>#984 Cyclomatic complexity should treat constructors like methods</description>
         <rule-property name="reportClasses">false</rule-property>
         <expected-problems>1</expected-problems>
         <expected-messages>
@@ -201,25 +183,19 @@
     </test-code>
 
     <test-code>
-        <description>Interfaces are not supported</description>
-        <expected-problems>0</expected-problems>
-        <code>
-            <![CDATA[
-            interface Koo {
-              void bar();
-            }
-            ]]>
-        </code>
-    </test-code>
-
-    <test-code>
-        <description>Avoid division by 0 when averaging a metric over no operations</description>
-        <rule-property name="reportLevel">-1</rule-property>
+        <description>Ternary expression counts 1 + boolean complexity</description>
         <expected-problems>1</expected-problems>
-        <expected-messages>'.Foo' has value NaN.</expected-messages>
+        <expected-messages>
+            <message>'.Foo#bar()' has value 3.</message>
+        </expected-messages>
         <code>
             <![CDATA[
-            public class Foo { }
+            class Foo {
+              void bar() {
+                boolean a, b;
+                boolean c = (a || b) ? a : b;
+              }
+            }
             ]]>
         </code>
     </test-code>