Skip to content

Commit 80a63bb

Browse files
maksim-grebeniuk-sonarsourcesonartech
authored andcommitted
SPNARPY-2933 Rule S7492: Unnecessary list comprehension in <any/all>() prevents short-circuiting - rewrite as a generator (#279)
GitOrigin-RevId: b06eb8e0ee3fe61706c21c4842419dbb655436f2
1 parent 2c8cc5c commit 80a63bb

7 files changed

Lines changed: 204 additions & 0 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
@@ -396,6 +396,7 @@ public Stream<Class<?>> getChecks() {
396396
UnencryptedSqsQueueCheck.class,
397397
UnencryptedEbsVolumeCheck.class,
398398
UnionTypeExpressionCheck.class,
399+
UnnecessaryListComprehensionArgumentCheck.class,
399400
UnnecessaryReversedCallCheck.class,
400401
UnnecessarySubscriptReversalCheck.class,
401402
UnquantifiedNonCapturingGroupCheck.class,
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
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.List;
20+
import java.util.Optional;
21+
import java.util.stream.Stream;
22+
import javax.annotation.Nullable;
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.ComprehensionExpression;
28+
import org.sonar.plugins.python.api.tree.Expression;
29+
import org.sonar.plugins.python.api.tree.Name;
30+
import org.sonar.plugins.python.api.tree.RegularArgument;
31+
import org.sonar.plugins.python.api.tree.Tree;
32+
import org.sonar.plugins.python.api.types.v2.TriBool;
33+
import org.sonar.python.checks.utils.Expressions;
34+
import org.sonar.python.semantic.v2.SymbolV2;
35+
import org.sonar.python.tree.TreeUtils;
36+
import org.sonar.python.types.v2.TypeCheckBuilder;
37+
38+
@Rule(key = "S7492")
39+
public class UnnecessaryListComprehensionArgumentCheck extends PythonSubscriptionCheck {
40+
private TypeCheckBuilder isAllTypeCheck;
41+
private TypeCheckBuilder isAnyTypeCheck;
42+
private TypeCheckBuilder isListTypeCheck;
43+
44+
@Override
45+
public void initialize(Context context) {
46+
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, this::initChecks);
47+
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, this::check);
48+
}
49+
50+
private void initChecks(SubscriptionContext ctx) {
51+
isAllTypeCheck = ctx.typeChecker().typeCheckBuilder().isTypeWithName("all");
52+
isAnyTypeCheck = ctx.typeChecker().typeCheckBuilder().isTypeWithName("any");
53+
isListTypeCheck = ctx.typeChecker().typeCheckBuilder().isInstanceOf("list");
54+
}
55+
56+
private void check(SubscriptionContext ctx) {
57+
var callExpression = (CallExpression) ctx.syntaxNode();
58+
if (!isSensitiveCall(callExpression)) {
59+
return;
60+
}
61+
TreeUtils.nthArgumentOrKeywordOptional(0, "", callExpression.arguments())
62+
.map(RegularArgument::expression)
63+
.ifPresent(argumentExpression -> {
64+
if (isListComprehensionExpression(argumentExpression) || isAssignedToListComprehensionExpression(argumentExpression)) {
65+
ctx.addIssue(callExpression, "Unpack this comprehension expression");
66+
}
67+
});
68+
}
69+
70+
private boolean isListComprehensionExpression(@Nullable Expression expression) {
71+
return expression instanceof ComprehensionExpression comprehensionExpression
72+
&& isListTypeCheck.check(comprehensionExpression.typeV2()) == TriBool.TRUE;
73+
}
74+
75+
private boolean isAssignedToListComprehensionExpression(Expression argumentExpression) {
76+
return argumentExpression instanceof Name name
77+
&& getUsageCount(name) == 2
78+
&& isListComprehensionExpression(Expressions.singleAssignedValue(name));
79+
}
80+
81+
private boolean isSensitiveCall(CallExpression callExpression) {
82+
return Stream.of(isAllTypeCheck, isAnyTypeCheck)
83+
.map(check -> check.check(callExpression.callee().typeV2()))
84+
.anyMatch(TriBool.TRUE::equals);
85+
}
86+
87+
private static int getUsageCount(Name name) {
88+
return Optional.of(name)
89+
.map(Name::symbolV2)
90+
.map(SymbolV2::usages)
91+
.map(List::size)
92+
.orElse(0);
93+
}
94+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<p>This rule raises an issue when list comprehensions are used as parameters to <code>any()</code> or <code>all()</code> instead of generator
2+
expressions as this prevents <code>any()</code> or <code>all()</code> from short-circuiting.</p>
3+
<h2>Why is this an issue?</h2>
4+
<p>Using a list comprehension inside <code>any()</code> or <code>all()</code> forces the entire list to be created in memory before the check begins.
5+
This prevents the short-circuiting behavior that these functions are designed to leverage, where <code>any()</code> stops at the first
6+
<code>True</code> and <code>all()</code> stops at the first <code>False</code>.</p>
7+
<p>Using a generator expression provides the same functionality while preserving the short-circuiting behavior of these functions. This could save
8+
both processing time and memory, especially for large iterables or when the condition has side effects or is computationally expensive.</p>
9+
<h2>How to fix it</h2>
10+
<p>Use a generator expression instead of a list comprehension.</p>
11+
<h3>Code examples</h3>
12+
<h4>Noncompliant code example</h4>
13+
<pre data-diff-id="1" data-diff-type="noncompliant">
14+
numbers = [1, 5, 0, 10]
15+
res_all = all([x &gt; 2 for x in numbers]) # Noncompliant: will evaluate all numbers instead of stopping at "5"
16+
</pre>
17+
<h4>Compliant solution</h4>
18+
<pre data-diff-id="1" data-diff-type="compliant">
19+
numbers = [1, 5, 0, 10]
20+
res_all = all(x &gt; 2 for x in numbers) # Compliant
21+
</pre>
22+
<h2>Resources</h2>
23+
<h3>Documentation</h3>
24+
<ul>
25+
<li> Python Wiki - <a href="https://wiki.python.org/moin/Generators">Generators</a> </li>
26+
<li> Python Documentation - <a href="https://docs.python.org/3/glossary.html#term-generator">Generator Glossary Entry</a> </li>
27+
</ul>
28+
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{
2+
"title": "List comprehensions should not be used with \"any()\" or \"all()\"",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [],
10+
"defaultSeverity": "Minor",
11+
"ruleSpecification": "RSPEC-7492",
12+
"sqKey": "S7492",
13+
"scope": "All",
14+
"quickfix": "targeted",
15+
"code": {
16+
"impacts": {
17+
"MAINTAINABILITY": "LOW"
18+
},
19+
"attribute": "EFFICIENT"
20+
}
21+
}

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
@@ -260,6 +260,7 @@
260260
"S7488",
261261
"S7489",
262262
"S7491",
263+
"S7492",
263264
"S7493",
264265
"S7494",
265266
"S7498",
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 UnnecessaryListComprehensionArgumentCheckTest {
23+
24+
@Test
25+
void test() {
26+
PythonCheckVerifier.verify("src/test/resources/checks/unnecessaryListComprehensionCallArgument.py", new UnnecessaryListComprehensionArgumentCheck());
27+
}
28+
29+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
2+
def case1():
3+
numbers = [1, 5, 0, 10]
4+
all([x > 2 for x in numbers]) # Noncompliant
5+
any([x > 2 for x in numbers]) # Noncompliant
6+
7+
def case2():
8+
numbers = [1, 5, 0, 10]
9+
d1 = [x > 2 for x in numbers]
10+
all(d1) # Noncompliant
11+
d2 = [x > 2 for x in numbers]
12+
d2.append(3)
13+
any(d2)
14+
15+
def case3():
16+
numbers = [1, 5, 0, 10]
17+
all(x > 2 for x in numbers)
18+
any(x > 2 for x in numbers)
19+
20+
def case4():
21+
numbers = [1, 5, 0, 10]
22+
d1 = {x > 2 for x in numbers}
23+
all(d1)
24+
d2 = {x > 2 for x in numbers}
25+
any(d2)
26+
27+
def case5():
28+
numbers = [1, 5, 0, 10]
29+
all({x > 2 for x in numbers})
30+
any({x > 2 for x in numbers})

0 commit comments

Comments
 (0)