Skip to content

Commit c43b2a1

Browse files
SONARPY-253: Rule S2761: Double prefix operators not and ~ should not be used (#1096)
* SONARPY-253: Rule 2761: Double prefix operators not and ~ should not be used * Correct test * SONARPY-253: Rule S2761: Double prefix operators not and ~ should not be used * Add updated rules * Update rule and add exception for overloaded invert function * Uncomment tests * Adjust code according to review
1 parent d1d1e51 commit c43b2a1

File tree

8 files changed

+180
-0
lines changed

8 files changed

+180
-0
lines changed
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
'project:buildbot-0.8.6p1/buildbot/process/properties.py':[
3+
75,
4+
],
5+
'project:numpy-1.16.4/numpy/distutils/command/build_clib.py':[
6+
151,
7+
],
8+
'project:twisted-12.1.0/twisted/names/server.py':[
9+
49,
10+
],
11+
}

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
@@ -112,6 +112,7 @@ public static Iterable<Class> getChecks() {
112112
DictionaryDuplicateKeyCheck.class,
113113
DisabledHtmlAutoEscapeCheck.class,
114114
DisabledHtmlAutoEscapeLegacyCheck.class,
115+
DoublePrefixOperatorCheck.class,
115116
DuplicateArgumentCheck.class,
116117
DuplicatedMethodFieldNamesCheck.class,
117118
DuplicatedMethodImplementationCheck.class,
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2022 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.Expression;
26+
import org.sonar.plugins.python.api.tree.ParenthesizedExpression;
27+
import org.sonar.plugins.python.api.tree.Tree;
28+
import org.sonar.plugins.python.api.tree.UnaryExpression;
29+
import org.sonar.python.types.InferredTypes;
30+
31+
@Rule(key = "S2761")
32+
33+
public class DoublePrefixOperatorCheck extends PythonSubscriptionCheck {
34+
35+
private static final String MESSAGE = "Use the \"%s\" operator just once or not at all.";
36+
private static final String MESSAGE_NOT = "Use the \"bool()\" builtin function instead of calling \"not\" twice.";
37+
38+
@Override
39+
public void initialize(Context context) {
40+
context.registerSyntaxNodeConsumer(Tree.Kind.BITWISE_COMPLEMENT, ctx -> doubleInversionCheck(ctx, (UnaryExpression) ctx.syntaxNode()));
41+
context.registerSyntaxNodeConsumer(Tree.Kind.NOT, ctx -> doubleInversionCheck(ctx, (UnaryExpression) ctx.syntaxNode()));
42+
}
43+
44+
private static void doubleInversionCheck(SubscriptionContext ctx, UnaryExpression original) {
45+
Expression invertedExpr = original.expression();
46+
boolean doubleInversionFollowed = true;
47+
while (invertedExpr.is(Tree.Kind.PARENTHESIZED)) {
48+
doubleInversionFollowed = false;
49+
invertedExpr = ((ParenthesizedExpression) invertedExpr).expression();
50+
}
51+
52+
if (invertedExpr.is(Tree.Kind.NOT, Tree.Kind.BITWISE_COMPLEMENT) && original.is(invertedExpr.getKind())) {
53+
if (doubleInversionFollowed) {
54+
// Overloaded __invert__ should not raise any warning
55+
if (invertedExpr.is(Tree.Kind.NOT)) {
56+
ctx.addIssue(original, MESSAGE_NOT);
57+
} else {
58+
if (((UnaryExpression) invertedExpr).expression().type() == InferredTypes.INT) {
59+
ctx.addIssue(original, String.format(MESSAGE, original.operator().value()));
60+
}
61+
}
62+
} else {
63+
ctx.addIssue(original, String.format(MESSAGE, original.operator().value()));
64+
}
65+
}
66+
}
67+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<p>Calling the <code>not</code> or <code>~</code> prefix operator twice might be redundant: the second invocation undoes the first. Such mistakes are
2+
typically caused by accidentally double-tapping the key in question without noticing. Either this is a bug, if the operator was actually meant to be
3+
called once, or misleading if done on purpose. Calling <code>not</code> twice is commonly done instead of using the dedicated "bool()" builtin
4+
function. However, the latter one increases the code readability and should be used.</p>
5+
<h2>Noncompliant Code Example</h2>
6+
<pre>
7+
a = 0
8+
b = False
9+
10+
c = not not a # Noncompliant
11+
d = ~~b # Noncompliant
12+
</pre>
13+
<h2>Compliant Solution</h2>
14+
<pre>
15+
a = 0
16+
b = False
17+
18+
c = bool(a)
19+
d = ~b
20+
</pre>
21+
<h2>Exceptions</h2>
22+
<p>If the <code>~</code> function has been overloaded in a customised class and has been called twice, no warning is raised as it is assumed to be an
23+
expected usage.</p>
24+
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"title": "Doubled prefix operators \"not\" and \"~\" should not be used",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [],
10+
"defaultSeverity": "Major",
11+
"ruleSpecification": "RSPEC-2761",
12+
"sqKey": "S2761",
13+
"scope": "All",
14+
"quickfix": "unknown"
15+
}

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
@@ -62,6 +62,7 @@
6262
"S2737",
6363
"S2755",
6464
"S2757",
65+
"S2761",
6566
"S2772",
6667
"S2823",
6768
"S2836",
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-2022 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 DoublePrefixOperatorCheckTest {
26+
27+
@Test
28+
public void test() {
29+
PythonCheckVerifier.verify("src/test/resources/checks/doublePrefixOperator.py", new DoublePrefixOperatorCheck());
30+
}
31+
32+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
def start():
2+
a = 1
3+
b = not not a # Noncompliant {{Use the "bool()" builtin function instead of calling "not" twice.}}
4+
# ^^^^^^^^^
5+
6+
c = ~~a # Noncompliant {{Use the "~" operator just once or not at all.}}
7+
# ^^^
8+
d = not (not (not (not a))) # Noncompliant 3
9+
10+
e = ~~~~~a # Noncompliant
11+
f = ~(((((~a))))) # Noncompliant {{Use the "~" operator just once or not at all.}}
12+
13+
g = not (a == b)
14+
h = ~(a and not b)
15+
i = not a and ~b
16+
17+
j = not (not (a is not b)) # Noncompliant {{Use the "not" operator just once or not at all.}}
18+
# ^^^^^^^^^^^^^^^^^^^^^^
19+
20+
k = not (~a)
21+
22+
i = not(not(a)) # Noncompliant {{Use the "not" operator just once or not at all.}}
23+
24+
class Foo:
25+
def __invert__(self):
26+
return self.bar == 42
27+
28+
foo = Foo()
29+
~~foo # here ~~ it might have a different semantic

0 commit comments

Comments
 (0)