Skip to content

Commit d0e4e23

Browse files
guillaume-dequenneguillaume-dequenne
committed
SONARPY-778 Rule S5890: Values assigned to variables should match their type annotations
1 parent abcd793 commit d0e4e23

7 files changed

Lines changed: 251 additions & 1 deletion

File tree

‎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
@@ -129,6 +129,7 @@ public static Iterable<Class> getChecks() {
129129
IgnoredPureOperationsCheck.class,
130130
ImplicitStringConcatenationCheck.class,
131131
IncompatibleOperandsCheck.class,
132+
InconsistentTypeHintCheck.class,
132133
IncorrectExceptionTypeCheck.class,
133134
InequalityUsageCheck.class,
134135
InfiniteRecursionCheck.class,

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

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
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.sonar.check.Rule;
23+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
24+
import org.sonar.plugins.python.api.SubscriptionContext;
25+
import org.sonar.plugins.python.api.symbols.Symbol;
26+
import org.sonar.plugins.python.api.tree.AnnotatedAssignment;
27+
import org.sonar.plugins.python.api.tree.Expression;
28+
import org.sonar.plugins.python.api.tree.Name;
29+
import org.sonar.plugins.python.api.tree.Tree;
30+
import org.sonar.plugins.python.api.tree.TypeAnnotation;
31+
import org.sonar.plugins.python.api.types.InferredType;
32+
import org.sonar.python.tree.TreeUtils;
33+
import org.sonar.python.types.InferredTypes;
34+
import org.sonar.python.types.TypeShed;
35+
36+
@Rule(key = "S5890")
37+
public class InconsistentTypeHintCheck extends PythonSubscriptionCheck {
38+
39+
@Override
40+
public void initialize(Context context) {
41+
context.registerSyntaxNodeConsumer(Tree.Kind.ANNOTATED_ASSIGNMENT, ctx -> {
42+
AnnotatedAssignment annotatedAssignment = (AnnotatedAssignment) ctx.syntaxNode();
43+
Expression assignedExpression = annotatedAssignment.assignedValue();
44+
if (assignedExpression == null) {
45+
return;
46+
}
47+
checkAnnotatedAssignment(ctx, annotatedAssignment, assignedExpression);
48+
});
49+
}
50+
51+
private static void checkAnnotatedAssignment(SubscriptionContext ctx, AnnotatedAssignment annotatedAssignment, Expression assignedExpression) {
52+
InferredType inferredType = assignedExpression.type();
53+
TypeAnnotation annotation = annotatedAssignment.annotation();
54+
InferredType expectedType = InferredTypes.fromTypeAnnotation(annotation);
55+
if (!inferredType.isCompatibleWith(expectedType) || isTypeUsedInsteadOfInstance(assignedExpression, expectedType)) {
56+
String inferredTypeName = InferredTypes.typeName(inferredType);
57+
String inferredTypeNameMessage = inferredTypeName != null ? String.format(" instead of \"%s\"", inferredTypeName) : "";
58+
String nameFromExpression = TreeUtils.nameFromExpression(annotatedAssignment.variable());
59+
String variableMessage = nameFromExpression != null ? String.format("\"%s\"", nameFromExpression) : "this expression";
60+
ctx.addIssue(assignedExpression,
61+
String.format("Assign to %s a value of type \"%s\"%s or update its type hint.",
62+
variableMessage,
63+
InferredTypes.typeName(expectedType),
64+
inferredTypeNameMessage))
65+
.secondary(annotation.expression(), null);
66+
}
67+
}
68+
69+
private static boolean isTypeUsedInsteadOfInstance(Expression assignedExpression, InferredType expectedType) {
70+
if (assignedExpression.is(Tree.Kind.NAME)) {
71+
Name name = (Name) assignedExpression;
72+
Symbol symbol = name.symbol();
73+
return symbol != null && symbol.is(Symbol.Kind.CLASS) &&
74+
!expectedType.isCompatibleWith(InferredTypes.runtimeType(TypeShed.typeShedClass("type")));
75+
}
76+
return false;
77+
}
78+
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<p>Type hints can be used to communicate the intended type of a given variable. These are not enforced at runtime and not respecting them might not
2+
necessarily lead to runtime errors.</p>
3+
<p>It is however confusing and could lead to maintainability issues.</p>
4+
<h2>Noncompliant Code Example</h2>
5+
<pre>
6+
def my_function():
7+
my_int: int = "string" # Noncompliant
8+
</pre>
9+
<h2>Compliant Solution</h2>
10+
<pre>
11+
def my_function():
12+
my_str: str = "string"
13+
</pre>
14+

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"title": "Values assigned to variables should match their type annotations",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [
10+
11+
],
12+
"defaultSeverity": "Major",
13+
"ruleSpecification": "RSPEC-5890",
14+
"sqKey": "S5890",
15+
"scope": "All"
16+
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@
129129
"S5807",
130130
"S5828",
131131
"S5864",
132-
"S5886"
132+
"S5886",
133+
"S5890"
133134
]
134135
}

