Skip to content

Commit ab9864d

Browse files
ghislainpiotsonartech
ghislainpiot
authored and
sonartech
committed
SONARPY-2997 Rule S7513: TaskGroup/Nursery should not be used for a single start call (#313)
GitOrigin-RevId: ef7116e6b6c1c98b7ffafb1e1a6965f69fd06487
1 parent e126b7f commit ab9864d

File tree

7 files changed

+514
-0
lines changed

7 files changed

+514
-0
lines changed

‎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
@@ -377,6 +377,7 @@ public Stream<Class<?>> getChecks() {
377377
SynchronousOsCallsInAsyncCheck.class,
378378
TempFileCreationCheck.class,
379379
ImplicitlySkippedTestCheck.class,
380+
TaskGroupNurseryUsedOnlyOnceCheck.class,
380381
TimeSleepInAsyncCheck.class,
381382
ToDoCommentCheck.class,
382383
TooManyLinesInFileCheck.class,

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

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
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+
18+
package org.sonar.python.checks;
19+
20+
import java.util.Optional;
21+
import java.util.Set;
22+
23+
import org.sonar.check.Rule;
24+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
25+
import org.sonar.plugins.python.api.SubscriptionContext;
26+
import org.sonar.plugins.python.api.tree.CallExpression;
27+
import org.sonar.plugins.python.api.tree.Name;
28+
import org.sonar.plugins.python.api.tree.QualifiedExpression;
29+
import org.sonar.plugins.python.api.tree.Tree;
30+
import org.sonar.plugins.python.api.tree.WithItem;
31+
import org.sonar.plugins.python.api.tree.WithStatement;
32+
import org.sonar.plugins.python.api.types.v2.TriBool;
33+
import org.sonar.python.semantic.v2.SymbolV2;
34+
import org.sonar.python.tree.TreeUtils;
35+
import org.sonar.python.types.v2.TypeCheckBuilder;
36+
37+
@Rule(key = "S7513")
38+
public class TaskGroupNurseryUsedOnlyOnceCheck extends PythonSubscriptionCheck {
39+
private static final String MESSAGE = "Replace the %s with a direct function call when it only ever spawns one task.";
40+
private static final String SECONDARY_MESSAGE = "Only task created here";
41+
42+
private TypeCheckBuilder asyncioTaskGroupTypeChecker;
43+
private TypeCheckBuilder trioNurseryTypeChecker;
44+
private TypeCheckBuilder anyioTaskGroupTypeChecker;
45+
private static final Set<String> START_METHOD_NAMES = Set.of("start_soon", "start", "create_task");
46+
47+
@Override
48+
public void initialize(Context context) {
49+
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, this::initTypeCheckers);
50+
context.registerSyntaxNodeConsumer(Tree.Kind.WITH_STMT, this::checkWithStatement);
51+
}
52+
53+
private void initTypeCheckers(SubscriptionContext ctx) {
54+
asyncioTaskGroupTypeChecker = ctx.typeChecker().typeCheckBuilder().isTypeOrInstanceWithName("asyncio.TaskGroup");
55+
trioNurseryTypeChecker = ctx.typeChecker().typeCheckBuilder().isTypeWithFqn("trio.open_nursery");
56+
anyioTaskGroupTypeChecker = ctx.typeChecker().typeCheckBuilder().isTypeOrInstanceWithName("anyio.create_task_group");
57+
}
58+
59+
private void checkWithStatement(SubscriptionContext ctx) {
60+
var withStmt = (WithStatement) ctx.syntaxNode();
61+
if (!withStmt.isAsync()) {
62+
return;
63+
}
64+
for (var item : withStmt.withItems()) {
65+
handleWithItem(ctx, item, withStmt);
66+
}
67+
}
68+
69+
private void handleWithItem(SubscriptionContext ctx, WithItem item, WithStatement withStmt) {
70+
var test = item.test();
71+
if (!(test instanceof CallExpression callExpr) || isTaskGroupOrNurseryCall(callExpr).isEmpty()) {
72+
return;
73+
}
74+
75+
var aliasExpr = item.expression();
76+
if (!(aliasExpr instanceof Name aliasNameTree)) {
77+
return;
78+
}
79+
80+
var aliasSym = aliasNameTree.symbolV2();
81+
if (aliasSym == null) {
82+
return;
83+
}
84+
85+
shouldRaise(aliasSym, withStmt)
86+
.ifPresent(spawnCall -> ctx.addIssue(aliasExpr, String.format(MESSAGE, isTaskGroupOrNurseryCall(callExpr).get())).secondary(spawnCall, SECONDARY_MESSAGE));
87+
}
88+
89+
private Optional<String> isTaskGroupOrNurseryCall(CallExpression callExpr) {
90+
var calleeType = callExpr.callee().typeV2();
91+
if (asyncioTaskGroupTypeChecker.check(calleeType) == TriBool.TRUE || anyioTaskGroupTypeChecker.check(calleeType) == TriBool.TRUE) {
92+
return Optional.of("TaskGroup");
93+
}
94+
if (trioNurseryTypeChecker.check(calleeType) == TriBool.TRUE) {
95+
return Optional.of("Nursery");
96+
}
97+
return Optional.empty();
98+
}
99+
100+
private static boolean isSpawnCall(QualifiedExpression qualifiedExpression) {
101+
return START_METHOD_NAMES.contains(qualifiedExpression.name().name());
102+
}
103+
104+
private static Optional<Tree> shouldRaise(SymbolV2 aliasSym, WithStatement withStmt) {
105+
var usagesInWithScope = aliasSym.usages().stream()
106+
.filter(u -> !u.isBindingUsage())
107+
.filter(u -> TreeUtils.firstAncestor(u.tree(), t -> t == withStmt) != null)
108+
.toList();
109+
110+
Tree spawnCall = null;
111+
for (var usage : usagesInWithScope) {
112+
var parent = usage.tree().parent();
113+
114+
if (parent instanceof QualifiedExpression qe && isSpawnCall(qe)) {
115+
if (spawnCall != null || isInsideLoop(qe)) {
116+
return Optional.empty();
117+
}
118+
spawnCall = qe;
119+
} else {
120+
return Optional.empty();
121+
}
122+
}
123+
124+
return Optional.ofNullable(spawnCall);
125+
}
126+
127+
private static boolean isInsideLoop(Tree tree) {
128+
return TreeUtils.firstAncestorOfKind(tree, Tree.Kind.FOR_STMT, Tree.Kind.WHILE_STMT) != null;
129+
}
130+
}

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

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
<p>This rule raises when a TaskGroup or Nursery body contains only a single call to <code>.start</code> or <code>.start_soon</code> without passing
2+
itself as a parameter.</p>
3+
<h2>Why is this an issue?</h2>
4+
<p>TaskGroup and Nursery are powerful tools for structured concurrency that automatically manage the lifecycle of multiple concurrent tasks. However,
5+
when they are used to spawn only a single task that doesn’t need the nursery/TaskGroup instance itself, they add unnecessary overhead and
6+
complexity.</p>
7+
<p>The main issues with using TaskGroup and Nursery for single tasks are:</p>
8+
<ul>
9+
<li> <strong>Unnecessary overhead</strong>: Creating a nursery or TaskGroup involves additional setup and teardown that serves no purpose when only
10+
one task is spawned </li>
11+
<li> <strong>Misleading code</strong>: The presence of a nursery suggests that multiple tasks will be managed, which can be confusing </li>
12+
<li> <strong>Reduced readability</strong>: The structured concurrency syntax is more verbose than a simple function call </li>
13+
</ul>
14+
<h3>What is the potential impact?</h3>
15+
<ul>
16+
<li> <strong>Performance</strong>: Minor performance overhead from unnecessary nursery/TaskGroup creation </li>
17+
<li> <strong>Maintainability</strong>: Code is more complex than necessary, making it harder to understand and maintain </li>
18+
<li> <strong>Code clarity</strong>: The intent of the code is obscured by unnecessary structured concurrency constructs </li>
19+
</ul>
20+
<h2>How to fix it in Asyncio</h2>
21+
<p>Replace the TaskGroup with a direct function call when the TaskGroup body contains only a single <code>create_task()</code> call.</p>
22+
<h3>Code examples</h3>
23+
<h4>Noncompliant code example</h4>
24+
<pre data-diff-id="2" data-diff-type="noncompliant">
25+
import asyncio
26+
27+
async def worker_task():
28+
await asyncio.sleep(1)
29+
30+
async def main():
31+
# Unnecessary TaskGroup for a single task
32+
async with asyncio.TaskGroup() as tg:
33+
tg.create_task(worker_task())
34+
</pre>
35+
<h4>Compliant solution</h4>
36+
<pre data-diff-id="2" data-diff-type="compliant">
37+
import asyncio
38+
39+
async def worker_task():
40+
await asyncio.sleep(1)
41+
42+
async def main():
43+
# Direct function call is simpler and more efficient
44+
await worker_task()
45+
</pre>
46+
<h2>How to fix it in Trio</h2>
47+
<p>Replace the nursery with a direct function call when the nursery body contains only a single <code>start_soon()</code> or <code>start()</code>
48+
call.</p>
49+
<h3>Code examples</h3>
50+
<h4>Noncompliant code example</h4>
51+
<pre data-diff-id="1" data-diff-type="noncompliant">
52+
import trio
53+
54+
async def worker_task():
55+
await trio.sleep(1)
56+
57+
async def main():
58+
# Unnecessary nursery for a single task
59+
async with trio.open_nursery() as nursery:
60+
nursery.start_soon(worker_task)
61+
</pre>
62+
<h4>Compliant solution</h4>
63+
<pre data-diff-id="1" data-diff-type="compliant">
64+
import trio
65+
66+
async def worker_task():
67+
await trio.sleep(1)
68+
69+
async def main():
70+
# Direct function call is simpler and more efficient
71+
await worker_task()
72+
</pre>
73+
<h2>How to fix it in AnyIO</h2>
74+
<p>Replace the task group with a direct function call when the task group body contains only a single <code>start_soon()</code> or
75+
<code>start()</code> call.</p>
76+
<h3>Code examples</h3>
77+
<h4>Noncompliant code example</h4>
78+
<pre data-diff-id="3" data-diff-type="noncompliant">
79+
import anyio
80+
81+
async def worker_task():
82+
await anyio.sleep(1)
83+
84+
async def main():
85+
# Unnecessary task group for a single task
86+
async with anyio.create_task_group() as tg:
87+
tg.start_soon(worker_task)
88+
</pre>
89+
<h4>Compliant solution</h4>
90+
<pre data-diff-id="3" data-diff-type="compliant">
91+
import anyio
92+
93+
async def worker_task():
94+
await anyio.sleep(1)
95+
96+
async def main():
97+
# Direct function call is simpler and more efficient
98+
await worker_task()
99+
</pre>
100+
<h2>Resources</h2>
101+
<h3>Documentation</h3>
102+
<ul>
103+
<li> Asyncio documentation - <a href="https://docs.python.org/3/library/asyncio-task.html#task-groups">Task Groups</a> </li>
104+
<li> Trio documentation - <a href="https://trio.readthedocs.io/en/latest/reference-core.html#nurseries-and-spawning">Nurseries and spawning</a>
105+
</li>
106+
<li> AnyIO documentation - <a href="https://anyio.readthedocs.io/en/stable/tasks.html#creating-and-managing-tasks">Creating and managing tasks</a>
107+
</li>
108+
</ul>
109+

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
{
2+
"title": "TaskGroup\/Nursery should not be used for a single start call",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [
10+
"async",
11+
"asyncio",
12+
"trio",
13+
"anyio"
14+
],
15+
"defaultSeverity": "Minor",
16+
"ruleSpecification": "RSPEC-7513",
17+
"sqKey": "S7513",
18+
"scope": "All",
19+
"quickfix": "unknown",
20+
"code": {
21+
"impacts": {
22+
"MAINTAINABILITY": "LOW"
23+
},
24+
"attribute": "CONVENTIONAL"
25+
}
26+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@
281281
"S7510",
282282
"S7511",
283283
"S7512",
284+
"S7513",
284285
"S7514",
285286
"S7516",
286287
"S7517",

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
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+
18+
package org.sonar.python.checks;
19+
20+
import org.junit.jupiter.api.Test;
21+
import org.sonar.python.checks.utils.PythonCheckVerifier;
22+
23+
class TaskGroupNurseryUsedOnlyOnceCheckTest {
24+
25+
@Test
26+
void test() {
27+
PythonCheckVerifier.verify("src/test/resources/checks/taskGroupNurseryUsedOnlyOnce.py", new TaskGroupNurseryUsedOnlyOnceCheck());
28+
}
29+
30+
}

0 commit comments

Comments
 (0)