Skip to content

Commit bf24302

Browse files
ghislainpiotsonartech
authored andcommitted
SONARPY-2922 Rule S7491: Checkpoints should be used instead of sleep(0) (#256)
GitOrigin-RevId: 10b4ed8e60bc4c5668e0901e986ada010c6a9bc9
1 parent 0f208ce commit bf24302

9 files changed

Lines changed: 448 additions & 3 deletions

File tree

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
@@ -338,6 +338,7 @@ public Stream<Class<?>> getChecks() {
338338
SingleCharCharacterClassCheck.class,
339339
SkippedTestNoReasonCheck.class,
340340
SkLearnEstimatorDontInitializeEstimatedValuesCheck.class,
341+
SleepZeroInAsyncCheck.class,
341342
SpecialMethodParamListCheck.class,
342343
SpecialMethodReturnTypeCheck.class,
343344
SQLQueriesCheck.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) 2011-2025 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 Sonar Source-Available License Version 1, as published by SonarSource SA.
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 java.util.Map;
20+
import org.sonar.check.Rule;
21+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
22+
import org.sonar.plugins.python.api.SubscriptionContext;
23+
import org.sonar.plugins.python.api.tree.CallExpression;
24+
import org.sonar.plugins.python.api.tree.Tree;
25+
import org.sonar.plugins.python.api.tree.Tree.Kind;
26+
import org.sonar.python.tree.TreeUtils;
27+
import org.sonar.python.types.v2.TypeCheckMap;
28+
29+
import static org.sonar.python.checks.hotspots.CommonValidationUtils.isEqualTo;
30+
31+
@Rule(key = "S7491")
32+
public class SleepZeroInAsyncCheck extends PythonSubscriptionCheck {
33+
34+
private static final String MESSAGE = "Use %s instead of %s.";
35+
private static final String SECONDARY_MESSAGE = "This function is async.";
36+
37+
private TypeCheckMap<MessageHolder> asyncSleepFunctions;
38+
39+
@Override
40+
public void initialize(Context context) {
41+
context.registerSyntaxNodeConsumer(Kind.FILE_INPUT, this::initializeTypeCheckMap);
42+
context.registerSyntaxNodeConsumer(Kind.CALL_EXPR, this::checkCallExpr);
43+
}
44+
45+
private void initializeTypeCheckMap(SubscriptionContext context) {
46+
asyncSleepFunctions = TypeCheckMap.ofEntries(
47+
Map.entry(context.typeChecker().typeCheckBuilder().isTypeWithFqn("trio.sleep"),
48+
new MessageHolder("trio.sleep", "trio.lowlevel.checkpoint()", "seconds")),
49+
Map.entry(context.typeChecker().typeCheckBuilder().isTypeOrInstanceWithName("anyio.sleep"),
50+
new MessageHolder("anyio.sleep", "anyio.lowlevel.checkpoint()", "delay")));
51+
}
52+
53+
private void checkCallExpr(SubscriptionContext context) {
54+
var callExpr = (CallExpression) context.syntaxNode();
55+
var asyncKeyword = TreeUtils.asyncTokenOfEnclosingFunction(callExpr).orElse(null);
56+
if (asyncKeyword == null) {
57+
return;
58+
}
59+
60+
var callee = callExpr.callee();
61+
asyncSleepFunctions.getOptionalForType(callee.typeV2()).ifPresent(
62+
messageHolder -> messageHolder.handleCallExpr(context, callExpr, asyncKeyword));
63+
}
64+
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);
73+
}
74+
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);
81+
}
82+
83+
}
84+
}

python-checks/src/main/java/org/sonar/python/checks/hotspots/CommonValidationUtils.java

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,29 @@ static boolean isLessThanExponent(Expression expression, int exponent) {
6060
return false;
6161
}
6262

