Skip to content

Commit d3b703d

Browse files
thomas-serre-sonarsourcesonartech
thomas-serre-sonarsource
authored and
sonartech
committed
SONARPY-2938 Rule S7496: Unnecessary <list/tuple> passed to tuple() - rewrite as literal or remove call (#282)
GitOrigin-RevId: 2ef85ea1418fb9733cced7391365fa8ee02ea24b
1 parent afc0e22 commit d3b703d

9 files changed

Lines changed: 451 additions & 66 deletions

File tree

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

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
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.EnumSet;
20+
import java.util.List;
21+
import java.util.Map;
22+
import java.util.Set;
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.Argument;
27+
import org.sonar.plugins.python.api.tree.CallExpression;
28+
import org.sonar.plugins.python.api.tree.ComprehensionExpression;
29+
import org.sonar.plugins.python.api.tree.Expression;
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.tree.Tree.Kind;
33+
import org.sonar.python.checks.utils.IsComprehensionTransformedChecker;
34+
import org.sonar.python.types.v2.TypeCheckBuilder;
35+
import org.sonar.python.types.v2.TypeCheckMap;
36+
37+
@Rule(key = "S7496")
38+
public class CollectionCreationWrappedInConstructorCheck extends PythonSubscriptionCheck {
39+
40+
private static final String TUPLE = "tuple";
41+
private static final String SET = "set";
42+
private static final String LIST = "list";
43+
private static final String DICT = "dict";
44+
45+
private static final Set<String> COLLECTION_TYPES = Set.of(LIST, SET, DICT, TUPLE);
46+
47+
private static final Set<Kind> COLLECTION_KINDS = EnumSet.of(
48+
Kind.LIST_LITERAL, Kind.SET_LITERAL, Kind.DICTIONARY_LITERAL, Kind.TUPLE,
49+
Kind.LIST_COMPREHENSION, Kind.SET_COMPREHENSION, Kind.DICT_COMPREHENSION
50+
);
51+
52+
private static final Map<String, List<Kind>> SAME_INNER_TYPES_MAP = Map.of(
53+
LIST, List.of(Kind.LIST_LITERAL, Kind.LIST_COMPREHENSION),
54+
SET, List.of(Kind.SET_LITERAL, Kind.SET_COMPREHENSION),
55+
DICT, List.of(Kind.DICTIONARY_LITERAL, Kind.DICT_COMPREHENSION),
56+
TUPLE, List.of(Kind.TUPLE)
57+
);
58+
59+
private final TypeCheckMap<String> collectionTypeCheckerMap = new TypeCheckMap<>();
60+
private IsComprehensionTransformedChecker isComprehensionTransformedChecker = null;
61+
62+
@Override
63+
public void initialize(Context context) {
64+
isComprehensionTransformedChecker = new IsComprehensionTransformedChecker(context);
65+
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, this::initializeTypeCheckers);
66+
context.registerSyntaxNodeConsumer(Kind.CALL_EXPR, this::checkCalls);
67+
}
68+
69+
private void initializeTypeCheckers(SubscriptionContext ctx) {
70+
for (var collectionType : COLLECTION_TYPES) {
71+
TypeCheckBuilder typeChecker = ctx.typeChecker().typeCheckBuilder().isTypeWithFqn(collectionType);
72+
collectionTypeCheckerMap.put(typeChecker, collectionType);
73+
}
74+
}
75+
76+
private void checkCalls(SubscriptionContext ctx) {
77+
CallExpression callExpression = (CallExpression) ctx.syntaxNode();
78+
Expression callee = callExpression.callee();
79+
RegularArgument regularArgument = getSingleRegularArgument(callExpression);
80+
if (regularArgument == null) {
81+
return;
82+
}
83+
84+
if (regularArgument.expression() instanceof ComprehensionExpression comprehensionExpression
85+
&& !isComprehensionTransformedChecker.isGeneratorTransformingData(comprehensionExpression, callee.typeV2())) {
86+
return;
87+
}
88+
89+
var collectionType = collectionTypeCheckerMap.getForType(callee.typeV2());
90+
if (collectionType == null) {
91+
return;
92+
}
93+
94+
if (isSameInnerType(collectionType, regularArgument)) {
95+
ctx.addIssue(callExpression.callee(), getSameInnerTypeCollectionMessage(collectionType));
96+
} else if (isSensitiveDifferentLiteralInnerType(collectionType, regularArgument)) {
97+
ctx.addIssue(callExpression.callee(), getDifferentInnerTypeCollectionMessage(collectionType));
98+
} else if (collectionType.equals(TUPLE)) {
99+
handleTupleConstructorSpecialCases(ctx, regularArgument);
100+
}
101+
}
102+
103+
private static RegularArgument getSingleRegularArgument(CallExpression callExpression) {
104+
if (callExpression.arguments().size() != 1) {
105+
return null;
106+
}
107+
Argument argument = callExpression.arguments().get(0);
108+
if (!(argument instanceof RegularArgument regularArgument)) {
109+
return null;
110+
}
111+
return regularArgument;
112+
}
113+
114+
private static boolean isSameInnerType(String collectionType, RegularArgument regularArgument) {
115+
return SAME_INNER_TYPES_MAP.get(collectionType)
116+
.stream()
117+
.anyMatch(kind -> regularArgument.expression().is(kind));
118+
}
119+
120+
private static boolean isSensitiveDifferentLiteralInnerType(String collectionType, RegularArgument regularArgument) {
121+
if (!isCollectionKind(regularArgument)) {
122+
return false;
123+
}
124+
125+
if (isSameInnerType(collectionType, regularArgument)) {
126+
return false;
127+
}
128+
129+
// It is common and valid to check for duplicates using set comprehension before creating another collection
130+
if (regularArgument.expression().is(Kind.SET_COMPREHENSION)) {
131+
return false;
132+
}
133+
134+
// When tuple are created from comprehensions, the issue is handled in handleTupleConstructorSpecialCases as it requires a special message
135+
EnumSet<Kind> comprehensions = EnumSet.of(Kind.LIST_COMPREHENSION, Kind.SET_COMPREHENSION, Kind.DICT_COMPREHENSION);
136+
return !collectionType.equals(TUPLE) || !comprehensions.contains(regularArgument.expression().getKind());
137+
}
138+
139+
private static String getSameInnerTypeCollectionMessage(String type) {
140+
return "Remove the redundant " + type + " constructor call.";
141+
}
142+
143+
private static String getDifferentInnerTypeCollectionMessage(String type) {
144+
return "Replace this " + type + " constructor call by a " + type + " literal.";
145+
}
146+
147+
private static void handleTupleConstructorSpecialCases(SubscriptionContext ctx, RegularArgument regularArgument) {
148+
if (regularArgument.expression().is(Kind.LIST_COMPREHENSION)) {
149+
ctx.addIssue(regularArgument, "Replace this list comprehension by a generator.");
150+
} else if (regularArgument.expression().is(Kind.SET_COMPREHENSION)) {
151+
ctx.addIssue(regularArgument, "Replace this set comprehension by a generator.");
152+
} else if (regularArgument.expression().is(Kind.DICT_COMPREHENSION)) {
153+
ctx.addIssue(regularArgument, "Replace this dict comprehension by a generator.");
154+
}
155+
}
156+
157+
private static boolean isCollectionKind(RegularArgument regularArgument) {
158+
return COLLECTION_KINDS.contains(regularArgument.expression().getKind());
159+
}
160+
}

