Skip to content

Commit 8cd361f

Browse files
ghislainpiotsonartech
ghislainpiot
authored and
sonartech
committed
SONARPY-2957 Implement quickfix for S7491 (#291)
GitOrigin-RevId: 7a1c5dcd6e03e7e0c883c05ca7d7ef17bd01b8cc
1 parent d3b703d commit 8cd361f

5 files changed

Lines changed: 469 additions & 249 deletions

File tree

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

Lines changed: 75 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,38 +16,71 @@
1616
*/
1717
package org.sonar.python.checks;
1818

19+
import static org.sonar.python.checks.hotspots.CommonValidationUtils.isEqualTo;
20+
21+
import java.util.HashMap;
22+
import java.util.List;
1923
import java.util.Map;
24+
import java.util.Optional;
25+
2026
import org.sonar.check.Rule;
2127
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
2228
import org.sonar.plugins.python.api.SubscriptionContext;
29+
import org.sonar.plugins.python.api.quickfix.PythonQuickFix;
30+
import org.sonar.plugins.python.api.tree.AliasedName;
2331
import org.sonar.plugins.python.api.tree.CallExpression;
32+
import org.sonar.plugins.python.api.tree.ImportName;
33+
import org.sonar.plugins.python.api.tree.Name;
2434
import org.sonar.plugins.python.api.tree.Tree;
2535
import org.sonar.plugins.python.api.tree.Tree.Kind;
36+
import org.sonar.python.quickfix.TextEditUtils;
2637
import org.sonar.python.tree.TreeUtils;
2738
import org.sonar.python.types.v2.TypeCheckMap;
2839

29-
import static org.sonar.python.checks.hotspots.CommonValidationUtils.isEqualTo;
30-
3140
@Rule(key = "S7491")
3241
public class SleepZeroInAsyncCheck extends PythonSubscriptionCheck {
3342

3443
private static final String MESSAGE = "Use %s instead of %s.";
3544
private static final String SECONDARY_MESSAGE = "This function is async.";
36-
45+
private static final String QUICK_FIX_MESSAGE = "Replace with %s";
46+
3747
private TypeCheckMap<MessageHolder> asyncSleepFunctions;
48+
private final Map<String, String> asyncLibraryAliases = new HashMap<>();
3849

3950
@Override
4051
public void initialize(Context context) {
41-
context.registerSyntaxNodeConsumer(Kind.FILE_INPUT, this::initializeTypeCheckMap);
52+
context.registerSyntaxNodeConsumer(Kind.FILE_INPUT, this::initializeAnalysis);
53+
context.registerSyntaxNodeConsumer(Kind.IMPORT_NAME, this::handleImportName);
4254
context.registerSyntaxNodeConsumer(Kind.CALL_EXPR, this::checkCallExpr);
4355
}
4456

45-
private void initializeTypeCheckMap(SubscriptionContext context) {
57+
private void initializeAnalysis(SubscriptionContext context) {
4658
asyncSleepFunctions = TypeCheckMap.ofEntries(
4759
Map.entry(context.typeChecker().typeCheckBuilder().isTypeWithFqn("trio.sleep"),
48-
new MessageHolder("trio.sleep", "trio.lowlevel.checkpoint()", "seconds")),
60+
new MessageHolder("trio.sleep", "%s.lowlevel.checkpoint()", "seconds", "trio")),
4961
Map.entry(context.typeChecker().typeCheckBuilder().isTypeOrInstanceWithName("anyio.sleep"),
50-
new MessageHolder("anyio.sleep", "anyio.lowlevel.checkpoint()", "delay")));
62+
new MessageHolder("anyio.sleep", "%s.lowlevel.checkpoint()", "delay", "anyio")));
63+
64+
asyncLibraryAliases.clear();
65+
}
66+
67+
private void handleImportName(SubscriptionContext context) {
68+
var importName = (ImportName) context.syntaxNode();
69+
importName.modules().forEach(this::trackModuleImport);
70+
}
71+
72+
private void trackModuleImport(AliasedName aliasedName) {
73+
List<Name> names = aliasedName.dottedName().names();
74+
if (names.size() > 1) {
75+
return;
76+
}
77+
78+
String moduleName = names.get(0).name();
79+
if ("trio".equals(moduleName) || "anyio".equals(moduleName)) {
80+
Name alias = aliasedName.alias();
81+
String moduleAlias = alias != null ? alias.name() : moduleName;
82+
asyncLibraryAliases.put(moduleName, moduleAlias);
83+
}
5184
}
5285

