You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2026/09/13 10:03:29 UTC
(groovy) branch master updated: GROOVY-12406: report duplicate case labels in a switch statement
This is an automated email from the ASF dual-hosted git repository.
paulk-asert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new c7e1856477 GROOVY-12406: report duplicate case labels in a switch statement
c7e1856477 is described below
commit c7e1856477ab5fda9632650d63a040ab16cd4321
Author: Paul King <pa...@asert.com.au>
AuthorDate: Sun Sep 13 17:15:40 2026 +1000
GROOVY-12406: report duplicate case labels in a switch statement
GROOVY-12289 rejects a repeated constant case label in a switch
expression, under @TypeChecked and @CompileStatic alike. It left switch
statements out, and marked that boundary with a test asserting the
permissive behaviour. Java rejects a duplicate label in either position,
so the gap was arbitrary:
@CompileStatic
def m(int x) {
switch (x) {
case 1: return 'a'
case 1: return 'b' // dead: the first arm always wins
default: return 'c'
}
}
The check reads only the selector and the case list, both of which a
SwitchStatement has, so it is now shared rather than duplicated. Only
labels whose value is statically known are compared, which is the same
set the jump-table optimizers key on: int-family, String and enum
constants. A label the compiler cannot evaluate, such as a variable or a
call, is left to sequential first-match-wins dispatch as before.
Dynamic Groovy is unchanged and still takes the first matching arm, as
it is for a switch expression; this is a type-checking diagnostic.
The scope-marking test from GROOVY-12289 is replaced by its inverse plus
a dynamic-mode test, and the statement form now has the int, String and
enum coverage the expression form already had. No other test in the
codebase needed changing, in core or in any subproject.
---
.../transform/stc/StaticTypeCheckingVisitor.java | 26 +++++++------
.../classgen/Jep361SwitchExpressionTest.groovy | 44 +++++++++++++++++++++-
2 files changed, 57 insertions(+), 13 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index 83ce23c485..1accc410da 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -5113,6 +5113,7 @@ trying: for (ClassNode[] signature : signatures) {
Map<VariableExpression, List<ClassNode>> oldTracker = pushAssignmentTracking();
try {
super.visitSwitch(statement);
+ checkSwitchDuplicateLabels(statement.getExpression(), statement.getCaseStatements());
} finally {
popAssignmentTracking(oldTracker);
}
@@ -5156,7 +5157,7 @@ trying: for (ClassNode[] signature : signatures) {
expression.setType(resultType);
typeCheckSwitchExpressionIsCase(expression);
- checkSwitchExpressionDuplicateLabels(expression);
+ checkSwitchDuplicateLabels(expression.getExpression(), expression.getCaseStatements());
checkSwitchExpressionExhaustiveness(expression);
} finally {
typeCheckingContext.popTemporaryTypeInfo();
@@ -5211,21 +5212,22 @@ trying: for (ClassNode[] signature : signatures) {
}
/**
- * Reports a repeated constant case label in a switch expression. Sequential
- * {@code isCase} semantics make the second arm dead code, and the optimized
- * {@code tableswitch}/{@code lookupswitch} forms cannot represent it at all,
- * so it is rejected here, uniformly for type-checked and statically-compiled
- * code (GROOVY-12289). Labels compared are the same ones the optimizers key
- * on: int-family, String and enum constants; anything else (GStrings, calls,
- * regex or collection labels) cannot be proven duplicated statically and is
- * left to sequential first-match-wins dispatch.
+ * Reports a repeated constant case label, in a switch statement as well as
+ * a switch expression. Sequential {@code isCase} semantics make the second
+ * arm dead code, and the optimized {@code tableswitch}/{@code lookupswitch}
+ * forms cannot represent it at all, so it is rejected here, uniformly for
+ * type-checked and statically-compiled code (GROOVY-12289, GROOVY-12406).
+ * Labels compared are the same ones the optimizers key on: int-family,
+ * String and enum constants; anything else (GStrings, calls, regex or
+ * collection labels) cannot be proven duplicated statically and is left to
+ * sequential first-match-wins dispatch.
*
* @since 6.0.0
*/
- private void checkSwitchExpressionDuplicateLabels(final SwitchExpression expression) {
- ClassNode enumType = unwrapEnumType(getType(expression.getExpression()));
+ private void checkSwitchDuplicateLabels(final Expression selector, final List<CaseStatement> caseStatements) {
+ ClassNode enumType = unwrapEnumType(getType(selector));
Set<Object> seen = new HashSet<>();
- for (CaseStatement caseStatement : expression.getCaseStatements()) {
+ for (CaseStatement caseStatement : caseStatements) {
Expression label = caseStatement.getExpression();
Object key = null;
if (enumType != null && enumType.isEnum()) {
diff --git a/src/test/groovy/org/codehaus/groovy/classgen/Jep361SwitchExpressionTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/Jep361SwitchExpressionTest.groovy
index 289b7aaedc..b9d4308af8 100644
--- a/src/test/groovy/org/codehaus/groovy/classgen/Jep361SwitchExpressionTest.groovy
+++ b/src/test/groovy/org/codehaus/groovy/classgen/Jep361SwitchExpressionTest.groovy
@@ -675,8 +675,50 @@ final class Jep361SwitchExpressionTest {
}
@Test
- void duplicateLabelInSwitchStatementIsNotAnError() {
+ void duplicateLabelInSwitchStatementIsTypeCheckingError() {
+ // GROOVY-12289 left statements out; GROOVY-12406 closes the gap, so the
+ // diagnostic no longer depends on position, any more than Java's does
+ for (mode in ['@groovy.transform.TypeChecked', '@groovy.transform.CompileStatic']) {
+ for (dup in [
+ [decl: 'int x', arms: "case 1: return 'a'; case 1: return 'b'", label: '1'],
+ [decl: 'String x', arms: 'case "a": return 1; case "a": return 2', label: 'a'],
+ [decl: 'int x', arms: "case 1: return 'a'; case 2: return 'b'; case 1: return 'c'", label: '1']]) {
+ def err = shouldFail MultipleCompilationErrorsException, """
+ $mode
+ def m(${dup.decl}) { switch (x) { ${dup.arms}; default: return 'z' } }
+ """
+ assert err.message.contains('[Static type checking] - Duplicate case label: ' + dup.label)
+ }
+ }
+ }
+
+ @Test
+ void duplicateEnumLabelInSwitchStatementIsTypeCheckingError() {
+ for (mode in ['@groovy.transform.TypeChecked', '@groovy.transform.CompileStatic']) {
+ def err = shouldFail MultipleCompilationErrorsException, """
+ enum E { X, Y }
+ $mode
+ def m(E e) { switch (e) { case E.X: return 1; case E.X: return 2; default: return 0 } }
+ """
+ assert err.message.contains('[Static type checking] - Duplicate case label: E.X')
+ }
+ }
+
+ @Test
+ void repeatedNonConstantLabelInSwitchStatementIsAllowed() {
+ // two labels can only be compared when both values are known statically
assertBoth '''
+ @groovy.transform.TypeChecked
+ def m(Object o, Object a, Object b) {
+ switch (o) { case a: return 'x'; case b: return 'y'; default: return 'z' }
+ }
+ assert m('q', 'q', 'q') == 'x'
+ '''
+ }
+
+ @Test
+ void duplicateLabelInSwitchStatementStillRunsDynamically() {
+ assertScript '''
def m(int x) { switch (x) { case 1: return 'a'; case 1: return 'b'; default: return 'c' } }
assert m(1) == 'a'
assert m(9) == 'c'