Skip to content

Commit 1a1444c

Browse files
committed
Add check for redundant jumps
1 parent 5275a31 commit 1a1444c

File tree

7 files changed

+565
-0
lines changed

7 files changed

+565
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111

12+
- `RedundantJump` analysis rule, which flags redundant jump statements, e.g., `Continue`, `Exit`.
1213
- **API:** `RepeatStatementNode::getGuardExpression` method.
1314
- **API:** `RepeatStatementNode::getStatementList` method.
1415
- **API:** `CaseStatementNode::getSelectorExpression` 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
@@ -128,6 +128,7 @@ public final class CheckList {
128128
RedundantBooleanCheck.class,
129129
RedundantCastCheck.class,
130130
RedundantInheritedCheck.class,
131+
RedundantJumpCheck.class,
131132
RedundantParenthesesCheck.class,
132133
RoutineNameCheck.class,
133134
RoutineNestingDepthCheck.class,
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
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 au.com.integradev.delphi.antlr.ast.node.RoutineImplementationNodeImpl;
22+
import au.com.integradev.delphi.cfg.ControlFlowGraphFactory;
23+
import au.com.integradev.delphi.cfg.api.Block;
24+
import au.com.integradev.delphi.cfg.api.ControlFlowGraph;
25+
import au.com.integradev.delphi.cfg.api.Finally;
26+
import au.com.integradev.delphi.cfg.api.Linear;
27+
import au.com.integradev.delphi.cfg.api.UnconditionalJump;
28+
import org.sonar.check.Rule;
29+
import org.sonar.plugins.communitydelphi.api.ast.AnonymousMethodNode;
30+
import org.sonar.plugins.communitydelphi.api.ast.ArgumentListNode;
31+
import org.sonar.plugins.communitydelphi.api.ast.CompoundStatementNode;
32+
import org.sonar.plugins.communitydelphi.api.ast.DelphiNode;
33+
import org.sonar.plugins.communitydelphi.api.ast.NameReferenceNode;
34+
import org.sonar.plugins.communitydelphi.api.ast.RoutineImplementationNode;
35+
import org.sonar.plugins.communitydelphi.api.check.DelphiCheck;
36+
import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext;
37+
import org.sonar.plugins.communitydelphi.api.symbol.declaration.NameDeclaration;
38+
import org.sonar.plugins.communitydelphi.api.symbol.declaration.RoutineNameDeclaration;
39+
40+
@Rule(key = "RedundantJump")
41+
public class RedundantJumpCheck extends DelphiCheck {
42+
private static final String MESSAGE = "Remove this redundant jump.";
43+
44+
@Override
45+
public DelphiCheckContext visit(RoutineImplementationNode routine, DelphiCheckContext context) {
46+
ControlFlowGraph cfg = ((RoutineImplementationNodeImpl) routine).getControlFlowGraph();
47+
if (cfg != null) {
48+
cfg.getBlocks().forEach(block -> checkBlock(block, context));
49+
}
50+
51+
return super.visit(routine, context);
52+
}
53+
54+
@Override
55+
public DelphiCheckContext visit(AnonymousMethodNode routine, DelphiCheckContext context) {
56+
CompoundStatementNode compoundStatementNode =
57+
routine.getFirstChildOfType(CompoundStatementNode.class);
58+
if (compoundStatementNode != null) {
59+
ControlFlowGraph cfg = ControlFlowGraphFactory.create(compoundStatementNode);
60+
cfg.getBlocks().forEach(block -> checkBlock(block, context));
61+
}
62+
63+
return super.visit(routine, context);
64+
}
65+
66+
private void checkBlock(Block block, DelphiCheckContext context) {
67+
if (!(block instanceof UnconditionalJump)) {
68+
return;
69+
}
70+
71+
UnconditionalJump jump = (UnconditionalJump) block;
72+
Block successorWithoutJump = jump.getSuccessorIfRemoved();
73+
DelphiNode terminator = jump.getTerminator();
74+
75+
RoutineNameDeclaration routineNameDeclaration = getRoutineNameDeclaration(terminator);
76+
if (!isContinueOrExit(routineNameDeclaration)
77+
|| isExitWithExpression(routineNameDeclaration, terminator)) {
78+
return;
79+
}
80+
81+
Block successor = jump.getSuccessor();
82+
successorWithoutJump = nonEmptySuccessor(successorWithoutJump);
83+
84+
if (!successorWithoutJump.equals(successor)) {
85+
return;
86+
}
87+
88+
Block finallyBlock = getFinallyBlock(block);
89+
if (finallyBlock != null) {
90+
if (onlyFinallyBlocksBeforeEnd(finallyBlock)) {
91+
reportIssue(context, terminator, MESSAGE);
92+
}
93+
return;
94+
}
95+
96+
reportIssue(context, terminator, MESSAGE);
97+
}
98+
99+
private static Block getFinallyBlock(Block block) {
100+
return block.getSuccessors().stream()
101+
.filter(Finally.class::isInstance)
102+
.findFirst()
103+
.orElse(null);
104+
}
105+
106+
private static boolean onlyFinallyBlocksBeforeEnd(Block finallyBlock) {
107+
while (finallyBlock.getSuccessors().size() == 1) {
108+
Block finallySuccessor = finallyBlock.getSuccessors().iterator().next();
109+
if (!(finallySuccessor instanceof Finally)) {
110+
break;
111+
}
112+
finallyBlock = finallySuccessor;
113+
}
114+
return finallyBlock.getSuccessors().size() == 1
115+
&& finallyBlock.getSuccessors().iterator().next().getSuccessors().isEmpty();
116+
}
117+
118+
private static RoutineNameDeclaration getRoutineNameDeclaration(DelphiNode node) {
119+
if (!(node instanceof NameReferenceNode)) {
120+
return null;
121+
}
122+
NameDeclaration nameDeclaration = ((NameReferenceNode) node).getNameDeclaration();
123+
if (!(nameDeclaration instanceof RoutineNameDeclaration)) {
124+
return null;
125+
}
126+
return (RoutineNameDeclaration) nameDeclaration;
127+
}
128+
129+
private static boolean isContinueOrExit(RoutineNameDeclaration routineNameDeclaration) {
130+
if (routineNameDeclaration == null) {
131+
return false;
132+
}
133+
String fullyQualifiedName = routineNameDeclaration.fullyQualifiedName();
134+
return fullyQualifiedName.equals("System.Continue") || fullyQualifiedName.equals("System.Exit");
135+
}
136+
137+
private static boolean isExitWithExpression(
138+
RoutineNameDeclaration routineNameDeclaration, DelphiNode statement) {
139+
if (routineNameDeclaration == null) {
140+
return false;
141+
}
142+
String fullyQualifiedName = routineNameDeclaration.fullyQualifiedName();
143+
if (!fullyQualifiedName.equals("System.Exit")) {
144+
return false;
145+
}
146+
ArgumentListNode argumentList =
147+
statement.getParent().getFirstChildOfType(ArgumentListNode.class);
148+
if (argumentList == null) {
149+
return false;
150+
}
151+
return argumentList.getArgumentNodes().size() == 1;
152+
}
153+
154+
private static Block nonEmptySuccessor(Block initialBlock) {
155+
Block result = initialBlock;
156+
while (result.getElements().isEmpty() && result instanceof Linear) {
157+
result = ((Linear) result).getSuccessor();
158+
}
159+
return result;
160+
}
161+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<h2>Why is this an issue?</h2>
2+
<p>
3+
Unnecessarily annotating the default control flow of a program using statements such as
4+
<code>Continue</code>, <code>Break</code>, and <code>Exit</code> makes the code harder to read and
5+
understand. Although these statements appear to alter the flow of a program, the control flow is
6+
identical without it.
7+
</p>
8+
<h2>How to fix it</h2>
9+
<p>Remove the redundant jump:</p>
10+
<pre data-diff-id="1" data-diff-type="noncompliant">
11+
procedure Example;
12+
begin
13+
while Condition do begin
14+
// ...
15+
Continue; // Noncompliant
16+
end;
17+
end;
18+
</pre>
19+
<pre data-diff-id="1" data-diff-type="compliant">
20+
procedure Example;
21+
begin
22+
while Condition do begin
23+
// ...
24+
end;
25+
end;
26+
</pre>
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"title": "Redundant jumps should not be used",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant/Issue",
7+
"constantCost": "2min"
8+
},
9+
"code": {
10+
"attribute": "CLEAR",
11+
"impacts": {
12+
"MAINTAINABILITY": "MEDIUM"
13+
}
14+
},
15+
"tags": ["clumsy"],
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
@@ -70,6 +70,7 @@
7070
"RedundantBoolean",
7171
"RedundantCast",
7272
"RedundantInherited",
73+
"RedundantJump",
7374
"RedundantParentheses",
7475
"RoutineName",
7576
"RoutineNestingDepth",

0 commit comments

Comments
 (0)