Skip to content

Commit e82c423

Browse files
author
Jeremi Do Dinh
authored
SONARPY-1462 Rule S6725: Add quick fix for equality checks against np.nan. (#1576)
1 parent 1372bb9 commit e82c423

File tree

6 files changed

+156
-61
lines changed

6 files changed

+156
-61
lines changed

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

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

22+
import java.util.Optional;
2223
import org.sonar.check.Rule;
2324
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
2425
import org.sonar.plugins.python.api.SubscriptionContext;
26+
import org.sonar.plugins.python.api.quickfix.PythonQuickFix;
2527
import org.sonar.plugins.python.api.symbols.Symbol;
2628
import org.sonar.plugins.python.api.tree.BinaryExpression;
2729
import org.sonar.plugins.python.api.tree.Expression;
28-
import org.sonar.plugins.python.api.tree.HasSymbol;
2930
import org.sonar.plugins.python.api.tree.Name;
3031
import org.sonar.plugins.python.api.tree.QualifiedExpression;
3132
import org.sonar.plugins.python.api.tree.Tree;
33+
import org.sonar.python.quickfix.TextEditUtils;
34+
import org.sonar.python.tree.TreeUtils;
3235

3336
@Rule(key = "S6725")
3437
public class NumpyIsNanCheck extends PythonSubscriptionCheck {
38+
39+
private static final String MESSAGE = "Don't perform an equality check against \"numpy.nan\".";
40+
private static final String QUICK_FIX_MESSAGE_EQUALITY = "Replace this equality check with numpy.isnan().";
41+
private static final String QUICK_FIX_MESSAGE_INEQUALITY = "Replace this inequality check with !numpy.isnan().";
42+
3543
@Override
3644
public void initialize(Context context) {
3745
context.registerSyntaxNodeConsumer(Tree.Kind.COMPARISON, NumpyIsNanCheck::checkForIsNan);
@@ -43,22 +51,47 @@ private static void checkForIsNan(SubscriptionContext ctx) {
4351
if (!("==".equals(value) || "!=".equals(value))) {
4452
return;
4553
}
46-
checkOperand(ctx, be.leftOperand(), be);
47-
checkOperand(ctx, be.rightOperand(), be);
54+
checkOperand(ctx, be.leftOperand(), be.rightOperand(), be);
55+
checkOperand(ctx, be.rightOperand(), be.leftOperand(), be);
56+
}
57+
58+
private static void checkOperand(SubscriptionContext ctx, Expression operand, Expression otherOperand, BinaryExpression be) {
59+
TreeUtils.getSymbolFromTree(operand)
60+
.map(Symbol::fullyQualifiedName)
61+
.filter("numpy.nan"::equals)
62+
.ifPresent(fqn -> {
63+
PreciseIssue issue = ctx.addIssue(be, MESSAGE);
64+
addQuickFix(issue, operand, otherOperand, be);
65+
});
66+
}
67+
68+
private static void addQuickFix(PreciseIssue issue, Expression nanOperand, Expression otherOperand, BinaryExpression be) {
69+
Optional.of(nanOperand)
70+
.flatMap(TreeUtils.toOptionalInstanceOfMapper(QualifiedExpression.class))
71+
.map(QualifiedExpression::qualifier)
72+
.flatMap(TreeUtils.toOptionalInstanceOfMapper(Name.class))
73+
.map(Name::name)
74+
.map(symbName -> addPrefix(be) + symbName + ".isnan(" + TreeUtils.treeToString(otherOperand, true) + ")")
75+
.ifPresent(replacement -> issue
76+
.addQuickFix(PythonQuickFix
77+
.newQuickFix(operatorToMessage(be))
78+
.addTextEdit(TextEditUtils.replace(be, replacement))
79+
.build()));
4880
}
4981

50-
private static void checkOperand(SubscriptionContext ctx, Expression operand, BinaryExpression be) {
51-
HasSymbol cast;
52-
if (operand.is(Tree.Kind.QUALIFIED_EXPR)) {
53-
cast = (QualifiedExpression) operand;
54-
} else if (operand.is(Tree.Kind.NAME)) {
55-
cast = (Name) operand;
82+
private static String addPrefix(BinaryExpression be) {
83+
if ("==".equals(be.operator().value())) {
84+
return "";
5685
} else {
57-
return;
86+
return "!";
5887
}
59-
Symbol symbol = cast.symbol();
60-
if (symbol != null && "numpy.nan".equals(symbol.fullyQualifiedName())) {
61-
ctx.addIssue(be, "Don't perform an equality check against \"numpy.nan\".");
88+
}
89+
90+
private static String operatorToMessage(BinaryExpression be) {
91+
if ("==".equals(be.operator().value())) {
92+
return QUICK_FIX_MESSAGE_EQUALITY;
93+
} else {
94+
return QUICK_FIX_MESSAGE_INEQUALITY;
6295
}
6396
}
6497
}

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

Lines changed: 68 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,21 +20,83 @@
2020
package org.sonar.python.checks;
2121

2222
import org.junit.jupiter.api.Test;
23+
import org.sonar.python.checks.quickfix.PythonQuickFixVerifier;
2324
import org.sonar.python.checks.utils.PythonCheckVerifier;
2425

2526
class NumpyIsNanCheckTest {
27+
28+
NumpyIsNanCheck check = new NumpyIsNanCheck();
29+
private static final String QUICK_FIX_MESSAGE_EQUALITY = "Replace this equality check with numpy.isnan().";
30+
private static final String QUICK_FIX_MESSAGE_INEQUALITY = "Replace this inequality check with !numpy.isnan().";
31+
2632
@Test
27-
void test1() {
28-
PythonCheckVerifier.verify("src/test/resources/checks/numpyIsNan1.py", new NumpyIsNanCheck());
33+
void test() {
34+
PythonCheckVerifier.verify("src/test/resources/checks/numpyIsNan.py", new NumpyIsNanCheck());
2935
}
3036

3137
@Test
32-
void test2() {
33-
PythonCheckVerifier.verify("src/test/resources/checks/numpyIsNan2.py", new NumpyIsNanCheck());
38+
void quickFixTestEqual() {
39+
40+
final String nonCompliant1 = "import numpy as np\n" +
41+
"def foo(x):\n" +
42+
" if x == np.nan: print(1)";
43+
44+
final String nonCompliant2 = "import numpy as np\n" +
45+
"def foo(x):\n" +
46+
" if np.nan == x: print(1)";
47+
48+
final String compliant = "import numpy as np\n" +
49+
"def foo(x):\n" +
50+
" if np.isnan(x): print(1)";
51+
52+
performVerification(nonCompliant1, compliant, QUICK_FIX_MESSAGE_EQUALITY);
53+
performVerification(nonCompliant2, compliant, QUICK_FIX_MESSAGE_EQUALITY);
54+
}
55+
56+
@Test
57+
void quickFixTestNotEqual() {
58+
59+
final String nonCompliant1 = "import numpy as np\n" +
60+
"def foo(x):\n" +
61+
" if x != np.nan: print(1)";
62+
63+
final String nonCompliant2 = "import numpy as np\n" +
64+
"def foo(x):\n" +
65+
" if np.nan != x: print(1)";
66+
67+
final String compliant = "import numpy as np\n" +
68+
"def foo(x):\n" +
69+
" if !np.isnan(x): print(1)";
70+
71+
performVerification(nonCompliant1, compliant, QUICK_FIX_MESSAGE_INEQUALITY);
72+
performVerification(nonCompliant2, compliant, QUICK_FIX_MESSAGE_INEQUALITY);
73+
}
74+
75+
private void performVerification(String nonCompliant, String compliant, String message) {
76+
PythonQuickFixVerifier.verify(check, nonCompliant, compliant);
77+
PythonQuickFixVerifier.verifyQuickFixMessages(check, nonCompliant, message);
78+
}
79+
80+
@Test
81+
void quickFixTestQualifiedExpression() {
82+
83+
final String nonCompliant = "import numpy as np\n" +
84+
"def foo(x):\n" +
85+
" if np.max(2,5) == np.nan: print(1)";
86+
87+
final String compliant = "import numpy as np\n" +
88+
"def foo(x):\n" +
89+
" if np.isnan(np.max(2,5)): print(1)";
90+
91+
performVerification(nonCompliant, compliant, QUICK_FIX_MESSAGE_EQUALITY);
3492
}
3593

3694
@Test
37-
void test3() {
38-
PythonCheckVerifier.verify("src/test/resources/checks/numpyIsNan3.py", new NumpyIsNanCheck());
95+
void noQuickFixTest() {
96+
// Here we offer no quick fixes, because we do not have a call to numpy.nan.
97+
final String compliant = "from numpy import nan\n" +
98+
"def foo(x):\n" +
99+
" if x != nan: print(1)";
100+
PythonQuickFixVerifier.verifyNoQuickFixes(check, compliant);
39101
}
40102
}

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
def foo1(x):
2+
import numpy as np
3+
if x == np.nan: print(1) # Noncompliant {{Don't perform an equality check against "numpy.nan".}}
4+
# ^^^^^^^^^^^
5+
if np.nan == x: print(1) # Noncompliant
6+
if x != np.nan: print(1) # Noncompliant
7+
if np.nan != x: print(1) # Noncompliant
8+
if zz.nan != x: print(1)
9+
if x == zz.nan: print(1)
10+
if np.isnan(x): print(1)
11+
if x == np.zeros(42): print(1)
12+
if np.nan < x: print(1)
13+
if np.nan == np.array([1, 2, 3]): print(1) # Noncompliant
14+
if np.nan == np.max(5, 3): print(1) # Noncompliant
15+
16+
17+
def foo2(x):
18+
import numpy as xx
19+
if x == xx.nan: print(1) # Noncompliant
20+
# ^^^^^^^^^^^
21+
if xx.nan == x: print(1) # Noncompliant
22+
if x != xx.nan: print(1) # Noncompliant
23+
if xx.nan != x: print(1) # Noncompliant
24+
if zz.nan != x: print(1)
25+
if x == zz.nan: print(1)
26+
if xx.isnan(x): print(1)
27+
if x == xx.zeros(42): print(1)
28+
if xx.nan < x: print(1)
29+
30+
31+
def foo3(x):
32+
from numpy import nan, isnan, zeros
33+
if x == nan: print(1) # Noncompliant
34+
# ^^^^^^^^
35+
if nan == x: print(1) # Noncompliant
36+
if x != nan: print(1) # Noncompliant
37+
if nan != x: print(1) # Noncompliant
38+
if zz.nan != x: print(1)
39+
if x == zz.nan: print(1)
40+
if isnan(x): print(1)
41+
if x == zeros(42): print(1)
42+
if nan < x: print(1)

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

Lines changed: 0 additions & 14 deletions
This file was deleted.

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

Lines changed: 0 additions & 14 deletions
This file was deleted.

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

Lines changed: 0 additions & 14 deletions
This file was deleted.

0 commit comments

Comments
 (0)