Skip to content

Commit afc0e22

Browse files
joke1196Seppli11
authored andcommitted
SONARPY-2943: Rule S7504 When iterating over an iterable object, using \\"list()\\" should be avoided (#285)
Co-authored-by: Sebastian Zumbrunn <sebastian.zumbrunn@sonarsource.com> GitOrigin-RevId: a99b228c0c6801349c291111096c389d10888bb5
1 parent 393d5ba commit afc0e22

7 files changed

Lines changed: 209 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
@@ -402,6 +402,7 @@ public Stream<Class<?>> getChecks() {
402402
UnnecessaryListComprehensionArgumentCheck.class,
403403
UnnecessaryComprehensionCheck.class,
404404
UnnecessaryLambdaMapCallCheck.class,
405+
UnnecessaryListCastCheck.class,
405406
UnnecessaryReversedCallCheck.class,
406407
UnnecessarySubscriptReversalCheck.class,
407408
UnquantifiedNonCapturingGroupCheck.class,
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
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 org.sonar.check.Rule;
22+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
23+
import org.sonar.plugins.python.api.SubscriptionContext;
24+
import org.sonar.plugins.python.api.tree.CallExpression;
25+
import org.sonar.plugins.python.api.tree.ComprehensionFor;
26+
import org.sonar.plugins.python.api.tree.Expression;
27+
import org.sonar.plugins.python.api.tree.ForStatement;
28+
import org.sonar.plugins.python.api.tree.RegularArgument;
29+
import org.sonar.plugins.python.api.tree.Tree;
30+
import org.sonar.plugins.python.api.types.v2.TriBool;
31+
import org.sonar.python.types.v2.TypeCheckBuilder;
32+
33+
@Rule(key = "S7504")
34+
public class UnnecessaryListCastCheck extends PythonSubscriptionCheck {
35+
private TypeCheckBuilder isListCallCheck;
36+
37+
@Override
38+
public void initialize(Context context) {
39+
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, this::initChecks);
40+
context.registerSyntaxNodeConsumer(Tree.Kind.FOR_STMT, this::checkForStatements);
41+
context.registerSyntaxNodeConsumer(Tree.Kind.COMP_FOR, this::checkComprehensions);
42+
}
43+
44+
private void initChecks(SubscriptionContext ctx) {
45+
isListCallCheck = ctx.typeChecker().typeCheckBuilder().isBuiltinWithName("list");
46+
}
47+
48+
private void checkForStatements(SubscriptionContext ctx) {
49+
ForStatement stmt = ((ForStatement) ctx.syntaxNode());
50+
checkListCastCheck(stmt.testExpressions(), ctx);
51+
}
52+
53+
private void checkComprehensions(SubscriptionContext ctx) {
54+
ComprehensionFor comprehensionFor = ((ComprehensionFor) ctx.syntaxNode());
55+
checkListCastCheck(List.of(comprehensionFor.iterable()), ctx);
56+
}
57+
58+
private void checkListCastCheck(List<Expression> expressions, SubscriptionContext ctx) {
59+
hasListCallOnIterable(expressions)
60+
.ifPresent(listCall -> ctx.addIssue(listCall.callee(), "Remove this unnecessary `list()` call on an already iterable object."));
61+
}
62+
63+
private Optional<CallExpression> hasListCallOnIterable(List<Expression> testExpressions) {
64+
if (testExpressions.size() == 1
65+
&& testExpressions.get(0) instanceof CallExpression callExpression
66+
&& isListCall(callExpression)
67+
&& hasOnlyOneRegularArg(callExpression)) {
68+
return Optional.of(callExpression);
69+
}
70+
return Optional.empty();
71+
}
72+
73+
private boolean isListCall(CallExpression callExpression) {
74+
return isListCallCheck.check(callExpression.callee().typeV2()) == TriBool.TRUE;
75+
}
76+
77+
private static boolean hasOnlyOneRegularArg(CallExpression callExpression) {
78+
return callExpression.arguments().size() == 1 && callExpression.arguments().get(0) instanceof RegularArgument;
79+
}
80+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<p>This rule raises an issue when <code>list()</code> calls are applied to types that are already directly iterable with for-loops or
2+
comprehensions.</p>
3+
<h2>Why is this an issue?</h2>
4+
<p>When iterating over an already iterable object with a for loop or a comprehension, wrapping it with <code>list()</code> adds meaningless clutter
5+
that doesn’t provide any functional value. Additionally, it creates unnecessary overhead by generating an intermediate list in memory, which
6+
inefficiently consumes memory and can degrade performance, especially with large data structures. Iterating directly over the original object is
7+
cleaner and more efficient.</p>
8+
<h2>How to fix it</h2>
9+
<p>Remove the redundant <code>list()</code> call and iterate directly over the original iterable.</p>
10+
<h3>Code examples</h3>
11+
<h4>Noncompliant code example</h4>
12+
<pre data-diff-id="1" data-diff-type="noncompliant">
13+
some_iterable = range(10)
14+
for i in list(some_iterable): # Noncompliant: unnecessary list() call
15+
print(i)
16+
</pre>
17+
<h4>Compliant solution</h4>
18+
<pre data-diff-id="1" data-diff-type="compliant">
19+
some_iterable = range(10)
20+
for i in some_iterable: # Compliant
21+
print(i)
22+
</pre>
23+
<h2>Resources</h2>
24+
<h3>Documentation</h3>
25+
<ul>
26+
<li> Python Documentation - <a href="https://docs.python.org/3/glossary.html#term-iterable">Iterable Glossary Entry</a> </li>
27+
<li> Python Documentation - <a href="https://docs.python.domainunion.de/3/library/stdtypes.html#list">list()</a> </li>
28+
</ul>
29+
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{
2+
"title": "When iterating over an iterable object, using \"list()\" should be avoided",
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-7504",
12+
"sqKey": "S7504",
13+
"scope": "All",
14+
"quickfix": "targeted",
15+
"code": {
16+
"impacts": {
17+
"MAINTAINABILITY": "LOW"
18+
},
19+
"attribute": "CONVENTIONAL"
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
@@ -272,6 +272,7 @@
272272
"S7501",
273273
"S7502",
274274
"S7503",
275+
"S7504",
275276
"S7505",
276277
"S7506",
277278
"S7507",
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 UnnecessaryListCastCheckTest {
23+
24+
@Test
25+
void test() {
26+
PythonCheckVerifier.verify("src/test/resources/checks/unnecessaryListCast.py", new UnnecessaryListCastCheck());
27+
}
28+
29+
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
2+
[i for i in list([1,2,3])] # Noncompliant {{Remove this unnecessary `list()` call on an already iterable object.}}
3+
# ^^^^
4+
5+
{i for i in list([1,2,3])} # Noncompliant
6+
# ^^^^
7+
8+
for i in list(range(2)): # Noncompliant
9+
# ^^^^
10+
print(i)
11+
12+
13+
some_iter = [1,2,5]
14+
for i in list(some_iter): # Noncompliant
15+
# ^^^^
16+
print(i)
17+
18+
def foo(a: Union[str, Iterable[str]]):
19+
for i in list(a): # Noncompliant
20+
print(i)
21+
22+
23+
list(range(32)) # Ok we only raise in for loops and comprehensions
24+
25+
26+
for i in list(*some_iter): # Ok
27+
print(i)
28+
29+
from module import some_val
30+
31+
for i in list(some_val): # Noncompliant
32+
print(i)
33+
34+
#=============== COVERAGE =============
35+
36+
for i,y in list(range(3)), list(range(4)):
37+
print(i)
38+
39+
for i in list([3,5], [1,3]): # incorrect syntax
40+
print(i)
41+
42+
for i in [1,2]:
43+
print(i)
44+
45+
from module import test
46+
47+
for i in test("1"):
48+
print(i)

0 commit comments

Comments
 (0)