‎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
@@ -146,6 +146,7 @@ public Stream<Class<?>> getChecks() {
146146
ClearTextProtocolsCheck.class,
147147
CognitiveComplexityFunctionCheck.class,
148148
CollapsibleIfStatementsCheck.class,
149+
CollectionCreationWrappedInConstructorCheck.class,
149150
CollectionLengthComparisonCheck.class,
150151
CommandLineArgsCheck.class,
151152
CommentedCodeCheck.class,

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

Lines changed: 3 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,15 @@
1919
import java.util.List;
2020
import java.util.Map;
2121
import java.util.Optional;
22-
import javax.annotation.Nullable;
2322
import org.sonar.check.Rule;
2423
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
2524
import org.sonar.plugins.python.api.tree.Argument;
2625
import org.sonar.plugins.python.api.tree.CallExpression;
27-
import org.sonar.plugins.python.api.tree.ComprehensionClause;
2826
import org.sonar.plugins.python.api.tree.ComprehensionExpression;
2927
import org.sonar.plugins.python.api.tree.Expression;
30-
import org.sonar.plugins.python.api.tree.Name;
3128
import org.sonar.plugins.python.api.tree.RegularArgument;
3229
import org.sonar.plugins.python.api.tree.Tree;
33-
import org.sonar.plugins.python.api.tree.Tuple;
34-
import org.sonar.plugins.python.api.types.v2.PythonType;
35-
import org.sonar.plugins.python.api.types.v2.TriBool;
36-
import org.sonar.python.semantic.v2.SymbolV2;
30+
import org.sonar.python.checks.utils.IsComprehensionTransformedChecker;
3731
import org.sonar.python.types.v2.TypeCheckBuilder;
3832
import org.sonar.python.types.v2.TypeCheckMap;
3933

@@ -47,19 +41,15 @@ public class RewriteCollectionConstructorAsComprehensionCheck extends PythonSubs
4741
);
4842

