Skip to content

Commit 7c6c74a

Browse files
maksim-grebeniuk-sonarsourcesonartech
maksim-grebeniuk-sonarsource
authored and
sonartech
committed
SONARPY-3014 Create QuickFix for Rule S7508 (#316)
GitOrigin-RevId: f00ea29415103a47275544e5aa418fd53d98efb7
1 parent bbf6d05 commit 7c6c74a

File tree

2 files changed

+131
-17
lines changed

2 files changed

+131
-17
lines changed

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

Lines changed: 57 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,34 +25,40 @@
2525
import org.sonar.check.Rule;
2626
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
2727
import org.sonar.plugins.python.api.SubscriptionContext;
28+
import org.sonar.plugins.python.api.quickfix.PythonQuickFix;
2829
import org.sonar.plugins.python.api.tree.CallExpression;
2930
import org.sonar.plugins.python.api.tree.Expression;
3031
import org.sonar.plugins.python.api.tree.Name;
3132
import org.sonar.plugins.python.api.tree.RegularArgument;
33+
import org.sonar.plugins.python.api.tree.Statement;
3234
import org.sonar.plugins.python.api.tree.Tree;
3335
import org.sonar.plugins.python.api.types.v2.TriBool;
3436
import org.sonar.python.checks.utils.Expressions;
37+
import org.sonar.python.quickfix.TextEditUtils;
3538
import org.sonar.python.semantic.v2.SymbolV2;
3639
import org.sonar.python.tree.TreeUtils;
3740
import org.sonar.python.types.v2.TypeCheckBuilder;
3841
import org.sonar.python.types.v2.TypeCheckMap;
3942

4043
@Rule(key = "S7508")
4144
public class NestedCollectionsCreationCheck extends PythonSubscriptionCheck {
42-
public static final String LIST_FQN = "list";
43-
public static final String SORTED_FQN = "sorted";
44-
public static final String SET_FQN = "set";
45-
public static final String TUPLE_FQN = "tuple";
46-
public static final String REVERSED_FQN = "reversed";
45+
private static final String MESSAGE = "Remove this redundant call.";
46+
private static final String QUICK_FIX_MESSAGE = MESSAGE;
47+
48+
private static final String LIST_FQN = "list";
49+
private static final String SORTED_FQN = "sorted";
50+
private static final String SET_FQN = "set";
51+
private static final String TUPLE_FQN = "tuple";
52+
private static final String REVERSED_FQN = "reversed";
4753

4854
private static final Map<String, Set<String>> SENSITIVE_NESTED_CALL_COMBINATIONS = Map.ofEntries(
4955
Map.entry(LIST_FQN, Set.of(LIST_FQN, TUPLE_FQN, SORTED_FQN)),
5056
Map.entry(SET_FQN, Set.of(LIST_FQN, SET_FQN, TUPLE_FQN, REVERSED_FQN, SORTED_FQN)),
5157
Map.entry(SORTED_FQN, Set.of(LIST_FQN, TUPLE_FQN, SORTED_FQN)),
5258
Map.entry(TUPLE_FQN, Set.of(LIST_FQN, TUPLE_FQN))
5359
);
54-
private TypeCheckMap<Set<TypeCheckBuilder>> sensitiveCallCombinationChecks;
5560

61+
private TypeCheckMap<Set<TypeCheckBuilder>> sensitiveCallCombinationChecks;
5662

5763
@Override
5864
public void initialize(Context context) {
@@ -77,22 +83,33 @@ private void check(SubscriptionContext ctx) {
7783
.ifPresent(nestedCallTypeChecks -> TreeUtils.nthArgumentOrKeywordOptional(0, "", callExpression.arguments())
7884
.map(RegularArgument::expression)
7985
.ifPresent(argumentExpression -> {
80-
if (isSensitiveMethodCall(argumentExpression, nestedCallTypeChecks) || isAssignedToSensitiveMethodCall(argumentExpression, nestedCallTypeChecks)) {
81-
ctx.addIssue(callExpression, "Remove this redundant call.");
82-
}
86+
findSensitiveMethodCall(argumentExpression, nestedCallTypeChecks)
87+
.ifPresent(redundantCallExpression -> {
88+
var issue = ctx.addIssue(callExpression, MESSAGE);
89+
createQuickFix(redundantCallExpression).ifPresent(issue::addQuickFix);
90+
});
91+
findAssignedToSensitiveMethodCall(argumentExpression, nestedCallTypeChecks)
92+
.ifPresent(argumentAssignedCall -> {
93+
var issue = ctx.addIssue(callExpression, MESSAGE);
94+
createAssignedQuickFix(argumentExpression, argumentAssignedCall).ifPresent(issue::addQuickFix);
95+
});
96+
97+
8398
}));
8499
}
85100

86-
private static boolean isSensitiveMethodCall(@Nullable Expression expression, Set<TypeCheckBuilder> sensitiveMethodsTypeChecks) {
87-
return expression instanceof CallExpression callExpression && sensitiveMethodsTypeChecks.stream()
88-
.map(check -> check.check(callExpression.callee().typeV2()))
89-
.anyMatch(TriBool.TRUE::equals);
101+
private static Optional<CallExpression> findSensitiveMethodCall(@Nullable Expression expression,
102+
Set<TypeCheckBuilder> sensitiveMethodsTypeChecks) {
103+
return TreeUtils.toOptionalInstanceOf(CallExpression.class, expression)
104+
.filter(callExpression -> sensitiveMethodsTypeChecks.stream()
105+
.map(check -> check.check(callExpression.callee().typeV2()))
106+
.anyMatch(TriBool.TRUE::equals));
90107
}
91108

92-
private static boolean isAssignedToSensitiveMethodCall(Expression argumentExpression, Set<TypeCheckBuilder> sensitiveMethodsTypeChecks) {
93-
return argumentExpression instanceof Name name
94-
&& getUsageCount(name) == 2
95-
&& isSensitiveMethodCall(Expressions.singleAssignedValue(name), sensitiveMethodsTypeChecks);
109+
private static Optional<CallExpression> findAssignedToSensitiveMethodCall(Expression argumentExpression, Set<TypeCheckBuilder> sensitiveMethodsTypeChecks) {
110+
return TreeUtils.toOptionalInstanceOf(Name.class, argumentExpression)
111+
.filter(name -> getUsageCount(name) == 2)
112+
.flatMap(name -> findSensitiveMethodCall(Expressions.singleAssignedValue(name), sensitiveMethodsTypeChecks));
96113
}
97114

98115
private static int getUsageCount(Name name) {
@@ -101,4 +118,27 @@ private static int getUsageCount(Name name) {
101118
.map(List::size)
102119
.orElse(0);
103120
}
121+
122+
private static Optional<PythonQuickFix> createQuickFix(Expression argumentCall) {
123+
return TreeUtils.toOptionalInstanceOf(CallExpression.class, argumentCall)
124+
.map(CallExpression::argumentList)
125+
.map(argList -> TreeUtils.treeToString(argList, false))
126+
.map(replacementText -> TextEditUtils.replace(argumentCall, replacementText))
127+
.map(textEdit -> PythonQuickFix.newQuickFix(QUICK_FIX_MESSAGE).addTextEdit(textEdit))
128+
.map(PythonQuickFix.Builder::build);
129+
}
130+
131+
private static Optional<PythonQuickFix> createAssignedQuickFix(Expression argumentExpression, Expression argumentAssignedValueCall) {
132+
return TreeUtils.toOptionalInstanceOf(CallExpression.class, argumentAssignedValueCall)
133+
.map(CallExpression::argumentList)
134+
.map(argList -> TreeUtils.treeToString(argList, false))
135+
.map(replacementText -> {
136+
var quickFix = PythonQuickFix.newQuickFix(QUICK_FIX_MESSAGE)
137+
.addTextEdit(TextEditUtils.replace(argumentExpression, replacementText));
138+
TreeUtils.toOptionalInstanceOf(Statement.class, TreeUtils.firstAncestor(argumentAssignedValueCall, Statement.class::isInstance))
139+
.map(TextEditUtils::removeStatement)
140+
.ifPresent(quickFix::addTextEdit);
141+
return quickFix.build();
142+
});
143+
}
104144
}

‎python-checks/src/test/java/org/sonar/python/checks/NestedCollectionsCreationCheckTest.java‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,12 @@
1616
*/
1717
package org.sonar.python.checks;
1818

19+
import java.util.stream.Stream;
1920
import org.junit.jupiter.api.Test;
21+
import org.junit.jupiter.params.ParameterizedTest;
22+
import org.junit.jupiter.params.provider.Arguments;
23+
import org.junit.jupiter.params.provider.MethodSource;
24+
import org.sonar.python.checks.quickfix.PythonQuickFixVerifier;
2025
import org.sonar.python.checks.utils.PythonCheckVerifier;
2126

2227
class NestedCollectionsCreationCheckTest {
@@ -26,4 +31,73 @@ void test() {
2631
PythonCheckVerifier.verify("src/test/resources/checks/nestedCollectionsCreation.py", new NestedCollectionsCreationCheck());
2732
}
2833

34+
@ParameterizedTest
35+
@MethodSource("quickFixTestCases")
36+
void quickFixTest(String before, String after, String expectedMessage) {
37+
var check = new NestedCollectionsCreationCheck();
38+
PythonQuickFixVerifier.verify(check, before, after);
39+
PythonQuickFixVerifier.verifyQuickFixMessages(check, before, expectedMessage);
40+
}
41+
42+
@ParameterizedTest
43+
@MethodSource("noQuickFixTestCases")
44+
void noQuickFixTest(String before) {
45+
PythonQuickFixVerifier.verifyNoQuickFixes(new NestedCollectionsCreationCheck(), before);
46+
}
47+
48+
static Stream<Arguments> quickFixTestCases() {
49+
return Stream.of(
50+
Arguments.of(
51+
"""
52+
iterable = (3, 1, 4, 1)
53+
list(list(iterable))
54+
""",
55+
"""
56+
iterable = (3, 1, 4, 1)
57+
list(iterable)
58+
""",
59+
"Remove this redundant call."),
60+
Arguments.of(
61+
"""
62+
iterable = (3, 1, 4, 1)
63+
set(list(iterable))
64+
""",
65+
"""
66+
iterable = (3, 1, 4, 1)
67+
set(iterable)
68+
""",
69+
"Remove this redundant call."),
70+
Arguments.of(
71+
"""
72+
set(list((3, 1, 4, 1)))
73+
""",
74+
"""
75+
set((3, 1, 4, 1))
76+
""",
77+
"Remove this redundant call."),
78+
Arguments.of(
79+
"""
80+
iterable = (3, 1, 4, 1)
81+
l = list(iterable)
82+
set(l)
83+
""",
84+
"""
85+
iterable = (3, 1, 4, 1)
86+
set(iterable)
87+
""",
88+
"Remove this redundant call.")
89+
);
90+
}
91+
92+
static Stream<Arguments> noQuickFixTestCases() {
93+
return Stream.of(
94+
Arguments.of(
95+
"""
96+
set(list((
97+
3, 1,
98+
4, 1)))
99+
""")
100+
);
101+
}
102+
29103
}

0 commit comments

Comments
 (0)