‎python-checks/src/test/java/org/sonar/python/checks/InconsistentTypeHintCheckTest.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 InconsistentTypeHintCheckTest {
26+
27+
@Test
28+
public void test() {
29+
PythonCheckVerifier.verify("src/test/resources/checks/inconsistentTypeHint.py", new InconsistentTypeHintCheck());
30+
}
31+
}

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

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
from typing import SupportsFloat, List, Iterable, Generator, Set, Union, Type
2+
3+
def assigned_directly():
4+
my_int_nok: int = "hello" # Noncompliant {{Assign to "my_int_nok" a value of type "int" instead of "str" or update its type hint.}}
5+
# ^^^> ^^^^^^^
6+
my_str_ok: str = 42 # Noncompliant
7+
my_int_ok: int = 42 # OK
8+
my_str_ok: str = "hello" # OK
9+
10+
def return_union() -> Union[str, float]:
11+
...
12+
13+
def assigned_to_union(cond):
14+
my_int_nok: int = return_union() # Noncompliant {{Assign to "my_int_nok" a value of type "int" instead of "Union[str, float]" or update its type hint.}}
15+
if cond:
16+
x = "hello"
17+
else:
18+
x = 42.5
19+
my_int_nok: int = x # Noncompliant {{Assign to "my_int_nok" a value of type "int" or update its type hint.}}
20+
21+
22+
def assigned_later(param):
23+
a: int
24+
a = "hello" # FN
25+
b: int
26+
if param:
27+
b = 42
28+
else:
29+
b = "42" # FN
30+
31+
c: int
32+
c = 1 if a else 2 # OK
33+
d: int
34+
d = 1 if a else "hello" # FN
35+
36+
37+
def custom_classes():
38+
class A:
39+
def method(): ...
40+
41+
class B(A):
42+
def additional_method(): ...
43+
44+
my_a_ok: A = A() # OK
45+
my_a_ok2: A = B() # OK
46+
my_a_nok: A = A # Noncompliant
47+
my_b_nok: B = A() # Noncompliant {{Assign to "my_b_nok" a value of type "B" instead of "A" or update its type hint.}}
48+
# ^> ^^^
49+
my_b_ok: B = B()
50+
51+
52+
def get_generator():
53+
yield 1
54+
55+
56+
def type_aliases():
57+
"""We should avoid raising FPs on type aliases"""
58+
my_float: SupportsFloat = 42 # OK
59+
my_iterable: Iterable = [] # OK
60+
my_generator: Generator = get_generator() # OK
61+
62+
63+
def collections():
64+
my_list: List = {} # Noncompliant
65+
66+
my_str_list_nok: List[str] = [1, 2, 3] # FN
67+
68+
my_str_list_ok: List[str] = ["a", "b", "c"] # OK
69+
70+
my_set_nok: Set = {} # Noncompliant {{Assign to "my_set_nok" a value of type "set" instead of "dict" or update its type hint.}}
71+
72+
my_set_nok2: Set = set # Noncompliant {{Assign to "my_set_nok2" a value of type "set" or update its type hint.}}
73+
74+
my_set_ok: Set = set() # OK
75+
76+
77+
def function_params():
78+
def overwritten_param(param: int):
79+
param = "hello" # Out of scope (S1226)
80+
81+
def used_param(param: int):
82+
print(param)
83+
param = "hello" # FN
84+
print(param)
85+
86+
87+
class ClassAttributes:
88+
my_attr: str = "hello" # OK
89+
my_attr_2: str = 42 # Noncompliant
90+
91+
my_attr_3: str
92+
93+
def __init__(self):
94+
self.my_attr_3 = 42 # FN
95+
self.my_attr_4: int = "hello" # Noncompliant {{Assign to this expression a value of type "int" instead of "str" or update its type hint.}}
96+
97+
class Meta(type): ...
98+
99+
class MyClassWithMeta(metaclass=Meta): ...
100+
101+
def metaclasses():
102+
my_var: Meta = set # Accepted FN
103+
my_other_var: Meta = MyClassWithMeta # OK
104+
my_other_var: MyClassWithMeta = MyClassWithMeta # Noncompliant {{Assign to "my_other_var" a value of type "MyClassWithMeta" or update its type hint.}}
105+
another_var: Type = MyClassWithMeta
106+
another_var: Type = set
107+
def a_function(): ...
108+
another_var: Type = a_function # Accepted FN
109+
another_var: Type = unknown_symbol

0 commit comments

Comments
 (0)