Skip to content

Commit b890b27

Browse files
fourlsfourls
committed
Implement ExhaustiveEnumCase check
1 parent 2fa0766 commit b890b27

File tree

6 files changed

+493
-0
lines changed

6 files changed

+493
-0
lines changed

‎CHANGELOG.md

+1
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+
- `ExhaustiveEnumCase` analysis rule, which flags `case` statements that do not handle all values in an enumeration.
1213
- **API:** `EnumeratorOccurrence` type.
1314
- **API:** `EnumeratorOccurrence::getGetEnumerator` method.
1415
- **API:** `EnumeratorOccurrence::getMoveNext` method.

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

+1
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ public final class CheckList {
6666
EmptyRoutineCheck.class,
6767
EmptyVisibilitySectionCheck.class,
6868
EnumNameCheck.class,
69+
ExhaustiveEnumCaseCheck.class,
6970
ExplicitDefaultPropertyReferenceCheck.class,
7071
ExplicitTObjectInheritanceCheck.class,
7172
FieldNameCheck.class,

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

+140
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
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 java.util.List;
22+
import java.util.Objects;
23+
import java.util.Set;
24+
import java.util.stream.Collectors;
25+
import org.sonar.check.Rule;
26+
import org.sonar.plugins.communitydelphi.api.ast.CaseItemStatementNode;
27+
import org.sonar.plugins.communitydelphi.api.ast.CaseStatementNode;
28+
import org.sonar.plugins.communitydelphi.api.ast.DelphiNode;
29+
import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode;
30+
import org.sonar.plugins.communitydelphi.api.ast.NameReferenceNode;
31+
import org.sonar.plugins.communitydelphi.api.ast.PrimaryExpressionNode;
32+
import org.sonar.plugins.communitydelphi.api.check.DelphiCheck;
33+
import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext;
34+
import org.sonar.plugins.communitydelphi.api.check.FilePosition;
35+
import org.sonar.plugins.communitydelphi.api.symbol.Invocable;
36+
import org.sonar.plugins.communitydelphi.api.symbol.declaration.EnumElementNameDeclaration;
37+
import org.sonar.plugins.communitydelphi.api.symbol.declaration.NameDeclaration;
38+
import org.sonar.plugins.communitydelphi.api.symbol.declaration.TypedDeclaration;
39+
import org.sonar.plugins.communitydelphi.api.type.Type;
40+
import org.sonar.plugins.communitydelphi.api.type.Type.EnumType;
41+
42+
@Rule(key = "ExhaustiveEnumCase")
43+
public class ExhaustiveEnumCaseCheck extends DelphiCheck {
44+
45+
@Override
46+
public DelphiCheckContext visit(CaseStatementNode node, DelphiCheckContext context) {
47+
if (node.getElseBlockNode() == null) {
48+
EnumType enumType = getSelectorExpressionType(node);
49+
if (enumType != null) {
50+
Set<EnumElementNameDeclaration> enumElements = getEnumElements(enumType);
51+
52+
List<ExpressionNode> expressions =
53+
node.getCaseItems().stream()
54+
.map(CaseItemStatementNode::getExpressions)
55+
.flatMap(List::stream)
56+
.map(ExpressionNode::skipParentheses)
57+
.collect(Collectors.toList());
58+
59+
if (expressions.stream().allMatch(PrimaryExpressionNode.class::isInstance)) {
60+
expressions.stream()
61+
.map(this::getElementNameDeclaration)
62+
.filter(Objects::nonNull)
63+
.forEach(enumElements::remove);
64+
} else {
65+
// If there are more complex expressions (e.g. subrange), with the information we have
66+
// we can't determine if all elements are handled.
67+
enumElements.clear();
68+
}
69+
70+
if (!enumElements.isEmpty()) {
71+
context
72+
.newIssue()
73+
.onFilePosition(FilePosition.from(node.getToken()))
74+
.withMessage(
75+
String.format(
76+
"Make this case statement exhaustive (%d unhandled value%s)",
77+
enumElements.size(), enumElements.size() == 1 ? "" : "s"))
78+
.report();
79+
}
80+
}
81+
}
82+
83+
return super.visit(node, context);
84+
}
85+
86+
private NameDeclaration unpackExpressionDeclaration(ExpressionNode expression) {
87+
expression = expression.skipParentheses();
88+
if (!(expression instanceof PrimaryExpressionNode)) {
89+
return null;
90+
}
91+
92+
DelphiNode maybeNameReference = expression.getChild(0);
93+
if (!(maybeNameReference instanceof NameReferenceNode)) {
94+
return null;
95+
}
96+
97+
NameReferenceNode nameReference = ((NameReferenceNode) maybeNameReference).getLastName();
98+
return nameReference.getNameDeclaration();
99+
}
100+
101+
private EnumElementNameDeclaration getElementNameDeclaration(ExpressionNode expression) {
102+
var declaration = unpackExpressionDeclaration(expression);
103+
if (declaration instanceof EnumElementNameDeclaration) {
104+
return (EnumElementNameDeclaration) declaration;
105+
} else {
106+
return null;
107+
}
108+
}
109+
110+
private EnumType getSelectorExpressionType(CaseStatementNode node) {
111+
var declaration = unpackExpressionDeclaration(node.getSelectorExpression());
112+
Type type;
113+
114+
if (declaration instanceof Invocable) {
115+
var invocable = (Invocable) declaration;
116+
if (invocable.getRequiredParametersCount() != 0) {
117+
return null;
118+
}
119+
120+
type = invocable.getReturnType();
121+
} else if (declaration instanceof TypedDeclaration) {
122+
type = ((TypedDeclaration) declaration).getType();
123+
} else {
124+
return null;
125+
}
126+
127+
if (type == null || !type.isEnum()) {
128+
return null;
129+
}
130+
131+
return (EnumType) type;
132+
}
133+
134+
private Set<EnumElementNameDeclaration> getEnumElements(EnumType enumType) {
135+
return enumType.typeScope().getAllDeclarations().stream()
136+
.filter(EnumElementNameDeclaration.class::isInstance)
137+
.map(EnumElementNameDeclaration.class::cast)
138+
.collect(Collectors.toSet());
139+
}
140+
}

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

+82
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
<h2>Why is this an issue?</h2>
2+
<p>
3+
When using a <code>case</code> statement to alternate between different values of an enumeration,
4+
all values should be handled. This could be done explicitly by including all values in the
5+
<code>case</code> arms, or implicitly by adding a <code>default</code> branch.
6+
</p>
7+
<p>
8+
An exhaustive <code>case</code> statement makes it clear that all behaviour is intentionally
9+
defined for all values, and guards against accidental omissions - for example, forgetting to
10+
update the case statement when a new value is added to the enumeration.
11+
</p>
12+
<p>
13+
Note that this rule currently ignores any <code>case</code> statement with a subrange
14+
expression due to analysis constraints.
15+
</p>
16+
<h2>How to fix it</h2>
17+
<p>
18+
Add the missing enumeration values to the <code>case</code>:
19+
</p>
20+
<pre data-diff-id="1" data-diff-type="noncompliant">
21+
type
22+
TBeverageKind = (bvCold, bvFrozen, bvHot, bvRoomTemp);
23+
24+
procedure PrepareBeverage(Kind: TBeverageKind);
25+
begin
26+
case Kind of
27+
bvCold, bvFrozen:
28+
Refrigerate;
29+
bvHot:
30+
Microwave;
31+
end;
32+
end;
33+
</pre>
34+
<pre data-diff-id="1" data-diff-type="compliant">
35+
type
36+
TBeverageKind = (bvCold, bvFrozen, bvHot, bvRoomTemp);
37+
38+
procedure PrepareBeverage(Kind: TBeverageKind);
39+
begin
40+
case Kind of
41+
bvCold, bvFrozen:
42+
Refrigerate;
43+
bvHot:
44+
Microwave;
45+
bvRoomTemp:
46+
// No action required
47+
end;
48+
end;
49+
</pre>
50+
<p>
51+
Alternatively, add an <code>else</code> block to implicitly handle all remaining values:
52+
</p>
53+
<pre data-diff-id="2" data-diff-type="noncompliant">
54+
type
55+
TBeverageKind = (bvCold, bvFrozen, bvHot, bvRoomTemp);
56+
57+
procedure PrepareBeverage(Kind: TBeverageKind);
58+
begin
59+
case Kind of
60+
bvCold, bvFrozen:
61+
Refrigerate;
62+
bvHot:
63+
Microwave;
64+
end;
65+
end;
66+
</pre>
67+
<pre data-diff-id="2" data-diff-type="compliant">
68+
type
69+
TBeverageKind = (bvCold, bvFrozen, bvHot, bvRoomTemp);
70+
71+
procedure PrepareBeverage(Kind: TBeverageKind);
72+
begin
73+
case Kind of
74+
bvCold, bvFrozen:
75+
Refrigerate;
76+
bvHot:
77+
Microwave;
78+
else
79+
// No action required
80+
end;
81+
end;
82+
</pre>

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

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"title": "Case statements should be exhaustive",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant/Issue",
7+
"constantCost": "3min"
8+
},
9+
"code": {
10+
"attribute": "COMPLETE",
11+
"impacts": {
12+
"MAINTAINABILITY": "MEDIUM"
13+
}
14+
},
15+
"tags": ["pitfall"],
16+
"defaultSeverity": "Major",
17+
"scope": "ALL",
18+
"quickfix": "unknown"
19+
}

0 commit comments

Comments
 (0)