Skip to content

Commit 3cd35fd

Browse files
guillaume-dequennesonartech
guillaume-dequenne
authored and
sonartech
committed
SONARPY-3003 Rule S7514 Control flow statements should not be used inside TaskGroup or Nursery blocks (#311)
GitOrigin-RevId: 5c5c3533afd6c39cc783f8138496cf11f0bba89b
1 parent c282d36 commit 3cd35fd

File tree

7 files changed

+410
-0
lines changed

7 files changed

+410
-0
lines changed

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

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
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.ArrayList;
20+
import java.util.List;
21+
import java.util.Optional;
22+
import org.sonar.check.Rule;
23+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
24+
import org.sonar.plugins.python.api.SubscriptionContext;
25+
import org.sonar.plugins.python.api.tree.BaseTreeVisitor;
26+
import org.sonar.plugins.python.api.tree.BreakStatement;
27+
import org.sonar.plugins.python.api.tree.CallExpression;
28+
import org.sonar.plugins.python.api.tree.ContinueStatement;
29+
import org.sonar.plugins.python.api.tree.Expression;
30+
import org.sonar.plugins.python.api.tree.FunctionDef;
31+
import org.sonar.plugins.python.api.tree.ReturnStatement;
32+
import org.sonar.plugins.python.api.tree.Tree;
33+
import org.sonar.plugins.python.api.tree.WithItem;
34+
import org.sonar.plugins.python.api.tree.WithStatement;
35+
import org.sonar.plugins.python.api.types.v2.TriBool;
36+
import org.sonar.python.types.v2.TypeCheckBuilder;
37+
38+
@Rule(key = "S7514")
39+
public class ControlFlowInTaskGroupCheck extends PythonSubscriptionCheck {
40+
41+
private static final String MESSAGE = "Refactor the block to eliminate this \"%s\" statement.";
42+
private static final String SECONDARY_MESSAGE = "This is an async task group context.";
43+
private List<TypeCheckBuilder> taskGroupTypeChecks = new ArrayList<>();
44+
45+
@Override
46+
public void initialize(Context context) {
47+
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, this::setupTypeChecks);
48+
context.registerSyntaxNodeConsumer(Tree.Kind.WITH_STMT, this::checkWithStatement);
49+
}
50+
51+
private void setupTypeChecks(SubscriptionContext ctx) {
52+
taskGroupTypeChecks = new ArrayList<>();
53+
taskGroupTypeChecks.add(ctx.typeChecker().typeCheckBuilder().isTypeOrInstanceWithName("asyncio.TaskGroup"));
54+
taskGroupTypeChecks.add(ctx.typeChecker().typeCheckBuilder().isTypeWithFqn("trio.open_nursery"));
55+
taskGroupTypeChecks.add(ctx.typeChecker().typeCheckBuilder().isTypeOrInstanceWithName("anyio.create_task_group"));
56+
}
57+
58+
private void checkWithStatement(SubscriptionContext ctx) {
59+
WithStatement withStatement = (WithStatement) ctx.syntaxNode();
60+
if (!withStatement.isAsync()) {
61+
return;
62+
}
63+
64+
Optional<Expression> taskGroupItem = withStatement.withItems().stream()
65+
.map(WithItem::test)
66+
.filter(this::isTaskGroupOrNursery).findFirst();
67+
taskGroupItem.ifPresent(t -> {
68+
ControlFlowVisitor visitor = new ControlFlowVisitor(ctx, t);
69+
withStatement.statements().accept(visitor);
70+
}
71+
);
72+
}
73+
74+
private boolean isTaskGroupOrNursery(Expression expression) {
75+
boolean result = taskGroupTypeChecks.stream()
76+
.anyMatch(typeCheck -> typeCheck.check(expression.typeV2()) == TriBool.TRUE);
77+
if (!result && expression instanceof CallExpression callExpression) {
78+
result = taskGroupTypeChecks.stream()
79+
.anyMatch(typeCheck -> typeCheck.check(callExpression.callee().typeV2()) == TriBool.TRUE);
80+
}
81+
return result;
82+
}
83+
84+
private static class ControlFlowVisitor extends BaseTreeVisitor {
85+
private final SubscriptionContext ctx;
86+
private final Expression taskGroupItem;
87+
88+
public ControlFlowVisitor(SubscriptionContext ctx, Expression taskGroupItem) {
89+
this.ctx = ctx;
90+
this.taskGroupItem = taskGroupItem;
91+
}
92+
93+
@Override
94+
public void visitReturnStatement(ReturnStatement returnStatement) {
95+
ctx.addIssue(returnStatement, String.format(MESSAGE, "return"))
96+
.secondary(taskGroupItem, SECONDARY_MESSAGE);
97+
}
98+
99+
@Override
100+
public void visitBreakStatement(BreakStatement breakStatement) {
101+
ctx.addIssue(breakStatement, String.format(MESSAGE, "break"))
102+
.secondary(taskGroupItem, SECONDARY_MESSAGE);
103+
}
104+
105+
@Override
106+
public void visitContinueStatement(ContinueStatement continueStatement) {
107+
ctx.addIssue(continueStatement, String.format(MESSAGE, "continue"))
108+
.secondary(taskGroupItem, SECONDARY_MESSAGE);
109+
}
110+
111+
@Override
112+
public void visitFunctionDef(FunctionDef functionDef) {
113+
// Skip nested functions
114+
}
115+
}
116+
}

