Skip to content

Commit a5c9299

Browse files
guillaume-dequenneguillaume-dequenne
authored
SONARPY-586 Rule S2710: The first argument to class methods should follow the naming convention (SonarSource#647)
1 parent ed10bc3 commit a5c9299

8 files changed

Lines changed: 254 additions & 0 deletions

File tree

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
{
2+
'project:buildbot-0.8.6p1/buildbot/test/fake/remotecommand.py':[
3+
172,
4+
176,
5+
],
6+
'project:django-2.2.3/django/conf/__init__.py':[
7+
38,
8+
],
9+
'project:django-cms-3.7.1/cms/plugin_base.py':[
10+
504,
11+
],
12+
'project:numpy-1.16.4/numpy/core/defchararray.py':[
13+
1967,
14+
],
15+
'project:numpy-1.16.4/numpy/core/memmap.py':[
16+
207,
17+
],
18+
'project:numpy-1.16.4/numpy/core/records.py':[
19+
416,
20+
],
21+
'project:numpy-1.16.4/numpy/core/tests/test_umath.py':[
22+
2664,
23+
],
24+
'project:numpy-1.16.4/numpy/ma/core.py':[
25+
6064,
26+
],
27+
'project:numpy-1.16.4/numpy/matrixlib/defmatrix.py':[
28+
117,
29+
],
30+
'project:twisted-12.1.0/twisted/test/test_pb.py':[
31+
341,
32+
],
33+
'project:twisted-12.1.0/twisted/web/client.py':[
34+
533,
35+
],
36+
}

‎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
@@ -67,6 +67,7 @@ public static Iterable<Class> getChecks() {
6767
ChangeMethodContractCheck.class,
6868
ChildAndParentExceptionCaughtCheck.class,
6969
ClassComplexityCheck.class,
70+
ClassMethodFirstArgumentNameCheck.class,
7071
ClassNameCheck.class,
7172
ClearTextProtocolsCheck.class,
7273
CognitiveComplexityFunctionCheck.class,

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

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2020 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.Arrays;
23+
import java.util.List;
24+
import java.util.stream.Collectors;
25+
import java.util.stream.Stream;
26+
import org.sonar.check.Rule;
27+
import org.sonar.check.RuleProperty;
28+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
29+
import org.sonar.plugins.python.api.SubscriptionContext;
30+
import org.sonar.plugins.python.api.tree.Decorator;
31+
import org.sonar.plugins.python.api.tree.FunctionDef;
32+
import org.sonar.plugins.python.api.tree.Name;
33+
import org.sonar.plugins.python.api.tree.Parameter;
34+
import org.sonar.plugins.python.api.tree.ParameterList;
35+
import org.sonar.plugins.python.api.tree.Tree;
36+
37+
@Rule(key = "S2710")
38+
public class ClassMethodFirstArgumentNameCheck extends PythonSubscriptionCheck {
39+
40+
private static final String DEFAULT_CLASS_PARAMETER_NAMES = "cls,mcs,metacls";
41+
private static final List<String> DEFAULT_CLASS_METHODS = Arrays.asList("__init_subclass__", "__class_getitem__", "__new__");
42+
List<String> classParameterNamesList;
43+
44+
@RuleProperty(
45+
key = "classParameterNames",
46+
description = "Comma separated list of valid class parameter names",
47+
defaultValue = DEFAULT_CLASS_PARAMETER_NAMES)
48+
public String classParameterNames = DEFAULT_CLASS_PARAMETER_NAMES;
49+
50+
private List<String> classParameterNames() {
51+
if (classParameterNamesList == null) {
52+
classParameterNamesList = Stream.of(classParameterNames.split(","))
53+
.map(String::trim).collect(Collectors.toList());
54+
}
55+
return classParameterNamesList;
56+
}
57+
58+
@Override
59+
public void initialize(Context context) {
60+
context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, ctx -> {
61+
FunctionDef functionDef = (FunctionDef) ctx.syntaxNode();
62+
if (!functionDef.isMethodDefinition()) {
63+
return;
64+
}
65+
if (DEFAULT_CLASS_METHODS.contains(functionDef.name().name())) {
66+
checkFirstParameterName(functionDef, ctx);
67+
return;
68+
}
69+
for (Decorator decorator : functionDef.decorators()) {
70+
if (decorator.name().names().size() == 1 && decorator.name().names().get(0).name().equals("classmethod")) {
71+
checkFirstParameterName(functionDef, ctx);
72+
}
73+
}
74+
});
75+
}
76+
77+
private void checkFirstParameterName(FunctionDef functionDef, SubscriptionContext ctx) {
78+
ParameterList parameterList = functionDef.parameters();
79+
if (parameterList == null || !parameterList.all().get(0).is(Tree.Kind.PARAMETER)) {
80+
// Those cases belong to S5719 scope
81+
return;
82+
}
83+
Parameter parameter = (Parameter) parameterList.all().get(0);
84+
Name parameterName = parameter.name();
85+
if (parameterName == null || parameter.starToken() != null) {
86+
// S5719 scope
87+
return;
88+
}
89+
if (!classParameterNames().contains(parameterName.name())) {
90+
ctx.addIssue(parameterName, String.format("Rename \"%s\" to a valid class parameter name or add the missing class parameter.", parameterName.name()));
91+
}
92+
}
93+
}

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<p>By convention, the first argument to class methods, i.e. methods decorated with @classmethod, is named <code>cls</code> as a representation and a
2+
reminder that the argument is the class itself. Name the argument something else, and you stand a good chance of confusing both users and maintainers
3+
of the code. It might also indicate that the <code>cls</code> parameter was forgotten, in which case calling the method will most probably fail. This
4+
rule also applies to methods <code>__init_subclass__</code>, <code>__class_getitem__</code> and <code>__new__</code> as their first argument is always
5+
the class instead of "self".</p>
6+
<p>By default this rule accepts <code>cls</code> and <code>mcs</code>, which is sometime used in metaclasses, as valid names for class parameters. You
7+
can set your own list of accepted names via the parameter <code>classParameterNames</code>.</p>
8+
<p>This rule raises an issue when the first parameter of a class method is not an accepted name.</p>
9+
<h2>Noncompliant Code Example</h2>
10+
<pre>
11+
class Rectangle(object):
12+
13+
@classmethod
14+
def area(bob, height, width): #Noncompliant
15+
return height * width
16+
</pre>
17+
<h2>Compliant Solution</h2>
18+
<pre>
19+
class Rectangle(object):
20+
21+
@classmethod
22+
def area(cls, height, width):
23+
return height * width
24+
</pre>
25+
<h2>See</h2>
26+
<ul>
27+
<li> PEP8 - <a href="https://www.python.org/dev/peps/pep-0008/#function-and-method-arguments">Function and Method Arguments</a> </li>
28+
</ul>
29+

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
{
2+
"title": "The first argument to class methods should follow the naming convention",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [
10+
"convention",
11+
"confusing",
12+
"pitfall"
13+
],
14+
"defaultSeverity": "Critical",
15+
"ruleSpecification": "RSPEC-2710",
16+
"sqKey": "S2710",
17+
"scope": "Main"
18+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
"S2190",
4747
"S2245",
4848
"S2638",
49+
"S2710",
4950
"S2711",
5051
"S2733",
5152
"S2734",

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2020 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 ClassMethodFirstArgumentNameCheckTest {
26+
27+
@Test
28+
public void test() {
29+
PythonCheckVerifier.verify("src/test/resources/checks/classMethodFirstArgumentNameCheck.py", new ClassMethodFirstArgumentNameCheck());
30+
}
31+
}

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
class A(object):
2+
@classmethod
3+
def first_param_wrong_name(bob, a, b): ... # Noncompliant
4+
5+
@classmethod
6+
def cls_ok(cls, height, width): ... # OK
7+
8+
@classmethod
9+
def mcs_ok(mcs, height, width): ... # OK
10+
11+
@classmethod
12+
def self_first_param(self, a, b): ... # Noncompliant
13+
14+
def __init_subclass__(bob, height, width): ... # Noncompliant
15+
16+
def __class_getitem__(bob, a, b): ... # Noncompliant
17+
18+
def __new__(bob, a, b): ... # Noncompliant
19+
20+
@random.decorator
21+
def some_method(a, b): ... # OK
22+
23+
@classmethod.unrelated
24+
def unpacking(a, b): ... # OK
25+
26+
@unrelated_classmethod
27+
def unpacking(a, b): ... # OK
28+
29+
@classmethod
30+
def unpacking(*args): ... # OK
31+
32+
@classmethod
33+
def cls_keyword_only(*, cls, x): ... # handled by S5719
34+
35+
@classmethod
36+
def no_cls_keyword_only(*, a, b): ... # handled by S5719
37+
38+
@classmethod
39+
def no_parameters(): ... # handled by S5719
40+
41+
@classmethod
42+
def tuple_first_param((a, b), c): ... # This would be a bug to be handled by S5719
43+
44+
45+
def some_function(): ... # OK

0 commit comments

Comments
 (0)