Skip to content

Commit acbe895

Browse files
jgardn3rjgardn3r
committed
Add DeadStoreCheck 
1 parent 1fe9069 commit acbe895

7 files changed

Lines changed: 1274 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
- **API:** `VariableNameDeclaration::isExceptItem` method.
1415

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

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<h2>Why is this an issue?</h2>
2+
<p>
3+
Dead stores refer to assignments made to 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+
8+
<h2>How to fix it</h2>
9+
<p>
10+
Remove the unnecessary assignment.
11+
</p>
12+
<pre data-diff-id="1" data-diff-type="noncompliant">
13+
function Test(Y: Integer): Integer;
14+
begin
15+
var I := 100; // Noncompliant
16+
I := 150; // Noncompliant
17+
I := 200;
18+
Exit(I + Y);
19+
end;
20+
</pre>
21+
<h4>Compliant solution</h4>
22+
<pre data-diff-id="1" data-diff-type="compliant">
23+
function Test(Y: Integer): Integer;
24+
begin
25+
var I := 200; // Compliant: no unnecessary assignment
26+
Exit(I + Y);
27+
end;
28+
</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)