‎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
@@ -157,6 +157,7 @@ public Stream<Class<?>> getChecks() {
157157
ConsistentReturnCheck.class,
158158
ConstantConditionCheck.class,
159159
ConstantValueDictComprehensionCheck.class,
160+
ControlFlowInTaskGroupCheck.class,
160161
CorsCheck.class,
161162
CsrfDisabledCheck.class,
162163
DataEncryptionCheck.class,

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

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
<p>This rule raises when a control flow statement (<code>return</code>, <code>break</code>, <code>continue</code>) is used inside a TaskGroup or
2+
Nursery context manager.</p>
3+
<h2>Why is this an issue?</h2>
4+
<p>Using control flow statements like <code>return</code>, <code>break</code>, or <code>continue</code> inside async TaskGroup or Nursery blocks leads
5+
to counterintuitive behavior that can confuse developers and introduce bugs.</p>
6+
<h3>What is the potential impact?</h3>
7+
<h4>Deferred execution in TaskGroup</h4>
8+
<p>In asyncio’s TaskGroup, control flow statements don’t take immediate effect. Instead, they wait for all tasks in the group to complete before
9+
executing. This can lead to:</p>
10+
<ul>
11+
<li> Unexpected delays when tasks run longer than anticipated </li>
12+
<li> Code that appears to exit early but actually continues running </li>
13+
<li> Potential infinite loops if background tasks never complete </li>
14+
</ul>
15+
<h4>Lost return values in Nurseries</h4>
16+
<p>In Trio and AnyIO nurseries, return values can be lost if other tasks in the nursery raise exceptions:</p>
17+
<ul>
18+
<li> When a background task raises an exception, the return value from the main flow is discarded </li>
19+
<li> The nursery’s exception handling takes precedence over return values </li>
20+
<li> Silent data loss that’s difficult to debug </li>
21+
</ul>
22+
<h2>How to fix it in Asyncio</h2>
23+
<p>Move the control flow statement outside the TaskGroup or Nursery block, and use the appropriate cancellation mechanism before exiting the
24+
block.</p>
25+
<h3>Code examples</h3>
26+
<h4>Noncompliant code example</h4>
27+
<pre data-diff-id="1" data-diff-type="noncompliant">
28+
import asyncio
29+
30+
async def process():
31+
async with asyncio.TaskGroup() as tg:
32+
tg.create_task(background_task())
33+
34+
if condition():
35+
return "result" # Noncompliant: waits for background_task() to complete
36+
</pre>
37+
<h4>Compliant solution</h4>
38+
<pre data-diff-id="1" data-diff-type="compliant">
39+
import asyncio
40+
41+
async def process():
42+
result = None
43+
async with asyncio.TaskGroup() as tg:
44+
task = tg.create_task(background_task())
45+
46+
if condition():
47+
result = "result"
48+
task.cancel()
49+
50+
return result
51+
</pre>
52+
<h2>How to fix it in Trio</h2>
53+
<p>Move the control flow statement outside the Nursery block, and use the appropriate cancellation mechanism before exiting the block.</p>
54+
<h3>Code examples</h3>
55+
<h4>Noncompliant code example</h4>
56+
<pre data-diff-id="2" data-diff-type="noncompliant">
57+
import trio
58+
59+
async def process():
60+
async with trio.open_nursery() as nursery:
61+
nursery.start_soon(background_task)
62+
63+
if condition():
64+
return "result" # Noncompliant: value may be lost
65+
</pre>
66+
<h4>Compliant solution</h4>
67+
<pre data-diff-id="2" data-diff-type="compliant">
68+
import trio
69+
70+
async def process():
71+
result = None
72+
async with trio.open_nursery() as nursery:
73+
nursery.start_soon(background_task)
74+
75+
if condition():
76+
result = "result"
77+
nursery.cancel_scope.cancel()
78+
79+
return result
80+
</pre>
81+
<h2>How to fix it in AnyIO</h2>
82+
<p>Move the control flow statement outside the TaskGroup block, and use the appropriate cancellation mechanism before exiting the block.</p>
83+
<h3>Code examples</h3>
84+
<h4>Noncompliant code example</h4>
85+
<pre data-diff-id="3" data-diff-type="noncompliant">
86+
import anyio
87+
88+
async def process():
89+
async with anyio.create_task_group() as tg:
90+
tg.start_soon(background_task)
91+
92+
if condition():
93+
return "result" # Noncompliant: waits for background_task
94+
</pre>
95+
<h4>Compliant solution</h4>
96+
<pre data-diff-id="3" data-diff-type="compliant">
97+
import anyio
98+
99+
async def process():
100+
result = None
101+
async with anyio.create_task_group() as tg:
102+
tg.start_soon(background_task)
103+
104+
if condition():
105+
result = "result"
106+
tg.cancel_scope.cancel()
107+
108+
return result
109+
</pre>
110+
<h2>Resources</h2>
111+
<h3>Documentation</h3>
112+
<ul>
113+
<li> Asyncio documentation - <a href="https://docs.python.org/3/library/asyncio-task.html#task-groups">Task Groups</a> </li>
114+
<li> Trio documentation - <a href="https://trio.readthedocs.io/en/latest/reference-core.html#nurseries-and-spawning">Nurseries and spawning</a>
115+
</li>
116+
<li> AnyIO documentation - <a href="https://anyio.readthedocs.io/en/stable/tasks.html#creating-and-managing-tasks">Creating and managing tasks</a>
117+
</li>
118+
<li> Trio issue #1493 - <a href="https://github.com/python-trio/trio/issues/1493">Returning a value from inside a nursery block behaves
119+
counterintuitively</a> </li>
120+
</ul>
121+

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
{
2+
"title": "Control flow statements should not be used inside TaskGroup or Nursery blocks",
3+
"type": "BUG",
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": "Major",
16+
"ruleSpecification": "RSPEC-7514",
17+
"sqKey": "S7514",
18+
"scope": "All",
19+
"quickfix": "unknown",
20+
"code": {
21+
"impacts": {
22+
"RELIABILITY": "HIGH"
23+
},
24+
"attribute": "LOGICAL"
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+
"S7514",
284285
"S7516",
285286
"S7517",
286287
"S7519"

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
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 ControlFlowInTaskGroupCheckTest {
23+
24+
@Test
25+
void test() {
26+
PythonCheckVerifier.verify("src/test/resources/checks/controlFlowInTaskGroup.py", new ControlFlowInTaskGroupCheck());
27+
}
28+
29+
}

0 commit comments

Comments
 (0)