8212982: Rule cases in switch expression accepted even if complete normally

Ensure an error is reported if switch expression does not correctly produce a value.

Reviewed-by: mcimadamore

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java

@@ -357,9 +357,9 @@ public class Flow {
         }
 
         /** Resolve all jumps of this statement. */
-        private boolean resolveJump(JCTree tree,
+        private Liveness resolveJump(JCTree tree,
-                        ListBuffer<P> oldPendingExits,
+                         ListBuffer<P> oldPendingExits,
-                        JumpKind jk) {
+                         JumpKind jk) {
             boolean resolved = false;
             List<P> exits = pendingExits.toList();
             pendingExits = oldPendingExits;
@@ -373,16 +373,16 @@ public class Flow {
                     pendingExits.append(exit);
                 }
             }
-            return resolved;
+            return Liveness.from(resolved);
         }
 
         /** Resolve all continues of this statement. */
-        boolean resolveContinues(JCTree tree) {
+        Liveness resolveContinues(JCTree tree) {
             return resolveJump(tree, new ListBuffer<P>(), JumpKind.CONTINUE);
         }
 
         /** Resolve all breaks of this statement. */
-        boolean resolveBreaks(JCTree tree, ListBuffer<P> oldPendingExits) {
+        Liveness resolveBreaks(JCTree tree, ListBuffer<P> oldPendingExits) {
             return resolveJump(tree, oldPendingExits, JumpKind.BREAK);
         }
 
@@ -417,11 +417,11 @@ public class Flow {
         /** A flag that indicates whether the last statement could
          *  complete normally.
          */
-        private boolean alive;
+        private Liveness alive;
 
         @Override
         void markDead() {
-            alive = false;
+            alive = Liveness.DEAD;
         }
 
     /*************************************************************************
@@ -432,7 +432,7 @@ public class Flow {
          */
         void scanDef(JCTree tree) {
             scanStat(tree);
-            if (tree != null && tree.hasTag(JCTree.Tag.BLOCK) && !alive) {
+            if (tree != null && tree.hasTag(JCTree.Tag.BLOCK) && alive == Liveness.DEAD) {
                 log.error(tree.pos(),
                           Errors.InitializerMustBeAbleToCompleteNormally);
             }
@@ -441,9 +441,9 @@ public class Flow {
         /** Analyze a statement. Check that statement is reachable.
          */
         void scanStat(JCTree tree) {
-            if (!alive && tree != null) {
+            if (alive == Liveness.DEAD && tree != null) {
                 log.error(tree.pos(), Errors.UnreachableStmt);
-                if (!tree.hasTag(SKIP)) alive = true;
+                if (!tree.hasTag(SKIP)) alive = Liveness.RECOVERY;
             }
             scan(tree);
         }
@@ -460,7 +460,7 @@ public class Flow {
 
         public void visitClassDef(JCClassDecl tree) {
             if (tree.sym == null) return;
-            boolean alivePrev = alive;
+            Liveness alivePrev = alive;
             ListBuffer<PendingExit> pendingExitsPrev = pendingExits;
             Lint lintPrev = lint;
 
@@ -506,10 +506,10 @@ public class Flow {
             Assert.check(pendingExits.isEmpty());
 
             try {
-                alive = true;
+                alive = Liveness.ALIVE;
                 scanStat(tree.body);
 
-                if (alive && !tree.sym.type.getReturnType().hasTag(VOID))
+                if (alive == Liveness.ALIVE && !tree.sym.type.getReturnType().hasTag(VOID))
                     log.error(TreeInfo.diagEndPos(tree.body), Errors.MissingRetStmt);
 
                 List<PendingExit> exits = pendingExits.toList();
@@ -544,21 +544,21 @@ public class Flow {
             ListBuffer<PendingExit> prevPendingExits = pendingExits;
             pendingExits = new ListBuffer<>();
             scanStat(tree.body);
-            alive |= resolveContinues(tree);
+            alive = alive.or(resolveContinues(tree));
             scan(tree.cond);
-            alive = alive && !tree.cond.type.isTrue();
+            alive = alive.and(!tree.cond.type.isTrue());
-            alive |= resolveBreaks(tree, prevPendingExits);
+            alive = alive.or(resolveBreaks(tree, prevPendingExits));
         }
 
         public void visitWhileLoop(JCWhileLoop tree) {
             ListBuffer<PendingExit> prevPendingExits = pendingExits;
             pendingExits = new ListBuffer<>();
             scan(tree.cond);
-            alive = !tree.cond.type.isFalse();
+            alive = Liveness.from(!tree.cond.type.isFalse());
             scanStat(tree.body);
-            alive |= resolveContinues(tree);
+            alive = alive.or(resolveContinues(tree));
-            alive = resolveBreaks(tree, prevPendingExits) ||
+            alive = resolveBreaks(tree, prevPendingExits).or(
-                !tree.cond.type.isTrue();
+                !tree.cond.type.isTrue());
         }
 
         public void visitForLoop(JCForLoop tree) {
@@ -567,15 +567,15 @@ public class Flow {
             pendingExits = new ListBuffer<>();
             if (tree.cond != null) {
                 scan(tree.cond);
-                alive = !tree.cond.type.isFalse();
+                alive = Liveness.from(!tree.cond.type.isFalse());
             } else {
-                alive = true;
+                alive = Liveness.ALIVE;
             }
             scanStat(tree.body);
-            alive |= resolveContinues(tree);
+            alive = alive.or(resolveContinues(tree));
             scan(tree.step);
-            alive = resolveBreaks(tree, prevPendingExits) ||
+            alive = resolveBreaks(tree, prevPendingExits).or(
-                tree.cond != null && !tree.cond.type.isTrue();
+                tree.cond != null && !tree.cond.type.isTrue());
         }
 
         public void visitForeachLoop(JCEnhancedForLoop tree) {
@@ -584,16 +584,16 @@ public class Flow {
             scan(tree.expr);
             pendingExits = new ListBuffer<>();
             scanStat(tree.body);
-            alive |= resolveContinues(tree);
+            alive = alive.or(resolveContinues(tree));
             resolveBreaks(tree, prevPendingExits);
-            alive = true;
+            alive = Liveness.ALIVE;
         }
 
         public void visitLabelled(JCLabeledStatement tree) {
             ListBuffer<PendingExit> prevPendingExits = pendingExits;
             pendingExits = new ListBuffer<>();
             scanStat(tree.body);
-            alive |= resolveBreaks(tree, prevPendingExits);
+            alive = alive.or(resolveBreaks(tree, prevPendingExits));
         }
 
         public void visitSwitch(JCSwitch tree) {
@@ -602,7 +602,7 @@ public class Flow {
             scan(tree.selector);
             boolean hasDefault = false;
             for (List<JCCase> l = tree.cases; l.nonEmpty(); l = l.tail) {
-                alive = true;
+                alive = Liveness.ALIVE;
                 JCCase c = l.head;
                 if (c.pats.isEmpty())
                     hasDefault = true;
@@ -612,13 +612,13 @@ public class Flow {
                     }
                 }
                 scanStats(c.stats);
-                c.completesNormally = alive;
+                c.completesNormally = alive != Liveness.DEAD;
-                if (alive && c.caseKind == JCCase.RULE) {
+                if (alive != Liveness.DEAD && c.caseKind == JCCase.RULE) {
                     scanSyntheticBreak(make, tree);
-                    alive = false;
+                    alive = Liveness.DEAD;
                 }
                 // Warn about fall-through if lint switch fallthrough enabled.
-                if (alive &&
+                if (alive == Liveness.ALIVE &&
                     lint.isEnabled(Lint.LintCategory.FALLTHROUGH) &&
                     c.stats.nonEmpty() && l.tail.nonEmpty())
                     log.warning(Lint.LintCategory.FALLTHROUGH,
@@ -626,9 +626,9 @@ public class Flow {
                                 Warnings.PossibleFallThroughIntoCase);
             }
             if (!hasDefault) {
-                alive = true;
+                alive = Liveness.ALIVE;
             }
-            alive |= resolveBreaks(tree, prevPendingExits);
+            alive = alive.or(resolveBreaks(tree, prevPendingExits));
         }
 
         @Override
@@ -644,9 +644,9 @@ public class Flow {
                 }
             }
             boolean hasDefault = false;
-            boolean prevAlive = alive;
+            Liveness prevAlive = alive;
             for (List<JCCase> l = tree.cases; l.nonEmpty(); l = l.tail) {
-                alive = true;
+                alive = Liveness.ALIVE;
                 JCCase c = l.head;
                 if (c.pats.isEmpty())
                     hasDefault = true;
@@ -662,13 +662,22 @@ public class Flow {
                     }
                 }
                 scanStats(c.stats);
-                c.completesNormally = alive;
+                if (alive == Liveness.ALIVE) {
+                    if (c.caseKind == JCCase.RULE) {
+                        log.error(TreeInfo.diagEndPos(c.body),
+                                  Errors.RuleCompletesNormally);
+                    } else if (l.tail.isEmpty()) {
+                        log.error(TreeInfo.diagEndPos(tree),
+                                  Errors.SwitchExpressionCompletesNormally);
+                    }
+                }
+                c.completesNormally = alive != Liveness.DEAD;
             }
             if ((constants == null || !constants.isEmpty()) && !hasDefault) {
                 log.error(tree, Errors.NotExhaustive);
             }
             alive = prevAlive;
-            alive |= resolveBreaks(tree, prevPendingExits);
+            alive = alive.or(resolveBreaks(tree, prevPendingExits));
         }
 
         public void visitTry(JCTry tree) {
@@ -686,22 +695,22 @@ public class Flow {
             }
 
             scanStat(tree.body);
-            boolean aliveEnd = alive;
+            Liveness aliveEnd = alive;
 
             for (List<JCCatch> l = tree.catchers; l.nonEmpty(); l = l.tail) {
-                alive = true;
+                alive = Liveness.ALIVE;
                 JCVariableDecl param = l.head.param;
                 scan(param);
                 scanStat(l.head.body);
-                aliveEnd |= alive;
+                aliveEnd = aliveEnd.or(alive);
             }
             if (tree.finalizer != null) {
                 ListBuffer<PendingExit> exits = pendingExits;
                 pendingExits = prevPendingExits;
-                alive = true;
+                alive = Liveness.ALIVE;
                 scanStat(tree.finalizer);
-                tree.finallyCanCompleteNormally = alive;
+                tree.finallyCanCompleteNormally = alive != Liveness.DEAD;
-                if (!alive) {
+                if (alive == Liveness.DEAD) {
                     if (lint.isEnabled(Lint.LintCategory.FINALLY)) {
                         log.warning(Lint.LintCategory.FINALLY,
                                 TreeInfo.diagEndPos(tree.finalizer),
@@ -726,12 +735,12 @@ public class Flow {
             scan(tree.cond);
             scanStat(tree.thenpart);
             if (tree.elsepart != null) {
-                boolean aliveAfterThen = alive;
+                Liveness aliveAfterThen = alive;
-                alive = true;
+                alive = Liveness.ALIVE;
                 scanStat(tree.elsepart);
-                alive = alive | aliveAfterThen;
+                alive = alive.or(aliveAfterThen);
             } else {
-                alive = true;
+                alive = Liveness.ALIVE;
             }
         }
 
@@ -776,12 +785,12 @@ public class Flow {
             }
 
             ListBuffer<PendingExit> prevPending = pendingExits;
-            boolean prevAlive = alive;
+            Liveness prevAlive = alive;
             try {
                 pendingExits = new ListBuffer<>();
-                alive = true;
+                alive = Liveness.ALIVE;
                 scanStat(tree.body);
-                tree.canCompleteNormally = alive;
+                tree.canCompleteNormally = alive != Liveness.DEAD;
             }
             finally {
                 pendingExits = prevPending;
@@ -807,7 +816,7 @@ public class Flow {
                 attrEnv = env;
                 Flow.this.make = make;
                 pendingExits = new ListBuffer<>();
-                alive = true;
+                alive = Liveness.ALIVE;
                 scan(tree);
             } finally {
                 pendingExits = null;
@@ -2776,4 +2785,58 @@ public class Flow {
             }
         }
     }
+
+    enum Liveness {
+        ALIVE {
+            @Override
+            public Liveness or(Liveness other) {
+                return this;
+            }
+            @Override
+            public Liveness and(Liveness other) {
+                return other;
+            }
+        },
+        DEAD {
+            @Override
+            public Liveness or(Liveness other) {
+                return other;
+            }
+            @Override
+            public Liveness and(Liveness other) {
+                return this;
+            }
+        },
+        RECOVERY {
+            @Override
+            public Liveness or(Liveness other) {
+                if (other == ALIVE) {
+                    return ALIVE;
+                } else {
+                    return this;
+                }
+            }
+            @Override
+            public Liveness and(Liveness other) {
+                if (other == DEAD) {
+                    return DEAD;
+                } else {
+                    return this;
+                }
+            }
+        };
+
+        public abstract Liveness or(Liveness other);
+        public abstract Liveness and(Liveness other);
+        public Liveness or(boolean value) {
+            return or(from(value));
+        }
+        public Liveness and(boolean value) {
+            return and(from(value));
+        }
+        public static Liveness from(boolean value) {
+            return value ? ALIVE : DEAD;
+        }
+    }
+
 }

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java

@@ -1392,6 +1392,7 @@ public class JavacParser implements Parser {
                 case RBRACE: case EOF:
                     JCSwitchExpression e = to(F.at(switchPos).SwitchExpression(selector,
                                                                                cases.toList()));
+                    e.endpos = token.pos;
                     accept(RBRACE);
                     return e;
                 default:

src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties

@@ -200,6 +200,14 @@ compiler.err.continue.outside.switch.expression=\
 compiler.err.return.outside.switch.expression=\
     return outside of enclosing switch expression
 
+compiler.err.rule.completes.normally=\
+    switch rule completes without providing a value\n\
+    (switch rules in switch expressions must either provide a value or throw)
+
+compiler.err.switch.expression.completes.normally=\
+    switch expression completes without providing a value\n\
+    (switch expressions must either provide a value or throw for all possible input values)
+
 # 0: name
 compiler.err.break.ambiguous.target=\
     ambiguous reference to ''{0}''\n\

src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java

@@ -1305,6 +1305,8 @@ public abstract class JCTree implements Tree, Cloneable, DiagnosticPosition {
     public static class JCSwitchExpression extends JCPolyExpression implements SwitchExpressionTree {
         public JCExpression selector;
         public List<JCCase> cases;
+        /** Position of closing brace, optional. */
+        public int endpos = Position.NOPOS;
         protected JCSwitchExpression(JCExpression selector, List<JCCase> cases) {
             this.selector = selector;
             this.cases = cases;

src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java

@@ -383,6 +383,9 @@ public class TreeInfo {
             JCTry t = (JCTry) tree;
             return endPos((t.finalizer != null) ? t.finalizer
                           : (t.catchers.nonEmpty() ? t.catchers.last().body : t.body));
+        } else if (tree.hasTag(SWITCH_EXPRESSION) &&
+                   ((JCSwitchExpression) tree).endpos != Position.NOPOS) {
+            return ((JCSwitchExpression) tree).endpos;
         } else
             return tree.pos;
     }

test/langtools/tools/javac/diags/examples/RuleCompletesNormally.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code 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 General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+// key: compiler.err.rule.completes.normally
+// key: compiler.note.preview.filename
+// key: compiler.note.preview.recompile
+// options: --enable-preview -source 12
+
+class RuleCompletesNormally {
+    public String convert(int i) {
+        return switch (i) {
+            default -> {}
+        };
+    }
+}

test/langtools/tools/javac/diags/examples/SwitchExpressionCompletesNormally.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code 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 General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+// key: compiler.err.switch.expression.completes.normally
+// key: compiler.note.preview.filename
+// key: compiler.note.preview.recompile
+// options: --enable-preview -source 12
+
+class SwitchExpressionCompletesNormally {
+    public String convert(int i) {
+        return switch (i) {
+            default:
+        };
+    }
+}

test/langtools/tools/javac/expswitch/ExpSwitchNestingTest.java

@@ -46,7 +46,7 @@ public class ExpSwitchNestingTest extends JavacTemplateTestBase {
     private static final String ESWITCH_S = "String res_string = switch (x) { case 0 -> { # } default -> \"default\"; };";
     private static final String INT_FN_ESWITCH = "java.util.function.IntSupplier r = switch (x) { case 0 -> { # } default -> null; };";
     private static final String INT_ESWITCH_DEFAULT = "int res = switch (x) { default -> { # } };";
-    private static final String IF = "if (cond) { # }";
+    private static final String IF = "if (cond) { # } else throw new RuntimeException();";
     private static final String BLOCK = "{ # }";
     private static final String BREAK_Z = "break 0;";
     private static final String BREAK_S = "break \"hello world\";";

test/langtools/tools/javac/switchexpr/ExpressionSwitchFlow.java

@@ -0,0 +1,66 @@
+/*
+ * @test /nodynamiccopyright/
+ * @bug 8212982
+ * @summary Verify a compile-time error is produced if switch expression does not provide a value
+ * @compile/fail/ref=ExpressionSwitchFlow.out --enable-preview -source 12 -XDrawDiagnostics ExpressionSwitchFlow.java
+ */
+
+public class ExpressionSwitchFlow {
+    private String test1(int i) {
+        return switch (i) {
+            case 0 -> {}
+            default -> { break "other"; }
+        };
+    }
+    private String test2(int i) {
+        return switch (i) {
+            case 0 -> {
+            }
+            default -> "other";
+        };
+    }
+    private String test3(int i) {
+        return switch (i) {
+            case 0 -> {}
+            default -> throw new IllegalStateException();
+        };
+    }
+    private String test4(int i) {
+        return switch (i) {
+            case 0 -> { break "other"; }
+            default -> {}
+        };
+    }
+    private String test5(int i) {
+        return switch (i) {
+            case 0 -> "other";
+            default -> {}
+        };
+    }
+    private String test6(int i) {
+        return switch (i) {
+            case 0 -> throw new IllegalStateException();
+            default -> {}
+        };
+    }
+    private String test7(int i) {
+        return switch (i) {
+            case 0: throw new IllegalStateException();
+            default:
+        };
+    }
+    private String test8(int i) {
+        return switch (i) {
+            case 0: i++;
+            default: {
+            }
+        };
+    }
+    private String test9(int i) {
+        return switch (i) {
+            case 0:
+            default:
+                System.err.println();
+        };
+    }
+}

test/langtools/tools/javac/switchexpr/ExpressionSwitchFlow.out

@@ -0,0 +1,12 @@
+ExpressionSwitchFlow.java:11:24: compiler.err.rule.completes.normally
+ExpressionSwitchFlow.java:18:13: compiler.err.rule.completes.normally
+ExpressionSwitchFlow.java:24:24: compiler.err.rule.completes.normally
+ExpressionSwitchFlow.java:31:25: compiler.err.rule.completes.normally
+ExpressionSwitchFlow.java:37:25: compiler.err.rule.completes.normally
+ExpressionSwitchFlow.java:43:25: compiler.err.rule.completes.normally
+ExpressionSwitchFlow.java:50:9: compiler.err.switch.expression.completes.normally
+ExpressionSwitchFlow.java:57:9: compiler.err.switch.expression.completes.normally
+ExpressionSwitchFlow.java:64:9: compiler.err.switch.expression.completes.normally
+- compiler.note.preview.filename: ExpressionSwitchFlow.java
+- compiler.note.preview.recompile
+9 errors