Skip to content

Commit 64691c2

Browse files
joke1196joke1196
authored
SONARPY-1331 Rule S6545: Add quick fix to replace typing module type hints with built-in types (SonarSource#1436)
1 parent 1196b0b commit 64691c2

File tree

3 files changed

+170
-14
lines changed

3 files changed

+170
-14
lines changed

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

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,21 @@
2525
import org.sonar.check.Rule;
2626
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
2727
import org.sonar.plugins.python.api.SubscriptionContext;
28+
import org.sonar.plugins.python.api.quickfix.PythonQuickFix;
2829
import org.sonar.plugins.python.api.symbols.Symbol;
2930
import org.sonar.plugins.python.api.tree.Expression;
3031
import org.sonar.plugins.python.api.tree.HasSymbol;
3132
import org.sonar.plugins.python.api.tree.SubscriptionExpression;
3233
import org.sonar.plugins.python.api.tree.Tree;
3334
import org.sonar.plugins.python.api.tree.TypeAnnotation;
35+
import org.sonar.python.quickfix.TextEditUtils;
3436

3537
import static java.util.Map.entry;
3638

3739
@Rule(key = "S6545")
3840
public class BuiltinGenericsOverTypingModuleCheck extends PythonSubscriptionCheck {
3941

40-
private static final String MESSAGE = "Use the built-in generic type `%s` instead of its typing counterpart.";
42+
public static final String MESSAGE = "Use the built-in generic type `%s` instead of its typing counterpart.";
4143
private static final Map<String, String> GENERICS_NAME = Map.ofEntries(
4244
entry("typing.List", "list"),
4345
entry("typing.Dict", "dict"),
@@ -70,22 +72,38 @@ private static void checkForTypingModule(SubscriptionContext subscriptionContext
7072
}
7173

7274
private static void checkForGenericsFromTypingModule(SubscriptionContext subscriptionContext, Expression expression) {
73-
getGenericsCounterPartFromTypingModule(subscriptionContext, expression)
74-
.ifPresent(preferredGenerics -> subscriptionContext.addIssue(expression, String.format(MESSAGE, preferredGenerics)));
75-
}
76-
77-
private static Optional<String> getGenericsCounterPartFromTypingModule(SubscriptionContext context, Expression expression) {
7875
if (expression instanceof SubscriptionExpression) {
7976
SubscriptionExpression subscriptionExpression = (SubscriptionExpression) expression;
80-
subscriptionExpression.subscripts().expressions()
81-
.forEach(nestedExpression -> checkForGenericsFromTypingModule(context, nestedExpression));
82-
Expression mainExpression = subscriptionExpression.object();
83-
return Optional.of(mainExpression)
84-
.map(HasSymbol.class::cast)
85-
.map(HasSymbol::symbol)
86-
.flatMap(BuiltinGenericsOverTypingModuleCheck::getBuiltinGenericsType);
77+
getGenericsCounterPartFromTypingModule(subscriptionContext, subscriptionExpression)
78+
.ifPresent(preferredGenerics -> raiseIssueForGenerics(subscriptionContext, subscriptionExpression, preferredGenerics));
8779
}
88-
return Optional.empty();
80+
}
81+
82+
private static void raiseIssueForGenerics(SubscriptionContext context, SubscriptionExpression expression, String preferredGenerics) {
83+
String specificMessage = String.format(MESSAGE, preferredGenerics);
84+
PreciseIssue preciseIssue = context.addIssue(expression, specificMessage);
85+
addQuickFix(preciseIssue, expression, preferredGenerics, specificMessage);
86+
}
87+
88+
private static void addQuickFix(PreciseIssue issue, SubscriptionExpression expression, String preferredGenerics, String message) {
89+
// Ignoring quick fix if the change would require an import
90+
if (!preferredGenerics.contains(".")) {
91+
PythonQuickFix quickFix = PythonQuickFix.newQuickFix(message)
92+
.addTextEdit(
93+
TextEditUtils.replaceRange(expression.firstToken(), expression.leftBracket(), preferredGenerics + "["))
94+
.build();
95+
issue.addQuickFix(quickFix);
96+
}
97+
}
98+
99+
private static Optional<String> getGenericsCounterPartFromTypingModule(SubscriptionContext context, SubscriptionExpression expression) {
100+
// Recursive check on nested types
101+
expression.subscripts().expressions()
102+
.forEach(nestedExpression -> checkForGenericsFromTypingModule(context, nestedExpression));
103+
return Optional.of(expression.object())
104+
.map(HasSymbol.class::cast)
105+
.map(HasSymbol::symbol)
106+
.flatMap(BuiltinGenericsOverTypingModuleCheck::getBuiltinGenericsType);
89107
}
90108

91109
private static Optional<String> getBuiltinGenericsType(@Nullable Symbol maybeSymbol) {

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

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,12 @@
2020
package org.sonar.python.checks;
2121

2222
import org.junit.Test;
23+
import org.sonar.python.checks.quickfix.PythonQuickFixVerifier;
2324
import org.sonar.python.checks.utils.PythonCheckVerifier;
2425

26+
import static org.sonar.python.checks.BuiltinGenericsOverTypingModuleCheck.MESSAGE;
27+
import static org.sonar.python.checks.utils.CodeTestUtils.code;
28+
2529
public class BuiltinGenericsOverTypingModuleCheckTest {
2630

2731
@Test
@@ -36,4 +40,135 @@ public void checkCollections() {
3640
new BuiltinGenericsOverTypingModuleCheck());
3741
}
3842

43+
@Test
44+
public void quick_fix_generics_in_param() {
45+
String codeWithIssue = code(
46+
"from typing import List",
47+
"def foo(test: List[int]) -> str:",
48+
" return \"bar\"");
49+
String codeFixed = code(
50+
"from typing import List",
51+
"def foo(test: list[int]) -> str:",
52+
" return \"bar\"");
53+
BuiltinGenericsOverTypingModuleCheck check = new BuiltinGenericsOverTypingModuleCheck();
54+
PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
55+
String specificMessage = String.format(MESSAGE, "list");
56+
PythonQuickFixVerifier.verifyQuickFixMessages(check, codeWithIssue, specificMessage);
57+
}
58+
59+
@Test
60+
public void quick_fix_generics_in_return_type() {
61+
String codeWithIssue = code(
62+
"from typing import Dict",
63+
"def foo(test: list[int]) -> Dict[str, int]:",
64+
" return {}");
65+
String codeFixed = code(
66+
"from typing import Dict",
67+
"def foo(test: list[int]) -> dict[str, int]:",
68+
" return {}");
69+
BuiltinGenericsOverTypingModuleCheck check = new BuiltinGenericsOverTypingModuleCheck();
70+
PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
71+
String specificMessage = String.format(MESSAGE, "dict");
72+
PythonQuickFixVerifier.verifyQuickFixMessages(check, codeWithIssue, specificMessage);
73+
}
74+
75+
@Test
76+
public void quick_fix_in_return_type_multiline() {
77+
String codeWithIssue = code(
78+
"from typing import Dict",
79+
"def foo(test: list[int]) -> Dict[str,",
80+
" int]:",
81+
" return {}");
82+
String codeFixed = code(
83+
"from typing import Dict",
84+
"def foo(test: list[int]) -> dict[str,",
85+
" int]:",
86+
" return {}");
87+
BuiltinGenericsOverTypingModuleCheck check = new BuiltinGenericsOverTypingModuleCheck();
88+
PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
89+
String specificMessage = String.format(MESSAGE, "dict");
90+
PythonQuickFixVerifier.verifyQuickFixMessages(check, codeWithIssue, specificMessage);
91+
}
92+
93+
@Test
94+
public void quick_fix_generics_in_variable() {
95+
String codeWithIssue = code(
96+
"from typing import Set",
97+
"def foo():",
98+
" my_var: Set[str]",
99+
" return my_var");
100+
String codeFixed = code(
101+
"from typing import Set",
102+
"def foo():",
103+
" my_var: set[str]",
104+
" return my_var");
105+
BuiltinGenericsOverTypingModuleCheck check = new BuiltinGenericsOverTypingModuleCheck();
106+
PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
107+
String specificMessage = String.format(MESSAGE, "set");
108+
PythonQuickFixVerifier.verifyQuickFixMessages(check, codeWithIssue, specificMessage);
109+
}
110+
111+
@Test
112+
public void quick_fix_fully_qualified() {
113+
String codeWithIssue = code(
114+
"import typing",
115+
"def foo():",
116+
" my_var: typing.Set[int]",
117+
" return my_var");
118+
String codeFixed = code(
119+
"import typing",
120+
"def foo():",
121+
" my_var: set[int]",
122+
" return my_var");
123+
BuiltinGenericsOverTypingModuleCheck check = new BuiltinGenericsOverTypingModuleCheck();
124+
PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
125+
String specificMessage = String.format(MESSAGE, "set");
126+
PythonQuickFixVerifier.verifyQuickFixMessages(check, codeWithIssue, specificMessage);
127+
}
128+
129+
@Test
130+
public void no_quick_fix_for_change_requiring_import() {
131+
String codeWithIssue = code(
132+
"from typing import Iterable",
133+
"def foo(test: Iterable[int]) -> dict[str, int]:",
134+
" return {}");
135+
BuiltinGenericsOverTypingModuleCheck check = new BuiltinGenericsOverTypingModuleCheck();
136+
PythonQuickFixVerifier.verifyNoQuickFixes(check, codeWithIssue);
137+
}
138+
139+
@Test
140+
public void quick_fix_nested_types() {
141+
String codeWithIssue = code(
142+
"from typing import Dict",
143+
"def foo() -> tuple[str, Dict[int]]:",
144+
" return (\"foo\", list())");
145+
String codeFixed = code(
146+
"from typing import Dict",
147+
"def foo() -> tuple[str, dict[int]]:",
148+
" return (\"foo\", list())");
149+
BuiltinGenericsOverTypingModuleCheck check = new BuiltinGenericsOverTypingModuleCheck();
150+
PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
151+
String specificMessage = String.format(MESSAGE, "dict");
152+
PythonQuickFixVerifier.verifyQuickFixMessages(check, codeWithIssue, specificMessage);
153+
}
154+
155+
@Test
156+
public void quick_fix_nested_function() {
157+
String codeWithIssue = code(
158+
"from typing import Tuple",
159+
"def foo() -> tuple[str, int]:",
160+
" def bar(test: Tuple[str, int]) -> str:",
161+
" return \"bar\"",
162+
" return (\"foo\", list())");
163+
String codeFixed = code(
164+
"from typing import Tuple",
165+
"def foo() -> tuple[str, int]:",
166+
" def bar(test: tuple[str, int]) -> str:",
167+
" return \"bar\"",
168+
" return (\"foo\", list())");
169+
BuiltinGenericsOverTypingModuleCheck check = new BuiltinGenericsOverTypingModuleCheck();
170+
PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
171+
String specificMessage = String.format(MESSAGE, "tuple");
172+
PythonQuickFixVerifier.verifyQuickFixMessages(check, codeWithIssue, specificMessage);
173+
}
39174
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ def with_var() -> set[str]:
7272
my_var: frozenset[int] = None
7373
pass
7474

75+
def without_type_arg() -> typing.Set:
76+
pass
7577

7678
class CheckCollections:
7779
class_var: typing.Iterable[str] # Noncompliant {{Use the built-in generic type `collections.abc.Iterable` instead of its typing counterpart.}}
@@ -131,4 +133,5 @@ def nested(m:ABCMapping[Type[str], int]): # Noncompliant
131133

132134
def success():
133135
my_var: type[int] = None
136+
other_var: Dict
134137
pass

0 commit comments

Comments
 (0)