4943
private TypeCheckMap<String> collectionTypeCheckerMap = null;
50-
private TypeCheckBuilder dictTypeChecker = null;
5144

5245
@Override
5346
public void initialize(Context context) {
47+
IsComprehensionTransformedChecker isComprehensionTransformedChecker = new IsComprehensionTransformedChecker(context);
5448
collectionTypeCheckerMap = new TypeCheckMap<>();
5549
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, ctx -> {
5650
for (var collectionEntry : COLLECTION_MESSAGES.entrySet()) {
5751
TypeCheckBuilder typeChecker = ctx.typeChecker().typeCheckBuilder().isTypeWithFqn(collectionEntry.getKey());
5852
collectionTypeCheckerMap.put(typeChecker, collectionEntry.getValue());
59-
60-
if ("dict".equals(collectionEntry.getKey())) {
61-
dictTypeChecker = typeChecker;
62-
}
6353
}
6454
});
6555

@@ -77,7 +67,7 @@ public void initialize(Context context) {
7767
return;
7868
}
7969

80-
if (isGeneratorTransformingData(generator, callExpression.callee().typeV2())) {
70+
if (isComprehensionTransformedChecker.isGeneratorTransformingData(generator, callExpression.callee().typeV2())) {
8171
ctx.addIssue(callExpression.callee(), message);
8272
}
8373
});
@@ -103,57 +93,4 @@ private static Optional<ComprehensionExpression> getSingleGeneratorArg(List<Argu
10393

10494
return Optional.of((ComprehensionExpression) argument);
10595
}
106-
107-
private boolean isGeneratorTransformingData(ComprehensionExpression generator, PythonType methodType) {
108-
if (hasMultipleNestedClauses(generator.comprehensionFor())) {
109-
return true;
110-
}
111-
if (dictTypeChecker.check(methodType) == TriBool.TRUE) {
112-
return isDictTransformingData(generator);
113-
} else {
114-
return isListOrSetComprehensionTransformingData(generator);
115-
}
116-
}
117-
118-
private static boolean hasMultipleNestedClauses(ComprehensionClause clause) {
119-
return clause.nestedClause() != null;
120-
}
121-
122-
private static boolean isDictTransformingData(ComprehensionExpression generator) {
123-
Expression resultExpr = generator.resultExpression();
124-
Expression loopVarExpr = generator.comprehensionFor().loopExpression();
125-
126-
var resultTuple = getTwoNamedTuple(resultExpr);
127-
var varTuple = getTwoNamedTuple(loopVarExpr);
128-
129-
if (resultTuple != null && varTuple != null) {
130-
return !resultTuple.equals(varTuple);
131-
}
132-
133-
return true;
134-
}
135-
136-
private static TwoNamedTuple getTwoNamedTuple(Expression expr) {
137-
if (expr instanceof Tuple tuple && tuple.elements().size() == 2) {
138-
var elements = tuple.elements();
139-
if (elements.get(0) instanceof Name name1 && elements.get(1) instanceof Name name2) {
140-
return new TwoNamedTuple(name1.symbolV2(), name2.symbolV2());
141-
}
142-
}
143-
return null;
144-
}
145-
146-
private record TwoNamedTuple(@Nullable SymbolV2 symbol1, @Nullable SymbolV2 symbol2) {
147-
}
148-
149-
private static boolean isListOrSetComprehensionTransformingData(ComprehensionExpression generator) {
150-
Expression elementExpr = generator.resultExpression();
151-
Expression loopVarExpr = generator.comprehensionFor().loopExpression();
152-
153-
if (elementExpr instanceof Name elementName && loopVarExpr instanceof Name loopVarName) {
154-
return elementName.symbolV2() != loopVarName.symbolV2();
155-
}
156-
157-
return true;
158-
}
15996
}

