Skip to content

Commit c86dd55

Browse files
marco-bearzi-sonarsourcemarco-bearzi-sonarsource
authored
SONARPY-1031: Add quick fix for S1854 (DeadStoreCheck) (#1145)
1 parent d83642f commit c86dd55

File tree

3 files changed

+143
-6
lines changed

3 files changed

+143
-6
lines changed

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

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,26 +19,33 @@
1919
*/
2020
package org.sonar.python.checks;
2121

22+
import java.util.List;
23+
import java.util.Optional;
2224
import java.util.Set;
2325
import org.sonar.check.Rule;
2426
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
2527
import org.sonar.plugins.python.api.SubscriptionContext;
2628
import org.sonar.plugins.python.api.cfg.CfgBlock;
2729
import org.sonar.plugins.python.api.cfg.ControlFlowGraph;
30+
import org.sonar.plugins.python.api.symbols.Symbol;
31+
import org.sonar.plugins.python.api.symbols.Usage;
2832
import org.sonar.plugins.python.api.tree.AnnotatedAssignment;
2933
import org.sonar.plugins.python.api.tree.AssignmentStatement;
3034
import org.sonar.plugins.python.api.tree.Expression;
3135
import org.sonar.plugins.python.api.tree.FunctionDef;
3236
import org.sonar.plugins.python.api.tree.Name;
3337
import org.sonar.plugins.python.api.tree.NumericLiteral;
38+
import org.sonar.plugins.python.api.tree.Token;
3439
import org.sonar.plugins.python.api.tree.Tree;
3540
import org.sonar.plugins.python.api.tree.UnaryExpression;
3641
import org.sonar.python.cfg.fixpoint.LiveVariablesAnalysis;
37-
import org.sonar.plugins.python.api.symbols.Symbol;
38-
import org.sonar.plugins.python.api.symbols.Usage;
42+
import org.sonar.python.quickfix.IssueWithQuickFix;
43+
import org.sonar.python.quickfix.PythonQuickFix;
44+
import org.sonar.python.quickfix.PythonTextEdit;
3945
import org.sonar.python.tree.TreeUtils;
4046

4147
import static org.sonar.python.checks.DeadStoreUtils.isUsedInSubFunction;
48+
import static org.sonar.python.quickfix.PythonTextEdit.remove;
4249

4350
@Rule(key = "S1854")
4451
public class DeadStoreCheck extends PythonSubscriptionCheck {
@@ -65,15 +72,21 @@ public void initialize(Context context) {
6572
* Bottom-up approach, keeping track of which variables will be read by successor elements.
6673
*/
6774
private static void verifyBlock(SubscriptionContext ctx, CfgBlock block, LiveVariablesAnalysis.LiveVariables blockLiveVariables,
68-
Set<Symbol> readSymbols, FunctionDef functionDef) {
75+
Set<Symbol> readSymbols, FunctionDef functionDef) {
6976

7077
DeadStoreUtils.findUnnecessaryAssignments(block, blockLiveVariables, functionDef)
7178
.stream()
7279
// symbols should have at least one read usage (otherwise will be reported by S1481)
7380
.filter(unnecessaryAssignment -> readSymbols.contains(unnecessaryAssignment.symbol))
7481
.filter((unnecessaryAssignment -> !isException(unnecessaryAssignment.symbol, unnecessaryAssignment.element, functionDef)))
75-
.forEach(unnecessaryAssignment ->
76-
ctx.addIssue(unnecessaryAssignment.element, String.format(MESSAGE_TEMPLATE, unnecessaryAssignment.symbol.name())));
82+
.forEach(unnecessaryAssignment -> {
83+
Tree element = unnecessaryAssignment.element;
84+
String message = String.format(MESSAGE_TEMPLATE, unnecessaryAssignment.symbol.name());
85+
IssueWithQuickFix issue = (IssueWithQuickFix) separatorToken(element)
86+
.map(separator -> ctx.addIssue(element.firstToken(), separator, message))
87+
.orElseGet(() -> ctx.addIssue(element, message));
88+
createQuickFix(issue, unnecessaryAssignment.element);
89+
});
7790
}
7891

7992
private static boolean isMultipleAssignement(Tree element) {
@@ -138,4 +151,38 @@ private static boolean isNumericLiteralOne(Expression expression) {
138151
private static boolean isFunctionDeclarationSymbol(Symbol symbol) {
139152
return symbol.usages().stream().anyMatch(u -> u.kind() == Usage.Kind.FUNC_DECLARATION);
140153
}
154+
155+
private static Optional<Token> separatorToken(Tree element) {
156+
if (element instanceof AssignmentStatement) {
157+
Token seperator = ((AssignmentStatement) element).separator();
158+
return "\n".equals(seperator.value()) ? Optional.empty() : Optional.of(seperator);
159+
}
160+
return Optional.empty();
161+
}
162+
163+
private static PythonTextEdit removeDeadStore(Tree tree) {
164+
List<Tree> childrenOfParent = tree.parent().children();
165+
if (childrenOfParent.size() == 1) {
166+
return remove(tree);
167+
}
168+
Token current = tree.firstToken();
169+
int i = childrenOfParent.indexOf(tree);
170+
if (i == childrenOfParent.size() - 1) {
171+
Token previous = childrenOfParent.get(i - 1).lastToken();
172+
current = tree.lastToken();
173+
// Replace from the end of the previous token (will also remove the separator and trailing whitespaces) until the end of the current token
174+
return new PythonTextEdit("", previous.line(), previous.column() + previous.value().length(), current.line(), current.column() + current.value().length());
175+
}
176+
Token next = childrenOfParent.get(i + 1).firstToken();
177+
// Remove from the start of the current tokenuntil the next token
178+
return new PythonTextEdit("", current.line(), current.column(), next.line(), next.column());
179+
}
180+
181+
private static void createQuickFix(IssueWithQuickFix issue, Tree unnecessaryAssignment) {
182+
PythonTextEdit edit = removeDeadStore(unnecessaryAssignment);
183+
PythonQuickFix quickFix = PythonQuickFix.newQuickFix("Remove the line(s)")
184+
.addTextEdit(edit)
185+
.build();
186+
issue.addQuickFix(quickFix);
187+
}
141188
}

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

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,99 @@
2020
package org.sonar.python.checks;
2121

2222
import org.junit.Test;
23+
import org.sonar.plugins.python.api.PythonCheck;
24+
import org.sonar.python.checks.quickfix.PythonQuickFixVerifier;
2325
import org.sonar.python.checks.utils.PythonCheckVerifier;
2426

2527
public class DeadStoreCheckTest {
2628

29+
private final PythonCheck check = new DeadStoreCheck();
30+
2731
@Test
2832
public void test() {
29-
PythonCheckVerifier.verify("src/test/resources/checks/deadStore.py", new DeadStoreCheck());
33+
PythonCheckVerifier.verify("src/test/resources/checks/deadStore.py", check);
34+
}
35+
36+
@Test
37+
public void quickfix() {
38+
String codeWithIssue = "def foo():\n" +
39+
" x = 42\n" +
40+
" x = 0\n" +
41+
" print(x)";
42+
String codeFixed = "def foo():\n" +
43+
" x = 0\n" +
44+
" print(x)";
45+
PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
46+
}
47+
48+
@Test
49+
public void semicolon() {
50+
String codeWithIssue = "def foo():\n" +
51+
" x = 42 ;\n" +
52+
" x = 0\n" +
53+
" print(x)";
54+
String codeFixed = "def foo():\n" +
55+
" x = 0\n" +
56+
" print(x)";
57+
PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
58+
}
59+
60+
@Test
61+
public void space() {
62+
String codeWithIssue = "def foo():\n" +
63+
" x = 42 \n" +
64+
" x = 0\n" +
65+
" print(x)";
66+
String codeFixed = "def foo():\n" +
67+
" x = 0\n" +
68+
" print(x)";
69+
PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
70+
}
71+
72+
@Test
73+
public void quickfix_after_non_issue() {
74+
String codeWithIssue = "def foo():\n" +
75+
" a = 1\n" +
76+
" x = 10 ;\n" +
77+
" x = 0\n" +
78+
" print(x)";
79+
String codeFixed = "def foo():\n" +
80+
" a = 1\n" +
81+
" x = 0\n" +
82+
" print(x)";
83+
PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
84+
}
85+
86+
@Test
87+
public void quickfix_oneline() {
88+
String codeWithIssue = "def dead_store(): unused = 24; unused = 42; print(unused)";
89+
String codeFixed = "def dead_store(): unused = 42; print(unused)";
90+
PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
91+
}
92+
93+
@Test
94+
public void quickfix_in_condition() {
95+
String codeWithIssue = "def simple_conditional():\n" +
96+
" x = 10 # Noncompliant\n" +
97+
" if p:\n" +
98+
" x = 11\n" +
99+
" print(x)";
100+
String codeFixed = "def simple_conditional():\n" +
101+
" if p:\n" +
102+
" x = 11\n" +
103+
" print(x)";
104+
PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
105+
}
106+
107+
@Test
108+
public void unused_after_reassignment() {
109+
String codeWithIssue = "def tuple_assign():\n" +
110+
" c = foo()\n" +
111+
" print(c)\n" +
112+
" c = foo()\n";
113+
String codeFixed = "def tuple_assign():\n" +
114+
" c = foo()\n" +
115+
" print(c)\n";
116+
PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
30117
}
31118
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,3 +319,6 @@ def match_statement_no_fp(value):
319319
case a.CONST:
320320
a = 42
321321
print(a)
322+
323+
def dead_store(): unused = 24; unused = 42; print(unused); # NonCompliant
324+
# ^^^^^^^^^^^^

0 commit comments

Comments
 (0)