Skip to content

Commit be557bd

Browse files
maksim-grebeniuk-sonarsourcesonartech
maksim-grebeniuk-sonarsource
authored and
sonartech
committed
SONARPY-2966 Rule S7512: Inefficient dictionary iteration method (#293)
GitOrigin-RevId: 8003cf4855dee239dc1f9ae8ddadd9288109e602
1 parent c078bc1 commit be557bd

File tree

7 files changed

+238
-1
lines changed

7 files changed

+238
-1
lines changed

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

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
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 javax.annotation.Nullable;
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.CallExpression;
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.Name;
29+
import org.sonar.plugins.python.api.tree.Tree;
30+
import org.sonar.plugins.python.api.types.v2.TriBool;
31+
import org.sonar.python.checks.utils.Expressions;
32+
import org.sonar.python.semantic.v2.SymbolV2;
33+
import org.sonar.python.types.v2.TypeCheckBuilder;
34+
35+
@Rule(key = "S7512")
36+
public class InefficientDictIterationCheck extends PythonSubscriptionCheck {
37+
private TypeCheckBuilder dictItemsTypeCheck;
38+
39+
@Override
40+
public void initialize(Context context) {
41+
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, this::initChecks);
42+
context.registerSyntaxNodeConsumer(Tree.Kind.FOR_STMT, this::check);
43+
}
44+
45+
private void initChecks(SubscriptionContext ctx) {
46+
dictItemsTypeCheck = ctx.typeChecker().typeCheckBuilder().isTypeWithName("dict.items");
47+
}
48+
49+
private void check(SubscriptionContext ctx) {
50+
var forStatement = (ForStatement) ctx.syntaxNode();
51+
var hasIgnoredKey = hasIgnoredKey(forStatement);
52+
var hasIgnoredValue = hasIgnoredValue(forStatement);
53+
if (forStatement.testExpressions().size() == 1
54+
&& (hasIgnoredKey || hasIgnoredValue)
55+
&& (isSensitiveMethodCall(forStatement.testExpressions().get(0)) || isAssignedToSensitiveMethodCall(forStatement.testExpressions().get(0)))) {
56+
var message = hasIgnoredKey ? "Make this loop to iterate over the dict values." : "Make this loop to iterate directly over the dict.";
57+
ctx.addIssue(forStatement.testExpressions().get(0), message);
58+
}
59+
}
60+
61+
private static boolean hasIgnoredKey(ForStatement forStatement) {
62+
return forStatement.expressions().size() == 2
63+
&& forStatement.expressions().get(0) instanceof Name keyName
64+
&& "_".equals(keyName.name());
65+
}
66+
67+
private static boolean hasIgnoredValue(ForStatement forStatement) {
68+
return forStatement.expressions().size() == 2
69+
&& forStatement.expressions().get(1) instanceof Name keyName
70+
&& "_".equals(keyName.name());
71+
}
72+
73+
74+
private boolean isSensitiveMethodCall(@Nullable Expression expression) {
75+
return expression instanceof CallExpression callExpression
76+
&& dictItemsTypeCheck.check(callExpression.callee().typeV2()) == TriBool.TRUE;
77+
}
78+
79+
private boolean isAssignedToSensitiveMethodCall(Expression argumentExpression) {
80+
return argumentExpression instanceof Name name
81+
&& getUsageCount(name) == 2
82+
&& isSensitiveMethodCall(Expressions.singleAssignedValue(name));
83+
}
84+
85+
private static int getUsageCount(Name name) {
86+
return Optional.ofNullable(name.symbolV2())
87+
.map(SymbolV2::usages)
88+
.map(List::size)
89+
.orElse(0);
90+
}
91+
}

