Merge pull request #75792 from ahoppen/no-editor-mode-switch

[Sema/IDE] Emit same diagnostics for missing switch cases independent of editor mode

Author: ahoppen

lib/Sema/TypeCheckSwitchStmt.cpp

@@ -1124,8 +1124,6 @@ namespace {
       llvm::SmallString<128> buffer;
       llvm::raw_svector_ostream OS(buffer);
 
-      bool InEditor = Context.LangOpts.DiagnosticsEditorMode;
-
       // Decide whether we want an error or a warning.
       std::optional<decltype(diag::non_exhaustive_switch)> mainDiagType =
           diag::non_exhaustive_switch;
@@ -1274,61 +1272,49 @@ namespace {
         }
       };
 
-      // If editing is enabled, emit a formatted error of the form:
+      // Emit a formatted note of the form, which has a Fix-It associated with
+      // it, primarily to be used in IDEs:
       //
       // switch must be exhaustive, do you want to add missing cases?
       //     case (.none, .some(_)):
       //       <#code#>
       //     case (.some(_), .none):
       //       <#code#>
       //
-      // else:
-      //
-      // switch must be exhaustive, consider adding missing cases:
+      // To also provide actionable output for command line errors, emit notes
+      // like the following:
       //
       // missing case '(.none, .some(_))'
       // missing case '(.some(_), .none)'
