Skip to content

Commit fca007c

Browse files
nils-werner-sonarsourcenils-werner-sonarsource
authored
SONARPY-259 Rule S138: Functions should not have too many lines of code (#1092)
* SONARPY-259 Rule S138: Functions should not have too many lines of code * Address review suggestions
1 parent 6509220 commit fca007c

File tree

9 files changed

+1135
-12
lines changed

9 files changed

+1135
-12
lines changed

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

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

‎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
@@ -229,6 +229,7 @@ public static Iterable<Class> getChecks() {
229229
TempFileCreationCheck.class,
230230
ToDoCommentCheck.class,
231231
TooManyLinesInFileCheck.class,
232+
TooManyLinesInFunctionCheck.class,
232233
TooManyParametersCheck.class,
233234
TooManyReturnsCheck.class,
234235
TrailingCommentCheck.class,

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

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2022 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 java.util.ArrayDeque;
23+
import java.util.Deque;
24+
import java.util.HashSet;
25+
import java.util.Objects;
26+
import java.util.Set;
27+
import org.sonar.check.Rule;
28+
import org.sonar.check.RuleProperty;
29+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
30+
import org.sonar.plugins.python.api.tree.FunctionDef;
31+
import org.sonar.plugins.python.api.tree.Token;
32+
import org.sonar.plugins.python.api.tree.Tree;
33+
34+
import static org.sonar.python.metrics.FileLinesVisitor.countDocstringLines;
35+
import static org.sonar.python.metrics.FileLinesVisitor.tokenLineNumbers;
36+
37+
@Rule(key = "S138")
38+
public class TooManyLinesInFunctionCheck extends PythonSubscriptionCheck {
39+
40+
private static final String MESSAGE = "This %1$s \"%2$s\" has %3$d lines of code, " +
41+
"which is greater than the %4$d authorized. Split it into smaller %1$ss.";
42+
43+
private static final int DEFAULT = 100;
44+
45+
@RuleProperty(
46+
key = "max",
47+
description = "Maximum authorized lines of code in a function",
48+
defaultValue = "" + DEFAULT)
49+
public int max = DEFAULT;
50+
51+
@Override
52+
public void initialize(Context context) {
53+
context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, ctx -> {
54+
FunctionDef functionDef = (FunctionDef) ctx.syntaxNode();
55+
FunctionLineVisitor visitor = new FunctionLineVisitor();
56+
visitor.scan(functionDef.body());
57+
58+
Set<Integer> linesOfCode = visitor.linesOfCode;
59+
Set<Integer> linesOfDocstring = countDocstringLines(functionDef.docstring());
60+
61+
for (Integer line : linesOfDocstring) {
62+
linesOfCode.remove(line);
63+
}
64+
65+
if (linesOfCode.size() > max) {
66+
ctx.addIssue(functionDef.name(), getIssueMessage(functionDef, linesOfCode.size()));
67+
}
68+
});
69+
}
70+
71+
private String getIssueMessage(FunctionDef functionDef, int numberOfLines) {
72+
String type = functionDef.isMethodDefinition() ? "method" : "function";
73+
return String.format(MESSAGE, type, functionDef.name().name(), numberOfLines, max);
74+
}
75+
76+
private static class FunctionLineVisitor {
77+
78+
private final Set<Integer> linesOfCode = new HashSet<>();
79+
80+
private void scan(Tree element) {
81+
Deque<Tree> stack = new ArrayDeque<>();
82+
stack.push(element);
83+
while (!stack.isEmpty()) {
84+
Tree currentElement = stack.pop();
85+
if (currentElement.is(Tree.Kind.TOKEN)) {
86+
linesOfCode.addAll(tokenLineNumbers((Token) currentElement));
87+
}
88+
89+
currentElement.children()
90+
.stream().filter(Objects::nonNull)
91+
.forEach(stack::push);
92+
}
93+
}
94+
}
95+
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<p>A function that grows too large tends to aggregate too many responsibilities.</p>
2+
<p>Such functions inevitably become harder to understand and therefore harder to maintain.</p>
3+
<p>Above a specific threshold, it is strongly advised to refactor into smaller functions which focus on well-defined tasks.</p>
4+
<p>Those smaller functions will not only be easier to understand, but also probably easier to test.</p>
5+

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S138.json

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
"title": "Functions should not have too many lines of code",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "20min"
8+
},
9+
"tags": [
10+
"brain-overload"
11+
],
12+
"defaultSeverity": "Major",
13+
"ruleSpecification": "RSPEC-138",
14+
"sqKey": "S138",
15+
"scope": "Main",
16+
"quickfix": "unknown"
17+
}

