Skip to content

Commit 65a587a

Browse files
joke1196sonartech
authored andcommitted
SONARPY-2994 Rule S7516: Unnecessary sort inside set (#305)
GitOrigin-RevId: 0666265776081f8127c1ce5b13e777337c61f638
1 parent 87285fa commit 65a587a

File tree

7 files changed

+208
-0
lines changed

7 files changed

+208
-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
@@ -408,6 +408,7 @@ public Stream<Class<?>> getChecks() {
408408
UnnecessaryLambdaMapCallCheck.class,
409409
UnnecessaryListCastCheck.class,
410410
UnnecessaryReversedCallCheck.class,
411+
UnnecessarySortInsideSetCheck.class,
411412
UnnecessarySubscriptReversalCheck.class,
412413
UnquantifiedNonCapturingGroupCheck.class,
413414
UnreachableExceptCheck.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.Optional;
20+
import javax.annotation.Nullable;
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.Expression;
26+
import org.sonar.plugins.python.api.tree.Name;
27+
import org.sonar.plugins.python.api.tree.RegularArgument;
28+
import org.sonar.plugins.python.api.tree.Tree;
29+
import org.sonar.plugins.python.api.types.v2.TriBool;
30+
import org.sonar.python.checks.utils.Expressions;
31+
import org.sonar.python.tree.TreeUtils;
32+
import org.sonar.python.types.v2.TypeCheckBuilder;
33+
34+
@Rule(key = "S7516")
35+
public class UnnecessarySortInsideSetCheck extends PythonSubscriptionCheck {
36+
37+
private static final String MESSAGE = "Remove either the call to set or sorted.";
38+
private TypeCheckBuilder isSetTypeCheck;
39+
private TypeCheckBuilder isSortedTypeCheck;
40+
41+
@Override
42+
public void initialize(Context context) {
43+
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, this::initChecks);
44+
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, this::check);
45+
}
46+
47+
private void initChecks(SubscriptionContext ctx) {
48+
isSetTypeCheck = ctx.typeChecker().typeCheckBuilder().isBuiltinWithName("set");
49+
isSortedTypeCheck = ctx.typeChecker().typeCheckBuilder().isBuiltinWithName("sorted");
50+
}
51+
52+
private void check(SubscriptionContext ctx) {
53+
if (ctx.syntaxNode() instanceof CallExpression callExpression && isSetCall(callExpression)) {
54+
TreeUtils.nthArgumentOrKeywordOptional(0, "", callExpression.arguments())
55+
.map(RegularArgument::expression)
56+
.ifPresent(argumentExpression -> {
57+
if (isSortedCall(argumentExpression)) {
58+
ctx.addIssue(callExpression.callee(), MESSAGE);
59+
} else {
60+
isAssignedToSortedCall(argumentExpression).ifPresent(sortedCallExpr -> {
61+
PreciseIssue issue = ctx.addIssue(callExpression.callee(), MESSAGE);
62+
issue.secondary(sortedCallExpr.callee(), "The list is sorted here.");
63+
});
64+
}
65+
});
66+
}
67+
}
68+
69+
private Optional<CallExpression> isAssignedToSortedCall(Expression argumentExpression) {
70+
if (argumentExpression instanceof Name name && getUsageCount(name) == 2) {
71+
var assignedValue = Expressions.singleAssignedValue(name);
72+
if (isSortedCall(assignedValue)) {
73+
return Optional.of((CallExpression) assignedValue);
74+
}
75+
}
76+
return Optional.empty();
77+
}
78+
79+
private boolean isSetCall(CallExpression callExpression) {
80+
return isSetTypeCheck.check(callExpression.callee().typeV2()) == TriBool.TRUE;
81+
}
82+
83+
private boolean isSortedCall(@Nullable Expression expression) {
84+
return expression instanceof CallExpression callExpression && isSortedTypeCheck.check(callExpression.callee().typeV2()) == TriBool.TRUE;
85+
}
86+
87+
private static int getUsageCount(Name name) {
88+
var symbol = name.symbolV2();
89+
if (symbol == null) {
90+
return 0;
91+
}
92+
return symbol.usages().size();
93+
}
94+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<p>This raises an issue when <code>set()</code> is directly around the output of <code>sorted()</code>.</p>
2+
<h2>Why is this an issue?</h2>
3+
<p>Calling <code>set(sorted(iterable))</code> is usually an indication of a misunderstanding of the desired outcome or an inefficient way to achieve
4+
it. The <code>sorted()</code> function produces a <strong>list</strong> of items in sorted order. Applying <code>set()</code> to this sorted list
5+
converts it into a set, which is an <strong>unordered</strong> collection of unique elements. The effort spent on sorting is immediately negated if
6+
the final result is an unordered set, as the order established by <code>sorted()</code> is discarded.</p>
7+
<p>If the intention is to obtain a sorted list of unique elements from an iterable, the pattern <code>set(sorted(iterable))</code> is inefficient. It
8+
first sorts all elements, including duplicates (which can be computationally expensive for large lists with many duplicates), and then removes these
9+
duplicates while also discarding the order established by <code>sorted()</code>. The more efficient and standard idiom for getting unique, sorted
10+
items is to deduplicate <strong>first</strong> using <code>set()</code>, and then sort the unique items: <code>sorted(set(iterable))</code>. This way,
11+
<code>sorted()</code> operates on a potentially smaller collection of unique items.</p>
12+
<h2>How to fix it</h2>
13+
<p>To fix this issue remove the call to either <code>set()</code> or <code>sorted()</code>, or call <code>sorted()</code> on the output of
14+
<code>set()</code>. If the goal is just to obtain unique items (and order is not important), then <code>set(iterable)</code> is sufficient; the
15+
<code>sorted()</code> call can be removed. If the goal is to obtain sorted items with duplicates preserved, then <code>sorted(iterable)</code> is
16+
sufficient; the <code>set()</code> call can be removed. If the goal is to obtain sorted items with duplicates removed, then
17+
<code>sorted(set(iterable))</code> is the correct way to proceed.</p>
18+
<h3>Code examples</h3>
19+
<h4>Noncompliant code example</h4>
20+
<pre data-diff-id="1" data-diff-type="noncompliant">
21+
data = [3, 4, 1 , 2]
22+
set(sorted(data)) # Noncompliant: set is called on a sorted list
23+
</pre>
24+
<h4>Compliant solution</h4>
25+
<pre data-diff-id="1" data-diff-type="compliant">
26+
data = [3, 4, 1 , 2]
27+
sorted(data) # Compliant
28+
</pre>
29+
<h2>Resources</h2>
30+
<h3>Documentation</h3>
31+
<ul>
32+
<li> Python Documentation - <a href="https://docs.python.org/3/library/functions.html#sorted">sorted</a> </li>
33+
<li> Python Documentation - <a href="https://docs.python.org/3/tutorial/datastructures.html#sets">sets</a> </li>
34+
</ul>
35+
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{
2+
"title": "\"sorted\" should not be wrapped directly inside \"set\"",
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-7516",
12+
"sqKey": "S7516",
13+
"scope": "All",
14+
"quickfix": "targeted",
15+
"code": {
16+
"impacts": {
17+
"RELIABILITY": "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
@@ -281,6 +281,7 @@
281281
"S7510",
282282
"S7511",
283283
"S7512",
284+
"S7516",
284285
"S7517"
285286
]
286287
}
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 UnnecessarySortInsideSetCheckTest {
23+
24+
@Test
25+
void test() {
26+
PythonCheckVerifier.verify("src/test/resources/checks/unnecessarySortInsideSet.py", new UnnecessarySortInsideSetCheck());
27+
}
28+
29+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
data = [3, 1, 4, 1, 5, 9]
2+
result = set(sorted(data)) # Noncompliant {{Remove either the call to set or sorted.}}
3+
# ^^^
4+
5+
sorted_data = sorted(data)
6+
# ^^^^^^> {{The list is sorted here.}}
7+
assigned = set(sorted_data) # Noncompliant
8+
# ^^^
9+
just_set = set(data)
10+
11+
unique_sorted = sorted(set(data))
12+
13+
# ============== COVERAGE ================
14+
15+
a = list(sorted(data))
16+
a = set([1,2])
17+
18+
multiple_assignement = sorted(data)
19+
multiple_assignement = [1,3,3]
20+
b = set(multiple_assignement)
21+
22+
unsorted = list(data)
23+
c = set(unsorted)
24+
25+
from module import something
26+
assignment = something[0:1]
27+
d = set(assignement)

0 commit comments

Comments
 (0)