Skip to content

Commit e74a5ac

Browse files
fourlsCirras
fourls
authored and
Cirras
committed
Add check for ambiguous bitwise not operations
1 parent 4799e37 commit e74a5ac

File tree

7 files changed

+307
-0
lines changed

7 files changed

+307
-0
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
- Support for MSBuild item and item metadata expressions in project files.
1313
- `ExhaustiveEnumCase` analysis rule, which flags `case` statements that do not handle all values in an enumeration.
1414
- `IterationPastHighBound` analysis rule, which flags `for` loops that iterate past the end of the collection.
15+
- `ExplicitBitwiseNot` analysis rule, which flags potentially incorrect bitwise `not` operations.
1516
- **API:** `EnumeratorOccurrence` type.
1617
- **API:** `ForInStatementNode::getEnumeratorOccurrence` method.
1718
- **API:** `TypeOfTypeNode::getTypeReferenceNode` method.

‎delphi-checks/src/main/java/au/com/integradev/delphi/checks/CheckList.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ public final class CheckList {
6767
EmptyVisibilitySectionCheck.class,
6868
EnumNameCheck.class,
6969
ExhaustiveEnumCaseCheck.class,
70+
ExplicitBitwiseNotCheck.class,
7071
ExplicitDefaultPropertyReferenceCheck.class,
7172
ExplicitTObjectInheritanceCheck.class,
7273
FieldNameCheck.class,

‎delphi-checks/src/main/java/au/com/integradev/delphi/checks/ExplicitBitwiseNotCheck.java

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
* Sonar Delphi Plugin
3+
* Copyright (C) 2025 Integrated Application Development
4+
*
5+
* This program is free software; you can redistribute it and/or
6+
* modify it under the terms of the GNU Lesser General Public
7+
* License as published by the Free Software Foundation; either
8+
* version 3 of the License, or (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
13+
* Lesser General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU Lesser General Public
16+
* License along with this program; if not, write to the Free Software
17+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02
18+
*/
19+
package au.com.integradev.delphi.checks;
20+
21+
import org.sonar.check.Rule;
22+
import org.sonar.plugins.communitydelphi.api.ast.BinaryExpressionNode;
23+
import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode;
24+
import org.sonar.plugins.communitydelphi.api.ast.UnaryExpressionNode;
25+
import org.sonar.plugins.communitydelphi.api.check.DelphiCheck;
26+
import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext;
27+
import org.sonar.plugins.communitydelphi.api.check.FilePosition;
28+
import org.sonar.plugins.communitydelphi.api.operator.BinaryOperator;
29+
import org.sonar.plugins.communitydelphi.api.operator.UnaryOperator;
30+
import org.sonar.plugins.communitydelphi.api.reporting.QuickFix;
31+
import org.sonar.plugins.communitydelphi.api.reporting.QuickFixEdit;
32+
33+
@Rule(key = "ExplicitBitwiseNot")
34+
public class ExplicitBitwiseNotCheck extends DelphiCheck {
35+
@Override
36+
public DelphiCheckContext visit(BinaryExpressionNode node, DelphiCheckContext context) {
37+
if (node.getOperator() == BinaryOperator.IN) {
38+
checkBitwiseNot(node.getLeft(), context);
39+
checkBitwiseNot(node.getRight(), context);
40+
}
41+
42+
return super.visit(node, context);
43+
}
44+
45+
private void checkBitwiseNot(ExpressionNode node, DelphiCheckContext context) {
46+
if (!(node instanceof UnaryExpressionNode)) {
47+
return;
48+
}
49+
UnaryExpressionNode unaryNode = (UnaryExpressionNode) node;
50+
51+
if (isBitwiseNot(unaryNode)) {
52+
context
53+
.newIssue()
54+
.onFilePosition(FilePosition.from(unaryNode.getToken()))
55+
.withMessage("Parenthesize this bitwise 'not' operation.")
56+
.withQuickFixes(
57+
QuickFix.newFix("Parenthesize bitwise 'not'")
58+
.withEdits(
59+
QuickFixEdit.insertBefore("(", node), QuickFixEdit.insertAfter(")", node)))
60+
.report();
61+
}
62+
}
63+
64+
private boolean isBitwiseNot(UnaryExpressionNode node) {
65+
return node.getOperator() == UnaryOperator.NOT && node.getType().isInteger();
66+
}
67+
}

‎delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/ExplicitBitwiseNot.html

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<h2>Why is this an issue?</h2>
2+
<p>
3+
The Delphi bitwise <code>not</code> operator binds stronger than the <code>in</code> binary
4+
operator, which can lead to subtly incorrect code. For example, in the code below the bitwise
5+
<code>not</code> has been confused for a logical <code>not</code>, which has introduced a bug:
6+
</p>
7+
<pre>
8+
var MyByte: Byte := 3;
9+
10+
if not MyByte in [252, 253, 254, 255] then
11+
raise Exception.Create('MyByte must not be above 251!'); // This is raised!
12+
</pre>
13+
<p>
14+
To avoid this pitfall, complex expressions involving <code>not</code> and <code>in</code> should
15+
be parenthesized appropriately, so the precedence is obvious at a glance.
16+
</p>
17+
<h2>How to fix it</h2>
18+
<p>If the bitwise <code>not</code> is intentional, parenthesize it:</p>
19+
<pre data-diff-id="1" data-diff-type="noncompliant">
20+
var MyByte: Byte := 3;
21+
if not MyByte in [252] then
22+
WriteLn('error: MyByte must not be 252!');
23+
</pre>
24+
<pre data-diff-id="1" data-diff-type="compliant">
25+
var MyByte: Byte := 3;
26+
if (not MyByte) in [252] then
27+
WriteLn('error: MyByte must not be 3!');
28+
</pre>
29+
<p>Otherwise, parenthesize the binary expression that should be negated:</p>
30+
<pre data-diff-id="1" data-diff-type="noncompliant">
31+
var MyByte: Byte := 3;
32+
if not MyByte in [252] then
33+
WriteLn('error: MyByte must not be 252!');
34+
</pre>
35+
<pre data-diff-id="1" data-diff-type="compliant">
36+
var MyByte: Byte := 3;
37+
if not (MyByte in [252]) then
38+
WriteLn('error: MyByte must be 252!');
39+
</pre>
40+
<h2>Resources</h2>
41+
<ul>
42+
<li>
43+
<a href="https://docwiki.embarcadero.com/RADStudio/en/Expressions_(Delphi)">
44+
RAD Studio documentation: Expressions (Delphi)
45+
</a>
46+
</li>
47+
</ul>

‎delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/ExplicitBitwiseNot.json

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"title": "Ambiguous bitwise 'not' operations should be parenthesized",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant/Issue",
7+
"constantCost": "2min"
8+
},
9+
"code": {
10+
"attribute": "LOGICAL",
11+
"impacts": {
12+
"RELIABILITY": "MEDIUM"
13+
}
14+
},
15+
"tags": ["suspicious"],
16+
"defaultSeverity": "Minor",
17+
"scope": "ALL",
18+
"quickfix": "unknown"
19+
}

