Skip to content

Commit c7462d2

Browse files
pynicolasandrea-guarino-sonarsource
pynicolas
authored and
andrea-guarino-sonarsource
committed
SONARPY-615 Rule S5727: Comparison to None should not be constant
1 parent 8101b84 commit c7462d2

File tree

8 files changed

+209
-0
lines changed

8 files changed

+209
-0
lines changed

‎its/ruling/src/test/resources/expected/python-S5727.json‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
'project:twisted-12.1.0/twisted/mail/imap4.py':[
3+
557,
4+
1088,
5+
1111,
6+
1377,
7+
1419,
8+
1951,
9+
2000,
10+
2103,
11+
2115,
12+
],
13+
'project:twisted-12.1.0/twisted/persisted/sob.py':[
14+
183,
15+
],
16+
'project:twisted-12.1.0/twisted/plugin.py':[
17+
107,
18+
],
19+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ public static Iterable<Class> getChecks() {
7373
CommandLineArgsCheck.class,
7474
CommentedCodeCheck.class,
7575
CommentRegularExpressionCheck.class,
76+
ComparisonToNoneCheck.class,
7677
CorsCheck.class,
7778
DataEncryptionCheck.class,
7879
DbNoPasswordCheck.class,

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2020 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
package org.sonar.python.checks;
21+
22+
import org.sonar.check.Rule;
23+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
24+
import org.sonar.plugins.python.api.SubscriptionContext;
25+
import org.sonar.plugins.python.api.tree.BinaryExpression;
26+
import org.sonar.plugins.python.api.tree.IsExpression;
27+
import org.sonar.plugins.python.api.tree.Tree;
28+
import org.sonar.plugins.python.api.tree.Tree.Kind;
29+
import org.sonar.plugins.python.api.types.InferredType;
30+
31+
@Rule(key = "S5727")
32+
public class ComparisonToNoneCheck extends PythonSubscriptionCheck {
33+
34+
@Override
35+
public void initialize(Context context) {
36+
context.registerSyntaxNodeConsumer(Kind.IS, ctx -> checkIdentityComparison(ctx, (IsExpression) ctx.syntaxNode()));
37+
context.registerSyntaxNodeConsumer(Kind.COMPARISON, ctx -> checkEqualityComparison(ctx, (BinaryExpression) ctx.syntaxNode()));
38+
}
39+
40+
private static void checkEqualityComparison(SubscriptionContext ctx, BinaryExpression comparison) {
41+
String operator = comparison.operator().value();
42+
if (!"==".equals(operator) && !"!=".equals(operator)) {
43+
return;
44+
}
45+
InferredType left = comparison.leftOperand().type();
46+
InferredType right = comparison.rightOperand().type();
47+
if (isNone(left) && isNone(right)) {
48+
addIssue(ctx, comparison, operator + " comparison", "==".equals(operator));
49+
} else if ((isNone(left) && cannotBeNone(right)) || (cannotBeNone(left) && isNone(right))) {
50+
addIssue(ctx, comparison, operator + " comparison", "!=".equals(operator));
51+
}
52+
}
53+
54+
private static void checkIdentityComparison(SubscriptionContext ctx, IsExpression comparison) {
55+
InferredType left = comparison.leftOperand().type();
56+
InferredType right = comparison.rightOperand().type();
57+
if (!left.isIdentityComparableWith(right) && (isNone(left) || isNone(right))) {
58+
addIssue(ctx, comparison, "identity check", comparison.notToken() != null);
59+
} else if (isNone(left) && isNone(right)) {
60+
addIssue(ctx, comparison, "identity check", comparison.notToken() == null);
61+
}
62+
}
63+
64+
private static void addIssue(SubscriptionContext ctx, Tree tree, String comparisonKind, boolean result) {
65+
String resultAsString = result ? "True" : "False";
66+
ctx.addIssue(tree, String.format("Remove this %s; it will always be %s.", comparisonKind, resultAsString));
67+
}
68+
69+
private static boolean isNone(InferredType type) {
70+
return type.canOnlyBe("NoneType");
71+
}
72+
73+
private static boolean cannotBeNone(InferredType type) {
74+
return !type.canBeOrExtend("NoneType");
75+
}
76+
}

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S5727.html‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<p>Checking if a variable or parameter is <code>None</code> should only be done when you expect that it can be <code>None</code>. Doing so when the
2+
variable is always None or never None is confusing at best. At worse, there is a bug and the variable is not updated properly.</p>
3+
<p>This rule raises an issue when expressions <code>X is None</code>, <code>X is not None</code>, <code>X == None</code> or <code>X != None</code> are
4+
constant, i.e. <code>X</code> is always None or never None.</p>
5+
<h2>Noncompliant Code Example</h2>
6+
<pre>
7+
mynone = None
8+
result = mynone is None: # Noncompliant. Always True.
9+
10+
if mynone == None: # Noncompliant. Always True.
11+
pass
12+
13+
if mynone is not None: # Noncompliant. Always False.
14+
pass
15+
16+
if mynone == None: # Noncompliant. Always False.
17+
pass
18+
19+
myint = 42
20+
result = myint is None: # Noncompliant. Always False.
21+
22+
if myint == None: # Noncompliant. Always False.
23+
pass
24+
25+
if myint is not None: # Noncompliant. Always True.
26+
pass
27+
28+
if myint == None: # Noncompliant. Always True.
29+
pass
30+
</pre>
31+
<h2>See</h2>
32+
<ul>
33+
<li> Python documentation - <a href="https://docs.python.org/3/reference/expressions.html#is-not">Identity comparisons</a> </li>
34+
<li> Python documentation - <a href="https://docs.python.org/3/reference/datamodel.html#object.__eq__">__eq__ operator</a> </li>
35+
</ul>
36+

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S5727.json‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"title": "Comparison to None should not be constant",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"tags": [
6+
"suspicious"
7+
],
8+
"defaultSeverity": "Critical",
9+
"ruleSpecification": "RSPEC-5727",
10+
"sqKey": "S5727",
11+
"scope": "All"
12+
}

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_way_profile.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@
8787
"S5708",
8888
"S5712",
8989
"S5714",
90+
"S5727",
9091
"S5747"
9192
]
9293
}

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2020 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
package org.sonar.python.checks;
21+
22+
import org.junit.Test;
23+
import org.sonar.python.checks.utils.PythonCheckVerifier;
24+
25+
public class ComparisonToNoneCheckTest {
26+
27+
@Test
28+
public void test() {
29+
PythonCheckVerifier.verify("src/test/resources/checks/comparisonToNoneCheck.py", new ComparisonToNoneCheck());
30+
}
31+
32+
}

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
def identity_check(param):
2+
a = None
3+
b = 42
4+
if a is param: pass
5+
if a is None: pass # Noncompliant {{Remove this identity check; it will always be True.}}
6+
if a is not None: pass # Noncompliant {{Remove this identity check; it will always be False.}}
7+
if b is None: pass # Noncompliant {{Remove this identity check; it will always be False.}}
8+
if b is not None: pass # Noncompliant {{Remove this identity check; it will always be True.}}
9+
if a is b: pass # Noncompliant {{Remove this identity check; it will always be False.}}
10+
c = None
11+
if a is c: pass # Noncompliant {{Remove this identity check; it will always be True.}}
12+
d = "abc"
13+
if b is d: pass
14+
if None is a: pass # Noncompliant
15+
if None is b: pass # Noncompliant
16+
17+
def equality_check(param):
18+
a = None
19+
b = 42
20+
if a == param: pass
21+
if a > None: pass
22+
if a >= None: pass
23+
if a == None: pass # Noncompliant {{Remove this == comparison; it will always be True.}}
24+
if a != None: pass # Noncompliant {{Remove this != comparison; it will always be False.}}
25+
if b == None: pass # Noncompliant {{Remove this == comparison; it will always be False.}}
26+
if b != None: pass # Noncompliant {{Remove this != comparison; it will always be True.}}
27+
if a == b: pass # Noncompliant {{Remove this == comparison; it will always be False.}}
28+
c = None
29+
if a == c: pass # Noncompliant {{Remove this == comparison; it will always be True.}}
30+
d = "abc"
31+
if b == d: pass
32+
if b != d: pass

0 commit comments

Comments
 (0)