Merge pull request #21708 from jrose-apple/5.0-into-the-unknown

[5.0] Special-case diagnostic for when you just need `@unknown default`

Author: AnnaZaks

include/swift/AST/DiagnosticsSema.def

@@ -4276,6 +4276,9 @@ NOTE(redundant_particular_literal_case_here,none,
      "first occurrence of identical literal pattern is here", ())
 
 WARNING(non_exhaustive_switch_warn,none, "switch must be exhaustive", ())
+WARNING(non_exhaustive_switch_unknown_only,none,
+        "switch covers known cases, but %0 may have additional unknown values"
+        "%select{|, possibly added in future versions}1", (Type, bool))
 
 WARNING(override_nsobject_hashvalue,none,
         "override of 'NSObject.hashValue' is deprecated; "

lib/Sema/TypeCheckSwitchStmt.cpp

@@ -1150,7 +1150,8 @@ namespace {
       bool InEditor = TC.Context.LangOpts.DiagnosticsEditorMode;
 
       // Decide whether we want an error or a warning.
-      auto mainDiagType = diag::non_exhaustive_switch;
+      Optional<decltype(diag::non_exhaustive_switch)> mainDiagType =
+          diag::non_exhaustive_switch;
       if (unknownCase) {
         switch (defaultReason) {
         case RequiresDefault::EmptySwitchBody:
@@ -1177,16 +1178,25 @@ namespace {
       switch (uncovered.checkDowngradeToWarning()) {
       case DowngradeToWarning::No:
         break;
-      case DowngradeToWarning::ForUnknownCase:
+      case DowngradeToWarning::ForUnknownCase: {
         if (TC.Context.LangOpts.DebuggerSupport ||
             TC.Context.LangOpts.Playground ||
             !TC.getLangOpts().EnableNonFrozenEnumExhaustivityDiagnostics) {
           // Don't require covering unknown cases in the debugger or in
           // playgrounds.
           return;
         }
-        // Missing '@unknown' is just a warning.
-        mainDiagType = diag::non_exhaustive_switch_warn;
+        assert(defaultReason == RequiresDefault::No);
+        Type subjectType = Switch->getSubjectExpr()->getType();
+        bool shouldIncludeFutureVersionComment = false;
+        if (auto *theEnum = subjectType->getEnumOrBoundGenericEnum()) {
+          shouldIncludeFutureVersionComment =
+              theEnum->getParentModule()->isSystemModule();
+        }
+        TC.diagnose(startLoc, diag::non_exhaustive_switch_unknown_only,
+                    subjectType, shouldIncludeFutureVersionComment);
+        mainDiagType = None;
+      }
         break;
       }
 
@@ -1201,7 +1211,7 @@ namespace {
         return;
       case RequiresDefault::UncoveredSwitch: {
         OS << tok::kw_default << ":\n" << placeholder << "\n";
-        TC.diagnose(startLoc, mainDiagType);
+        TC.diagnose(startLoc, mainDiagType.getValue());
         TC.diagnose(startLoc, diag::missing_several_cases, /*default*/true)
           .fixItInsert(insertLoc, buffer.str());
       }
@@ -1218,7 +1228,9 @@ namespace {
       // If there's nothing else to diagnose, bail.
       if (uncovered.isEmpty()) return;
 
-      TC.diagnose(startLoc, mainDiagType);
+      // Check if we still have to emit the main diganostic.
+      if (mainDiagType.hasValue())
+        TC.diagnose(startLoc, mainDiagType.getValue());
 
       // Add notes to explain what's missing.
       auto processUncoveredSpaces =

test/ClangImporter/enum-error.swift

@@ -87,11 +87,9 @@ func testError() {
 // CHECK: sil_witness_table shared [serialized] ExhaustiveError: _BridgedStoredNSError module __ObjC
   let terr = getErr()
   switch (terr) { case .TENone, .TEOne, .TETwo: break }
-  // EXHAUSTIVE: [[@LINE-1]]:{{.+}}: warning: switch must be exhaustive
+  // EXHAUSTIVE: [[@LINE-1]]:{{.+}}: warning: switch covers known cases, but 'TestError.Code' may have additional unknown values
   // EXHAUSTIVE: [[@LINE-2]]:{{.+}}: note: handle unknown values using "@unknown default"
 
-  // FIXME: This should still be an error because there are /known/ cases that
-  // aren't covered.
   switch (terr) { case .TENone, .TEOne: break }
   // EXHAUSTIVE: [[@LINE-1]]:{{.+}}: error: switch must be exhaustive
   // EXHAUSTIVE: [[@LINE-2]]:{{.+}}: note: add missing case: '.TETwo'

test/ClangImporter/enum-exhaustivity-system.swift

@@ -0,0 +1,50 @@
+// RUN: %target-swift-frontend -typecheck %s -Xcc -isystem -Xcc %S/Inputs/custom-modules -verify -enable-nonfrozen-enum-exhaustivity-diagnostics
+
+import EnumExhaustivity
+
+func test(_ value: RegularEnum, _ exhaustiveValue: ExhaustiveEnum) {
+  switch value { // expected-warning {{switch covers known cases, but 'RegularEnum' may have additional unknown values, possibly added in future versions}} expected-note {{handle unknown values using "@unknown default"}}
+  case .A: break
+  case .B: break
+  }
+
+  switch exhaustiveValue { // always okay
+  case .A: break
+  case .B: break
+  }
+}
+
+func testAttributes(
+  _ rete: RegularEnumTurnedExhaustive,
+  _ arete: AnotherRegularEnumTurnedExhaustive,
+  _ retetb: RegularEnumTurnedExhaustiveThenBackViaAPINotes,
+  _ fdte: ForwardDeclaredTurnedExhaustive,
+  _ fdo: ForwardDeclaredOnly
+) {
+  switch rete {
+  case .A, .B: break
+  }
+
+  switch arete {
+  case .A, .B: break
+  }
+
+  switch retetb { // expected-warning {{switch covers known cases, but 'RegularEnumTurnedExhaustiveThenBackViaAPINotes' may have additional unknown values, possibly added in future versions}} expected-note {{handle unknown values using "@unknown default"}}
+  case .A, .B: break
+  }
+
+  switch fdte {
+  case .A, .B: break
+  }
+
+  switch fdo {
+  case .A, .B: break
+  }
+}
+
+func testUnavailableCases(_ value: UnavailableCases) {
+  switch value { // okay
+  case .A: break
+  case .B: break
+  }
+}

test/ClangImporter/enum-exhaustivity.swift

@@ -17,7 +17,7 @@
 import EnumExhaustivity
 
 func test(_ value: RegularEnum, _ exhaustiveValue: ExhaustiveEnum) {
-  switch value { // expected-error {{switch must be exhaustive}} expected-note {{handle unknown values using "@unknown default"}}
+  switch value { // expected-error {{switch covers known cases, but 'RegularEnum' may have additional unknown values}} expected-note {{handle unknown values using "@unknown default"}}
   case .A: break
   case .B: break
   }
@@ -43,7 +43,7 @@ func testAttributes(
   case .A, .B: break
   }
 
-  switch retetb { // expected-error {{switch must be exhaustive}} expected-note {{handle unknown values using "@unknown default"}}
+  switch retetb { // expected-error {{switch covers known cases, but 'RegularEnumTurnedExhaustiveThenBackViaAPINotes' may have additional unknown values}} expected-note {{handle unknown values using "@unknown default"}}
   case .A, .B: break
   }
 

test/ClangImporter/enum-inferred-exhaustivity.swift

@@ -6,14 +6,14 @@
 
 func test(_ value: EnumWithDefaultExhaustivity) {
   // We want to assume such enums are non-frozen.
-  switch value { // expected-error {{switch must be exhaustive}} expected-note {{handle unknown values using "@unknown default"}}
+  switch value { // expected-error {{switch covers known cases, but 'EnumWithDefaultExhaustivity' may have additional unknown values}} expected-note {{handle unknown values using "@unknown default"}}
   case .loneCase: break
   }
 }
 
 func test(_ value: EnumWithSpecialAttributes) {
   // Same, but with the attributes macro shipped in the Xcode 9 SDKs.
-  switch value { // expected-error {{switch must be exhaustive}} expected-note {{handle unknown values using "@unknown default"}}
+  switch value { // expected-error {{switch covers known cases, but 'EnumWithSpecialAttributes' may have additional unknown values}} expected-note {{handle unknown values using "@unknown default"}}
   case .loneCase: break
   }
 }

test/ClangImporter/enum-new.swift

@@ -5,11 +5,11 @@ _ = .Red as Color
 _ = .Cyan as MoreColor
 
 func test() {
-  switch getColor() { // expected-warning {{switch must be exhaustive}} expected-note{{handle unknown values using "@unknown default"}}
+  switch getColor() { // expected-warning {{switch covers known cases, but 'Color' may have additional unknown values}} expected-note{{handle unknown values using "@unknown default"}}
   case .Red, .Blue, .Green: break
   }
 
-  switch getMoreColor() { // expected-warning {{switch must be exhaustive}} expected-note{{handle unknown values using "@unknown default"}}
+  switch getMoreColor() { // expected-warning {{switch covers known cases, but 'MoreColor' may have additional unknown values}} expected-note{{handle unknown values using "@unknown default"}}
   case .Yellow, .Magenta, .Black, .Cyan: break
   }
 

test/ClangImporter/enum-objc.swift

@@ -3,7 +3,7 @@
 // REQUIRES: objc_interop
 
 func test(_ value: SwiftEnum, _ exhaustiveValue: ExhaustiveEnum) {
-  switch value { // expected-warning {{switch must be exhaustive}} expected-note {{handle unknown values using "@unknown default"}}
+  switch value { // expected-warning {{switch covers known cases, but 'SwiftEnum' may have additional unknown values}} expected-note {{handle unknown values using "@unknown default"}}
   case .one: break
   case .two: break
   case .three: break

test/Sema/exhaustive_switch.swift

@@ -883,7 +883,7 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
   case .a: break
   }
 
-  switch value { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{handle unknown values using "@unknown default"}} {{3-3=@unknown default:\n<#fatalError#>()\n}}
+  switch value { // expected-warning {{switch covers known cases, but 'NonExhaustive' may have additional unknown values}} {{none}} expected-note {{handle unknown values using "@unknown default"}} {{3-3=@unknown default:\n<#fatalError#>()\n}}
   case .a: break
   case .b: break
   }
@@ -915,7 +915,7 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
   }
 
   // Test being part of other spaces.
-  switch value as Optional { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '.some(_)'}}
+  switch value as Optional { // expected-warning {{switch covers known cases, but 'Optional<NonExhaustive>' may have additional unknown values}} {{none}} expected-note {{add missing case: '.some(_)'}}
   case .a?: break
   case .b?: break
   case nil: break
@@ -933,7 +933,7 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
   case nil: break
   } // no-warning
 
-  switch (value, flag) { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '(_, false)'}}
+  switch (value, flag) { // expected-warning {{switch covers known cases, but '(NonExhaustive, Bool)' may have additional unknown values}} {{none}} expected-note {{add missing case: '(_, false)'}}
   case (.a, _): break
   case (.b, false): break
   case (_, true): break
@@ -946,7 +946,7 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
   @unknown case _: break
   } // no-warning
 
-  switch (flag, value) { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '(false, _)'}}
+  switch (flag, value) { // expected-warning {{switch covers known cases, but '(Bool, NonExhaustive)' may have additional unknown values}} {{none}} expected-note {{add missing case: '(false, _)'}}
   case (_, .a): break
   case (false, .b): break
   case (true, _): break
@@ -959,7 +959,7 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
   @unknown case _: break
   } // no-warning
 
-  switch (value, value) { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{add missing case: '(_, _)'}}
+  switch (value, value) { // expected-warning {{switch covers known cases, but '(NonExhaustive, NonExhaustive)' may have additional unknown values}} {{none}} expected-note {{add missing case: '(_, _)'}}
   case (.a, _), (_, .a): break
   case (.b, _), (_, .b): break
   }
@@ -975,7 +975,7 @@ public func testNonExhaustive(_ value: NonExhaustive, _ payload: NonExhaustivePa
   case .a: break
   }
 
-  switch payload { // expected-warning {{switch must be exhaustive}} {{none}} expected-note {{handle unknown values using "@unknown default"}} {{3-3=@unknown default:\n<#fatalError#>()\n}}
+  switch payload { // expected-warning {{switch covers known cases, but 'NonExhaustivePayload' may have additional unknown values}} {{none}} expected-note {{handle unknown values using "@unknown default"}} {{3-3=@unknown default:\n<#fatalError#>()\n}}
   case .a: break
   case .b: break
   }

test/Sema/exhaustive_switch_testable.swift

@@ -17,7 +17,7 @@ func testFrozen(_ e: FrozenEnum) -> Int {
 }
 
 func testNonFrozen(_ e: NonFrozenEnum) -> Int {
-  // VERIFY-NON-FROZEN: exhaustive_switch_testable.swift:[[@LINE+1]]:{{[0-9]+}}: warning: switch must be exhaustive
+  // VERIFY-NON-FROZEN: exhaustive_switch_testable.swift:[[@LINE+1]]:{{[0-9]+}}: warning: switch covers known cases, but 'NonFrozenEnum' may have additional unknown values
   switch e {
   case .a: return 1
   case .b, .c: return 2