phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'pr-2297'

Browse Source 
[apex] Cognitive complexity metrics
This commit is contained in:
Andreas Dangel
2020-02-22 12:07:57 +01:00
parent 4b832b24fb ddd338dc78
commit 8b08e5449c
13 changed files with 867 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
docs/pages/release_notes.md
View File

@ -19,15 +19,24 @@ This is a {{ site.pmd.release_type }} release.
This PMD release ships a new version of the pmd-designer.
For the changes, see [PMD Designer Changelog](https://github.com/pmd/pmd-designer/releases/tag/6.21.0).
### Apex Suppressions
#### Apex Suppressions
In addition to suppressing violation with the `@SuppressWarnings` annotation, Apex now also supports
the suppressions with a `NOPMD` comment. See [Suppressing warnings](pmd_userdocs_suppressing_warnings.html).
#### New Rules
* The Rule {% rule "apex/design/CognitiveComplexity" %} (`apex-design`) finds methods and classes
that are highly complex and therefore difficult to read and more costly to maintain. In contrast
to cyclomatic complexity, this rule uses "Cognitive Complexity", which is a measure of how
difficult it is for humans to read and understand a method.
### Fixed Issues
* apex
* [#1087](https://github.com/pmd/pmd/issues/1087): \[apex] Support suppression via //NOPMD
* apex-design
* [#2162](https://github.com/pmd/pmd/issues/2162): \[apex] Cognitive Complexity rule
* doc
* [#2274](https://github.com/pmd/pmd/issues/2274): \[doc] Java API documentation for PMD
* java
@ -125,6 +134,7 @@ methods on {% jdoc apex::lang.apex.ast.ApexParserVisitor %} and its implementati
* [#2276](https://github.com/pmd/pmd/pull/2276): \[java] AppendCharacterWithCharRule ignore literals in expressions - [Kris Scheibe](https://github.com/kris-scheibe)
* [#2278](https://github.com/pmd/pmd/pull/2278): \[java] fix UnusedImports rule for ambiguous static on-demand imports - [Kris Scheibe](https://github.com/kris-scheibe)
* [#2279](https://github.com/pmd/pmd/pull/2279): \[apex] Add support for suppressing violations using the // NOPMD comment - [Gwilym Kuiper](https://github.com/gwilymatgearset)
* [#2297](https://github.com/pmd/pmd/pull/2297): \[apex] Cognitive complexity metrics - [Gwilym Kuiper](https://github.com/gwilymatgearset)
{% endtocmaker %}

2
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/api/ApexClassMetricKey.java
View File

@ -5,6 +5,7 @@
package net.sourceforge.pmd.lang.apex.metrics.api;
import net.sourceforge.pmd.lang.apex.ast.ASTUserClassOrInterface;
import net.sourceforge.pmd.lang.apex.metrics.impl.ClassCognitiveComplexityMetric;
import net.sourceforge.pmd.lang.apex.metrics.impl.WmcMetric;
import net.sourceforge.pmd.lang.metrics.MetricKey;
@ -12,6 +13,7 @@ import net.sourceforge.pmd.lang.metrics.MetricKey;
* @author Clément Fournier
*/
public enum ApexClassMetricKey implements MetricKey<ASTUserClassOrInterface<?>> {
COGNITIVE(new ClassCognitiveComplexityMetric()),
WMC(new WmcMetric());

4
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/api/ApexOperationMetricKey.java
View File

@ -5,6 +5,7 @@
package net.sourceforge.pmd.lang.apex.metrics.api;
import net.sourceforge.pmd.lang.apex.ast.ASTMethod;
import net.sourceforge.pmd.lang.apex.metrics.impl.CognitiveComplexityMetric;
import net.sourceforge.pmd.lang.apex.metrics.impl.CycloMetric;
import net.sourceforge.pmd.lang.metrics.MetricKey;
@ -12,7 +13,8 @@ import net.sourceforge.pmd.lang.metrics.MetricKey;
* @author Clément Fournier
*/
public enum ApexOperationMetricKey implements MetricKey<ASTMethod> {
CYCLO(new CycloMetric());
CYCLO(new CycloMetric()),
COGNITIVE(new CognitiveComplexityMetric());
private final ApexOperationMetric calculator;

23
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/impl/ClassCognitiveComplexityMetric.java Normal file
View File

@ -0,0 +1,23 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.apex.metrics.impl;
import net.sourceforge.pmd.lang.apex.ast.ASTUserClassOrInterface;
import net.sourceforge.pmd.lang.apex.metrics.ApexMetrics;
import net.sourceforge.pmd.lang.apex.metrics.api.ApexOperationMetricKey;
import net.sourceforge.pmd.lang.metrics.MetricOptions;
import net.sourceforge.pmd.lang.metrics.ResultOption;
/**
* The sum of the cognitive complexities of all the methods within a class.
*
* @author Gwilym Kuiper
*/
public class ClassCognitiveComplexityMetric extends AbstractApexClassMetric {
@Override
public double computeFor(ASTUserClassOrInterface<?> node, MetricOptions options) {
return ApexMetrics.get(ApexOperationMetricKey.COGNITIVE, node, ResultOption.SUM);
}
}

24
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/impl/CognitiveComplexityMetric.java Normal file
View File

@ -0,0 +1,24 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.apex.metrics.impl;
import net.sourceforge.pmd.lang.apex.ast.ASTMethod;
import net.sourceforge.pmd.lang.apex.metrics.impl.visitors.CognitiveComplexityVisitor;
import net.sourceforge.pmd.lang.metrics.MetricOptions;
/**
* Measures the cognitive complexity of a Class / Method in Apex.
*
* See https://www.sonarsource.com/docs/CognitiveComplexity.pdf for information about the metric
*
* @author Gwilym Kuiper
*/
public class CognitiveComplexityMetric extends AbstractApexOperationMetric {
@Override
public double computeFor(ASTMethod node, MetricOptions options) {
CognitiveComplexityVisitor.State resultingState = (CognitiveComplexityVisitor.State) node.jjtAccept(new CognitiveComplexityVisitor(), new CognitiveComplexityVisitor.State());
return resultingState.getComplexity();
}
}

234
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/impl/visitors/CognitiveComplexityVisitor.java Normal file
View File

@ -0,0 +1,234 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.apex.metrics.impl.visitors;
import net.sourceforge.pmd.lang.apex.ast.ASTBlockStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTBooleanExpression;
import net.sourceforge.pmd.lang.apex.ast.ASTBreakStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTCatchBlockStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTContinueStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTDoLoopStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTForEachStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTForLoopStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTIfElseBlockStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTMethod;
import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression;
import net.sourceforge.pmd.lang.apex.ast.ASTPrefixExpression;
import net.sourceforge.pmd.lang.apex.ast.ASTTernaryExpression;
import net.sourceforge.pmd.lang.apex.ast.ASTWhileLoopStatement;
import net.sourceforge.pmd.lang.apex.ast.ApexNode;
import net.sourceforge.pmd.lang.apex.ast.ApexParserVisitorAdapter;
import apex.jorje.data.ast.BooleanOp;
import apex.jorje.data.ast.PrefixOp;
/**
* @author Gwilym Kuiper
*/
public class CognitiveComplexityVisitor extends ApexParserVisitorAdapter {
public static class State {
private int complexity = 0;
private int nestingLevel = 0;
private BooleanOp currentBooleanOperation = null;
private String methodName = null;
public double getComplexity() {
return complexity;
}
void structureComplexity() {
complexity += 1;
}
void nestingComplexity() {
complexity += nestingLevel;
}
void booleanOperation(BooleanOp op) {
if (currentBooleanOperation != op) {
if (op != null) {
structureComplexity();
}
currentBooleanOperation = op;
}
}
void increaseNestingLevel() {
structureComplexity();
nestingComplexity();
nestingLevel++;
}
void decreaseNestingLevel() {
nestingLevel--;
}
void methodCall(String methodCalledName) {
if (methodCalledName.equals(methodName)) {
structureComplexity();
}
}
void setMethodName(String name) {
methodName = name;
}
}
@Override
public Object visit(ASTIfElseBlockStatement node, Object data) {
State state = (State) data;
boolean hasElseStatement = node.getNode().hasElseStatement();
for (ApexNode<?> child : node.children()) {
// If we don't have an else statement, we get an empty block statement which we shouldn't count
if (!hasElseStatement && child instanceof ASTBlockStatement) {
break;
}
state.increaseNestingLevel();
super.visit(child, data);
state.decreaseNestingLevel();
}
return data;
}
@Override
public Object visit(ASTForLoopStatement node, Object data) {
State state = (State) data;
state.increaseNestingLevel();
super.visit(node, data);
state.decreaseNestingLevel();
return data;
}
@Override
public Object visit(ASTForEachStatement node, Object data) {
State state = (State) data;
state.increaseNestingLevel();
super.visit(node, data);
state.decreaseNestingLevel();
return data;
}
@Override
public Object visit(ASTContinueStatement node, Object data) {
State state = (State) data;
state.structureComplexity();
return super.visit(node, data);
}
@Override
public Object visit(ASTBreakStatement node, Object data) {
State state = (State) data;
state.structureComplexity();
return super.visit(node, data);
}
@Override
public Object visit(ASTWhileLoopStatement node, Object data) {
State state = (State) data;
state.increaseNestingLevel();
super.visit(node, data);
state.decreaseNestingLevel();
return data;
}
@Override
public Object visit(ASTCatchBlockStatement node, Object data) {
State state = (State) data;
state.increaseNestingLevel();
super.visit(node, data);
state.decreaseNestingLevel();
return data;
}
@Override
public Object visit(ASTDoLoopStatement node, Object data) {
State state = (State) data;
state.increaseNestingLevel();
super.visit(node, data);
state.decreaseNestingLevel();
return data;
}
@Override
public Object visit(ASTTernaryExpression node, Object data) {
State state = (State) data;
state.increaseNestingLevel();
super.visit(node, data);
state.decreaseNestingLevel();
return data;
}
@Override
public Object visit(ASTBooleanExpression node, Object data) {
State state = (State) data;
BooleanOp op = node.getNode().getOp();
if (op == BooleanOp.AND || op == BooleanOp.OR) {
state.booleanOperation(op);
}
return super.visit(node, data);
}
@Override
public Object visit(ASTPrefixExpression node, Object data) {
State state = (State) data;
PrefixOp op = node.getNode().getOp();
if (op == PrefixOp.NOT) {
state.booleanOperation(null);
}
return super.visit(node, data);
}
@Override
public Object visit(ASTBlockStatement node, Object data) {
State state = (State) data;
for (ApexNode<?> child : node.children()) {
child.jjtAccept(this, data);
// This needs to happen because the current 'run' of boolean operations is terminated
// once we finish a statement.
state.booleanOperation(null);
}
return data;
}
@Override
public Object visit(ASTMethod node, Object data) {
State state = (State) data;
state.setMethodName(node.getNode().getMethodInfo().getCanonicalName());
return super.visit(node, data);
}
@Override
public Object visit(ASTMethodCallExpression node, Object data) {
State state = (State) data;
state.methodCall(node.getNode().getMethodName());
return super.visit(node, data);
}
}

107
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/design/CognitiveComplexityRule.java Normal file
View File

@ -0,0 +1,107 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.apex.rule.design;
import static net.sourceforge.pmd.properties.constraints.NumericConstraints.positive;
import java.util.Stack;
import net.sourceforge.pmd.lang.apex.ast.ASTMethod;
import net.sourceforge.pmd.lang.apex.ast.ASTUserClass;
import net.sourceforge.pmd.lang.apex.ast.ASTUserTrigger;
import net.sourceforge.pmd.lang.apex.metrics.ApexMetrics;
import net.sourceforge.pmd.lang.apex.metrics.api.ApexClassMetricKey;
import net.sourceforge.pmd.lang.apex.metrics.api.ApexOperationMetricKey;
import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
import net.sourceforge.pmd.lang.metrics.MetricsUtil;
import net.sourceforge.pmd.lang.metrics.ResultOption;
import net.sourceforge.pmd.properties.PropertyDescriptor;
import net.sourceforge.pmd.properties.PropertyFactory;
public class CognitiveComplexityRule extends AbstractApexRule {
private static final PropertyDescriptor<Integer> CLASS_LEVEL_DESCRIPTOR
= PropertyFactory.intProperty("classReportLevel")
.desc("Total class cognitive complexity reporting threshold")
.require(positive())
.defaultValue(50)
.build();
private static final PropertyDescriptor<Integer> METHOD_LEVEL_DESCRIPTOR
= PropertyFactory.intProperty("methodReportLevel")
.desc("Cognitive complexity reporting threshold")
.require(positive())
.defaultValue(15)
.build();
private Stack<String> classNames = new Stack<>();
private boolean inTrigger;
public CognitiveComplexityRule() {
definePropertyDescriptor(CLASS_LEVEL_DESCRIPTOR);
definePropertyDescriptor(METHOD_LEVEL_DESCRIPTOR);
}
@Override
public Object visit(ASTUserTrigger node, Object data) {
inTrigger = true;
super.visit(node, data);
inTrigger = false;
return data;
}
@Override
public Object visit(ASTUserClass node, Object data) {
classNames.push(node.getImage());
super.visit(node, data);
classNames.pop();
if (ApexClassMetricKey.COGNITIVE.supports(node)) {
int classCognitive = (int) MetricsUtil.computeMetric(ApexClassMetricKey.COGNITIVE, node);
if (classCognitive >= getProperty(CLASS_LEVEL_DESCRIPTOR)) {
int classHighest = (int) ApexMetrics.get(ApexOperationMetricKey.COGNITIVE, node, ResultOption.HIGHEST);
String[] messageParams = {
"class",
node.getImage(),
" total",
classCognitive + " (highest " + classHighest + ")",
};
addViolation(data, node, messageParams);
}
}
return data;
}
@Override
public final Object visit(ASTMethod node, Object data) {
if (ApexOperationMetricKey.COGNITIVE.supports(node)) {
int cognitive = (int) MetricsUtil.computeMetric(ApexOperationMetricKey.COGNITIVE, node);
if (cognitive >= getProperty(METHOD_LEVEL_DESCRIPTOR)) {
String opType = inTrigger ? "trigger"
: node.getImage().equals(classNames.peek()) ? "constructor"
: "method";
addViolation(data, node, new String[] {
opType,
node.getQualifiedName().getOperation(),
"",
"" + cognitive,
});
}
}
return data;
}
}

59
pmd-apex/src/main/resources/category/apex/design.xml
View File

@ -86,6 +86,65 @@ public class Complicated {
</example>
</rule>
<rule name="CognitiveComplexity"
message="The {0} ''{1}'' has a{2} cognitive complexity of {3}."
since="6.22.0"
class="net.sourceforge.pmd.lang.apex.rule.design.CognitiveComplexityRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_apex_design.html#cognitivecomplexity">
<description>
Methods that are highly complex are difficult to read and more costly to maintain. If you include too much decisional
logic within a single method, you make its behavior hard to understand and more difficult to modify.
Cognitive complexity is a measure of how difficult it is for humans to read and understand a method. Code that contains
a break in the control flow is more complex, whereas the use of language shorthands doesn't increase the level of
complexity. Nested control flows can make a method more difficult to understand, with each additional nesting of the
control flow leading to an increase in cognitive complexity.
Information about Cognitive complexity can be found in the original paper here:
https://www.sonarsource.com/docs/CognitiveComplexity.pdf
By default, this rule reports methods with a complexity of 15 or more. Reported methods should be broken down into less
complex components.
</description>
<priority>3</priority>
<example>
<![CDATA[
public class Foo {
// Has a cognitive complexity of 0
public void createAccount() {
Account account = new Account(Name = 'PMD');
insert account;
}
// Has a cognitive complexity of 1
public Boolean setPhoneNumberIfNotExisting(Account a, String phone) {
if (a.Phone == null) { // +1
a.Phone = phone;
update a;
}
}
// Has a cognitive complexity of 5
public void updateContacts(List<Contact> contacts) {
List<Contact> contactsToUpdate = new List<Contact>();
for (Contact contact : contacts) { // +1
if (contact.Department == 'Finance') { // +2
contact.Title = 'Finance Specialist';
contactsToUpdate.add(contact);
} else if (contact.Department == 'Sales') { // +2
contact.Title = 'Sales Specialist';
contactsToUpdate.add(contact);
}
}
update contactsToUpdate;
}
}
]]>
</example>
</rule>
<rule name="ExcessiveClassLength"
since="5.5.0"
message="Avoid really long classes."

1
pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/metrics/impl/AllMetricsTest.java
View File

@ -20,6 +20,7 @@ public class AllMetricsTest extends SimpleAggregatorTst {
public void setUp() {
addRule(RULESET, "CycloTest");
addRule(RULESET, "WmcTest");
addRule(RULESET, "CognitiveComplexityTest");
}
}

23
pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/metrics/impl/CognitiveComplexityTestRule.java Normal file
View File

@ -0,0 +1,23 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.apex.metrics.impl;
import net.sourceforge.pmd.lang.apex.metrics.api.ApexClassMetricKey;
import net.sourceforge.pmd.lang.apex.metrics.api.ApexOperationMetricKey;
/**
* @author Gwilym Kuiper
*/
public class CognitiveComplexityTestRule extends AbstractApexMetricTestRule {
@Override
protected ApexClassMetricKey getClassKey() {
return null;
}
@Override
protected ApexOperationMetricKey getOpKey() {
return ApexOperationMetricKey.COGNITIVE;
}
}

362
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/metrics/impl/xml/CognitiveComplexityTest.xml Normal file
View File

File diff suppressed because it is too large Load Diff

5
pmd-apex/src/test/resources/rulesets/apex/metrics_test.xml
View File

@ -19,4 +19,9 @@
class="net.sourceforge.pmd.lang.apex.metrics.impl.WmcTestRule">
</rule>
<rule name="CognitiveComplexityTest"
message = "''{0}'' has value {1}."
class="net.sourceforge.pmd.lang.apex.metrics.impl.CognitiveComplexityTestRule">
</rule>
</ruleset>

13
pmd-core/src/main/resources/rulesets/releases/6220.xml Normal file
View File

@ -0,0 +1,13 @@
<?xml version="1.0"?>
<ruleset name="6220"
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 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
<description>
This ruleset contains links to rules that are new in PMD v6.22.0
</description>
<rule ref="category/apex/design.xml/CognitiveComplexity"/>
</ruleset>