Semantic checks on switch cases and fall-through errors.

pkg/front_end/lib/src/fasta/kernel/body_builder.dart

Index: pkg/front_end/lib/src/fasta/kernel/body_builder.dart
diff --git a/pkg/front_end/lib/src/fasta/kernel/body_builder.dart b/pkg/front_end/lib/src/fasta/kernel/body_builder.dart
index a82e842c13d025698d266f62733658a4bb160a1c..56f926d5d1dffb39a5bb747a3b3cef59d5cd1a3d 100644
--- a/pkg/front_end/lib/src/fasta/kernel/body_builder.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/body_builder.dart
@@ -2481,7 +2481,7 @@ class BodyBuilder extends ScopeListener<JumpTarget> implements BuilderHelper {
         if (labelOrExpression is Label) {
           labels.add(labelOrExpression);
         } else {
-          expressions.add(toValue(labelOrExpression));
+          expressions.add(labelOrExpression);
         }
       }
     }
@@ -2542,7 +2542,44 @@ class BodyBuilder extends ScopeListener<JumpTarget> implements BuilderHelper {
           target.resolveGotos(current);
         }
       }
-      // TODO(ahe): Validate that there's only one default and it's last.
+    }
+    // Check all but the last case for the following:
+    // 1. That it isn't a default case (which should be last).
+    // 2. That it doesn't fall through to the next case.
+    for (int i = 0; i < caseCount - 1; i++) {
+      SwitchCase current = cases[i];
+      if (current.isDefault) {
+        addCompileTimeError(current.fileOffset,
+            "'default' switch case should be the last case.");
+        continue;
+      }
+      Block block = current.body;
+      TreeNode lastNode = block;
+      while (lastNode is Block || lastNode is LabeledStatement) {

[Paul Berry, 2017/06/06 18:32:26]

This isn't consistent with the spec.  The spec says we only unwrap a block once.
 Here's the relevant spec text:

Let $s$ be the last statement of the statement sequence $s_k$.
If $s$ is a non-empty block statement, let $s$ instead be the last statement of
the block statement.
It is a static warning $s$ is not a \BREAK{}, \CONTINUE{}, \RETHROW{} or
\RETURN{} statement or an expression statement where the expression is a
\THROW{} expression.

[ahe, 2017/06/06 18:40:05]

Then I should replace lines 2558-2569 with lines 2559-2565, agreed?

[Paul Berry, 2017/06/06 18:55:37]

I think we still need the LabeledStatement logic.  Can labeled statements nest? 
If so, then I think we need something like:

while (lastNode is LabeledStatement) {
  LabeledStatement statement = lastNode;
  lastNode = statement.body;
}
if (lastNode is Block && lastNode.statements.isNotEmpty) {
  Block block = lastNode;
  lastNode = lastNode.statements.last;
}
while (lastNode is LabeledStatement) {
  LabeledStatement statement = lastNode;
  lastNode = statement.body;
}

(If labeled statements can't nest, then those "while"s above can change to
"if"s)

[ahe, 2017/06/06 19:02:15]

That's what I thought at first. However, that doesn't match the specification
you quoted.

[Paul Berry, 2017/06/06 19:18:08]

Fair enough.  I've checked and analyzer's interpretation matches yours.

+        if (lastNode is Block) {
+          Block block = lastNode;
+          if (block.statements.isEmpty) {
+            lastNode = null;
+          } else {
+            lastNode = block.statements.last;
+          }
+        } else if (lastNode is LabeledStatement) {
+          LabeledStatement statement = lastNode;
+          lastNode = statement.body;
+        }
+      }
+      if (lastNode is ExpressionStatement) {
+        ExpressionStatement statement = lastNode;
+        lastNode = statement.expression;
+      }
+      if (lastNode is! BreakStatement &&
+          lastNode is! ContinueSwitchStatement &&
+          lastNode is! Rethrow &&
+          lastNode is! ReturnStatement &&
+          lastNode is! Throw) {
+        block.addStatement(
+            new ExpressionStatement(buildFallThroughError(current.fileOffset)));
+      }
     }
     JumpTarget target = exitBreakTarget();
     exitSwitchScope();
@@ -2717,6 +2754,14 @@ class BodyBuilder extends ScopeListener<JumpTarget> implements BuilderHelper {
       ..fileOffset = expression.fileOffset;
   }
 
+  Expression buildFallThroughError(int charOffset) {
+    warningNotError("Switch case may fall through to next case.", charOffset);
+    Builder constructor = library.loader.getFallThroughError();
+    return new Throw(buildStaticInvocation(
+        constructor.target, new Arguments.empty(),
+        charOffset: charOffset));
+  }
+
   Expression buildAbstractClassInstantiationError(String className,
       [int charOffset = -1]) {
     warning("The class '$className' is abstract and can't be instantiated.",