Skip to content

Commit 5c015d7

Browse files
vilchik-elenaivandalbosco
vilchik-elena
authored and
ivandalbosco
committed
SONARPY-186 Rule: Cognitive Complexity of functions should not be too high (#74)
* SONARPY-186 Rule: Cognitive Complexity of functions should not be too high * Remove FunctionComplexity rule from default profile * Update licenses * SONARPY-186 Fix wrong behaviour * Add unit test for 'except' with multiple exceptions * Add unit test with not complex function
1 parent a68e22b commit 5c015d7

File tree

12 files changed

+1723
-5
lines changed

12 files changed

+1723
-5
lines changed

‎its/ruling/src/test/resources/expected/python-S3776.json‎

Lines changed: 1258 additions & 0 deletions
Large diffs are not rendered by default.

‎its/ruling/src/test/resources/profile.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,5 +270,10 @@
270270
<key>S1764</key>
271271
<priority>INFO</priority>
272272
</rule>
273+
<rule>
274+
<repositoryKey>python</repositoryKey>
275+
<key>S3776</key>
276+
<priority>INFO</priority>
277+
</rule>
273278
</rules>
274279
</profile>

‎python-checks/src/main/java/org/sonar/python/checks/CheckList.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ private CheckList() {
3232

3333
public static Iterable<Class> getChecks() {
3434
return ImmutableList.<Class>of(
35+
CognitiveComplexityFunctionCheck.class,
3536
ParsingErrorCheck.class,
3637
CommentRegularExpressionCheck.class,
3738
LineLengthCheck.class,

‎python-checks/src/main/java/org/sonar/python/checks/CognitiveComplexityFunctionCheck.java‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2017 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
package org.sonar.python.checks;
21+
22+
import com.sonar.sslr.api.AstNode;
23+
import java.util.HashSet;
24+
import java.util.Set;
25+
import org.sonar.check.Priority;
26+
import org.sonar.check.Rule;
27+
import org.sonar.check.RuleProperty;
28+
import org.sonar.python.PythonCheck;
29+
import org.sonar.python.api.PythonGrammar;
30+
import org.sonar.python.api.PythonKeyword;
31+
import org.sonar.python.api.PythonPunctuator;
32+
import org.sonar.squidbridge.annotations.ActivatedByDefault;
33+
import org.sonar.squidbridge.annotations.SqaleLinearWithOffsetRemediation;
34+
35+
@Rule(
36+
key = CognitiveComplexityFunctionCheck.CHECK_KEY,
37+
name = "Cognitive Complexity of functions should not be too high",
38+
priority = Priority.CRITICAL,
39+
tags = Tags.BRAIN_OVERLOAD
40+
)
41+
@ActivatedByDefault
42+
@SqaleLinearWithOffsetRemediation(
43+
coeff = "1min",
44+
offset = "5min",
45+
effortToFixDescription = "per complexity point above the threshold")
46+
public class CognitiveComplexityFunctionCheck extends PythonCheck {
47+
48+
private static final String MESSAGE = "Refactor this method to reduce its Cognitive Complexity from %s to the %s allowed.";
49+
public static final String CHECK_KEY = "S3776";
50+
private static final int DEFAULT_THRESHOLD = 15;
51+
52+
private AstNode currentFunction = null;
53+
private int complexity;
54+
private int nestingLevel;
55+
private Set<IssueLocation> secondaryLocations = new HashSet<>();
56+
57+
@RuleProperty(
58+
key = "threshold",
59+
description = "The maximum authorized complexity.",
60+
defaultValue = "" + DEFAULT_THRESHOLD)
61+
private int threshold = DEFAULT_THRESHOLD;
62+
63+
public void setThreshold(int threshold) {
64+
this.threshold = threshold;
65+
}
66+
67+
@Override
68+
public void init() {
69+
subscribeTo(
70+
PythonGrammar.IF_STMT,
71+
PythonKeyword.ELIF,
72+
PythonKeyword.ELSE,
73+
74+
PythonGrammar.WHILE_STMT,
75+
PythonGrammar.FOR_STMT,
76+
PythonGrammar.EXCEPT_CLAUSE,
77+
78+
PythonGrammar.AND_TEST,
79+
PythonGrammar.OR_TEST,
80+
81+
PythonGrammar.TEST,
82+
83+
PythonGrammar.FUNCDEF,
84+
PythonGrammar.SUITE);
85+
}
86+
87+
@Override
88+
public void visitNode(AstNode astNode) {
89+
if (astNode.is(PythonGrammar.FUNCDEF) && currentFunction == null) {
90+
currentFunction = astNode;
91+
complexity = 0;
92+
nestingLevel = 0;
93+
secondaryLocations.clear();
94+
}
95+
96+
if (currentFunction != null) {
97+
if (astNode.is(PythonGrammar.SUITE) && incrementsNestingLevel(astNode)) {
98+
nestingLevel++;
99+
}
100+
101+
checkComplexity(astNode);
102+
}
103+
}
104+
105+
private void checkComplexity(AstNode astNode) {
106+
if (astNode.is(PythonGrammar.IF_STMT, PythonGrammar.WHILE_STMT, PythonGrammar.FOR_STMT, PythonGrammar.EXCEPT_CLAUSE)) {
107+
incrementWithNesting(astNode.getFirstChild());
108+
}
109+
110+
if (astNode.is(PythonKeyword.ELIF) || (astNode.is(PythonKeyword.ELSE) && astNode.getNextSibling().is(PythonPunctuator.COLON))) {
111+
incrementWithoutNesting(astNode);
112+
}
113+
114+
if (astNode.is(PythonGrammar.AND_TEST, PythonGrammar.OR_TEST)) {
115+
incrementWithoutNesting(astNode.getFirstChild(PythonKeyword.AND, PythonKeyword.OR));
116+
}
117+
118+
// conditional expression
119+
if (astNode.is(PythonGrammar.TEST) && astNode.hasDirectChildren(PythonKeyword.IF)) {
120+
incrementWithNesting(astNode.getFirstChild(PythonKeyword.IF));
121+
}
122+
}
123+
124+
@Override
125+
public void leaveNode(AstNode astNode) {
126+
if (currentFunction == null) {
127+
return;
128+
}
129+
130+
if (currentFunction.equals(astNode)) {
131+
if (complexity > threshold){
132+
raiseIssue();
133+
}
134+
currentFunction = null;
135+
}
136+
137+
if (astNode.is(PythonGrammar.SUITE) && incrementsNestingLevel(astNode)) {
138+
nestingLevel--;
139+
}
140+
}
141+
142+
private void raiseIssue() {
143+
String message = String.format(MESSAGE, complexity, threshold);
144+
PreciseIssue issue = addIssue(currentFunction.getFirstChild(PythonGrammar.FUNCNAME), message)
145+
.withCost(complexity - threshold);
146+
secondaryLocations.forEach(issue::secondary);
147+
}
148+
149+
private boolean incrementsNestingLevel(AstNode astNode) {
150+
AstNode previousSibling = astNode.getPreviousSibling().getPreviousSibling();
151+
if (previousSibling.is(PythonKeyword.TRY, PythonKeyword.FINALLY)) {
152+
return false;
153+
}
154+
155+
AstNode parent = astNode.getParent();
156+
157+
return !parent.is(PythonGrammar.WITH_STMT, PythonGrammar.CLASSDEF)
158+
&& (!parent.is(PythonGrammar.FUNCDEF) || !parent.equals(currentFunction));
159+
}
160+
161+
private void incrementWithNesting(AstNode secondaryLocationNode) {
162+
int currentNodeComplexity = nestingLevel + 1;
163+
incrementComplexity(secondaryLocationNode, currentNodeComplexity);
164+
}
165+
166+
private void incrementWithoutNesting(AstNode secondaryLocationNode) {
167+
incrementComplexity(secondaryLocationNode, 1);
168+
}
169+
170+
private void incrementComplexity(AstNode secondaryLocationNode, int currentNodeComplexity) {
171+
secondaryLocations.add(new IssueLocation(secondaryLocationNode, secondaryMessage(currentNodeComplexity)));
172+
complexity += currentNodeComplexity;
173+
}
174+
175+
private static String secondaryMessage(int complexity) {
176+
if (complexity == 1) {
177+
return "+1";
178+
179+
} else{
180+
return String.format("+%s (incl %s for nesting)", complexity, complexity - 1);
181+
}
182+
}
183+
184+
}

‎python-checks/src/main/java/org/sonar/python/checks/FunctionComplexityCheck.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.sonar.python.PythonCheck;
2727
import org.sonar.python.api.PythonGrammar;
2828
import org.sonar.python.api.PythonMetric;
29-
import org.sonar.squidbridge.annotations.ActivatedByDefault;
3029
import org.sonar.squidbridge.annotations.SqaleLinearWithOffsetRemediation;
3130
import org.sonar.squidbridge.api.SourceFunction;
3231

@@ -40,7 +39,6 @@
4039
coeff = "1min",
4140
offset = "10min",
4241
effortToFixDescription = "per complexity point above the threshold")
43-
@ActivatedByDefault
4442
public class FunctionComplexityCheck extends PythonCheck {
4543
private static final int DEFAULT_MAXIMUM_FUNCTION_COMPLEXITY_THRESHOLD = 15;
4644
private static final String MESSAGE = "Function has a complexity of %s which is greater than %s authorized.";

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S3776.html‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<p>Cognitive Complexity is a measure of how hard the control flow of a function is to understand. Functions with high Cognitive Complexity will be
2+
difficult to maintain.</p>
3+
<h2>See</h2>
4+
<ul>
5+
<li> <a href="http://redirect.sonarsource.com/doc/cognitive-complexity.html">Cognitive Complexity</a> </li>
6+
</ul>
7+

‎python-checks/src/test/java/org/sonar/python/checks/CognitiveComplexityFunctionCheckTest.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2017 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
package org.sonar.python.checks;
21+
22+
import org.junit.Test;
23+
import org.sonar.python.checks.utils.PythonCheckVerifier;
24+
25+
public class CognitiveComplexityFunctionCheckTest {
26+
27+
private final CognitiveComplexityFunctionCheck check = new CognitiveComplexityFunctionCheck();
28+
29+
@Test
30+
public void test() {
31+
check.setThreshold(0);
32+
PythonCheckVerifier.verify("src/test/resources/checks/cognitiveComplexityFunction.py", check);
33+
}
34+
35+
@Test
36+
public void default_threshold() throws Exception {
37+
PythonCheckVerifier.verify("src/test/resources/checks/cognitiveComplexityFunctionDefault.py", check);
38+
}
39+
}

‎python-checks/src/test/java/org/sonar/python/checks/utils/PythonCheckVerifier.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ private static void verifyIssue(TestIssue expected, PreciseIssue actual) {
8080
assertThat(actual.primaryLocation().message()).as("Bad message at line " + expected.line()).isEqualTo(expected.message());
8181
}
8282
if (expected.effortToFix() != null) {
83-
assertThat(actual.cost()).as("Bad effortToFix at line " + expected.line()).isEqualTo(expected.effortToFix());
83+
assertThat(actual.cost().intValue()).as("Bad effortToFix at line " + expected.line()).isEqualTo(expected.effortToFix());
8484
}
8585
if (expected.startColumn() != null) {
8686
assertThat(actual.primaryLocation().startLineOffset() + 1).as("Bad start column at line " + expected.line()).isEqualTo(expected.startColumn());

0 commit comments

Comments
 (0)