Skip to content

Commit 11925f4

Browse files
sonar-nigel[bot]sonartech
authored andcommitted
SONARPY-4011 Rule S8507 TypeVar, ParamSpec, and NewType names should match their assigned variable names (#1040)
GitOrigin-RevId: 6c637df3b657e7959d17bda21463f8ce4a4bb3f7
1 parent 912c362 commit 11925f4

File tree

7 files changed

+394
-0
lines changed

7 files changed

+394
-0
lines changed

python-checks/src/main/java/org/sonar/python/checks/OpenSourceCheckList.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,7 @@ public Stream<Class<?>> getChecks() {
451451
TrailingWhitespaceCheck.class,
452452
TypeAliasAnnotationCheck.class,
453453
TypeVarCovariantAndContravariantCheck.class,
454+
TypeVarNamingConventionCheck.class,
454455
TfFunctionDependOnOutsideVariableCheck.class,
455456
TfFunctionRecursivityCheck.class,
456457
TfInputShapeOnModelSubclassCheck.class,
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* You can redistribute and/or modify this program under the terms of
7+
* the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import org.sonar.check.Rule;
20+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
21+
import org.sonar.plugins.python.api.SubscriptionContext;
22+
import org.sonar.plugins.python.api.tree.AssignmentStatement;
23+
import org.sonar.plugins.python.api.tree.CallExpression;
24+
import org.sonar.plugins.python.api.tree.ExpressionList;
25+
import org.sonar.plugins.python.api.tree.Name;
26+
import org.sonar.plugins.python.api.tree.RegularArgument;
27+
import org.sonar.plugins.python.api.tree.StringLiteral;
28+
import org.sonar.plugins.python.api.tree.Tree;
29+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
30+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
31+
import org.sonar.python.tree.TreeUtils;
32+
33+
@Rule(key = "S8507")
34+
public class TypeVarNamingConventionCheck extends PythonSubscriptionCheck {
35+
36+
private static final String MESSAGE = "Rename this string to match the variable name \"%s\".";
37+
38+
private static final TypeMatcher TYPING_CONSTRUCT_MATCHER = TypeMatchers.any(
39+
TypeMatchers.isType("typing.TypeVar"),
40+
TypeMatchers.isType("typing_extensions.TypeVar"),
41+
TypeMatchers.isType("typing.ParamSpec"),
42+
TypeMatchers.isType("typing_extensions.ParamSpec"),
43+
TypeMatchers.isType("typing.NewType"),
44+
TypeMatchers.isType("typing_extensions.NewType")
45+
);
46+
47+
@Override
48+
public void initialize(Context context) {
49+
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, TypeVarNamingConventionCheck::checkCall);
50+
}
51+
52+
private static void checkCall(SubscriptionContext ctx) {
53+
CallExpression call = (CallExpression) ctx.syntaxNode();
54+
if (!TYPING_CONSTRUCT_MATCHER.isTrueFor(call.callee(), ctx)) {
55+
return;
56+
}
57+
RegularArgument firstArg = TreeUtils.nthArgumentOrKeyword(0, "name", call.arguments());
58+
if (firstArg == null || !firstArg.expression().is(Tree.Kind.STRING_LITERAL)) {
59+
return;
60+
}
61+
StringLiteral stringLiteral = (StringLiteral) firstArg.expression();
62+
String stringName = stringLiteral.trimmedQuotesValue();
63+
64+
Tree parent = call.parent();
65+
if (!(parent instanceof AssignmentStatement assignment)) {
66+
return;
67+
}
68+
if (assignment.lhsExpressions().size() != 1) {
69+
return;
70+
}
71+
ExpressionList lhsExprList = assignment.lhsExpressions().get(0);
72+
if (lhsExprList.expressions().size() != 1) {
73+
return;
74+
}
75+
Tree lhsExpr = lhsExprList.expressions().get(0);
76+
if (!lhsExpr.is(Tree.Kind.NAME)) {
77+
return;
78+
}
79+
String variableName = ((Name) lhsExpr).name();
80+
if (!stringName.equals(variableName)) {
81+
ctx.addIssue(stringLiteral, String.format(MESSAGE, variableName));
82+
}
83+
}
84+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<p>This rule raises an issue when a <code>TypeVar</code>, <code>ParamSpec</code>, or <code>NewType</code> is assigned to a variable whose name does
2+
not match the string name passed to its constructor.</p>
3+
<h2>Why is this an issue?</h2>
4+
<p>In Python’s typing system, <code>TypeVar</code>, <code>ParamSpec</code>, and <code>NewType</code> are special constructs used to define generic
5+
types and type aliases. When creating these objects, you provide a string name as the first argument to the constructor. The convention is that this
6+
string name should match the variable name to which the type is assigned.</p>
7+
<p>When the names do not match, type checkers and runtime error messages use the string name from the constructor, which can make error messages
8+
confusing. IDEs and other development tools may also rely on this naming convention to provide accurate type information.</p>
9+
<h3>What is the potential impact?</h3>
10+
<p>Mismatched names create confusion for developers reading the code and make error messages harder to understand. This can slow down debugging and
11+
make code reviews more difficult, reducing overall development efficiency.</p>
12+
<p>In larger codebases with multiple type variables, mismatched names can lead to mistakes when refactoring or extending generic classes and
13+
functions.</p>
14+
<h2>How to fix it</h2>
15+
<p>Ensure the string name passed to the constructor matches the variable name it is assigned to.</p>
16+
<h3>Code examples</h3>
17+
<h4>Noncompliant code example</h4>
18+
<pre data-diff-id="1" data-diff-type="noncompliant">
19+
from typing import TypeVar, ParamSpec, NewType
20+
21+
MyType = TypeVar("T") # Noncompliant: variable is "MyType" but string is "T"
22+
MyParams = ParamSpec("P") # Noncompliant: variable is "MyParams" but string is "P"
23+
MyInt = NewType("Integer", int) # Noncompliant: variable is "MyInt" but string is "Integer"
24+
</pre>
25+
<h4>Compliant solution</h4>
26+
<pre data-diff-id="1" data-diff-type="compliant">
27+
from typing import TypeVar, ParamSpec, NewType
28+
29+
MyType = TypeVar("MyType") # Compliant: names match
30+
MyParams = ParamSpec("MyParams") # Compliant: names match
31+
MyInt = NewType("MyInt", int) # Compliant: names match
32+
</pre>
33+
<h2>Resources</h2>
34+
<h3>Documentation</h3>
35+
<ul>
36+
<li>Python Documentation - <a href="https://docs.python.org/3/library/typing.html#typing.TypeVar"><code>TypeVar</code></a></li>
37+
<li>Python Documentation - <a href="https://docs.python.org/3/library/typing.html#typing.ParamSpec"><code>ParamSpec</code></a></li>
38+
<li>Python Documentation - <a href="https://docs.python.org/3/library/typing.html#typing.NewType"><code>NewType</code></a></li>
39+
<li>PEP 484 - <a href="https://peps.python.org/pep-0484/">Type Hints</a></li>
40+
</ul>
41+
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
{
2+
"title": "TypeVar, ParamSpec, and NewType names should match their assigned variable names",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [
10+
"convention",
11+
"type-hint",
12+
"readability"
13+
],
14+
"defaultSeverity": "Major",
15+
"ruleSpecification": "RSPEC-8507",
16+
"sqKey": "S8507",
17+
"scope": "All",
18+
"quickfix": "targeted",
19+
"code": {
20+
"impacts": {
21+
"MAINTAINABILITY": "MEDIUM"
22+
},
23+
"attribute": "CLEAR"
24+
}
25+
}

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
@@ -332,6 +332,7 @@
332332
"S8494",
333333
"S8495",
334334
"S8504",
335+
"S8507",
335336
"S8517",
336337
"S8520",
337338
"S8521",
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* You can redistribute and/or modify this program under the terms of
7+
* the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.python.checks.utils.PythonCheckVerifier;
21+
22+
class TypeVarNamingConventionCheckTest {
23+
24+
@Test
25+
void test() {
26+
PythonCheckVerifier.verify(
27+
"src/test/resources/checks/typeVarNamingConvention.py",
28+
new TypeVarNamingConventionCheck());
29+
}
30+
}

0 commit comments

Comments
 (0)