Semantic checks on switch cases and fall-through errors.

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

 // Copyright (c) 2016, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library fasta.body_builder;
 
 import '../fasta_codes.dart'
     show FastaMessage, codeExpectedButGot, codeExpectedFunctionBody;
 
 import '../parser/parser.dart' show FormalParameterType, MemberKind, optional;
@@ skipping 2463 matching lines @@
   void beginSwitchCase(int labelCount, int expressionCount, Token firstToken) {
     debugEvent("beginSwitchCase");
     List labelsAndExpressions = popList(labelCount + expressionCount);
     List<Label> labels = <Label>[];
     List<Expression> expressions = <Expression>[];
     if (labelsAndExpressions != null) {
       for (var labelOrExpression in labelsAndExpressions) {
         if (labelOrExpression is Label) {
           labels.add(labelOrExpression);
         } else {
-          expressions.add(toValue(labelOrExpression));
+          expressions.add(labelOrExpression);
         }
       }
     }
     assert(scope == switchScope);
     for (Label label in labels) {
       if (scope.hasLocalLabel(label.name)) {
         // TODO(ahe): Should validate this is a goto target and not duplicated.
         scope.claimLabel(label.name);
       } else {
         scope.declareLabel(label.name, createGotoTarget(firstToken.charOffset));
@@ skipping 40 matching lines @@
         new List<SwitchCase>.filled(caseCount, null, growable: true);
     for (int i = caseCount - 1; i >= 0; i--) {
       List<Label> labels = pop();
       SwitchCase current = cases[i] = pop();
       for (Label label in labels) {
         JumpTarget target = switchScope.lookupLabel(label.name);
         if (target != null) {
           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();
     exitLocalScope();
     Expression expression = popForValue();
     Statement result = new SwitchStatement(expression, cases);
     if (target.hasUsers) {
       result = new LabeledStatement(result);
       target.resolveBreaks(result);
     }
@@ skipping 154 matching lines @@
   }
 
   Expression wrapInCompileTimeError(Expression expression, String message) {
     return new Let(
         new VariableDeclaration.forValue(expression)
           ..fileOffset = expression.fileOffset,
         buildCompileTimeError(message, expression.fileOffset))
       ..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.",
         charOffset);
     Builder constructor = library.loader.getAbstractClassInstantiationError();
     return new Throw(buildStaticInvocation(constructor.target,
         new KernelArguments(<Expression>[new StringLiteral(className)])));
   }
 
   Statement buildCompileTimeErrorStatement(error, [int charOffset = -1]) {
@@ skipping 644 matching lines @@
     if (starToken == null) {
       return AsyncMarker.Async;
     } else {
       assert(identical(starToken.stringValue, "*"));
       return AsyncMarker.AsyncStar;
     }
   } else {
     return internalError("Unknown async modifier: $asyncToken");
   }
 }