5386
private void checkCallExpr(SubscriptionContext context) {
@@ -59,26 +92,46 @@ private void checkCallExpr(SubscriptionContext context) {
5992

6093
var callee = callExpr.callee();
6194
asyncSleepFunctions.getOptionalForType(callee.typeV2()).ifPresent(
62-
messageHolder -> messageHolder.handleCallExpr(context, callExpr, asyncKeyword));
95+
messageHolder -> handleCallExpr(context, messageHolder, callExpr, asyncKeyword, asyncLibraryAliases));
96+
}
97+
98+
private static void handleCallExpr(SubscriptionContext context, MessageHolder messageHolder, CallExpression callExpr, Tree asyncKeyword,
99+
Map<String, String> asyncLibraryAliases) {
100+
if (!isZero(callExpr, messageHolder.keywordArgumentName())) {
101+
return;
102+
}
103+
104+
var moduleAlias = asyncLibraryAliases.getOrDefault(messageHolder.libraryName(), messageHolder.libraryName());
105+
var formattedReplacement = messageHolder.replacement().formatted(moduleAlias);
106+
107+
var message = String.format(MESSAGE, formattedReplacement, messageHolder.fqn());
108+
var issue = context.addIssue(callExpr, message);
109+
issue.secondary(asyncKeyword, SECONDARY_MESSAGE);
110+
111+
createQuickFix(messageHolder, callExpr, formattedReplacement, asyncLibraryAliases).ifPresent(issue::addQuickFix);
63112
}
64113

