Merge branch 'pr-734'

This commit is contained in:
Andreas Dangel
2017-11-19 13:47:02 +01:00
8 changed files with 591 additions and 905 deletions

View File

@ -196,6 +196,11 @@ Notice this last scenario is slightly different to the Java syntax. This is due
Its report threshold can be configured via the property `reportLevel`, which replaces the now
deprecated property `minimum`.
* The rule `CyclomaticComplexity` (ruleset `java-codesize`) has been revamped to use the new metrics framework.
Its report threshold can be configured via the properties `classReportLevel` and `methodReportLevel` separately.
The old property `reportLevel`, which configured the level for both total class and method complexity,
is deprecated.
#### Deprecated Rules
* The rules `NcssConstructorCount`, `NcssMethodCount`, and `NcssTypeCount` (ruleset `java-codesize`) have been
@ -438,5 +443,6 @@ a warning will now be produced suggesting users to adopt it for better performan
* [#696](https://github.com/pmd/pmd/pull/696): \[core] Add remove operation over nodes - [Matias Comercio](https://github.com/MatiasComercio)
* [#722](https://github.com/pmd/pmd/pull/722): \[java] Move NPathComplexity from metrics to design - [Clément Fournier](https://github.com/oowekyala)
* [#726](https://github.com/pmd/pmd/pull/726): \[java] Fix issue #721 (NPE in InvalidSlf4jMessageFormat) - [Clément Fournier](https://github.com/oowekyala)
* [#734](https://github.com/pmd/pmd/pull/734): \[java] Move CyclomaticComplexity from metrics to design - [Clément Fournier](https://github.com/oowekyala)
* [#737](https://github.com/pmd/pmd/pull/737): \[doc] Fix NPathComplexity documentation bad rendering - [Clément Fournier](https://github.com/oowekyala)

View File

@ -1,120 +0,0 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.metrics.rule;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
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.JavaClassMetricKey;
import net.sourceforge.pmd.lang.java.metrics.api.JavaOperationMetricKey;
import net.sourceforge.pmd.lang.java.metrics.impl.CycloMetric.CycloOption;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaMetricsRule;
import net.sourceforge.pmd.lang.metrics.MetricOptions;
import net.sourceforge.pmd.lang.metrics.ResultOption;
import net.sourceforge.pmd.properties.EnumeratedMultiProperty;
import net.sourceforge.pmd.properties.IntegerProperty;
/**
* Cyclomatic complexity rule using metrics.
*
* @author Clément Fournier
*/
public class CyclomaticComplexityRule extends AbstractJavaMetricsRule {
private static final IntegerProperty CLASS_LEVEL_DESCRIPTOR
= IntegerProperty.named("classReportLevel")
.desc("Total class complexity reporting threshold")
.range(1, 600).defaultValue(80).uiOrder(1.0f).build();
private static final IntegerProperty METHOD_LEVEL_DESCRIPTOR
= IntegerProperty.named("methodReportLevel")
.desc("Cyclomatic complexity reporting threshold")
.range(1, 50).defaultValue(10).uiOrder(1.0f).build();
private static final Map<String, CycloOption> OPTION_MAP;
static {
OPTION_MAP = new HashMap<>();
OPTION_MAP.put(CycloOption.IGNORE_BOOLEAN_PATHS.valueName(), CycloOption.IGNORE_BOOLEAN_PATHS);
OPTION_MAP.put(CycloOption.CONSIDER_ASSERT.valueName(), CycloOption.CONSIDER_ASSERT);
}
private static final EnumeratedMultiProperty<CycloOption> CYCLO_OPTIONS_DESCRIPTOR
= EnumeratedMultiProperty.<CycloOption>named("cycloOptions").type(CycloOption.class)
.desc("Choose options for the computation of Cyclo")
.mappings(OPTION_MAP)
.defaultValues(Collections.<CycloOption>emptyList())
.uiOrder(3.0f).build();
private int methodReportLevel;
private int classReportLevel;
private MetricOptions cycloOptions;
public CyclomaticComplexityRule() {
definePropertyDescriptor(CLASS_LEVEL_DESCRIPTOR);
definePropertyDescriptor(METHOD_LEVEL_DESCRIPTOR);
definePropertyDescriptor(CYCLO_OPTIONS_DESCRIPTOR);
}
@Override
public Object visit(ASTCompilationUnit node, Object data) {
methodReportLevel = getProperty(METHOD_LEVEL_DESCRIPTOR);
classReportLevel = getProperty(CLASS_LEVEL_DESCRIPTOR);
cycloOptions = MetricOptions.ofOptions(getProperty(CYCLO_OPTIONS_DESCRIPTOR));
super.visit(node, data);
return data;
}
@Override
public Object visit(ASTAnyTypeDeclaration node, Object data) {
super.visit(node, data);
if (JavaClassMetricKey.WMC.supports(node)) {
int classWmc = (int) JavaMetrics.get(JavaClassMetricKey.WMC, node, cycloOptions);
if (classWmc >= classReportLevel) {
int classHighest = (int) JavaMetrics.get(JavaOperationMetricKey.CYCLO, node, cycloOptions, ResultOption.HIGHEST);
String[] messageParams = {node.getTypeKind().name().toLowerCase(),
node.getImage(),
" total",
classWmc + " (highest " + classHighest + ")", };
addViolation(data, node, messageParams);
}
}
return data;
}
@Override
public final Object visit(ASTMethodOrConstructorDeclaration node, Object data) {
int cyclo = (int) JavaMetrics.get(JavaOperationMetricKey.CYCLO, node, cycloOptions);
if (cyclo >= methodReportLevel) {
addViolation(data, node, new String[] {node instanceof ASTMethodDeclaration ? "method" : "constructor",
node.getQualifiedName().getOperation(),
"",
"" + cyclo, });
}
return data;
}
}

View File

@ -4,78 +4,151 @@
package net.sourceforge.pmd.lang.java.rule.design;
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.ASTWhileStatement;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.logging.Logger;
import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
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.JavaClassMetricKey;
import net.sourceforge.pmd.lang.java.metrics.api.JavaOperationMetricKey;
import net.sourceforge.pmd.lang.java.metrics.impl.CycloMetric;
import net.sourceforge.pmd.lang.java.metrics.impl.CycloMetric.CycloOption;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaMetricsRule;
import net.sourceforge.pmd.lang.metrics.MetricOptions;
import net.sourceforge.pmd.lang.metrics.ResultOption;
import net.sourceforge.pmd.properties.EnumeratedMultiProperty;
import net.sourceforge.pmd.properties.IntegerProperty;
/**
* @author Donald A. Leckie,
* Cyclomatic complexity rule using metrics.
*
* @version $Revision: 5956 $, $Date: 2008-04-04 04:59:25 -0500 (Fri, 04 Apr
* 2008) $
* @since January 14, 2003
* @author Clément Fournier, based on work by Alan Hohn and Donald A. Leckie
* @see CycloMetric
* @version 6.0.0
*/
public class CyclomaticComplexityRule extends StdCyclomaticComplexityRule {
public class CyclomaticComplexityRule extends AbstractJavaMetricsRule {
@Override
public Object visit(ASTIfStatement node, Object data) {
super.visit(node, data);
private static final Logger LOG = Logger.getLogger(CyclomaticComplexityRule.class.getName());
int boolCompIf = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class));
entryStack.peek().bumpDecisionPoints(boolCompIf);
return data;
// Deprecated, kept for backwards compatibility (6.0.0)
@Deprecated
private static final IntegerProperty REPORT_LEVEL_DESCRIPTOR
= IntegerProperty.named("reportLevel")
.desc("Deprecated! Cyclomatic Complexity reporting threshold")
.range(1, 30).defaultValue(10).uiOrder(1.0f).build();
private static final IntegerProperty CLASS_LEVEL_DESCRIPTOR
= IntegerProperty.named("classReportLevel")
.desc("Total class complexity reporting threshold")
.range(1, 600).defaultValue(80).uiOrder(1.0f).build();
private static final IntegerProperty METHOD_LEVEL_DESCRIPTOR
= IntegerProperty.named("methodReportLevel")
.desc("Cyclomatic complexity reporting threshold")
.range(1, 50).defaultValue(10).uiOrder(1.0f).build();
private static final Map<String, CycloOption> OPTION_MAP;
static {
OPTION_MAP = new HashMap<>();
OPTION_MAP.put(CycloOption.IGNORE_BOOLEAN_PATHS.valueName(), CycloOption.IGNORE_BOOLEAN_PATHS);
OPTION_MAP.put(CycloOption.CONSIDER_ASSERT.valueName(), CycloOption.CONSIDER_ASSERT);
}
private static final EnumeratedMultiProperty<CycloOption> CYCLO_OPTIONS_DESCRIPTOR
= EnumeratedMultiProperty.<CycloOption>named("cycloOptions").type(CycloOption.class)
.desc("Choose options for the computation of Cyclo")
.mappings(OPTION_MAP)
.defaultValues(Collections.<CycloOption>emptyList())
.uiOrder(3.0f).build();
private int methodReportLevel;
private int classReportLevel;
private MetricOptions cycloOptions;
public CyclomaticComplexityRule() {
definePropertyDescriptor(CLASS_LEVEL_DESCRIPTOR);
definePropertyDescriptor(METHOD_LEVEL_DESCRIPTOR);
definePropertyDescriptor(CYCLO_OPTIONS_DESCRIPTOR);
definePropertyDescriptor(REPORT_LEVEL_DESCRIPTOR);
}
// Kept for backwards compatibility // TODO remove the property sometime
private void assignReportLevelsCompat() {
int methodLevel = getProperty(METHOD_LEVEL_DESCRIPTOR);
int classLevel = getProperty(CLASS_LEVEL_DESCRIPTOR);
int commonLevel = getProperty(REPORT_LEVEL_DESCRIPTOR);
if (methodLevel == METHOD_LEVEL_DESCRIPTOR.defaultValue()
&& classLevel == CLASS_LEVEL_DESCRIPTOR.defaultValue()
&& commonLevel != REPORT_LEVEL_DESCRIPTOR.defaultValue()) {
LOG.warning("Rule CyclomaticComplexity uses deprecated property 'reportLevel'. "
+ "Future versions of PMD will remove support for this property. "
+ "Please use 'methodReportLevel' and 'classReportLevel' instead!");
methodLevel = commonLevel;
classLevel = commonLevel * 8;
}
methodReportLevel = methodLevel;
classReportLevel = classLevel;
}
@Override
public Object visit(ASTForStatement node, Object data) {
super.visit(node, data);
public Object visit(ASTCompilationUnit node, Object data) {
// methodReportLevel = getProperty(METHOD_LEVEL_DESCRIPTOR);
// classReportLevel = getProperty(CLASS_LEVEL_DESCRIPTOR);
assignReportLevelsCompat();
int boolCompFor = CycloMetric.booleanExpressionComplexity(node.getFirstDescendantOfType(ASTExpression.class));
entryStack.peek().bumpDecisionPoints(boolCompFor);
cycloOptions = MetricOptions.ofOptions(getProperty(CYCLO_OPTIONS_DESCRIPTOR));
super.visit(node, data);
return data;
}
@Override
public Object visit(ASTDoStatement node, Object data) {
public Object visit(ASTAnyTypeDeclaration node, Object data) {
super.visit(node, data);
int boolCompDo = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class));
entryStack.peek().bumpDecisionPoints(boolCompDo);
if (JavaClassMetricKey.WMC.supports(node)) {
int classWmc = (int) JavaMetrics.get(JavaClassMetricKey.WMC, node, cycloOptions);
if (classWmc >= classReportLevel) {
int classHighest = (int) JavaMetrics.get(JavaOperationMetricKey.CYCLO, node, cycloOptions, ResultOption.HIGHEST);
String[] messageParams = {node.getTypeKind().name().toLowerCase(),
node.getImage(),
" total",
classWmc + " (highest " + classHighest + ")", };
addViolation(data, node, messageParams);
}
}
return data;
}
@Override
public Object visit(ASTSwitchStatement node, Object data) {
super.visit(node, data);
public final Object visit(ASTMethodOrConstructorDeclaration node, Object data) {
int cyclo = (int) JavaMetrics.get(JavaOperationMetricKey.CYCLO, node, cycloOptions);
if (cyclo >= methodReportLevel) {
addViolation(data, node, new String[]{node instanceof ASTMethodDeclaration ? "method" : "constructor",
node.getQualifiedName().getOperation(),
"",
"" + cyclo, });
}
int boolCompSwitch = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class));
entryStack.peek().bumpDecisionPoints(boolCompSwitch);
return data;
}
@Override
public Object visit(ASTWhileStatement node, Object data) {
super.visit(node, data);
int boolCompWhile = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class));
entryStack.peek().bumpDecisionPoints(boolCompWhile);
return data;
}
@Override
public Object visit(ASTConditionalExpression node, Object data) {
super.visit(node, data);
if (node.isTernary()) {
int boolCompTern = CycloMetric.booleanExpressionComplexity(node.getFirstChildOfType(ASTExpression.class));
entryStack.peek().bumpDecisionPoints(boolCompTern);
}
return data;
}
}

View File

@ -372,52 +372,50 @@ public class Foo {
</rule>
<rule name="CyclomaticComplexity"
message="The {0} ''{1}'' has a{2} cyclomatic complexity of {3}."
since="1.03"
message = "The {0} ''{1}'' has a Cyclomatic Complexity of {2}."
class="net.sourceforge.pmd.lang.java.rule.design.CyclomaticComplexityRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#cyclomaticcomplexity"
deprecated="true">
<description>
Complexity directly affects maintenance costs is determined by the number of decision points in a method
plus one for the method entry. The decision points include 'if', 'while', 'for', and 'case labels' calls.
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#cyclomaticcomplexity">
<description><![CDATA[
The complexity of methods directly affects maintenance costs and readability. Concentrating too much decisional logic
in a single method makes its behaviour hard to read and change.
Cyclomatic complexity assesses the complexity of a method by counting the number of decision points in a method,
plus one for the method entry. Decision points are places where the control flow jumps to another place in the
program. As such, they include all control flow statements, such as `if`, `while`, `for`, and `case`. For more
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.]]>
</description>
<priority>3</priority>
<example>
<![CDATA[
public class Foo { // This has a Cyclomatic Complexity = 12
1 public void example() {
2 if (a == b) {
3 if (a1 == b1) {
fiddle();
4 } else if a2 == b2) {
fiddle();
class Foo {
void baseCyclo() { // Cyclo = 1
highCyclo();
}
void highCyclo() { // Cyclo = 10: reported!
int x = 0, y = 2;
boolean a = false, b = true;
if (a && (y == 1 ? b : true)) { // +3
if (y == x) { // +1
while (true) { // +1
if (x++ < 20) { // +1
break; // +1
}
}
} else if (y == t && !d) { // +2
x = a ? y : x; // +1
} else {
fiddle();
}
5 } else if (c == d) {
6 while (c == d) {
fiddle();
}
7 } else if (e == f) {
8 for (int n = 0; n < h; n++) {
fiddle();
}
} else{
switch (z) {
9 case 1:
fiddle();
break;
10 case 2:
fiddle();
break;
11 case 3:
fiddle();
break;
12 default:
fiddle();
break;
x = 2;
}
}
}

View File

@ -1,66 +0,0 @@
<?xml version="1.0"?>
<ruleset name="Metrics temporary ruleset"
xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
<description>
These are rules which use the Metrics Framework to calculate metrics.
</description>
<rule name="CyclomaticComplexity"
message="The {0} ''{1}'' has a{2} cyclomatic complexity of {3}."
since="1.03"
class="net.sourceforge.pmd.lang.java.metrics.rule.CyclomaticComplexityRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_metrics.html#cyclomaticcomplexity">
<description><![CDATA[
The complexity of methods directly affects maintenance costs and readability. Concentrating too much decisional logic
in a single method makes its behaviour hard to read and change.
Cyclomatic complexity assesses the complexity of a method by counting the number of decision points in a method,
plus one for the method entry. Decision points are places where the control flow jumps to another place in the
program. As such, they include all control flow statements, such as `if`, `while`, `for`, and `case`. For more
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. 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.]]>
</description>
<priority>3</priority>
<example>
<![CDATA[
class Foo {
void baseCyclo() { // Cyclo = 1
highCyclo();
}
void highCyclo() { // Cyclo = 10: reported!
int x = 0, y = 2;
boolean a = false, b = true;
if (a && (y == 1 ? b : true)) { // +3
if (y == x) { // +1
while (true) { // +1
if (x++ < 20) { // +1
break; // +1
}
}
} else if (y == t && !d) { // +2
x = a ? y : x; // +1
} else {
x = 2;
}
}
}
}
]]>
</example>
</rule>
</ruleset>

View File

@ -1,30 +0,0 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.metrics;
import net.sourceforge.pmd.Rule;
import net.sourceforge.pmd.lang.java.metrics.MetricsHook;
import net.sourceforge.pmd.testframework.SimpleAggregatorTst;
/**
* @author Clément Fournier
*/
public class MetricsRulesTest extends SimpleAggregatorTst {
private static final String RULESET = "java-metrics";
@Override
protected Rule reinitializeRule(Rule rule) {
MetricsHook.reset();
return super.reinitializeRule(rule);
}
@Override
public void setUp() {
addRule(RULESET, "CyclomaticComplexity");
}
}