63-
static boolean isEqualTo(Expression expression, int number) {
63+
public static boolean isEqualTo(Expression expression, int number) {
6464
try {
6565
if (expression.is(Tree.Kind.NAME)) {
6666
return Expressions.singleAssignedNonNameValue(((Name) expression)).map(value -> isEqualTo(value, number)).orElse(false);
6767
}
68-
return expression.is(Tree.Kind.NUMERIC_LITERAL) && ((NumericLiteral) expression).valueAsLong() == number;
68+
return expression.is(Tree.Kind.NUMERIC_LITERAL)
69+
&& (isNumericLiteralEqualToInt((NumericLiteral) expression, number) || isNumericLiteralEqualToDouble((NumericLiteral) expression, number));
70+
} catch (NumberFormatException nfe) {
71+
return false;
72+
}
73+
}
74+
75+
private static boolean isNumericLiteralEqualToInt(NumericLiteral numericLiteral, int number) {
76+
try {
77+
return numericLiteral.valueAsLong() == number;
78+
} catch (NumberFormatException nfe) {
79+
return false;
80+
}
81+
}
82+
83+
private static boolean isNumericLiteralEqualToDouble(NumericLiteral numericLiteral, double number) {
84+
try {
85+
return Double.parseDouble(numericLiteral.valueAsString()) == number;
6986
} catch (NumberFormatException nfe) {
7087
return false;
7188
}
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
<p>This rule raises issues when <code>trio.sleep(0)</code> or <code>anyio.sleep(0)</code> is used instead of the more explicit and descriptive
2+
<code>trio.lowlevel.checkpoint()</code> or <code>anyio.lowlevel.checkpoint()</code>.</p>
3+
<h2>Why is this an issue?</h2>
4+
<p>When using async libraries like <code>trio</code> or <code>anyio</code>, developers sometimes use <code>sleep(0)</code> as a technique to yield
5+
control back to the event loop, allowing other pending tasks to run. While this approach technically works, it obscures the actual intent.</p>
6+
<p>The <code>sleep(0)</code> pattern is confusing because it suggests waiting for a specific duration (which happens to be 0 seconds), when the actual
7+
intention is to create a checkpoint in the code where the current task can yield control to other tasks without any intended delay.</p>
8+
<p>Both <code>trio</code> and <code>anyio</code> provide dedicated functions for this exact purpose: <code>trio.lowlevel.checkpoint()</code> and
9+
<code>anyio.lowlevel.checkpoint()</code>. These functions clearly communicate the intent to create a checkpoint for task switching.</p>
10+
<h3>What is the potential impact?</h3>
11+
<ul>
12+
<li> <strong>Readability</strong>: Code using <code>sleep(0)</code> is less self-documenting and can be confusing to other developers who need to
13+
know the non-obvious intent of the code. </li>
14+
<li> <strong>Maintainability</strong>: The intent behind <code>sleep(0)</code> is not immediately clear to developers reviewing or maintaining the
15+
code. </li>
16+
</ul>
17+
<h2>How to fix it in Trio</h2>
18+
<p>Replace <code>trio.sleep(0)</code> with <code>trio.lowlevel.checkpoint()</code>.</p>
19+
<h3>Code examples</h3>
20+
<h4>Noncompliant code example</h4>
21+
<pre data-diff-id="1" data-diff-type="noncompliant">
22+
import trio
23+
24+
async def main():
25+
await trio.sleep(0) # Noncompliant
26+
27+
trio.run(main)
28+
</pre>
29+
<h4>Compliant solution</h4>
30+
<pre data-diff-id="1" data-diff-type="compliant">
31+
import trio
32+
33+
async def main():
34+
await trio.lowlevel.checkpoint() # Compliant
35+
36+
trio.run(main)
37+
</pre>
38+
<h2>How to fix it in AnyIO</h2>
39+
<p>Replace <code>anyio.sleep(0)</code> with <code>anyio.lowlevel.checkpoint()</code>.</p>
40+
<h3>Code examples</h3>
41+
<h4>Noncompliant code example</h4>
42+
<pre data-diff-id="2" data-diff-type="noncompliant">
43+
import anyio
44+
45+
async def main():
46+
await anyio.sleep(0) # Noncompliant
47+
48+
anyio.run(main)
49+
</pre>
50+
<h4>Compliant solution</h4>
51+
<pre data-diff-id="2" data-diff-type="compliant">
52+
import anyio
53+
54+
async def main():
55+
await anyio.lowlevel.checkpoint() # Compliant
56+
57+
anyio.run(main)
58+
</pre>
59+
<h2>Resources</h2>
60+
<h3>Documentation</h3>
61+
<ul>
62+
<li> Trio Documentation - <a
63+
href="https://trio.readthedocs.io/en/stable/reference-lowlevel.html#trio.lowlevel.checkpoint">trio.lowlevel.checkpoint</a> </li>
64+
<li> AnyIO Documentation - <a href="https://anyio.readthedocs.io/en/stable/api.html#anyio.lowlevel.checkpoint">anyio.lowlevel.checkpoint</a> </li>
65+
</ul>
66+
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
{
2+
"title": "Checkpoints should be used instead of sleep(0)",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [
10+
"async",
11+
"trio",
12+
"anyio"
13+
],
14+
"defaultSeverity": "Minor",
15+
"ruleSpecification": "RSPEC-7491",
16+
"sqKey": "S7491",
17+
"scope": "All",
18+
"quickfix": "unknown",
19+
"code": {
20+
"impacts": {
21+
"MAINTAINABILITY": "LOW"
22+
},
23+
"attribute": "CLEAR"
24+
}
25+
}

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
@@ -254,6 +254,7 @@
254254
"S6983",
255255
"S6984",
256256
"S6985",
257-
"S7488"
257+
"S7488",
258+
"S7491"
258259
]
259260
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 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 Sonar Source-Available License Version 1, as published by SonarSource SA.
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 SleepZeroInAsyncCheckTest {
23+
@Test
24+
void test() {
25+
PythonCheckVerifier.verify("src/test/resources/checks/sleepZeroInAsync.py", new SleepZeroInAsyncCheck());
26+
}
27+
}

python-checks/src/test/resources/checks/commonValidationUtils.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
callExpr(12, 0)
1313
callExpr(12, 10) # Noncompliant {{Argument is equal to 10}}
14+
callExpr(12, 10.0) # Noncompliant {{Argument is equal to 10}}
1415
ten = 10
1516
callExpr(isEqualTo=ten) # Noncompliant {{Argument is equal to 10}}
1617
not_ten = 11

0 commit comments

Comments
 (0)