‎python-checks/src/test/java/org/sonar/python/checks/TooManyLinesInFunctionCheckTest.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-2022 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 TooManyLinesInFunctionCheckTest {
26+
27+
@Test
28+
public void test() {
29+
PythonCheckVerifier.verify("src/test/resources/checks/tooManyLinesInFunction.py", new TooManyLinesInFunctionCheck());
30+
}
31+
32+
@Test
33+
public void custom() {
34+
TooManyLinesInFunctionCheck check = new TooManyLinesInFunctionCheck();
35+
check.max = 5;
36+
PythonCheckVerifier.verify("src/test/resources/checks/tooManyLinesInFunctionCustom.py", check);
37+
}
38+
39+
}

‎python-checks/src/test/resources/checks/tooManyLinesInFunction.py

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
# Noncompliant@+1 {{This function "function_101" has 101 lines of code, which is greater than the 100 authorized. Split it into smaller functions.}}
2+
def function_101():
3+
# ^^^^^^^^^^^^
4+
print(1)
5+
print(1)
6+
print(1)
7+
print(1)
8+
print(1)
9+
print(1)
10+
print(1)
11+
print(1)
12+
print(1)
13+
print(1)
14+
print(1)
15+
print(1)
16+
print(1)
17+
print(1)
18+
print(1)
19+
print(1)
20+
print(1)
21+
print(1)
22+
print(1)
23+
print(1)
24+
print(1)
25+
print(1)
26+
print(1)
27+
print(1)
28+
print(1)
29+
print(1)
30+
print(1)
31+
print(1)
32+
print(1)
33+
print(1)
34+
print(1)
35+
print(1)
36+
print(1)
37+
print(1)
38+
print(1)
39+
print(1)
40+
print(1)
41+
print(1)
42+
print(1)
43+
print(1)
44+
print(1)
45+
print(1)
46+
print(1)
47+
print(1)
48+
print(1)
49+
print(1)
50+
print(1)
51+
print(1)
52+
print(1)
53+
print(1)
54+
print(1)
55+
print(1)
56+
print(1)
57+
print(1)
58+
print(1)
59+
print(1)
60+
print(1)
61+
print(1)
62+
print(1)
63+
print(1)
64+
print(1)
65+
print(1)
66+
print(1)
67+
print(1)
68+
print(1)
69+
print(1)
70+
print(1)
71+
print(1)
72+
print(1)
73+
print(1)
74+
print(1)
75+
print(1)
76+
print(1)
77+
print(1)
78+
print(1)
79+
print(1)
80+
print(1)
81+
print(1)
82+
print(1)
83+
print(1)
84+
print(1)
85+
print(1)
86+
print(1)
87+
print(1)
88+
print(1)
89+
print(1)
90+
print(1)
91+
print(1)
92+
print(1)
93+
print(1)
94+
print(1)
95+
print(1)
96+
print(1)
97+
print(1)
98+
print(1)
99+
print(1)
100+
print(1)
101+
print(1)
102+
print(1)
103+
print(1)
104+
print(1)
105+
106+
class MyClass():
107+
# Noncompliant@+1 {{This method "method_101" has 101 lines of code, which is greater than the 100 authorized. Split it into smaller methods.}}
108+
def method_101(self):
109+
print(1)
110+
print(1)
111+
print(1)
112+
print(1)
113+
print(1)
114+
print(1)
115+
print(1)
116+
print(1)
117+
print(1)
118+
print(1)
119+
print(1)
120+
print(1)
121+
print(1)
122+
print(1)
123+
print(1)
124+
print(1)
125+
print(1)
126+
print(1)
127+
print(1)
128+
print(1)
129+
print(1)
130+
print(1)
131+
print(1)
132+
print(1)
133+
print(1)
134+
print(1)
135+
print(1)
136+
print(1)
137+
print(1)
138+
print(1)
139+
print(1)
140+
print(1)
141+
print(1)
142+
print(1)
143+
print(1)
144+
print(1)
145+
print(1)
146+
print(1)
147+
print(1)
148+
print(1)
149+
print(1)
150+
print(1)
151+
print(1)
152+
print(1)
153+
print(1)
154+
print(1)
155+
print(1)
156+
print(1)
157+
print(1)
158+
print(1)
159+
print(1)
160+
print(1)
161+
print(1)
162+
print(1)
163+
print(1)
164+
print(1)
165+
print(1)
166+
print(1)
167+
print(1)
168+
print(1)
169+
print(1)
170+
print(1)
171+
print(1)
172+
print(1)
173+
print(1)
174+
print(1)
175+
print(1)
176+
print(1)
177+
print(1)
178+
print(1)
179+
print(1)
180+
print(1)
181+
print(1)
182+
print(1)
183+
print(1)
184+
print(1)
185+
print(1)
186+
print(1)
187+
print(1)
188+
print(1)
189+
print(1)
190+
print(1)
191+
print(1)
192+
print(1)
193+
print(1)
194+
print(1)
195+
print(1)
196+
print(1)
197+
print(1)
198+
print(1)
199+
print(1)
200+
print(1)
201+
print(1)
202+
print(1)
203+
print(1)
204+
print(1)
205+
print(1)
206+
print(1)
207+
print(1)
208+
print(1)
209+
print(1)

0 commit comments

Comments
 (0)