Skip to content

Commit 4ce892c

Browse files
joke1196sonartech
authored andcommitted
SONARPY-3261: Rule S7508: Add secondary location to improve user experience (#455)
GitOrigin-RevId: 7327d29a9ff6c50e8f17bdcc5ac7a76ba985b84d
1 parent 3a4cc93 commit 4ce892c

2 files changed

Lines changed: 26 additions & 19 deletions

File tree

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

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
@Rule(key = "S7508")
4444
public class NestedCollectionsCreationCheck extends PythonSubscriptionCheck {
4545
private static final String MESSAGE = "Remove this redundant call.";
46+
private static final String SECONDARY_MESSAGE = "A redundant call is done here.";
4647
private static final String QUICK_FIX_MESSAGE = MESSAGE;
4748

4849
private static final String LIST_FQN = "list";
@@ -55,8 +56,7 @@ public class NestedCollectionsCreationCheck extends PythonSubscriptionCheck {
5556
Map.entry(LIST_FQN, Set.of(LIST_FQN, TUPLE_FQN, SORTED_FQN)),
5657
Map.entry(SET_FQN, Set.of(LIST_FQN, SET_FQN, TUPLE_FQN, REVERSED_FQN, SORTED_FQN)),
5758
Map.entry(SORTED_FQN, Set.of(LIST_FQN, TUPLE_FQN, SORTED_FQN)),
58-
Map.entry(TUPLE_FQN, Set.of(LIST_FQN, TUPLE_FQN))
59-
);
59+
Map.entry(TUPLE_FQN, Set.of(LIST_FQN, TUPLE_FQN)));
6060

6161
private TypeCheckMap<Set<TypeCheckBuilder>> sensitiveCallCombinationChecks;
6262

@@ -82,20 +82,23 @@ private void check(SubscriptionContext ctx) {
8282
sensitiveCallCombinationChecks.getOptionalForType(callExpression.callee().typeV2())
8383
.ifPresent(nestedCallTypeChecks -> TreeUtils.nthArgumentOrKeywordOptional(0, "", callExpression.arguments())
8484
.map(RegularArgument::expression)
85-
.ifPresent(argumentExpression -> {
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-
85+
.ifPresent(argumentExpression -> checkCallArgumentsForSensitiveMethods(callExpression, argumentExpression, nestedCallTypeChecks, ctx)));
86+
}
9787

98-
}));
88+
private static void checkCallArgumentsForSensitiveMethods(CallExpression callExpression, Expression argumentExpression, Set<TypeCheckBuilder> nestedCallTypeChecks,
89+
SubscriptionContext ctx) {
90+
findSensitiveMethodCall(argumentExpression, nestedCallTypeChecks)
91+
.ifPresent(redundantCallExpression -> {
92+
var issue = ctx.addIssue(redundantCallExpression.callee(), MESSAGE)
93+
.secondary(callExpression.callee(), SECONDARY_MESSAGE);
94+
createQuickFix(redundantCallExpression).ifPresent(issue::addQuickFix);
95+
});
96+
findAssignedToSensitiveMethodCall(argumentExpression, nestedCallTypeChecks)
97+
.ifPresent(argumentAssignedCall -> {
98+
var issue = ctx.addIssue(argumentAssignedCall.callee(), MESSAGE)
99+
.secondary(callExpression.callee(), SECONDARY_MESSAGE);
100+
createAssignedQuickFix(argumentExpression, argumentAssignedCall).ifPresent(issue::addQuickFix);
101+
});
99102
}
100103

101104
private static Optional<CallExpression> findSensitiveMethodCall(@Nullable Expression expression,

python-checks/src/test/resources/checks/nestedCollectionsCreation.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
def case1():
22
iterable = (3, 1, 4, 1)
3-
list(list(iterable)) # Noncompliant
3+
list(list(iterable)) # Noncompliant {{Remove this redundant call.}}
4+
# ^^^^
5+
# ^^^^@-1< {{A redundant call is done here.}}
46
list(tuple(iterable)) # Noncompliant
57
list(sorted(iterable)) # Noncompliant
68

@@ -35,8 +37,10 @@ def case5():
3537

3638
def case6():
3739
iterable = (3, 1, 4, 1)
38-
single_usage_assigned_list = list(iterable)
39-
list_of_list = list(single_usage_assigned_list) # Noncompliant
40+
single_usage_assigned_list = list(iterable) # Noncompliant
41+
# ^^^^
42+
list_of_list = list(single_usage_assigned_list)
43+
# ^^^^< {{A redundant call is done here.}}
4044
single_usage_assigned_not_sensitive = something_else(iterable)
4145
list_of_list = list(single_usage_assigned_not_sensitive)
4246
multiple_usage_assigned_list = list(iterable)
@@ -47,4 +51,4 @@ def case7():
4751
iterable = (3, 1, 4, 1)
4852
sorted(reversed(iterable)) # OK, it should be raided by S7510
4953
reversed(sorted(iterable)) # OK, it should be raided by S7510
50-
list(set(iterable))
54+
list(set(iterable))

0 commit comments

Comments
 (0)