Re: [PR] GROOVY-12399: arrow switch statement with expression arms is compiled as a switch expression and throws when unmatched [groovy]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #2922:
URL: https://github.com/apache/groovy/pull/2922#discussion_r3987321373


##########
src/main/java/org/codehaus/groovy/classgen/asm/SwitchExpressionWriter.java:
##########
@@ -213,12 +215,31 @@ protected void writeDefaultOrThrow(final SwitchExpression expression, final int
         if (defaultStatement != null && !defaultStatement.isEmpty()) {
             defaultStatement.visit(controller.getAcg());
             if (maybeFallsThrough(defaultStatement)) {
-                throwUnmatchedSelector(selectorIndex, selectorType);
+                writeUnmatchedSelector(expression, selectorIndex, selectorType);
             }
             return;
         }
         if (completeEnum) {
             throwIncompatibleClassChangeError();
+        } else {
+            writeUnmatchedSelector(expression, selectorIndex, selectorType);

Review Comment:
   This branch takes precedence over `UNMATCHED_YIELDS_NULL`. Consequently, a complete enum switch in implicit-return position still fails under `@CompileStatic`: an unmatched new enum constant reaches `IncompatibleClassChangeError`, and the static enum writer's null-selector path directly calls `throwUnmatchedSelector`. Both paths must consult the new metadata so implicit-return switches consistently yield `null`.



##########
src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java:
##########
@@ -2449,18 +2450,101 @@ public Statement visitBlock(final BlockContext ctx) {
     @Override
     public Statement visitCommandExprAlt(final CommandExprAltContext ctx) {
         Expression expr = this.visitCommandExpression(ctx.commandExpression());
-        // Statement-position `switch` with `->` is parsed as a SwitchExpression
-        // (switchStatement is colon-only). If an arrow block does not yield,
-        // rewrite it to SwitchStatement so the block need not yield (JEP 361
-        // statement rules). Expression-position incomplete arms already errored.
-        if (expr instanceof SwitchExpression se
-                && ctx.getParent() instanceof ExpressionStmtAltContext
-                && switchExpressionHasIncompleteArm(se)) {
-            return configureAST(switchExpressionAsStatement(se), ctx);
+        // A `switch` with `->` is parsed as a SwitchExpression (switchStatement is
+        // colon-only), but as in Java (JEP 361) the arrow only decides fall-through;
+        // the position decides whether a value is produced. A switch whose value is
+        // not used, or whose arrow block arms do not yield, is a SwitchStatement.
+        // In implicit-return position (the last statement of a method, closure or
+        // script body) the value is used, but an unmatched selector yields null
+        // rather than throwing, as it did in 4.x/5.x (GROOVY-12399).
+        if (expr instanceof SwitchExpression se && ctx.getParent() instanceof ExpressionStmtAltContext statement) {
+            if (switchExpressionHasIncompleteArm(se) || !isSwitchValueUsed(statement)) {
+                return configureAST(switchExpressionAsStatement(se), ctx);

Review Comment:
   The statement/expression decision occurs only after `visitCommandExpression` has visited every arm. During that visit, `visitReturnStmtAlt` still rejects `return` whenever the parser stack contains a `SwitchExpression`, so a genuine statement such as `switch (i) { case 1 -> { return 'one' } }` remains a compilation error. Determine statement position before visiting the arms, or make the control-transfer checks consult that position.
   
   This issue also appears in the following locations of the same file:
   - line 2478
   - line 2506



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org