‎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
@@ -245,6 +245,7 @@ public Stream<Class<?>> getChecks() {
245245
IncorrectExceptionTypeCheck.class,
246246
IncorrectParameterDatetimeConstructorsCheck.class,
247247
IndexMethodCheck.class,
248+
InefficientDictIterationCheck.class,
248249
InequalityUsageCheck.class,
249250
InfiniteRecursionCheck.class,
250251
InitReturnsValueCheck.class,

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<p>This rule raises an issue when <code>.items()</code> is used to iterate over a dictionary and then either the key or the value is discarded.</p>
2+
<h2>Why is this an issue?</h2>
3+
<p>Using <code>.items()</code> to iterate over a dictionary and then discarding either the key or the value in each iteration is less efficient than
4+
directly iterating over only the keys or values needed.</p>
5+
<p>Python dictionaries provide efficient ways to iterate over their contents:</p>
6+
<ul>
7+
<li> Iterating directly over the dictionary yields the keys: </li>
8+
</ul>
9+
<pre>
10+
for k in my_dict:
11+
...
12+
</pre>
13+
<ul>
14+
<li> Using <code>my_dict.keys()</code> explicitly yields the keys. </li>
15+
<li> Using <code>my_dict.values()</code> yields the values. </li>
16+
<li> Using <code>my_dict.items()</code> yields key-value pairs (as tuples). </li>
17+
</ul>
18+
<p>The <code>.items()</code> method is useful when you need both the key and the value within the loop. However, if your loop only uses the key,
19+
discarding the value, often with <code>_</code>, or only uses the value, discarding the key, calling <code>.items()</code> performs unnecessary work
20+
retrieving the part you don’t use.</p>
21+
<p>While the performance difference might be minor for small dictionaries, using the more specific method is clearer, more idiomatic, and avoids
22+
retrieving and unpacking data that is immediately ignored, like`.keys()` for keys, or <code>.values()</code> for values.</p>
23+
<h2>How to fix it</h2>
24+
<p>Adjust the loop to use the most appropriate dictionary view method based on whether you need keys, values, or both: * If only values are needed,
25+
iterate over <code>my_dict.values()</code>. * If only keys are needed, iterate directly over the dictionary (<code>for key in my_dict:</code>) or use
26+
<code>my_dict.keys()</code>. * If both key and value are needed, continue using <code>my_dict.items()</code>.</p>
27+
<h3>Code examples</h3>
28+
<h4>Noncompliant code example</h4>
29+
<pre data-diff-id="1" data-diff-type="noncompliant">
30+
fruit = {'a': 'Apple', 'b': 'Banana'}
31+
for _, value in fruit.items(): # Discards key
32+
print(value)
33+
</pre>
34+
<pre data-diff-id="2" data-diff-type="noncompliant">
35+
fruit = {'a': 'Apple', 'b': 'Banana'}
36+
for key, _ in fruit.items(): # Discards value
37+
print(key)
38+
</pre>
39+
<h4>Compliant solution</h4>
40+
<pre data-diff-id="1" data-diff-type="compliant">
41+
fruit = {'a': 'Apple', 'b': 'Banana'}
42+
for value in fruit.values(): # Iterates only on values
43+
print(value)
44+
</pre>
45+
<pre data-diff-id="2" data-diff-type="compliant">
46+
fruit = {'a': 'Apple', 'b': 'Banana'}
47+
for key in fruit: # Iterates directly over dictionary (yields keys)
48+
print(key)
49+
</pre>
50+
<h3>Documentation</h3>
51+
<ul>
52+
<li> Python Documentation - <a href="https://docs.python.org/3/library/stdtypes.html#dictionary-view-objects">Dictionary View Objects
53+
(<code>.keys()</code>, <code>.values()</code>, <code>.items()</code>)</a> </li>
54+
</ul>
55+

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{
2+
"title": "Using \".items()\" to iterate over a dictionary should be avoided if possible.",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "1min"
8+
},
9+
"tags": [],
10+
"defaultSeverity": "Major",
11+
"ruleSpecification": "RSPEC-7512",
12+
"sqKey": "S7512",
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: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@
278278
"S7506",
279279
"S7507",
280280
"S7510",
281-
"S7511"
281+
"S7511",
282+
"S7512"
282283
]
283284
}

‎python-checks/src/test/java/org/sonar/python/checks/InefficientDictIterationCheckTest.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 InefficientDictIterationCheckTest {
23+
24+
@Test
25+
void test() {
26+
PythonCheckVerifier.verify("src/test/resources/checks/inefficientDictIteration.py", new InefficientDictIterationCheck());
27+
}
28+
29+
}

‎python-checks/src/test/resources/checks/inefficientDictIteration.py‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
def case1():
2+
fruit = {'a': 'Apple', 'b': 'Banana'}
3+
4+
for _, value in fruit.items(): # Noncompliant
5+
...
6+
7+
for key, _ in fruit.items(): # Noncompliant
8+
...
9+
10+
items1 = fruit.items()
11+
for _, value in items1: # Noncompliant
12+
...
13+
14+
items2 = fruit.items()
15+
for key, _ in items2: # Noncompliant
16+
...
17+
18+
def case2():
19+
fruit = {'a': 'Apple', 'b': 'Banana'}
20+
21+
for key, value in fruit.items():
22+
...
23+
24+
for key, _ in fruit.something():
25+
...
26+
27+
items1 = fruit.items()
28+
something(items1)
29+
for _, value in items1:
30+
...
31+
32+
for key, _ in fruit.items(), fruit.items():
33+
...
34+
35+
for v, v, v in fruit.items():
36+
...
37+
38+
for k.v, v.v in fruit.items():
39+
...

0 commit comments

Comments
 (0)