Do not error on dead, mandated statements at end of switch cases

pkg/analyzer/lib/src/generated/resolver.dart

Index: pkg/analyzer/lib/src/generated/resolver.dart
diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
index ca02438d7ee4fb47d33baf18f7a934a0e4910bd9..a33bcf8442005fc704a431d30ba8dffb3a495449 100644
--- a/pkg/analyzer/lib/src/generated/resolver.dart
+++ b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -1913,13 +1913,13 @@ class DeadCodeVerifier extends RecursiveAstVisitor<Object> {
 
   @override
   Object visitSwitchCase(SwitchCase node) {
-    _checkForDeadStatementsInNodeList(node.statements);
+    _checkForDeadStatementsInNodeList(node.statements, allowMandated: true);
     return super.visitSwitchCase(node);
   }
 
   @override
   Object visitSwitchDefault(SwitchDefault node) {
-    _checkForDeadStatementsInNodeList(node.statements);
+    _checkForDeadStatementsInNodeList(node.statements, allowMandated: true);
     return super.visitSwitchDefault(node);
   }
 
@@ -2045,8 +2045,12 @@ class DeadCodeVerifier extends RecursiveAstVisitor<Object> {
    * [SwitchMember], this loops through the list searching for dead statements.
    *
    * @param statements some ordered list of statements in a [Block] or [SwitchMember]
+   * @param allowMandated allow dead statements mandated by the language spec.
+   *            This allows for a final break, continue, return, or throw statement
+   *            at the end of a switch case, that are mandated by the language spec.
    */
-  void _checkForDeadStatementsInNodeList(NodeList<Statement> statements) {
+  void _checkForDeadStatementsInNodeList(
+      NodeList<Statement> statements, {bool allowMandated: false}) {
     bool statementExits(Statement statement) {
       if (statement is BreakStatement) {
         return statement.label == null;
@@ -2063,6 +2067,16 @@ class DeadCodeVerifier extends RecursiveAstVisitor<Object> {
       if (statementExits(currentStatement) && i != size - 1) {
         Statement nextStatement = statements[i + 1];
         Statement lastStatement = statements[size - 1];
+        // If mandated statements are allowed, and only the last statement is
+        // dead, maybe we should _not_ report an error.
+        if (allowMandated && i == size - 2) {
+          if (nextStatement is BreakStatement ||

[Paul Berry, 2016/06/14 18:39:27]

IMHO this is overly permissive.  The important case we want to allow is a
mandated "break".  If someone puts some other statement at the end of a switch
case (a return, continue, return, or throw), then I think we should go ahead and
report a hint, because in those other cases it's highly likely that the user has
made a mistake.

[Brian Wilkerson, 2016/06/14 18:59:41]

I could buy that; we're essentially leaving a single idiom for getting around a
warning in the spec.

How does this deal with something like

switch(intValue) {
  case 0:
    return false;
    break;
}

It would be nice if we could still report the break statement as being dead
code, but maybe that's too much for now.

[srawlins, 2016/06/14 21:09:55]

Yeah... I think at this point we just won't be smart enough to catch simple
baddies like this. But imho, this is very ok: the DEAD_CODE warnings are for
_non-obvious_ dead code paths, and our brains can be used for the obvious ones
:P.

It _will_ still catch:

return false;
int a = 1;
break;

[srawlins, 2016/06/14 21:09:55]

Ah, good point. I had not thought this through all the way.

+              nextStatement is ContinueStatement ||
+              nextStatement is ReturnStatement ||
+              nextStatement is ThrowExpression) {
+            return;
+          }
+        }
         int offset = nextStatement.offset;
         int length = lastStatement.end - offset;
         _errorReporter.reportErrorForOffset(HintCode.DEAD_CODE, offset, length);