Skip to content

Commit 6d4bfd5

Browse files
joke1196sonartech
joke1196
authored and
sonartech
committed
SONARPY-5083: Implement rule S9462 - Sentinel values should be compared by identity
GitOrigin-RevId: 803788ff15f644570ed32153273c5b6fd12e6fb9
1 parent 919ae05 commit 6d4bfd5

8 files changed

Lines changed: 296 additions & 1 deletion

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,7 @@ public Stream<Class<?>> getChecks() {
523523
SynchronousHttpOperationsInAsyncCheck.class,
524524
SynchronousSubprocessOperationsInAsyncCheck.class,
525525
SynchronousOsCallsInAsyncCheck.class,
526+
SentinelValuesIdentityComparisonCheck.class,
526527
TaskGroupNurseryUsedOnlyOnceCheck.class,
527528
TempFileCreationCheck.class,
528529
TemplateAndStrConcatenationCheck.class,

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* You can redistribute and/or modify this program under the terms of
7+
* the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import org.sonar.check.Rule;
20+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
21+
import org.sonar.plugins.python.api.SubscriptionContext;
22+
import org.sonar.plugins.python.api.quickfix.PythonQuickFix;
23+
import org.sonar.plugins.python.api.tree.BinaryExpression;
24+
import org.sonar.plugins.python.api.tree.Tree;
25+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
26+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
27+
import org.sonar.python.quickfix.TextEditUtils;
28+
import org.sonar.python.types.v2.matchers.InternalTypeMatchers;
29+
30+
@Rule(key = "S9462")
31+
public class SentinelValuesIdentityComparisonCheck extends PythonSubscriptionCheck {
32+
33+
public static final String IS_QUICK_FIX_MESSAGE = "Replace with \"is\"";
34+
public static final String IS_NOT_QUICK_FIX_MESSAGE = "Replace with \"is not\"";
35+
private static final TypeMatcher TYPING_EXTENSIONS_SENTINEL = TypeMatchers.isObjectOfType("typing_extensions.sentinel");
36+
private static final TypeMatcher SENTINEL = TypeMatchers.isObjectOfType("sentinel");
37+
private static final TypeMatcher SENTINEL_VALUE = TypeMatchers.any(
38+
TypeMatchers.isObjectOfType("builtins.sentinel"),
39+
TYPING_EXTENSIONS_SENTINEL,
40+
SENTINEL,
41+
InternalTypeMatchers.isAnyTypeInUnionSatisfying(SENTINEL));
42+
43+
@Override
44+
public void initialize(Context context) {
45+
context.registerSyntaxNodeConsumer(Tree.Kind.COMPARISON, SentinelValuesIdentityComparisonCheck::checkComparison);
46+
}
47+
48+
private static void checkComparison(SubscriptionContext ctx) {
49+
var comparison = (BinaryExpression) ctx.syntaxNode();
50+
String operator = comparison.operator().value();
51+
if (("==".equals(operator) || "!=".equals(operator)) &&
52+
(SENTINEL_VALUE.isTrueFor(comparison.leftOperand(), ctx) || SENTINEL_VALUE.isTrueFor(comparison.rightOperand(), ctx))) {
53+
String replacement = "==".equals(operator) ? "is" : "is not";
54+
String quickFixMessage = "==".equals(operator) ? IS_QUICK_FIX_MESSAGE : IS_NOT_QUICK_FIX_MESSAGE;
55+
var quickFix = PythonQuickFix.newQuickFix(quickFixMessage)
56+
.addTextEdit(TextEditUtils.replace(comparison.operator(), replacementWithSpaces(comparison, replacement)))
57+
.build();
58+
ctx.addIssue(comparison.operator(), String.format("Use \"%s\" instead of \"%s\" when comparing with a sentinel value.", replacement, operator))
59+
.addQuickFix(quickFix);
60+
}
61+
}
62+
63+
private static String replacementWithSpaces(BinaryExpression comparison, String replacement) {
64+
var operator = comparison.operator();
65+
var left = comparison.leftOperand().lastToken();
66+
var right = comparison.rightOperand().firstToken();
67+
boolean hasSpaceBefore = left.line() != operator.line() || left.column() + left.value().length() < operator.column();
68+
boolean hasSpaceAfter = right.line() != operator.line() || operator.column() + operator.value().length() < right.column();
69+
return (hasSpaceBefore ? "" : " ") + replacement + (hasSpaceAfter ? "" : " ");
70+
}
71+
}

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<p>This rule raises an issue when <code>==</code> or <code>!=</code> compares a value with a sentinel created by the built-in <code>sentinel()</code>
2+
(Python 3.15, PEP 661) or by <code>typing_extensions.Sentinel</code>.</p>
3+
<h2>Why is this an issue?</h2>
4+
<p>A sentinel is a unique object used to represent a special state, such as a missing or unset value. Its meaning comes from its identity: the object
5+
itself, not a value it contains.</p>
6+
<p>Using an equality or inequality comparison asks the language to compare values. This is less clear than an identity comparison and can invoke
7+
custom equality behavior from the other operand.</p>
8+
<p>Use a reference-identity comparison when checking for a sentinel, and a negated identity comparison when checking that a value is not the sentinel.
9+
These comparisons clearly show that the code checks for the unique marker object.</p>
10+
<h3>What is the potential impact?</h3>
11+
<p>Value-equality comparisons can make sentinel checks less clear and may behave unexpectedly when the other operand defines custom equality behavior.
12+
Comparisons that test whether both references designate the same object express the intended test directly and reliably.</p>
13+
<h2>How to fix it</h2>
14+
<p>Replace <code>==</code> with <code>is</code> and <code>!=</code> with <code>is not</code> when comparing with a value created by the built-in
15+
<code>sentinel()</code> or by <code>typing_extensions.Sentinel</code>.</p>
16+
<h3>Code examples</h3>
17+
<h4>Noncompliant code example</h4>
18+
<pre data-diff-id="1" data-diff-type="noncompliant">
19+
MISSING = sentinel("MISSING")
20+
UNSET = sentinel("UNSET")
21+
22+
if value == MISSING: # Noncompliant
23+
handle_missing()
24+
25+
if value != UNSET: # Noncompliant
26+
process(value)
27+
</pre>
28+
<h4>Compliant solution</h4>
29+
<pre data-diff-id="1" data-diff-type="compliant">
30+
MISSING = sentinel("MISSING")
31+
UNSET = sentinel("UNSET")
32+
33+
if value is MISSING:
34+
handle_missing()
35+
36+
if value is not UNSET:
37+
process(value)
38+
</pre>
39+
<h2>Resources</h2>
40+
<h3>Documentation</h3>
41+
<ul>
42+
<li>Python Documentation - <a href="https://docs.python.org/3/reference/expressions.html#is">Identity comparisons</a></li>
43+
<li>Python documentation - <a href="https://docs.python.org/3.15/library/functions.html#sentinel">Built-in Functions — sentinel</a></li>
44+
</ul>
45+

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"title": "Sentinel values should be compared by identity",
3+
"type": "BUG",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5 min"
8+
},
9+
"tags": [
10+
"suspicious"
11+
],
12+
"defaultSeverity": "Critical",
13+
"ruleSpecification": "RSPEC-9462",
14+
"sqKey": "S9462",
15+
"scope": "All",
16+
"quickfix": "covered",
17+
"code": {
18+
"impacts": {
19+
"RELIABILITY": "HIGH"
20+
},
21+
"attribute": "LOGICAL"
22+
}
23+
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,7 @@
407407
"S9453",
408408
"S9454",
409409
"S9455",
410-
"S9460"
410+
"S9460",
411+
"S9462"
411412
]
412413
}

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

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* You can redistribute and/or modify this program under the terms of
7+
* the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import java.util.EnumSet;
20+
import org.junit.jupiter.api.AfterEach;
21+
import org.junit.jupiter.api.Test;
22+
import org.sonar.plugins.python.api.ProjectPythonVersion;
23+
import org.sonar.plugins.python.api.PythonVersionUtils;
24+
import org.sonar.python.checks.utils.PythonCheckVerifier;
25+
import org.sonar.python.checks.quickfix.PythonQuickFixVerifier;
26+
import org.sonar.python.types.TypeShed;
27+
28+
class SentinelValuesIdentityComparisonCheckTest {
29+
30+
@AfterEach
31+
void resetPythonVersion() {
32+
ProjectPythonVersion.setCurrentVersions(PythonVersionUtils.allVersions());
33+
TypeShed.resetBuiltinSymbols();
34+
}
35+
36+
@Test
37+
void test() {
38+
ProjectPythonVersion.setCurrentVersions(EnumSet.of(PythonVersionUtils.Version.V_315));
39+
PythonCheckVerifier.verify("src/test/resources/checks/sentinelValuesIdentityComparison.py", new SentinelValuesIdentityComparisonCheck());
40+
}
41+
42+
@Test
43+
void typing_extensions_sentinel() {
44+
PythonCheckVerifier.verify("src/test/resources/checks/typingExtensionsSentinelValuesIdentityComparison.py", new SentinelValuesIdentityComparisonCheck());
45+
}
46+
47+
@Test
48+
void equality_comparison_quick_fix() {
49+
var check = new SentinelValuesIdentityComparisonCheck();
50+
String codeWithIssue = """
51+
from typing_extensions import Sentinel
52+
missing = Sentinel("MISSING")
53+
if value == missing:
54+
pass
55+
""";
56+
String fixedCode = """
57+
from typing_extensions import Sentinel
58+
missing = Sentinel("MISSING")
59+
if value is missing:
60+
pass
61+
""";
62+
PythonQuickFixVerifier.verify(check, codeWithIssue, fixedCode);
63+
PythonQuickFixVerifier.verifyQuickFixMessages(check, codeWithIssue, SentinelValuesIdentityComparisonCheck.IS_QUICK_FIX_MESSAGE);
64+
}
65+
66+
@Test
67+
void equality_comparison_without_whitespace_quick_fix() {
68+
var check = new SentinelValuesIdentityComparisonCheck();
69+
String codeWithIssue = """
70+
from typing_extensions import Sentinel
71+
missing = Sentinel("MISSING")
72+
if value==missing:
73+
pass
74+
""";
75+
String fixedCode = """
76+
from typing_extensions import Sentinel
77+
missing = Sentinel("MISSING")
78+
if value is missing:
79+
pass
80+
""";
81+
PythonQuickFixVerifier.verify(check, codeWithIssue, fixedCode);
82+
}
83+
84+
@Test
85+
void inequality_comparison_quick_fix() {
86+
var check = new SentinelValuesIdentityComparisonCheck();
87+
String codeWithIssue = """
88+
from typing_extensions import Sentinel
89+
missing = Sentinel("MISSING")
90+
if value != missing:
91+
pass
92+
""";
93+
String fixedCode = """
94+
from typing_extensions import Sentinel
95+
missing = Sentinel("MISSING")
96+
if value is not missing:
97+
pass
98+
""";
99+
PythonQuickFixVerifier.verify(check, codeWithIssue, fixedCode);
100+
PythonQuickFixVerifier.verifyQuickFixMessages(check, codeWithIssue, SentinelValuesIdentityComparisonCheck.IS_NOT_QUICK_FIX_MESSAGE);
101+
}
102+
}

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
MISSING = sentinel("MISSING")
2+
UNSET = sentinel("UNSET")
3+
4+
5+
def equality_comparisons(value):
6+
if value == MISSING: # Noncompliant {{Use "is" instead of "==" when comparing with a sentinel value.}}
7+
# ^^
8+
pass
9+
if UNSET == value: # Noncompliant
10+
# ^^
11+
pass
12+
if value != MISSING: # Noncompliant {{Use "is not" instead of "!=" when comparing with a sentinel value.}}
13+
# ^^
14+
pass
15+
if UNSET != value: # Noncompliant
16+
# ^^
17+
pass
18+
19+
20+
def identity_comparisons(value):
21+
if value is MISSING: # OK
22+
pass
23+
if UNSET is not value: # OK
24+
pass
25+
26+
27+
def other_values(value):
28+
if value == None: # OK
29+
pass
30+
if value != object(): # OK
31+
pass
32+
33+
34+
def user_defined_sentinel(value):
35+
class sentinel:
36+
pass
37+
38+
custom = sentinel()
39+
if value == custom: # OK
40+
pass

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
from typing_extensions import Sentinel
2+
3+
MISSING = Sentinel("MISSING")
4+
5+
6+
def equality_comparisons(value):
7+
if value == MISSING: # Noncompliant {{Use "is" instead of "==" when comparing with a sentinel value.}}
8+
# ^^
9+
pass
10+
if value != MISSING: # Noncompliant {{Use "is not" instead of "!=" when comparing with a sentinel value.}}
11+
# ^^
12+
pass

0 commit comments

Comments
 (0)