Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Support for MSBuild item and item metadata expressions in project files.
- `ExhaustiveEnumCase` analysis rule, which flags `case` statements that do not handle all values in an enumeration.
- `IterationPastHighBound` analysis rule, which flags `for` loops that iterate past the end of the collection.
- `ExplicitBitwiseNot` analysis rule, which flags potentially incorrect bitwise `not` operations.
- **API:** `EnumeratorOccurrence` type.
- **API:** `ForInStatementNode::getEnumeratorOccurrence` method.
- **API:** `TypeOfTypeNode::getTypeReferenceNode` method.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ public final class CheckList {
EmptyVisibilitySectionCheck.class,
EnumNameCheck.class,
ExhaustiveEnumCaseCheck.class,
ExplicitBitwiseNotCheck.class,
ExplicitDefaultPropertyReferenceCheck.class,
ExplicitTObjectInheritanceCheck.class,
FieldNameCheck.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Sonar Delphi Plugin
* Copyright (C) 2025 Integrated Application Development
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02
*/
package au.com.integradev.delphi.checks;

import org.sonar.check.Rule;
import org.sonar.plugins.communitydelphi.api.ast.BinaryExpressionNode;
import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode;
import org.sonar.plugins.communitydelphi.api.ast.UnaryExpressionNode;
import org.sonar.plugins.communitydelphi.api.check.DelphiCheck;
import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext;
import org.sonar.plugins.communitydelphi.api.check.FilePosition;
import org.sonar.plugins.communitydelphi.api.operator.BinaryOperator;
import org.sonar.plugins.communitydelphi.api.operator.UnaryOperator;
import org.sonar.plugins.communitydelphi.api.reporting.QuickFix;
import org.sonar.plugins.communitydelphi.api.reporting.QuickFixEdit;

@Rule(key = "ExplicitBitwiseNot")
public class ExplicitBitwiseNotCheck extends DelphiCheck {
@Override
public DelphiCheckContext visit(BinaryExpressionNode node, DelphiCheckContext context) {
if (node.getOperator() == BinaryOperator.IN) {
checkBitwiseNot(node.getLeft(), context);
checkBitwiseNot(node.getRight(), context);
}

return super.visit(node, context);
}

private void checkBitwiseNot(ExpressionNode node, DelphiCheckContext context) {
if (!(node instanceof UnaryExpressionNode)) {
return;
}
UnaryExpressionNode unaryNode = (UnaryExpressionNode) node;

if (isBitwiseNot(unaryNode)) {
context
.newIssue()
.onFilePosition(FilePosition.from(unaryNode.getToken()))
.withMessage("Parenthesize this bitwise 'not' operation.")
.withQuickFixes(
QuickFix.newFix("Parenthesize bitwise 'not'")
.withEdits(
QuickFixEdit.insertBefore("(", node), QuickFixEdit.insertAfter(")", node)))
.report();
}
}

private boolean isBitwiseNot(UnaryExpressionNode node) {
return node.getOperator() == UnaryOperator.NOT && node.getType().isInteger();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<h2>Why is this an issue?</h2>
<p>
The Delphi bitwise <code>not</code> operator binds stronger than the <code>in</code> binary
operator, which can lead to subtly incorrect code. For example, in the code below the bitwise
<code>not</code> has been confused for a logical <code>not</code>, which has introduced a bug:
</p>
<pre>
var MyByte: Byte := 3;

if not MyByte in [252, 253, 254, 255] then
raise Exception.Create('MyByte must not be above 251!'); // This is raised!
</pre>
<p>
To avoid this pitfall, complex expressions involving <code>not</code> and <code>in</code> should
be parenthesized appropriately, so the precedence is obvious at a glance.
</p>
<h2>How to fix it</h2>
<p>If the bitwise <code>not</code> is intentional, parenthesize it:</p>
<pre data-diff-id="1" data-diff-type="noncompliant">
var MyByte: Byte := 3;
if not MyByte in [252] then
WriteLn('error: MyByte must not be 252!');
</pre>
<pre data-diff-id="1" data-diff-type="compliant">
var MyByte: Byte := 3;
if (not MyByte) in [252] then
WriteLn('error: MyByte must not be 3!');
</pre>
<p>Otherwise, parenthesize the binary expression that should be negated:</p>
<pre data-diff-id="1" data-diff-type="noncompliant">
var MyByte: Byte := 3;
if not MyByte in [252] then
WriteLn('error: MyByte must not be 252!');
</pre>
<pre data-diff-id="1" data-diff-type="compliant">
var MyByte: Byte := 3;
if not (MyByte in [252]) then
WriteLn('error: MyByte must be 252!');
</pre>
<h2>Resources</h2>
<ul>
<li>
<a href="https://docwiki.embarcadero.com/RADStudio/en/Expressions_(Delphi)">
RAD Studio documentation: Expressions (Delphi)
</a>
</li>
</ul>
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"title": "Ambiguous bitwise 'not' operations should be parenthesized",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant/Issue",
"constantCost": "2min"
},
"code": {
"attribute": "LOGICAL",
"impacts": {
"RELIABILITY": "MEDIUM"
}
},
"tags": ["suspicious"],
"defaultSeverity": "Minor",
"scope": "ALL",
"quickfix": "unknown"
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
"EmptyRoutine",
"EmptyVisibilitySection",
"EnumName",
"ExplicitBitwiseNot",
"ExplicitDefaultPropertyReference",
"FieldName",
"FormatArgumentCount",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
/*
* Sonar Delphi Plugin
* Copyright (C) 2019 Integrated Application Development
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02
*/
package au.com.integradev.delphi.checks;

import au.com.integradev.delphi.builders.DelphiTestUnitBuilder;
import au.com.integradev.delphi.checks.verifier.CheckVerifier;
import org.junit.jupiter.api.Test;

class ExplicitBitwiseNotCheckTest {
@Test
void testSimpleBitwiseNotShouldNotAddIssue() {
CheckVerifier.newVerifier()
.withCheck(new ExplicitBitwiseNotCheck())
.onFile(
new DelphiTestUnitBuilder()
.appendImpl("function Test: Integer;")
.appendImpl("begin")
.appendImpl(" Result := not 1;")
.appendImpl("end;"))
.verifyNoIssues();
}

@Test
void testOtherIntegerUnaryOperatorsShouldNotAddIssue() {
CheckVerifier.newVerifier()
.withCheck(new ExplicitBitwiseNotCheck())
.onFile(
new DelphiTestUnitBuilder()
.appendImpl("function Test: Integer;")
.appendImpl("begin")
.appendImpl(" Result := -1 in [2];")
.appendImpl(" Result := +1 in [2];")
.appendImpl("end;"))
.verifyNoIssues();
}

@Test
void testBitwiseNotInSetContainmentCheckShouldAddIssue() {
CheckVerifier.newVerifier()
.withCheck(new ExplicitBitwiseNotCheck())
.onFile(
new DelphiTestUnitBuilder()
.appendImpl("procedure Test;")
.appendImpl("begin")
.appendImpl(" // Fix@[+2:5 to +2:5] <<(>>")
.appendImpl(" // Fix@[+1:10 to +1:10] <<)>>")
.appendImpl(" if not 3 in [3, 252] then // Noncompliant")
.appendImpl(" Writeln('Foo');")
.appendImpl("end;"))
.verifyIssues();
}

@Test
void testBitwiseNotInBinaryExpressionShouldAddIssue() {
CheckVerifier.newVerifier()
.withCheck(new ExplicitBitwiseNotCheck())
.onFile(
new DelphiTestUnitBuilder()
.appendImpl("function Test: Integer;")
.appendImpl("begin")
.appendImpl(" Result := not 1 in [2]; // Noncompliant")
.appendImpl("end;"))
.verifyIssues();
}

@Test
void testParenthesizedBitwiseNotInBinaryExpressionShouldNotAddIssue() {
CheckVerifier.newVerifier()
.withCheck(new ExplicitBitwiseNotCheck())
.onFile(
new DelphiTestUnitBuilder()
.appendImpl("function Test: Integer;")
.appendImpl("begin")
.appendImpl(" Result := (not 1) in [2];")
.appendImpl("end;"))
.verifyNoIssues();
}

@Test
void testBooleanNotShouldNotAddIssue() {
CheckVerifier.newVerifier()
.withCheck(new ExplicitBitwiseNotCheck())
.onFile(
new DelphiTestUnitBuilder()
.appendImpl("function Test: Integer;")
.appendImpl("begin")
.appendImpl(" Result := not (1 in [2]);")
.appendImpl("end;"))
.verifyNoIssues();
}

@Test
void testUnresolvedNotShouldNotAddIssue() {
CheckVerifier.newVerifier()
.withCheck(new ExplicitBitwiseNotCheck())
.onFile(
new DelphiTestUnitBuilder()
.appendImpl("function Test: Integer;")
.appendImpl("begin")
.appendImpl(" Result := not UnresolvedIdentifier in [2];")
.appendImpl("end;"))
.verifyNoIssues();
}

@Test
void testIntegerProceduralTypeShouldAddIssue() {
CheckVerifier.newVerifier()
.withCheck(new ExplicitBitwiseNotCheck())
.onFile(
new DelphiTestUnitBuilder()
.appendImpl("function GetMyInt: Integer;")
.appendImpl("begin")
.appendImpl(" Result := 1;")
.appendImpl("end;")
.appendImpl("function Test: Integer;")
.appendImpl("begin")
.appendImpl(" Result := not GetMyInt in [2]; // Noncompliant")
.appendImpl("end;"))
.verifyIssues();
}

@Test
void testNonIntegerProceduralTypeShouldNotAddIssue() {
CheckVerifier.newVerifier()
.withCheck(new ExplicitBitwiseNotCheck())
.onFile(
new DelphiTestUnitBuilder()
.appendImpl("function GetMyStr: string;")
.appendImpl("begin")
.appendImpl(" Result := 'Hello world';")
.appendImpl("end;")
.appendImpl("function Test: Integer;")
.appendImpl("begin")
.appendImpl(" Result := not GetMyStr in [2];")
.appendImpl("end;"))
.verifyNoIssues();
}

@Test
void testParenthesizedIntegerProceduralTypeShouldNotAddIssue() {
CheckVerifier.newVerifier()
.withCheck(new ExplicitBitwiseNotCheck())
.onFile(
new DelphiTestUnitBuilder()
.appendImpl("function GetMyInt: Integer;")
.appendImpl("begin")
.appendImpl(" Result := 1;")
.appendImpl("end;")
.appendImpl("function Test: Integer;")
.appendImpl("begin")
.appendImpl(" Result := not (GetMyInt in [2]);")
.appendImpl("end;"))
.verifyNoIssues();
}
}