Skip to content

Commit b8a298e

Browse files
authored
Fix RefineSwitchCases generating invalid guard on enum constant labels (#1124)
`when` guards are only valid on pattern case labels, but the recipe also matched constant labels (e.g. enum constants) when the `if` condition referenced no label-bound variables, producing uncompilable code with a duplicate case. Require the case label to bind pattern variables before refining. Fixes #1123
1 parent c7f400b commit b8a298e

2 files changed

Lines changed: 41 additions & 2 deletions

File tree

src/main/java/org/openrewrite/java/migrate/lang/RefineSwitchCases.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,11 @@ public J.Switch visitSwitch(J.Switch sw, ExecutionContext ctx) {
6666
List<Statement> caseStatements = ((J.Block) case_.getBody()).getStatements();
6767
if (caseStatements.size() == 1 && caseStatements.get(0) instanceof J.If) {
6868
J.If if_ = (J.If) caseStatements.get(0);
69-
if (extractLabelVariables(case_)
70-
.containsAll(extractConditionVariables(if_.getIfCondition().getTree()))) {
69+
// Guards (`when`) are only valid on pattern case labels, not on
70+
// constant labels such as enum constants, so require bound variables
71+
Set<String> labelVariables = extractLabelVariables(case_);
72+
if (!labelVariables.isEmpty() &&
73+
labelVariables.containsAll(extractConditionVariables(if_.getIfCondition().getTree()))) {
7174
// Replace case with multiple cases
7275
return createGuardedCases(case_, if_);
7376
}

src/test/java/org/openrewrite/java/migrate/lang/RefineSwitchCasesTest.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,42 @@ else if (i >= 0 && i < 5)
293293
);
294294
}
295295

296+
@Test
297+
void enumConstantLabelWithGuardlessCondition() {
298+
rewriteRun(
299+
//language=java
300+
java(
301+
"""
302+
class Reproducer {
303+
enum EnumOptions {
304+
OPTION_ONE,
305+
OPTION_TWO
306+
}
307+
308+
boolean checkMethod() {
309+
return true;
310+
}
311+
312+
void handleOptionOne() {
313+
}
314+
315+
void buildMenu(EnumOptions item) {
316+
switch (item) {
317+
case OPTION_ONE -> {
318+
if (checkMethod()) {
319+
handleOptionOne();
320+
}
321+
}
322+
case OPTION_TWO -> {
323+
}
324+
}
325+
}
326+
}
327+
"""
328+
)
329+
);
330+
}
331+
296332
@Test
297333
void notFormattedWhenNotChanged() {
298334
rewriteRun(

0 commit comments

Comments
 (0)