Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 5c015d7

Browse filesBrowse files
vilchik-elenaivandalbosco
authored andcommitted
SONARPY-186 Rule: Cognitive Complexity of functions should not be too high (SonarSource#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
Copy full SHA for 5c015d7

File tree

Expand file treeCollapse file tree

12 files changed

+1723
-5
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

12 files changed

+1723
-5
lines changed
Open diff view settings
Collapse file

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

Copy file name to clipboardExpand all lines: its/ruling/src/test/resources/expected/python-S3776.json
+1,258Lines changed: 1258 additions & 0 deletions
Large diffs are not rendered by default.
Collapse file

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

Copy file name to clipboardExpand all lines: its/ruling/src/test/resources/profile.xml
+5Lines 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>
Collapse file

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

Copy file name to clipboardExpand all lines: python-checks/src/main/java/org/sonar/python/checks/CheckList.java
+1Lines 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,
Collapse file
+184Lines 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+
}
Collapse file

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

Copy file name to clipboardExpand all lines: python-checks/src/main/java/org/sonar/python/checks/FunctionComplexityCheck.java
-2Lines 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.";
Collapse file
+7Lines 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+
Collapse file
+39Lines 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+
}
Collapse file

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

Copy file name to clipboardExpand all lines: python-checks/src/test/java/org/sonar/python/checks/utils/PythonCheckVerifier.java
+1-1Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.