-      if (InEditor) {
-        buffer.clear();
-
-        bool alreadyEmittedSomething = false;
-        processUncoveredSpaces([&](const Space &space,
-                                   bool onlyOneUncoveredSpace) {
-          if (space.getKind() == SpaceKind::UnknownCase) {
-            OS << "@unknown " << tok::kw_default;
-            if (onlyOneUncoveredSpace) {
-              OS << ":\n<#fatalError()#>\n";
-              DE.diagnose(startLoc, diag::missing_unknown_case)
-                .fixItInsert(insertLoc, buffer.str());
-              alreadyEmittedSomething = true;
-              return;
-            }
-          } else {
-            OS << tok::kw_case << " ";
-            space.show(OS);
-          }
-          OS << ":\n" << placeholder << "\n";
-        });
-
-        if (!alreadyEmittedSomething) {
-          DE.diagnose(startLoc, diag::missing_several_cases, false)
-            .fixItInsert(insertLoc, buffer.str());
+      SmallString<128> missingSeveralCasesFixIt;
+      int diagnosedCases = 0;
+
+      processUncoveredSpaces([&](const Space &space,
+                                 bool onlyOneUncoveredSpace) {
+        llvm::SmallString<64> fixItBuffer;
+        llvm::raw_svector_ostream fixItOS(fixItBuffer);
+        if (space.getKind() == SpaceKind::UnknownCase) {
+          fixItOS << "@unknown " << tok::kw_default << ":\n<#fatalError()#>\n";
+          DE.diagnose(startLoc, diag::missing_unknown_case)
+              .fixItInsert(insertLoc, fixItBuffer.str());
+        } else {
+          llvm::SmallString<64> spaceBuffer;
+          llvm::raw_svector_ostream spaceOS(spaceBuffer);
+          space.show(spaceOS);
+
+          fixItOS << tok::kw_case << " " << spaceBuffer << ":\n"
+                  << placeholder << "\n";
+          DE.diagnose(startLoc, diag::missing_particular_case,
+                      spaceBuffer.str())
+              .fixItInsert(insertLoc, fixItBuffer);
         }
+        diagnosedCases += 1;
+        missingSeveralCasesFixIt += fixItBuffer;
+      });
 
-      } else {
-        processUncoveredSpaces([&](const Space &space,
-                                   bool onlyOneUncoveredSpace) {
-          if (space.getKind() == SpaceKind::UnknownCase) {
-            auto note = DE.diagnose(startLoc, diag::missing_unknown_case);
-            if (onlyOneUncoveredSpace)
-              note.fixItInsert(insertLoc, "@unknown default:\n<#fatalError#>()\n");
-            return;
-          }
-
-          buffer.clear();
-          space.show(OS);
-          DE.diagnose(startLoc, diag::missing_particular_case, buffer.str());
-        });
+      if (diagnosedCases > 1) {
+        DE.diagnose(startLoc, diag::missing_several_cases, false)
+            .fixItInsert(insertLoc, missingSeveralCasesFixIt.str());
       }
     }
 

test/ClangImporter/availability_open_enums.swift

@@ -10,6 +10,7 @@ func exhaustiveSwitch(e: NSEnumAddedCasesIn2017) {
   switch e { // expected-error{{switch must be exhaustive}}
     // expected-note@-1{{add missing case: '.newCaseOne'}}
     // expected-note@-2{{handle unknown values using "@unknown default"}}
+    // expected-note@-3 {{add missing cases}}
   case .existingCaseOne:
     return
   case .existingCaseTwo:

test/ClangImporter/enum-dataflow.swift

@@ -14,6 +14,7 @@ case .original:
 switch aliasOriginal { // expected-error {{switch must be exhaustive}}
 // expected-note@-1 {{add missing case: '.original'}}
 // expected-note@-2 {{add missing case: '.differentValue'}}
+// expected-note@-3 {{add missing cases}}
 case .bySameValue:
   break
 }

test/ClangImporter/swift2_warnings.swift

@@ -101,6 +101,7 @@ func useLowercasedEnumCase(x: NSRuncingMode) {
   switch x { // expected-error {{switch must be exhaustive}}
     // expected-note@-1 {{add missing case: '.mince'}}
     // expected-note@-2 {{add missing case: '.quince'}}
+    // expected-note@-3 {{add missing cases}}
     case .Mince: return // expected-error {{'Mince' has been renamed to 'mince'}} {{11-16=mince}}
     case .Quince: return // expected-error {{'Quince' has been renamed to 'quince'}} {{11-17=quince}}
   }

test/Compatibility/exhaustive_switch.swift

@@ -368,6 +368,7 @@ func checkDiagnosticMinimality(x: Runcible?) {
   // expected-note@-1 {{add missing case: '(.fork, _)'}}
   // expected-note@-2 {{add missing case: '(.hat, .hat)'}}
   // expected-note@-3 {{add missing case: '(_, .fork)'}}
+  // expected-note@-4 {{add missing cases}} {{+11:3-3=case (.fork, _):\n<#code#>\ncase (.hat, .hat):\n<#code#>\ncase (_, .fork):\n<#code#>\n}}
   case (.spoon, .spoon):
     break
   case (.spoon, .hat):
@@ -381,6 +382,7 @@ func checkDiagnosticMinimality(x: Runcible?) {
   // expected-note@-2 {{add missing case: '(.hat, .spoon)'}}
   // expected-note@-3 {{add missing case: '(.spoon, .hat)'}}
   // expected-note@-4 {{add missing case: '(_, .fork)'}}
+  // expected-note@-5 {{add missing cases}} {{+10:3-3=case (.fork, _):\n<#code#>\ncase (.hat, .spoon):\n<#code#>\ncase (.spoon, .hat):\n<#code#>\ncase (_, .fork):\n<#code#>\n}}
   case (.spoon, .spoon):
     break
   case (.hat, .hat):
@@ -405,6 +407,7 @@ enum LargeSpaceEnum {
 func notQuiteBigEnough() -> Bool {
   switch (LargeSpaceEnum.case1, LargeSpaceEnum.case2) { // expected-error {{switch must be exhaustive}}
   // expected-note@-1 110 {{add missing case:}}
+  // expected-note@-2 {{add missing cases}}
   case (.case0, .case0): return true
   case (.case1, .case1): return true
   case (.case2, .case2): return true
@@ -443,6 +446,7 @@ enum ContainsOverlyLargeEnum {
 func quiteBigEnough() -> Bool {
   switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-error {{switch must be exhaustive}}
   // expected-note@-1 132 {{add missing case:}}
+  // expected-note@-2 {{add missing cases}}
   case (.case0, .case0): return true
   case (.case1, .case1): return true
   case (.case2, .case2): return true
@@ -543,7 +547,7 @@ func quiteBigEnough() -> Bool {
   }
 
   // Make sure we haven't just stopped emitting diagnostics.
-  switch OverlyLargeSpaceEnum.case1 { // expected-error {{switch must be exhaustive}} expected-note 12 {{add missing case}}
+  switch OverlyLargeSpaceEnum.case1 { // expected-error {{switch must be exhaustive}} expected-note 12 {{add missing case}} expected-note {{add missing cases}}
   }
 }
 
@@ -564,12 +568,14 @@ indirect enum MutuallyRecursive {
 func infinitelySized() -> Bool {
   switch (InfinitelySized.one, InfinitelySized.one) { // expected-error {{switch must be exhaustive}}
   // expected-note@-1 8 {{add missing case:}}
+  // expected-note@-2 {{add missing cases}}
   case (.one, .one): return true
   case (.two, .two): return true
   }
 
   switch (MutuallyRecursive.one, MutuallyRecursive.one) { // expected-error {{switch must be exhaustive}}
   // expected-note@-1 8 {{add missing case:}}
+  // expected-note@-2 {{add missing cases}}
   case (.one, .one): return true
   case (.two, .two): return true
   }
@@ -840,7 +846,7 @@ public enum NonExhaustivePayload {
 // case.
 @inlinable
 public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePayload, for interval: TemporalProxy, flag: Bool) {
-  switch value { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b'}} {{none}}
+  switch value { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b'}} {{+2:3-3=case .b:\n<#code#>\n}} 
   case .a: break
   }
 
@@ -861,12 +867,16 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
   @unknown case _: break // no-warning
   }
 
-  switch value { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b'}} {{none}}
+  switch value { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b'}} {{+2:3-3=case .b:\n<#code#>\n}} 
   case .a: break
   @unknown case _: break
   }
 
-  switch value { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.a'}} {{none}} expected-note {{add missing case: '.b'}} {{none}}
+  switch value { 
+  // expected-warning@-1 {{switch must be exhaustive}} {{none}}
+  // expected-note@-2 {{add missing case: '.a'}} {{+5:3-3=case .a:\n<#code#>\n}} 
+  // expected-note@-3 {{add missing case: '.b'}} {{+5:3-3=case .b:\n<#code#>\n}} 
+  // expected-note@-4 {{add missing cases}} {{+5:3-3=case .a:\n<#code#>\ncase .b:\n<#code#>\n}}
   @unknown case _: break
   }
 
@@ -932,7 +942,7 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
   } // no-warning
 
   // Test payloaded enums.
-  switch payload { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(_)'}} {{none}}
+  switch payload { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(_)'}} {{+2:3-3=case .b(_):\n<#code#>\n}} 
   case .a: break
   }
 
@@ -953,17 +963,17 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
   @unknown case _: break // no-warning
   }
 
-  switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(_)'}} {{none}}
+  switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(_)'}} {{+2:3-3=case .b(_):\n<#code#>\n}} 
   case .a: break
   @unknown case _: break
   }
 
-  switch payload { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(true)'}} {{none}}
+  switch payload { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(true)'}} {{+3:3-3=case .b(true):\n<#code#>\n}} 
   case .a: break
   case .b(false): break
   }
 
-  switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(true)'}} {{none}}
+  switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(true)'}} {{+3:3-3=case .b(true):\n<#code#>\n}} 
   case .a: break
   case .b(false): break
   @unknown case _: break
@@ -1017,12 +1027,15 @@ public func testNonExhaustiveWithinModule(_ value: NonExhaustive, _ payload: Non
   @unknown case _: break // no-warning
   }
 
-  switch value { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b'}} {{none}}
+  switch value { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b'}} {{+2:3-3=case .b:\n<#code#>\n}} 
   case .a: break
   @unknown case _: break
   }
 
-  switch value { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.a'}} {{none}} expected-note {{add missing case: '.b'}} {{none}}
+  switch value { // expected-warning {{switch must be exhaustive}} {{none}} 
+  // expected-note@-1 {{add missing case: '.a'}} {{+4:3-3=case .a:\n<#code#>\n}} 
+  // expected-note@-2 {{add missing case: '.b'}} {{+4:3-3=case .b:\n<#code#>\n}} 
+  // expected-note@-3 {{add missing cases}} {{+4:3-3=case .a:\n<#code#>\ncase .b:\n<#code#>\n}}
   @unknown case _: break
   }
 
@@ -1071,7 +1084,7 @@ public func testNonExhaustiveWithinModule(_ value: NonExhaustive, _ payload: Non
   }
 
   // Test payloaded enums.
-  switch payload { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(_)'}} {{none}}
+  switch payload { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(_)'}} {{+2:3-3=case .b(_):\n<#code#>\n}}
   case .a: break
   }
 
@@ -1092,17 +1105,17 @@ public func testNonExhaustiveWithinModule(_ value: NonExhaustive, _ payload: Non
   @unknown case _: break // no-warning
   }
 
-  switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(_)'}} {{none}}
+  switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(_)'}} {{+2:3-3=case .b(_):\n<#code#>\n}}
   case .a: break
   @unknown case _: break
   }
 
-  switch payload { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(true)'}} {{none}}
+  switch payload { // expected-error {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(true)'}} {{+3:3-3=case .b(true):\n<#code#>\n}}
   case .a: break
   case .b(false): break
   }
 
-  switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(true)'}} {{none}}
+  switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.b(true)'}} {{+3:3-3=case .b(true):\n<#code#>\n}}
   case .a: break
   case .b(false): break
   @unknown case _: break

test/FixCode/fixits-empty-switch-multifile.swift

@@ -1,7 +1,11 @@
-// RUN: not %swift -typecheck -target %target-triple %s %S/Inputs/fixits-enum-multifile.swift -emit-fixits-path %t.remap -I %S/Inputs -diagnostics-editor-mode
-// RUN: c-arcmt-test %t.remap | arcmt-test -verify-transformed-files %s.result
+// RUN: %target-typecheck-verify-swift %s %S/Inputs/fixits-enum-multifile.swift -I %S/Inputs
 
 func foo1(_ e: EMulti) {
   switch e {
+// expected-error@-1 {{switch must be exhaustive}}
+// expected-note@-2 {{add missing case: '.e1'}} {{+6:3-3=case .e1:\n<#code#>\n}}
+// expected-note@-3 {{add missing case: '.e2'}} {{+6:3-3=case .e2:\n<#code#>\n}}
+// expected-note@-4 {{add missing case: '.e3(_)'}} {{+6:3-3=case .e3(_):\n<#code#>\n}}
+// expected-note@-5 {{add missing cases}} {{+6:3-3=case .e1:\n<#code#>\ncase .e2:\n<#code#>\ncase .e3(_):\n<#code#>\n}}
   }
 }

test/FixCode/fixits-empty-switch-multifile.swift.result

@@ -1,5 +1,4 @@
-// RUN: not %swift -typecheck -target %target-triple %s -emit-fixits-path %t.remap -I %S/Inputs -diagnostics-editor-mode
-// RUN: c-arcmt-test %t.remap | arcmt-test -verify-transformed-files %s.result
+// RUN: %target-typecheck-verify-swift %s -I %S/Inputs -diagnostics-editor-mode
 
 enum E1 {
   case e1
@@ -9,10 +8,15 @@ enum E1 {
 
 func foo1(_ e: E1) {
   switch e {
+//expected-error@-1 {{switch must be exhaustive}}
+//expected-note@-2 {{add missing case: '.e1'}} {{+6:3-3=case .e1:\n<#code#>\n}}
+//expected-note@-3 {{add missing case: '.e2'}} {{+6:3-3=case .e2:\n<#code#>\n}}
+//expected-note@-4 {{add missing case: '.e3'}} {{+6:3-3=case .e3:\n<#code#>\n}}
+//expected-note@-5 {{add missing cases}} {{+6:3-3=case .e1:\n<#code#>\ncase .e2:\n<#code#>\ncase .e3:\n<#code#>\n}}
   }
 }
 
 func foo1 (_ i : Int) {
-  switch i {
+  switch i { // expected-error {{'switch' statement body must have at least one 'case' or 'default' block; add a default case}} {{+1:3-3=default:\n<#code#>\n}} 
   }
 }