Skip to content

Commit 4b51064

Browse files
jgardn3rjgardn3r
committed
Add DeadStoreCheck 
1 parent 36d40db commit 4b51064

7 files changed

Lines changed: 1278 additions & 0 deletions

File tree

‎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+
- `DeadStore` analysis rule, which flags redundant assignments.
1213
- **API:** `RaiseStatementNode::getRaiseLocation` method.
1314

1415
## [1.21.0] - 2026-09-04

‎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
@@ -53,6 +53,7 @@ public final class CheckList {
5353
ConstructorWithoutInheritedCheck.class,
5454
CyclomaticComplexityRoutineCheck.class,
5555
DateFormatSettingsCheck.class,
56+
DeadStoreCheck.class,
5657
DestructorNameCheck.class,
5758
DestructorWithoutInheritedCheck.class,
5859
DigitGroupingCheck.class,

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

Lines changed: 295 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,295 @@
1+
/*
2+
* Sonar Delphi Plugin
3+
* Copyright (C) 2026 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.ControlFlowGraphUtils;
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.lva.BlockDataFlowVisitor;
26+
import au.com.integradev.delphi.cfg.lva.LiveVariable;
27+
import au.com.integradev.delphi.cfg.lva.LiveVariables;
28+
import au.com.integradev.delphi.symbol.declaration.VariableNameDeclarationImpl;
29+
import java.util.ArrayList;
30+
import java.util.Comparator;
31+
import java.util.HashMap;
32+
import java.util.HashSet;
33+
import java.util.List;
34+
import java.util.Map;
35+
import java.util.Set;
36+
import java.util.TreeMap;
37+
import java.util.function.Consumer;
38+
import java.util.function.Predicate;
39+
import java.util.stream.Collectors;
40+
import java.util.stream.Stream;
41+
import org.sonar.check.Rule;
42+
import org.sonar.plugins.communitydelphi.api.ast.AnonymousMethodNode;
43+
import org.sonar.plugins.communitydelphi.api.ast.CompoundStatementNode;
44+
import org.sonar.plugins.communitydelphi.api.ast.DelphiAst;
45+
import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode;
46+
import org.sonar.plugins.communitydelphi.api.ast.FinalizationSectionNode;
47+
import org.sonar.plugins.communitydelphi.api.ast.InitializationSectionNode;
48+
import org.sonar.plugins.communitydelphi.api.ast.LocalDeclarationSectionNode;
49+
import org.sonar.plugins.communitydelphi.api.ast.RoutineImplementationNode;
50+
import org.sonar.plugins.communitydelphi.api.ast.StatementListNode;
51+
import org.sonar.plugins.communitydelphi.api.ast.UnaryExpressionNode;
52+
import org.sonar.plugins.communitydelphi.api.check.DelphiCheck;
53+
import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext;
54+
import org.sonar.plugins.communitydelphi.api.symbol.declaration.NameDeclaration;
55+
import org.sonar.plugins.communitydelphi.api.symbol.declaration.PropertyNameDeclaration;
56+
import org.sonar.plugins.communitydelphi.api.symbol.declaration.RoutineNameDeclaration;
57+
import org.sonar.plugins.communitydelphi.api.symbol.declaration.VariableNameDeclaration;
58+
import org.sonar.plugins.communitydelphi.api.type.Type;
59+
import org.sonar.plugins.communitydelphi.api.type.Type.ArrayConstructorType;
60+
import org.sonar.plugins.communitydelphi.api.type.Type.BooleanType;
61+
import org.sonar.plugins.communitydelphi.api.type.Type.IntegerType;
62+
import org.sonar.plugins.communitydelphi.api.type.Type.PointerType;
63+
64+
@Rule(key = "DeadStore")
65+
public class DeadStoreCheck extends DelphiCheck {
66+
private static final Map<RoutineNameDeclaration, RoutineImplementationNodeImpl> subRoutines =
67+
new HashMap<>();
68+
69+
@Override
70+
public DelphiCheckContext visit(RoutineImplementationNode node, DelphiCheckContext data) {
71+
ControlFlowGraph cfg = ControlFlowGraphUtils.findContainingCFG(node);
72+
addIssues(cfg, data);
73+
return super.visit(node, data);
74+
}
75+
76+
@Override
77+
public DelphiCheckContext visit(StatementListNode node, DelphiCheckContext data) {
78+
if (node.getParent() instanceof InitializationSectionNode
79+
|| node.getParent() instanceof FinalizationSectionNode) {
80+
ControlFlowGraph cfg = ControlFlowGraphUtils.findContainingCFG(node);
81+
addIssues(cfg, data);
82+
}
83+
return super.visit(node, data);
84+
}
85+
86+
@Override
87+
public DelphiCheckContext visit(AnonymousMethodNode node, DelphiCheckContext data) {
88+
ControlFlowGraph cfg = ControlFlowGraphUtils.findContainingCFG(node);
89+
addIssues(cfg, data);
90+
return super.visit(node, data);
91+
}
92+
93+
@Override
94+
public DelphiCheckContext visit(CompoundStatementNode node, DelphiCheckContext data) {
95+
if (node.getParent() instanceof DelphiAst) {
96+
ControlFlowGraph cfg = ControlFlowGraphUtils.findContainingCFG(node);
97+
addIssues(cfg, data);
98+
}
99+
return super.visit(node, data);
100+
}
101+
102+
private static void addIssues(ControlFlowGraph cfg, DelphiCheckContext data) {
103+
if (cfg == null) {
104+
return;
105+
}
106+
107+
LiveVariables liveVariables = LiveVariables.analyze(cfg);
108+
Set<LiveVariable> deadStores = new HashSet<>();
109+
subRoutines.clear();
110+
streamSubRoutines(cfg)
111+
.forEach(subRoutine -> subRoutines.put(subRoutine.getRoutineNameDeclaration(), subRoutine));
112+
113+
for (Block block : cfg.getBlocks()) {
114+
deadStores.addAll(getDeadStores(block, liveVariables));
115+
}
116+
117+
raiseIssues(deadStores, data);
118+
}
119+
120+
private static List<LiveVariable> getDeadStores(Block block, LiveVariables liveVariables) {
121+
Set<LiveVariable> blockOutputs = liveVariables.getBlockOutputs(block);
122+
Map<NameDeclaration, LiveVariable> unusedAssignments = new TreeMap<>(Comparator.naturalOrder());
123+
List<LiveVariable> deadStores = new ArrayList<>();
124+
125+
Consumer<LiveVariable> onAssign =
126+
liveVariable -> {
127+
NameDeclaration declaration = getTargetDeclaration(liveVariable, true);
128+
if (declaration == null) {
129+
return;
130+
}
131+
132+
LiveVariable unusedAssignment = unusedAssignments.put(declaration, liveVariable);
133+
if (unusedAssignment != null && unusedAssignment != liveVariable) {
134+
deadStores.add(unusedAssignment);
135+
}
136+
};
137+
Consumer<LiveVariable> onReference =
138+
liveVariable -> {
139+
NameDeclaration declaration = getTargetDeclaration(liveVariable, false);
140+
if (declaration == null) {
141+
return;
142+
}
143+
144+
unusedAssignments.remove(declaration);
145+
if (declaration instanceof RoutineNameDeclaration
146+
|| (declaration instanceof VariableNameDeclaration
147+
&& ((VariableNameDeclaration) declaration).getType().isProcedural())) {
148+
handleRoutineReference(liveVariables, liveVariable, unusedAssignments);
149+
}
150+
};
151+
152+
new BlockDataFlowVisitor().setOnAssign(onAssign).setOnReference(onReference).visit(block);
153+
154+
// If an assignment is used in another block, it isn't unused
155+
unusedAssignments.values().stream()
156+
.filter(entry -> !blockOutputs.contains(entry))
157+
.filter(liveVariable -> !isExcludedDeadStore(liveVariables, liveVariable))
158+
.forEach(deadStores::add);
159+
160+
return deadStores;
161+
}
162+
163+
private static void raiseIssues(Set<LiveVariable> deadStores, DelphiCheckContext data) {
164+
for (LiveVariable deadStore : deadStores) {
165+
if (deadStore == null) continue;
166+
String name = deadStore.getNameDeclaration().getName();
167+
data.newIssue()
168+
.onFilePosition(deadStore.getFilePosition())
169+
.withMessage("Remove redundant assignment to '%s'", name)
170+
.report();
171+
}
172+
}
173+
174+
private static NameDeclaration getTargetDeclaration(LiveVariable liveVariable, boolean isAssign) {
175+
NameDeclaration declaration = liveVariable.getNameDeclaration();
176+
if (declaration instanceof PropertyNameDeclaration) {
177+
PropertyNameDeclaration propertyDeclaration = (PropertyNameDeclaration) declaration;
178+
NameDeclaration readDeclaration = propertyDeclaration.getReadDeclaration();
179+
NameDeclaration writeDeclaration = propertyDeclaration.getWriteDeclaration();
180+
if (readDeclaration instanceof VariableNameDeclaration
181+
&& writeDeclaration instanceof VariableNameDeclaration) {
182+
if (((VariableNameDeclaration) readDeclaration).getType().isArray()
183+
|| ((VariableNameDeclaration) writeDeclaration).getType().isArray()) {
184+
// Cannot discern array elements, therefore they are excluded
185+
return null;
186+
}
187+
if (isAssign) {
188+
return writeDeclaration;
189+
} else {
190+
return readDeclaration;
191+
}
192+
} else {
193+
// Properties that aren't just a passthrough to a variable are excluded
194+
return null;
195+
}
196+
}
197+
return declaration;
198+
}
199+
200+
private static void handleRoutineReference(
201+
LiveVariables liveVariables,
202+
LiveVariable liveVariable,
203+
Map<NameDeclaration, LiveVariable> unusedAssignments) {
204+
if (liveVariable.getNameDeclaration() instanceof RoutineNameDeclaration) {
205+
RoutineNameDeclaration routine = (RoutineNameDeclaration) liveVariable.getNameDeclaration();
206+
207+
if (subRoutines.containsKey(routine)) {
208+
Stream.ofNullable(subRoutines.get(routine).getControlFlowGraph())
209+
.flatMap(cfg -> LiveVariables.analyze(cfg).getBlockInputs(cfg.getEntryBlock()).stream())
210+
.map(LiveVariable::getNameDeclaration)
211+
.forEach(unusedAssignments::remove);
212+
return;
213+
}
214+
}
215+
216+
Set<NameDeclaration> localVariables =
217+
liveVariables.getLocalScopeVariables().stream()
218+
.map(LiveVariable::getNameDeclaration)
219+
.collect(Collectors.toSet());
220+
221+
new HashSet<>(unusedAssignments.keySet())
222+
.stream()
223+
.filter(variable -> !localVariables.contains(variable))
224+
.forEach(unusedAssignments::remove);
225+
}
226+
227+
private static boolean isExcludedDeadStore(
228+
LiveVariables liveVariables, LiveVariable liveVariable) {
229+
NameDeclaration nameDeclaration = liveVariable.getNameDeclaration();
230+
231+
// Exception handler declarations are excluded
232+
if (nameDeclaration instanceof VariableNameDeclarationImpl) {
233+
VariableNameDeclarationImpl declarationImpl = (VariableNameDeclarationImpl) nameDeclaration;
234+
if (declarationImpl.isExceptItem()) return true;
235+
}
236+
237+
// Excepted values are excluded
238+
if (!isExceptedValue(liveVariable)) return false;
239+
240+
// If the excluded value's variable isn't referenced anywhere, then it isn't excluded
241+
return liveVariables.getAllAssignments().stream()
242+
.filter(Predicate.not(liveVariable::equals))
243+
.map(LiveVariable::getNameDeclaration)
244+
.anyMatch(liveVariable.getNameDeclaration()::equals);
245+
}
246+
247+
private static boolean isExceptedValue(LiveVariable liveVariable) {
248+
ExpressionNode value = liveVariable.getExpressionNode();
249+
250+
if (value == null) return false;
251+
value = value.skipParentheses();
252+
Type valueType = value.getType();
253+
if (valueType instanceof BooleanType) {
254+
// `True`, `False`
255+
return true;
256+
} else if (valueType instanceof IntegerType) {
257+
while (value instanceof UnaryExpressionNode) {
258+
value = ((UnaryExpressionNode) value).getOperand().skipParentheses();
259+
}
260+
// `-1`, `0`, `1`
261+
return "1".equals(value.getImage()) || "0".equals(value.getImage());
262+
} else if (valueType instanceof PointerType) {
263+
// `nil`
264+
return ((PointerType) valueType).isNilPointer();
265+
} else if (valueType instanceof ArrayConstructorType) {
266+
// `[]`
267+
return ((ArrayConstructorType) valueType).elementTypes().isEmpty();
268+
} else {
269+
// `''`
270+
return "''".equals(value.getImage());
271+
}
272+
}
273+
274+
private static Stream<RoutineImplementationNodeImpl> streamSubRoutines(ControlFlowGraph cfg) {
275+
return streamLocalDeclarationSection(cfg)
276+
.flatMap(
277+
section -> section.findDescendantsOfType(RoutineImplementationNodeImpl.class).stream());
278+
}
279+
280+
private static Stream<LocalDeclarationSectionNode> streamLocalDeclarationSection(
281+
ControlFlowGraph cfg) {
282+
StatementListNode statementList = cfg.getStatementListNode();
283+
AnonymousMethodNode anonymous = statementList.getFirstParentOfType(AnonymousMethodNode.class);
284+
if (anonymous != null) {
285+
return Stream.ofNullable(anonymous.getDeclarationSection());
286+
}
287+
RoutineImplementationNode routine =
288+
statementList.getFirstParentOfType(RoutineImplementationNode.class);
289+
if (routine != null) {
290+
return Stream.ofNullable(routine.getDeclarationSection());
291+
}
292+
293+
return Stream.empty();
294+
}
295+
}

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<h2>Why is this an issue?</h2>
2+
<p>
3+
Dead stores refer to assignments made to local variables that are subsequently never used or immediately overwritten.
4+
Such assignments are unnecessary and impact the clarity and potentially performance of the code. Removing them
5+
improves clarity, intentionality, and readability.
6+
</p>
7+
<h3>Exceptions</h3>
8+
<p>
9+
This rule ignores initializations to <code>-1</code>, <code>0</code>, <code>1</code>, <code>nil</code>,
10+
<code>True</code>, <code>False</code>, <code>[]</code>, and <code>''</code>.
11+
</p>
12+
13+
<h2>How to fix it</h2>
14+
<p>
15+
Remove the unnecessary assignment.
16+
</p>
17+
<pre data-diff-id="1" data-diff-type="noncompliant">
18+
function Test(Y: Integer): Integer;
19+
begin
20+
var I := 100; // Noncompliant
21+
I := 150; // Noncompliant
22+
I := 200;
23+
Exit(I + Y);
24+
end;
25+
</pre>
26+
<h4>Compliant solution</h4>
27+
<pre data-diff-id="1" data-diff-type="compliant">
28+
function Test(Y: Integer): Integer;
29+
begin
30+
var I := 200; // Compliant: no unnecessary assignment
31+
Exit(I + Y);
32+
end;
33+
</pre>

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"title": "Redundant assignments should be removed.",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant/Issue",
7+
"constantCost": "1min"
8+
},
9+
"code": {
10+
"attribute": "CLEAR",
11+
"impacts": {
12+
"MAINTAINABILITY": "MEDIUM"
13+
}
14+
},
15+
"tags": ["clumsy"],
16+
"defaultSeverity": "Major",
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
@@ -21,6 +21,7 @@
2121
"ConstantName",
2222
"ConstructorName",
2323
"CyclomaticComplexityRoutine",
24+
"DeadStore",
2425
"DestructorName",
2526
"DestructorWithoutInherited",
2627
"DigitGrouping",

0 commit comments

Comments
 (0)