‎python-checks/src/main/java/org/sonar/python/checks/utils/IsComprehensionTransformedChecker.java‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
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.utils;
18+
19+
import javax.annotation.Nullable;
20+
import org.sonar.plugins.python.api.SubscriptionCheck;
21+
import org.sonar.plugins.python.api.tree.ComprehensionClause;
22+
import org.sonar.plugins.python.api.tree.ComprehensionExpression;
23+
import org.sonar.plugins.python.api.tree.Expression;
24+
import org.sonar.plugins.python.api.tree.Name;
25+
import org.sonar.plugins.python.api.tree.Tree;
26+
import org.sonar.plugins.python.api.tree.Tuple;
27+
import org.sonar.plugins.python.api.types.v2.PythonType;
28+
import org.sonar.plugins.python.api.types.v2.TriBool;
29+
import org.sonar.python.semantic.v2.SymbolV2;
30+
import org.sonar.python.types.v2.TypeCheckBuilder;
31+
32+
public class IsComprehensionTransformedChecker {
33+
34+
private TypeCheckBuilder dictTypeChecker = null;
35+
36+
public IsComprehensionTransformedChecker(SubscriptionCheck.Context context) {
37+
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, ctx -> dictTypeChecker = ctx.typeChecker().typeCheckBuilder().isTypeWithFqn("dict"));
38+
}
39+
40+
public boolean isGeneratorTransformingData(ComprehensionExpression generator, PythonType methodType) {
41+
if (hasMultipleNestedClauses(generator.comprehensionFor())) {
42+
return true;
43+
}
44+
if (dictTypeChecker.check(methodType) == TriBool.TRUE) {
45+
return isDictTransformingData(generator);
46+
} else {
47+
return isListOrSetComprehensionTransformingData(generator);
48+
}
49+
}
50+
51+
private static boolean hasMultipleNestedClauses(ComprehensionClause clause) {
52+
return clause.nestedClause() != null;
53+
}
54+
55+
private static boolean isDictTransformingData(ComprehensionExpression generator) {
56+
Expression resultExpr = generator.resultExpression();
57+
Expression loopVarExpr = generator.comprehensionFor().loopExpression();
58+
59+
var resultTuple = getTwoNamedTuple(resultExpr);
60+
var varTuple = getTwoNamedTuple(loopVarExpr);
61+
62+
if (resultTuple != null && varTuple != null) {
63+
return !resultTuple.equals(varTuple);
64+
}
65+
66+
return true;
67+
}
68+
69+
private static TwoNamedTuple getTwoNamedTuple(Expression expr) {
70+
if (expr instanceof Tuple tuple && tuple.elements().size() == 2) {
71+
var elements = tuple.elements();
72+
if (elements.get(0) instanceof Name name1 && elements.get(1) instanceof Name name2) {
73+
return new TwoNamedTuple(name1.symbolV2(), name2.symbolV2());
74+
}
75+
}
76+
return null;
77+
}
78+
79+
private record TwoNamedTuple(@Nullable SymbolV2 symbol1, @Nullable SymbolV2 symbol2) {
80+
}
81+
82+
private static boolean isListOrSetComprehensionTransformingData(ComprehensionExpression generator) {
83+
Expression elementExpr = generator.resultExpression();
84+
Expression loopVarExpr = generator.comprehensionFor().loopExpression();
85+
86+
if (elementExpr instanceof Name elementName && loopVarExpr instanceof Name loopVarName) {
87+
return elementName.symbolV2() != loopVarName.symbolV2();
88+
}
89+
90+
return true;
91+
}
92+
}

0 commit comments

Comments
 (0)