Skip to content

Commit eb180b9

Browse files
guillaume-dequennesonartech
authored andcommitted
SONARPY-2939 Rule S7503: Async functions should use async features (#262)
GitOrigin-RevId: 32d1c0f183dbb7a398b72bc5968bc15300fbeded
1 parent 2c1ea64 commit eb180b9

9 files changed

Lines changed: 371 additions & 3 deletions

File tree

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
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 javax.annotation.Nullable;
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.AwaitExpression;
24+
import org.sonar.plugins.python.api.tree.BaseTreeVisitor;
25+
import org.sonar.plugins.python.api.tree.ForStatement;
26+
import org.sonar.plugins.python.api.tree.FunctionDef;
27+
import org.sonar.plugins.python.api.tree.Statement;
28+
import org.sonar.plugins.python.api.tree.StatementList;
29+
import org.sonar.plugins.python.api.tree.Token;
30+
import org.sonar.plugins.python.api.tree.Tree;
31+
import org.sonar.plugins.python.api.tree.WithStatement;
32+
import org.sonar.plugins.python.api.tree.YieldExpression;
33+
import org.sonar.plugins.python.api.tree.YieldStatement;
34+
import org.sonar.python.checks.utils.CheckUtils;
35+
36+
@Rule(key = "S7503")
37+
public class AsyncFunctionNotAsyncCheck extends PythonSubscriptionCheck {
38+
39+
private static final String MESSAGE = "Use asynchronous features in this function or remove the `async` keyword.";
40+
41+
@Override
42+
public void initialize(Context context) {
43+
context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, AsyncFunctionNotAsyncCheck::checkAsyncFunction);
44+
}
45+
46+
private static void checkAsyncFunction(SubscriptionContext ctx) {
47+
FunctionDef functionDef = (FunctionDef) ctx.syntaxNode();
48+
49+
Token asyncKeyword = functionDef.asyncKeyword();
50+
if (asyncKeyword == null || isException(functionDef)) {
51+
return;
52+
}
53+
AsyncFeatureVisitor visitor = new AsyncFeatureVisitor();
54+
functionDef.body().accept(visitor);
55+
56+
if (!visitor.hasAsyncFeature()) {
57+
ctx.addIssue(functionDef.name(), MESSAGE).secondary(asyncKeyword, "This function is async.");
58+
}
59+
}
60+
61+
private static boolean isException(FunctionDef functionDef) {
62+
return CheckUtils.isAbstract(functionDef) || isEmptyFunction(functionDef.body());
63+
}
64+
65+
private static boolean isEmptyFunction(StatementList body) {
66+
for (Statement statement : body.statements()) {
67+
if (!CheckUtils.isEmptyStatement(statement)) {
68+
return false;
69+
}
70+
}
71+
return true;
72+
}
73+
74+
private static class AsyncFeatureVisitor extends BaseTreeVisitor {
75+
76+
private boolean asyncFeatureFound = false;
77+
78+
public boolean hasAsyncFeature() {
79+
return asyncFeatureFound;
80+
}
81+
82+
@Override
83+
public void visitAwaitExpression(AwaitExpression awaitExpression) {
84+
asyncFeatureFound = true;
85+
}
86+
87+
@Override
88+
public void visitForStatement(ForStatement forStatement) {
89+
if (forStatement.isAsync()) {
90+
asyncFeatureFound = true;
91+
return;
92+
}
93+
if (!asyncFeatureFound) {
94+
super.visitForStatement(forStatement);
95+
}
96+
}
97+
98+
@Override
99+
public void visitWithStatement(WithStatement withStatement) {
100+
if (withStatement.isAsync()) {
101+
asyncFeatureFound = true;
102+
}
103+
if (!asyncFeatureFound) {
104+
super.visitWithStatement(withStatement);
105+
}
106+
}
107+
108+
@Override
109+
public void visitYieldStatement(YieldStatement yieldStatement) {
110+
asyncFeatureFound = true;
111+
}
112+
113+
@Override
114+
public void visitYieldExpression(YieldExpression yieldExpression) {
115+
asyncFeatureFound = true;
116+
}
117+
118+
@Override
119+
public void visitFunctionDef(FunctionDef functionDef) {
120+
// Skip nested functions
121+
}
122+
123+
@Override
124+
protected void scan(@Nullable Tree tree) {
125+
// Stop scanning if we've already found an async feature
126+
if (!asyncFeatureFound && tree != null) {
127+
tree.accept(this);
128+
}
129+
}
130+
}
131+
}

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
@@ -121,6 +121,7 @@ public Stream<Class<?>> getChecks() {
121121
AssertOnDissimilarTypesCheck.class,
122122
AssertAfterRaiseCheck.class,
123123
AssertOnTupleLiteralCheck.class,
124+
AsyncFunctionNotAsyncCheck.class,
124125
AsyncFunctionWithTimeoutCheck.class,
125126
AsyncLongSleepCheck.class,
126127
BackslashInStringCheck.class,

python-checks/src/main/java/org/sonar/python/checks/utils/CheckUtils.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,14 @@
2626
import org.sonar.plugins.python.api.tree.ClassDef;
2727
import org.sonar.plugins.python.api.tree.DictionaryLiteral;
2828
import org.sonar.plugins.python.api.tree.Expression;
29+
import org.sonar.plugins.python.api.tree.ExpressionStatement;
2930
import org.sonar.plugins.python.api.tree.FunctionDef;
3031
import org.sonar.plugins.python.api.tree.ListLiteral;
3132
import org.sonar.plugins.python.api.tree.Name;
3233
import org.sonar.plugins.python.api.tree.Parameter;
3334
import org.sonar.plugins.python.api.tree.ParameterList;
3435
import org.sonar.plugins.python.api.tree.SetLiteral;
36+
import org.sonar.plugins.python.api.tree.Statement;
3537
import org.sonar.plugins.python.api.tree.Tree;
3638
import org.sonar.plugins.python.api.tree.Tuple;
3739
import org.sonar.plugins.python.api.types.BuiltinTypes;
@@ -180,6 +182,19 @@ public static boolean isAbstract(FunctionDef funDef) {
180182
.anyMatch(foundDeco -> ABC_ABSTRACTMETHOD_DECORATORS.stream().anyMatch(abcDeco -> abcDeco.equals(foundDeco)));
181183
}
182184

185+
public static boolean isEmptyStatement(Statement statement) {
186+
return statement.is(Tree.Kind.PASS_STMT) ||
187+
(statement.is(Tree.Kind.EXPRESSION_STMT) && isStringLiteralOrEllipsis((ExpressionStatement) statement));
188+
}
189+
190+
private static boolean isStringLiteralOrEllipsis(ExpressionStatement statement) {
191+
Tree expression = statement.expressions().get(0);
192+
if (expression.is(STRING_LITERAL)) {
193+
return true;
194+
}
195+
return expression.is(Tree.Kind.ELLIPSIS);
196+
}
197+
183198
/**
184199
* Simple check whether the given expression is the "self" name expression.
185200
*
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<p>This rule raises an issue when a function is declared async but does not use any asynchronous features.</p>
2+
<h2>Why is this an issue?</h2>
3+
<p>Declaring a function with <code>async def</code> signals that it’s a coroutine, typically because it needs to use <code>await</code> for
4+
non-blocking operations (like I/O), or employs other asynchronous features like <code>async for</code> or <code>async with</code>.</p>
5+
<p>If none of these asynchronous mechanisms are utilized within the function’s body, the <code>async</code> declaration is superfluous.</p>
6+
<p>Using <code>async</code> unnecessarily can:</p>
7+
<ul>
8+
<li> <strong>Reduce Code Clarity:</strong> It misleads developers into thinking the function performs asynchronous operations when it doesn’t,
9+
making the codebase harder to understand and reason about. </li>
10+
<li> <strong>Introduce Minor Overhead:</strong> Calling an <code>async def</code> function creates a coroutine object. While modern Python is
11+
efficient, this still incurs more overhead than a direct synchronous function call if the asynchronicity isn’t actually leveraged. The function’s
12+
body will execute synchronously until an <code>await</code> (if any) is encountered. If there’s no <code>await</code> or other async feature, it
13+
essentially runs synchronously but wrapped as a coroutine. </li>
14+
<li> <strong>Complicate Usage:</strong> Callers must use <code>await</code> (or schedule it as a task) to execute the coroutine and get its result.
15+
This is an unnecessary ceremony if the function itself isn’t truly asynchronous. </li>
16+
</ul>
17+
<h2>How to fix it</h2>
18+
<p>Either remove the <code>async</code> keyword, or start using the appropriate asynchronous features.</p>
19+
<h3>Code examples</h3>
20+
<h4>Noncompliant code example</h4>
21+
<pre data-diff-id="1" data-diff-type="noncompliant">
22+
async def my_function(): # Noncompliant
23+
print("Hello from my function")
24+
</pre>
25+
<h4>Compliant solution</h4>
26+
<pre data-diff-id="1" data-diff-type="compliant">
27+
def my_function(): # Compliant
28+
print("Hello from my function")
29+
</pre>
30+
<h4>Noncompliant code example</h4>
31+
<pre data-diff-id="2" data-diff-type="noncompliant">
32+
def another_function(): ...
33+
34+
async def my_function(): # Noncompliant
35+
return another_function()
36+
</pre>
37+
<h4>Compliant solution</h4>
38+
<pre data-diff-id="2" data-diff-type="compliant">
39+
async def another_function(): ...
40+
41+
async def my_function(): # Compliant
42+
return await another_function()
43+
</pre>
44+
<h2>Resources</h2>
45+
<h3>Articles &amp; blog posts</h3>
46+
<ul>
47+
<li> <a href="https://journal.stuffwithstuff.com/2015/02/01/what-color-is-your-function/">What Color Is Your Function? - Bob Nystrom</a> </li>
48+
</ul>
49+
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
"title": "Async functions should use async features",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [
10+
"async"
11+
],
12+
"defaultSeverity": "Minor",
13+
"ruleSpecification": "RSPEC-7503",
14+
"sqKey": "S7503",
15+
"scope": "All",
16+
"quickfix": "unknown",
17+
"code": {
18+
"impacts": {
19+
"MAINTAINABILITY": "MEDIUM",
20+
"RELIABILITY": "LOW"
21+
},
22+
"attribute": "CONVENTIONAL"
23+
}
24+
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,9 @@
265265
"S7501",
266266
"S7506",
267267
"S7510",
268-
"S7511"
268+
"S7511",
269+
"S7510",
270+
"S7501",
271+
"S7503"
269272
]
270273
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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 AsyncFunctionNotAsyncCheckTest {
23+
@Test
24+
void test() {
25+
PythonCheckVerifier.verify("src/test/resources/checks/asyncFunctionNotAsync.py", new AsyncFunctionNotAsyncCheck());
26+
}
27+
28+
}

python-checks/src/test/java/org/sonar/python/checks/utils/CheckUtilsTest.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,14 @@ void lambda_equivalence() {
9898
assertThat(CheckUtils.areEquivalent(parse("x = lambda a : a + 10"), parse("x = lambda a : a + 5"))).isFalse();
9999
}
100100

101+
@Test
102+
void isEmptyStatement() {
103+
assertThat(CheckUtils.isEmptyStatement(parse("pass").statements().statements().get(0))).isTrue();
104+
assertThat(CheckUtils.isEmptyStatement(parse("...").statements().statements().get(0))).isTrue();
105+
assertThat(CheckUtils.isEmptyStatement(parse("\"hello\"").statements().statements().get(0))).isTrue();
106+
assertThat(CheckUtils.isEmptyStatement(parse("x = \"hello\"").statements().statements().get(0))).isFalse();
107+
}
108+
101109
@Test
102110
void no_parent_class() {
103111
FileInput file = (FileInput) parse("" +
@@ -211,7 +219,7 @@ void findFirstParameterSymbolTest() throws IOException {
211219
}
212220
}
213221

214-
private static Tree parse(String content) {
222+
private static FileInput parse(String content) {
215223
PythonParser parser = PythonParser.create();
216224
AstNode astNode = parser.parse(content);
217225
FileInput parse = new PythonTreeMaker().fileInput(astNode);
@@ -220,7 +228,7 @@ private static Tree parse(String content) {
220228

221229
private static FileInput parseFile(String path) throws IOException {
222230
try (var sourceFile = new Scanner(new File(path)).useDelimiter("\\Z")) {
223-
return (FileInput) parse(sourceFile.next());
231+
return parse(sourceFile.next());
224232
}
225233
}
226234

0 commit comments

Comments
 (0)