65-
record MessageHolder(String fqn, String replacement, String keywordArgumentName) {
66-
public void handleCallExpr(SubscriptionContext ctx, CallExpression callExpr, Tree asyncKeyword) {
67-
if (!isZero(callExpr, keywordArgumentName)) {
68-
return;
69-
}
70-
var message = String.format(MESSAGE, replacement, fqn);
71-
var issue = ctx.addIssue(callExpr, message);
72-
issue.secondary(asyncKeyword, SECONDARY_MESSAGE);
114+
private static Optional<PythonQuickFix> createQuickFix(MessageHolder messageHolder, CallExpression callExpr, String formattedReplacement,
115+
Map<String, String> asyncLibraryAliases) {
116+
if (!asyncLibraryAliases.containsKey(messageHolder.libraryName())) {
117+
return Optional.empty();
73118
}
74119

75-
private static boolean isZero(CallExpression callExpr, String keywordArgumentName) {
76-
var argument = TreeUtils.nthArgumentOrKeyword(0, keywordArgumentName, callExpr.arguments());
77-
if (argument == null) {
78-
return false;
79-
}
80-
return isEqualTo(argument.expression(), 0);
120+
var quickFixMessage = String.format(QUICK_FIX_MESSAGE, formattedReplacement);
121+
var quickFix = PythonQuickFix.newQuickFix(quickFixMessage)
122+
.addTextEdit(TextEditUtils.replace(callExpr, formattedReplacement))
123+
.build();
124+
return Optional.of(quickFix);
125+
}
126+
127+
private static boolean isZero(CallExpression callExpr, String keywordArgumentName) {
128+
var argument = TreeUtils.nthArgumentOrKeyword(0, keywordArgumentName, callExpr.arguments());
129+
if (argument == null) {
130+
return false;
81131
}
132+
return isEqualTo(argument.expression(), 0);
133+
}
82134

135+
record MessageHolder(String fqn, String replacement, String keywordArgumentName, String libraryName) {
83136
}
84137
}

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

Lines changed: 164 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,172 @@
1616
*/
1717
package org.sonar.python.checks;
1818

19+
import java.util.stream.Stream;
20+
1921
import org.junit.jupiter.api.Test;
22+
import org.junit.jupiter.params.ParameterizedTest;
23+
import org.junit.jupiter.params.provider.Arguments;
24+
import org.junit.jupiter.params.provider.MethodSource;
25+
import org.sonar.python.checks.quickfix.PythonQuickFixVerifier;
2026
import org.sonar.python.checks.utils.PythonCheckVerifier;
2127

2228
class SleepZeroInAsyncCheckTest {
23-
@Test
24-
void test() {
25-
PythonCheckVerifier.verify("src/test/resources/checks/sleepZeroInAsync.py", new SleepZeroInAsyncCheck());
26-
}
29+
@Test
30+
void testTrio() {
31+
PythonCheckVerifier.verify("src/test/resources/checks/sleepZeroInAsyncTrio.py", new SleepZeroInAsyncCheck());
32+
}
33+
34+
@Test
35+
void testAnyio() {
36+
PythonCheckVerifier.verify("src/test/resources/checks/sleepZeroInAsyncAnyio.py", new SleepZeroInAsyncCheck());
37+
}
38+
39+
@ParameterizedTest
40+
@MethodSource("quickFixTestCases")
41+
void quickFixTest(String testName, String before, String after, String expectedMessage) {
42+
var check = new SleepZeroInAsyncCheck();
43+
PythonQuickFixVerifier.verify(check, before, after);
44+
PythonQuickFixVerifier.verifyQuickFixMessages(check, before, expectedMessage);
45+
}
46+
47+
@ParameterizedTest
48+
@MethodSource("noQuickFixTestCases")
49+
void noQuickFixTest(String testName, String before) {
50+
var check = new SleepZeroInAsyncCheck();
51+
PythonQuickFixVerifier.verifyNoQuickFixes(check, before);
52+
}
53+
54+
static Stream<Arguments> quickFixTestCases() {
55+
return Stream.of(
56+
Arguments.of(
57+
"trio sleep with integer zero",
58+
"""
59+
import trio
60+
async def test():
61+
await trio.sleep(0)""",
62+
"""
63+
import trio
64+
async def test():
65+
await trio.lowlevel.checkpoint()""",
66+
"Replace with trio.lowlevel.checkpoint()"),
67+
Arguments.of(
68+
"trio sleep with float zero",
69+
"""
70+
import trio
71+
async def test():
72+
await trio.sleep(0.0)""",
73+
"""
74+
import trio
75+
async def test():
76+
await trio.lowlevel.checkpoint()""",
77+
"Replace with trio.lowlevel.checkpoint()"),
78+
Arguments.of(
79+
"trio sleep with keyword argument",
80+
"""
81+
import trio
82+
async def test():
83+
await trio.sleep(seconds=0)""",
84+
"""
85+
import trio
86+
async def test():
87+
await trio.lowlevel.checkpoint()""",
88+
"Replace with trio.lowlevel.checkpoint()"),
89+
Arguments.of(
90+
"trio sleep with alias",
91+
"""
92+
import trio as t
93+
async def test():
94+
await t.sleep(0)""",
95+
"""
96+
import trio as t
97+
async def test():
98+
await t.lowlevel.checkpoint()""",
99+
"Replace with t.lowlevel.checkpoint()"),
100+
Arguments.of(
101+
"anyio sleep with integer zero",
102+
"""
103+
import anyio
104+
async def test():
105+
await anyio.sleep(0)""",
106+
"""
107+
import anyio
108+
async def test():
109+
await anyio.lowlevel.checkpoint()""",
110+
"Replace with anyio.lowlevel.checkpoint()"),
111+
Arguments.of(
112+
"anyio sleep with float zero",
113+
"""
114+
import anyio
115+
async def test():
116+
await anyio.sleep(0.0)""",
117+
"""
118+
import anyio
119+
async def test():
120+
await anyio.lowlevel.checkpoint()""",
121+
"Replace with anyio.lowlevel.checkpoint()"),
122+
Arguments.of(
123+
"anyio sleep with keyword argument",
124+
"""
125+
import anyio
126+
async def test():
127+
await anyio.sleep(delay=0)""",
128+
"""
129+
import anyio
130+
async def test():
131+
await anyio.lowlevel.checkpoint()""",
132+
"Replace with anyio.lowlevel.checkpoint()"),
133+
Arguments.of(
134+
"anyio sleep with alias",
135+
"""
136+
import anyio as a
137+
async def test():
138+
await a.sleep(0)""",
139+
"""
140+
import anyio as a
141+
async def test():
142+
await a.lowlevel.checkpoint()""",
143+
"Replace with a.lowlevel.checkpoint()"),
144+
Arguments.of(
145+
"multiple libraries imported - trio call",
146+
"""
147+
import trio
148+
import anyio
149+
async def test():
150+
await trio.sleep(0)""",
151+
"""
152+
import trio
153+
import anyio
154+
async def test():
155+
await trio.lowlevel.checkpoint()""",
156+
"Replace with trio.lowlevel.checkpoint()"),
157+
Arguments.of(
158+
"multiple libraries imported - anyio call",
159+
"""
160+
import trio
161+
import anyio
162+
async def test():
163+
await anyio.sleep(0)""",
164+
"""
165+
import trio
166+
import anyio
167+
async def test():
168+
await anyio.lowlevel.checkpoint()""",
169+
"Replace with anyio.lowlevel.checkpoint()"));
170+
}
171+
172+
static Stream<Arguments> noQuickFixTestCases() {
173+
return Stream.of(
174+
Arguments.of(
175+
"trio sleep imported directly",
176+
"""
177+
from trio import sleep
178+
async def test():
179+
await sleep(0)"""),
180+
Arguments.of(
181+
"anyio sleep imported directly",
182+
"""
183+
from anyio import sleep
184+
async def test():
185+
await sleep(0)"""));
186+
}
27187
}

0 commit comments

Comments
 (0)