Issue 40392. Report SWITCH_CASE_COMPLETES_NORMALLY when switch/case completes normally.

Bug: https://github.com/dart-lang/sdk/issues/40392
Change-Id: I15c80d0667be1c783cb21870b9f92ee98a4f3042
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/134245
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

pkg/analyzer/lib/error/error.dart

@@ -303,6 +303,7 @@ const List<ErrorCode> errorCodeValues = [
   CompileTimeErrorCode.SUPER_IN_EXTENSION,
   CompileTimeErrorCode.SUPER_IN_INVALID_CONTEXT,
   CompileTimeErrorCode.SUPER_IN_REDIRECTING_CONSTRUCTOR,
+  CompileTimeErrorCode.SWITCH_CASE_COMPLETES_NORMALLY,
   CompileTimeErrorCode.TYPE_ALIAS_CANNOT_REFERENCE_ITSELF,
   CompileTimeErrorCode.TYPE_ARGUMENT_NOT_MATCHING_BOUNDS,
   // ignore: deprecated_member_use_from_same_package

pkg/analyzer/lib/src/error/codes.dart

@@ -5096,6 +5096,16 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
       CompileTimeErrorCode('SUPER_INITIALIZER_IN_OBJECT',
           "The class 'Object' can't invoke a constructor from a superclass.");
 
+  /// It is an error if any case of a switch statement except the last case
+  /// (the default case if present) may complete normally. The previous
+  /// syntactic restriction requiring the last statement of each case to be
+  /// one of an enumerated list of statements (break, continue, return,
+  /// throw, or rethrow) is removed.
+  static const CompileTimeErrorCode SWITCH_CASE_COMPLETES_NORMALLY =
+      CompileTimeErrorCode('SWITCH_CASE_COMPLETES_NORMALLY',
+          "The 'case' should not complete normally.",
+          correction: "Try adding 'break', or 'return', etc.");
+
   /**
    * Parameters:
    * 0: the name of the type used in the instance creation that should be

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

@@ -1885,6 +1885,8 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
    * See [StaticWarningCode.CASE_BLOCK_NOT_TERMINATED].
    */
   void _checkForCaseBlocksNotTerminated(SwitchStatement statement) {
+    if (_isNonNullableByDefault) return;
+
     NodeList<SwitchMember> members = statement.members;
     int lastMember = members.length - 1;
     for (int i = 0; i < lastMember; i++) {

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

@@ -1692,6 +1692,17 @@ class ResolverVisitor extends ScopedVisitor {
     InferenceContext.setType(
         node.expression, _enclosingSwitchStatementExpressionType);
     super.visitSwitchCase(node);
+
+    var flow = _flowAnalysis?.flow;
+    if (flow != null && flow.isReachable) {
+      var switchStatement = node.parent as SwitchStatement;
+      if (switchStatement.members.last != node && node.statements.isNotEmpty) {
+        errorReporter.reportErrorForToken(
+          CompileTimeErrorCode.SWITCH_CASE_COMPLETES_NORMALLY,
+          node.keyword,
+        );
+      }
+    }
   }
 
   @override

pkg/analyzer/test/generated/non_error_resolver_test.dart

@@ -700,42 +700,6 @@ f() {
     ]);
   }
 
-  test_caseBlockNotTerminated() async {
-    await assertNoErrorsInCode(r'''
-f(int p) {
-  for (int i = 0; i < 10; i++) {
-    switch (p) {
-      case 0:
-        break;
-      case 1:
-        continue;
-      case 2:
-        return;
-      case 3:
-        throw new Object();
-      case 4:
-      case 5:
-        return;
-      case 6:
-      default:
-        return;
-    }
-  }
-}
-''');
-  }
-
-  test_caseBlockNotTerminated_lastCase() async {
-    await assertNoErrorsInCode(r'''
-f(int p) {
-  switch (p) {
-    case 0:
-      p = p + 1;
-  }
-}
-''');
-  }
-
   test_class_type_alias_documentationComment() async {
     await assertNoErrorsInCode('''
 /**

pkg/analyzer/test/src/diagnostics/case_block_not_terminated_test.dart

@@ -15,17 +15,96 @@ main() {
 
 @reflectiveTest
 class CaseBlockNotTerminatedTest extends DriverResolutionTest {
-  test_caseBlockNotTerminated() async {
-    await assertErrorsInCode('''
-f(int p) {
-  switch (p) {
+  test_lastCase() async {
+    await assertNoErrorsInCode(r'''
+f(int a) {
+  switch (a) {
     case 0:
-      f(p);
-    case 1:
-      break;
-  }
-}''', [
-      error(StaticWarningCode.CASE_BLOCK_NOT_TERMINATED, 30, 4),
-    ]);
+      print(0);
+  }
+}
+''');
+  }
+
+  test_notTerminated() async {
+    await assertErrorsInCode('''
+void f(int a) {
+  switch (a) {
+    case 0:
+      print(0);
+    default:
+      return;
+  }
+}''', [
+      error(StaticWarningCode.CASE_BLOCK_NOT_TERMINATED, 35, 4),
+    ]);
+  }
+
+  test_terminated_break() async {
+    await assertNoErrorsInCode(r'''
+void f(int a) {
+  switch (a) {
+    case 0:
+      break;
+    default:
+      return;
+  }
+}
+''');
+  }
+
+  test_terminated_continue_loop() async {
+    await assertNoErrorsInCode(r'''
+void f(int a) {
+  while (true) {
+    switch (a) {
+      case 0:
+        continue;
+      default:
+        return;
+    }
+  }
+}
+''');
+  }
+
+  test_terminated_return() async {
+    await assertNoErrorsInCode(r'''
+void f(int a) {
+  switch (a) {
+    case 0:
+      return;
+    default:
+      return;
+  }
+}
+''');
+  }
+
+  test_terminated_return2() async {
+    await assertNoErrorsInCode(r'''
+void f(int a) {
+  switch (a) {
+    case 0:
+    case 1:
+      return;
+    default:
+      return;
+  }
+}
+''');
+  }
+
+  test_terminated_throw() async {
+    await assertNoErrorsInCode(r'''
+void f(int a) {
+  switch (a) {
+    case 0:
+      throw 42;
+    default:
+      return;
+  }
+}
+''');
   }
 }

pkg/analyzer/test/src/diagnostics/switch_case_completes_normally_test.dart

@@ -0,0 +1,162 @@
+// Copyright (c) 2020, 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.
+
+import 'package:analyzer/dart/analysis/features.dart';
+import 'package:analyzer/src/error/codes.dart';
+import 'package:analyzer/src/generated/engine.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/driver_resolution.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(SwitchCaseCompletesNormallyTest);
+  });
+}
+
+@reflectiveTest
+class SwitchCaseCompletesNormallyTest extends DriverResolutionTest {
+  @override
+  AnalysisOptionsImpl get analysisOptions => AnalysisOptionsImpl()
+    ..contextFeatures = FeatureSet.forTesting(
+        sdkVersion: '2.7.0', additionalFeatures: [Feature.non_nullable]);
+
+  test_break() async {
+    await assertNoErrorsInCode(r'''
+void f(int a) {
+  switch (a) {
+    case 0:
+      break;
+    default:
+      return;
+  }
+}
+''');
+  }
+
+  test_completes() async {
+    await assertErrorsInCode('''
+void f(int a) {
+  switch (a) {
+    case 0:
+      print(0);
+    default:
+      return;
+  }
+}''', [
+      error(CompileTimeErrorCode.SWITCH_CASE_COMPLETES_NORMALLY, 35, 4),
+    ]);
+  }
+
+  test_continue_loop() async {
+    await assertNoErrorsInCode(r'''
+void f(int a) {
+  while (true) {
+    switch (a) {
+      case 0:
+        continue;
+      default:
+        return;
+    }
+  }
+}
+''');
+  }
+
+  test_for_whatever() async {
+    await assertNoErrorsInCode(r'''
+void f(int a) {
+  switch (a) {
+    case 0:
+      for (;;) {
+        print(0);
+      }
+    default:
+      return;
+  }
+}
+''');
+  }
+
+  test_lastCase() async {
+    await assertNoErrorsInCode(r'''
+f(int a) {
+  switch (a) {
+    case 0:
+      print(0);
+  }
+}
+''');
+  }
+
+  test_methodInvocation_never() async {
+    await assertNoErrorsInCode(r'''
+void f(int a) {
+  switch (a) {
+    case 0:
+      neverCompletes();
+    default:
+      return;
+  }
+}
+
+Never neverCompletes() {}
+''');
+  }
+
+  test_return() async {
+    await assertNoErrorsInCode(r'''
+void f(int a) {
+  switch (a) {
+    case 0:
+      return;
+    default:
+      return;
+  }
+}
+''');
+  }
+
+  test_return2() async {
+    await assertNoErrorsInCode(r'''
+void f(int a) {
+  switch (a) {
+    case 0:
+    case 1:
+      return;
+    default:
+      return;
+  }
+}
+''');
+  }
+
+  test_throw() async {
+    await assertNoErrorsInCode(r'''
+void f(int a) {
+  switch (a) {
+    case 0:
+      throw 42;
+    default:
+      return;
+  }
+}
+''');
+  }
+
+  test_while_true() async {
+    await assertNoErrorsInCode(r'''
+void f(int a) {
+  switch (a) {
+    case 0:
+      while (true) {
+        print(0);
+      }
+    default:
+      return;
+  }
+}
+''');
+  }
+}

pkg/analyzer/test/src/diagnostics/test_all.dart

@@ -412,6 +412,8 @@ import 'super_in_invalid_context_test.dart' as super_in_invalid_context;
 import 'super_in_redirecting_constructor_test.dart'
     as super_in_redirecting_constructor;
 import 'super_initializer_in_object_test.dart' as super_initializer_in_object;
+import 'switch_case_completes_normally_test.dart'
+    as switch_case_completes_normally;
 import 'switch_expression_not_assignable_test.dart'
     as switch_expression_not_assignable;
 import 'todo_test.dart' as todo_test;
@@ -764,6 +766,7 @@ main() {
     super_in_invalid_context.main();
     super_in_redirecting_constructor.main();
     super_initializer_in_object.main();
+    switch_case_completes_normally.main();
     switch_expression_not_assignable.main();
     todo_test.main();
     top_level_instance_getter.main();