Skip to content

Commit a985381

Browse files
pynicolasivandalbosco
authored andcommitted
SONARPY-205 SelfAssignmentCheck: fix FP on imported name
1 parent 865a0df commit a985381

File tree

2 files changed

+54
-11
lines changed

2 files changed

+54
-11
lines changed

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

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

22+
import com.google.common.collect.ImmutableSet;
23+
import com.sonar.sslr.api.AstNode;
24+
import com.sonar.sslr.api.AstNodeType;
25+
import java.util.HashSet;
2226
import java.util.Set;
23-
2427
import org.sonar.check.Rule;
2528
import org.sonar.python.PythonCheck;
2629
import org.sonar.python.api.PythonGrammar;
30+
import org.sonar.python.api.PythonKeyword;
2731
import org.sonar.python.api.PythonPunctuator;
2832
import org.sonar.sslr.ast.AstSelect;
2933

30-
import com.google.common.collect.ImmutableSet;
31-
import com.sonar.sslr.api.AstNode;
32-
import com.sonar.sslr.api.AstNodeType;
33-
3434
@Rule(key = SelfAssignmentCheck.CHECK_KEY)
3535
public class SelfAssignmentCheck extends PythonCheck {
3636

3737
public static final String CHECK_KEY = "S1656";
3838

3939
public static final String MESSAGE = "Remove or correct this useless self-assignment.";
4040

41+
private Set<String> importedNames = new HashSet<>();
42+
43+
@Override
44+
public void visitFile(AstNode node) {
45+
importedNames.clear();
46+
}
47+
4148
@Override
4249
public Set<AstNodeType> subscribedKinds() {
43-
return ImmutableSet.of(PythonGrammar.EXPRESSION_STMT);
50+
return ImmutableSet.of(
51+
PythonGrammar.EXPRESSION_STMT,
52+
PythonGrammar.IMPORT_NAME,
53+
PythonGrammar.IMPORT_AS_NAME);
4454
}
4555

4656
@Override
4757
public void visitNode(AstNode node) {
48-
for (AstNode assignOperator : node.getChildren(PythonPunctuator.ASSIGN)) {
49-
if (CheckUtils.equalNodes(assignOperator.getPreviousSibling(), assignOperator.getNextSibling()) && !isException(node)) {
50-
addIssue(assignOperator, MESSAGE);
58+
if (node.is(PythonGrammar.IMPORT_NAME)) {
59+
for (AstNode dottedAsName : node.select().children(PythonGrammar.DOTTED_AS_NAMES).children(PythonGrammar.DOTTED_AS_NAME)) {
60+
AstNode importedName = dottedAsName.getFirstChild().getLastChild(PythonGrammar.NAME);
61+
addImportedName(dottedAsName, importedName);
62+
}
63+
64+
} else if (node.is(PythonGrammar.IMPORT_AS_NAME)) {
65+
AstNode importedName = node.getFirstChild(PythonGrammar.NAME);
66+
addImportedName(node, importedName);
67+
68+
} else {
69+
for (AstNode assignOperator : node.getChildren(PythonPunctuator.ASSIGN)) {
70+
AstNode assigned = assignOperator.getPreviousSibling();
71+
if (CheckUtils.equalNodes(assigned, assignOperator.getNextSibling()) && !isException(node, assigned)) {
72+
addIssue(assignOperator, MESSAGE);
73+
}
5174
}
5275
}
5376
}
5477

55-
private static boolean isException(AstNode expressionStatement) {
56-
AstSelect potentialFunctionCalls = expressionStatement.select()
78+
private void addImportedName(AstNode node, AstNode importedName) {
79+
AstNode name = importedName;
80+
AstNode as = node.getFirstChild(PythonKeyword.AS);
81+
if (as != null) {
82+
name = as.getNextSibling();
83+
}
84+
importedNames.add(name.getTokenValue());
85+
}
86+
87+
private boolean isException(AstNode expressionStatement, AstNode assigned) {
88+
AstSelect potentialFunctionCalls = assigned.select()
5789
.descendants(PythonGrammar.TRAILER)
5890
.children(PythonPunctuator.LPARENTHESIS);
5991
if (!potentialFunctionCalls.isEmpty()) {
6092
return true;
6193
}
94+
if (assigned.getTokens().size() == 1 && importedNames.contains(assigned.getTokenValue())) {
95+
return true;
96+
}
6297
AstNode suite = expressionStatement.getFirstAncestor(PythonGrammar.SUITE);
6398
return suite != null && suite.getParent().is(PythonGrammar.CLASSDEF, PythonGrammar.TRY_STMT);
6499
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
import import1, x as import2
2+
from module1 import (import3, x as import4)
3+
14
x = 1
25
x = y
36
x = x # Noncompliant
@@ -21,3 +24,8 @@ def f():
2124
a.x = a.x # Noncompliant
2225
a[x] = a[x] # Noncompliant
2326
a[sideEffect()] = a[sideEffect()]
27+
28+
import1 = import1
29+
import2 = import2
30+
import3 = import3
31+
import4 = import4

0 commit comments

Comments
 (0)