Skip to content

Add check for exhaustive case statements over enums #370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 6, 2025
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 @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- `ExhaustiveEnumCase` analysis rule, which flags `case` statements that do not handle all values in an enumeration.
- **API:** `EnumeratorOccurrence` type.
- **API:** `EnumeratorOccurrence::getGetEnumerator` method.
- **API:** `EnumeratorOccurrence::getMoveNext` method.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public final class CheckList {
EmptyRoutineCheck.class,
EmptyVisibilitySectionCheck.class,
EnumNameCheck.class,
ExhaustiveEnumCaseCheck.class,
ExplicitDefaultPropertyReferenceCheck.class,
ExplicitTObjectInheritanceCheck.class,
FieldNameCheck.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
/*
* 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 java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
import org.sonar.check.Rule;
import org.sonar.plugins.communitydelphi.api.ast.CaseItemStatementNode;
import org.sonar.plugins.communitydelphi.api.ast.CaseStatementNode;
import org.sonar.plugins.communitydelphi.api.ast.DelphiNode;
import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode;
import org.sonar.plugins.communitydelphi.api.ast.NameReferenceNode;
import org.sonar.plugins.communitydelphi.api.ast.PrimaryExpressionNode;
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.symbol.Invocable;
import org.sonar.plugins.communitydelphi.api.symbol.declaration.EnumElementNameDeclaration;
import org.sonar.plugins.communitydelphi.api.symbol.declaration.NameDeclaration;
import org.sonar.plugins.communitydelphi.api.symbol.declaration.TypedDeclaration;
import org.sonar.plugins.communitydelphi.api.type.Type;
import org.sonar.plugins.communitydelphi.api.type.Type.EnumType;

@Rule(key = "ExhaustiveEnumCase")
public class ExhaustiveEnumCaseCheck extends DelphiCheck {

@Override
public DelphiCheckContext visit(CaseStatementNode node, DelphiCheckContext context) {
if (node.getElseBlockNode() == null) {
EnumType enumType = getSelectorExpressionType(node);
if (enumType != null) {
Set<EnumElementNameDeclaration> enumElements = getEnumElements(enumType);

List<ExpressionNode> expressions =
node.getCaseItems().stream()
.map(CaseItemStatementNode::getExpressions)
.flatMap(List::stream)
.map(ExpressionNode::skipParentheses)
.collect(Collectors.toList());

if (expressions.stream().allMatch(PrimaryExpressionNode.class::isInstance)) {
expressions.stream()
.map(this::getElementNameDeclaration)
.filter(Objects::nonNull)
.forEach(enumElements::remove);
} else {
// If there are more complex expressions (e.g. subrange), with the information we have
// we can't determine if all elements are handled.
enumElements.clear();
}

if (!enumElements.isEmpty()) {
context
.newIssue()
.onFilePosition(FilePosition.from(node.getToken()))
.withMessage(
String.format(
"Make this case statement exhaustive (%d unhandled value%s)",
enumElements.size(), enumElements.size() == 1 ? "" : "s"))
.report();
}
}
}

return super.visit(node, context);
}

private NameDeclaration unpackExpressionDeclaration(ExpressionNode expression) {
expression = expression.skipParentheses();
if (!(expression instanceof PrimaryExpressionNode)) {
return null;
}

DelphiNode maybeNameReference = expression.getChild(0);
if (!(maybeNameReference instanceof NameReferenceNode)) {
return null;
}

NameReferenceNode nameReference = ((NameReferenceNode) maybeNameReference).getLastName();
return nameReference.getNameDeclaration();
}

private EnumElementNameDeclaration getElementNameDeclaration(ExpressionNode expression) {
var declaration = unpackExpressionDeclaration(expression);
if (declaration instanceof EnumElementNameDeclaration) {
return (EnumElementNameDeclaration) declaration;
} else {
return null;
}
}

private EnumType getSelectorExpressionType(CaseStatementNode node) {
var declaration = unpackExpressionDeclaration(node.getSelectorExpression());
Type type;

if (declaration instanceof Invocable) {
var invocable = (Invocable) declaration;
if (invocable.getRequiredParametersCount() != 0) {
return null;
}

type = invocable.getReturnType();
} else if (declaration instanceof TypedDeclaration) {
type = ((TypedDeclaration) declaration).getType();
} else {
return null;
}

if (type == null || !type.isEnum()) {
return null;
}

return (EnumType) type;
}

private Set<EnumElementNameDeclaration> getEnumElements(EnumType enumType) {
return enumType.typeScope().getAllDeclarations().stream()
.filter(EnumElementNameDeclaration.class::isInstance)
.map(EnumElementNameDeclaration.class::cast)
.collect(Collectors.toSet());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<h2>Why is this an issue?</h2>
<p>
When using a <code>case</code> statement to alternate between different values of an enumeration,
all values should be handled. This could be done explicitly by including all values in the
<code>case</code> arms, or implicitly by adding a <code>default</code> branch.
</p>
<p>
An exhaustive <code>case</code> statement makes it clear that all behaviour is intentionally
defined for all values, and guards against accidental omissions - for example, forgetting to
update the case statement when a new value is added to the enumeration.
</p>
<p>
Note that this rule currently ignores any <code>case</code> statement with a subrange
expression due to analysis constraints.
</p>
<h2>How to fix it</h2>
<p>
Add the missing enumeration values to the <code>case</code>:
</p>
<pre data-diff-id="1" data-diff-type="noncompliant">
type
TBeverageKind = (bvCold, bvFrozen, bvHot, bvRoomTemp);

procedure PrepareBeverage(Kind: TBeverageKind);
begin
case Kind of
bvCold, bvFrozen:
Refrigerate;
bvHot:
Microwave;
end;
end;
</pre>
<pre data-diff-id="1" data-diff-type="compliant">
type
TBeverageKind = (bvCold, bvFrozen, bvHot, bvRoomTemp);

procedure PrepareBeverage(Kind: TBeverageKind);
begin
case Kind of
bvCold, bvFrozen:
Refrigerate;
bvHot:
Microwave;
bvRoomTemp:
// No action required
end;
end;
</pre>
<p>
Alternatively, add an <code>else</code> block to implicitly handle all remaining values:
</p>
<pre data-diff-id="2" data-diff-type="noncompliant">
type
TBeverageKind = (bvCold, bvFrozen, bvHot, bvRoomTemp);

procedure PrepareBeverage(Kind: TBeverageKind);
begin
case Kind of
bvCold, bvFrozen:
Refrigerate;
bvHot:
Microwave;
end;
end;
</pre>
<pre data-diff-id="2" data-diff-type="compliant">
type
TBeverageKind = (bvCold, bvFrozen, bvHot, bvRoomTemp);

procedure PrepareBeverage(Kind: TBeverageKind);
begin
case Kind of
bvCold, bvFrozen:
Refrigerate;
bvHot:
Microwave;
else
// No action required
end;
end;
</pre>
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"title": "Case statements should be exhaustive",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant/Issue",
"constantCost": "3min"
},
"code": {
"attribute": "COMPLETE",
"impacts": {
"MAINTAINABILITY": "MEDIUM"
}
},
"tags": ["pitfall"],
"defaultSeverity": "Major",
"scope": "ALL",
"quickfix": "unknown"
}
Loading