‎delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/Sonar_way_profile.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
"EmptyRoutine",
3434
"EmptyVisibilitySection",
3535
"EnumName",
36+
"ExplicitBitwiseNot",
3637
"ExplicitDefaultPropertyReference",
3738
"FieldName",
3839
"FormatArgumentCount",

‎delphi-checks/src/test/java/au/com/integradev/delphi/checks/ExplicitBitwiseNotCheckTest.java

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
/*
2+
* Sonar Delphi Plugin
3+
* Copyright (C) 2019 Integrated Application Development
4+
*
5+
* This program is free software; you can redistribute it and/or
6+
* modify it under the terms of the GNU Lesser General Public
7+
* License as published by the Free Software Foundation; either
8+
* version 3 of the License, or (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
13+
* Lesser General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU Lesser General Public
16+
* License along with this program; if not, write to the Free Software
17+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02
18+
*/
19+
package au.com.integradev.delphi.checks;
20+
21+
import au.com.integradev.delphi.builders.DelphiTestUnitBuilder;
22+
import au.com.integradev.delphi.checks.verifier.CheckVerifier;
23+
import org.junit.jupiter.api.Test;
24+
25+
class ExplicitBitwiseNotCheckTest {
26+
@Test
27+
void testSimpleBitwiseNotShouldNotAddIssue() {
28+
CheckVerifier.newVerifier()
29+
.withCheck(new ExplicitBitwiseNotCheck())
30+
.onFile(
31+
new DelphiTestUnitBuilder()
32+
.appendImpl("function Test: Integer;")
33+
.appendImpl("begin")
34+
.appendImpl(" Result := not 1;")
35+
.appendImpl("end;"))
36+
.verifyNoIssues();
37+
}
38+
39+
@Test
40+
void testOtherIntegerUnaryOperatorsShouldNotAddIssue() {
41+
CheckVerifier.newVerifier()
42+
.withCheck(new ExplicitBitwiseNotCheck())
43+
.onFile(
44+
new DelphiTestUnitBuilder()
45+
.appendImpl("function Test: Integer;")
46+
.appendImpl("begin")
47+
.appendImpl(" Result := -1 in [2];")
48+
.appendImpl(" Result := +1 in [2];")
49+
.appendImpl("end;"))
50+
.verifyNoIssues();
51+
}
52+
53+
@Test
54+
void testBitwiseNotInSetContainmentCheckShouldAddIssue() {
55+
CheckVerifier.newVerifier()
56+
.withCheck(new ExplicitBitwiseNotCheck())
57+
.onFile(
58+
new DelphiTestUnitBuilder()
59+
.appendImpl("procedure Test;")
60+
.appendImpl("begin")
61+
.appendImpl(" // Fix@[+2:5 to +2:5] <<(>>")
62+
.appendImpl(" // Fix@[+1:10 to +1:10] <<)>>")
63+
.appendImpl(" if not 3 in [3, 252] then // Noncompliant")
64+
.appendImpl(" Writeln('Foo');")
65+
.appendImpl("end;"))
66+
.verifyIssues();
67+
}
68+
69+
@Test
70+
void testBitwiseNotInBinaryExpressionShouldAddIssue() {
71+
CheckVerifier.newVerifier()
72+
.withCheck(new ExplicitBitwiseNotCheck())
73+
.onFile(
74+
new DelphiTestUnitBuilder()
75+
.appendImpl("function Test: Integer;")
76+
.appendImpl("begin")
77+
.appendImpl(" Result := not 1 in [2]; // Noncompliant")
78+
.appendImpl("end;"))
79+
.verifyIssues();
80+
}
81+
82+
@Test
83+
void testParenthesizedBitwiseNotInBinaryExpressionShouldNotAddIssue() {
84+
CheckVerifier.newVerifier()
85+
.withCheck(new ExplicitBitwiseNotCheck())
86+
.onFile(
87+
new DelphiTestUnitBuilder()
88+
.appendImpl("function Test: Integer;")
89+
.appendImpl("begin")
90+
.appendImpl(" Result := (not 1) in [2];")
91+
.appendImpl("end;"))
92+
.verifyNoIssues();
93+
}
94+
95+
@Test
96+
void testBooleanNotShouldNotAddIssue() {
97+
CheckVerifier.newVerifier()
98+
.withCheck(new ExplicitBitwiseNotCheck())
99+
.onFile(
100+
new DelphiTestUnitBuilder()
101+
.appendImpl("function Test: Integer;")
102+
.appendImpl("begin")
103+
.appendImpl(" Result := not (1 in [2]);")
104+
.appendImpl("end;"))
105+
.verifyNoIssues();
106+
}
107+
108+
@Test
109+
void testUnresolvedNotShouldNotAddIssue() {
110+
CheckVerifier.newVerifier()
111+
.withCheck(new ExplicitBitwiseNotCheck())
112+
.onFile(
113+
new DelphiTestUnitBuilder()
114+
.appendImpl("function Test: Integer;")
115+
.appendImpl("begin")
116+
.appendImpl(" Result := not UnresolvedIdentifier in [2];")
117+
.appendImpl("end;"))
118+
.verifyNoIssues();
119+
}
120+
121+
@Test
122+
void testIntegerProceduralTypeShouldAddIssue() {
123+
CheckVerifier.newVerifier()
124+
.withCheck(new ExplicitBitwiseNotCheck())
125+
.onFile(
126+
new DelphiTestUnitBuilder()
127+
.appendImpl("function GetMyInt: Integer;")
128+
.appendImpl("begin")
129+
.appendImpl(" Result := 1;")
130+
.appendImpl("end;")
131+
.appendImpl("function Test: Integer;")
132+
.appendImpl("begin")
133+
.appendImpl(" Result := not GetMyInt in [2]; // Noncompliant")
134+
.appendImpl("end;"))
135+
.verifyIssues();
136+
}
137+
138+
@Test
139+
void testNonIntegerProceduralTypeShouldNotAddIssue() {
140+
CheckVerifier.newVerifier()
141+
.withCheck(new ExplicitBitwiseNotCheck())
142+
.onFile(
143+
new DelphiTestUnitBuilder()
144+
.appendImpl("function GetMyStr: string;")
145+
.appendImpl("begin")
146+
.appendImpl(" Result := 'Hello world';")
147+
.appendImpl("end;")
148+
.appendImpl("function Test: Integer;")
149+
.appendImpl("begin")
150+
.appendImpl(" Result := not GetMyStr in [2];")
151+
.appendImpl("end;"))
152+
.verifyNoIssues();
153+
}
154+
155+
@Test
156+
void testParenthesizedIntegerProceduralTypeShouldNotAddIssue() {
157+
CheckVerifier.newVerifier()
158+
.withCheck(new ExplicitBitwiseNotCheck())
159+
.onFile(
160+
new DelphiTestUnitBuilder()
161+
.appendImpl("function GetMyInt: Integer;")
162+
.appendImpl("begin")
163+
.appendImpl(" Result := 1;")
164+
.appendImpl("end;")
165+
.appendImpl("function Test: Integer;")
166+
.appendImpl("begin")
167+
.appendImpl(" Result := not (GetMyInt in [2]);")
168+
.appendImpl("end;"))
169+
.verifyNoIssues();
170+
}
171+
}

0 commit comments

Comments
 (0)