Special-case diagnostic for when you just need @unknown default (#21695)

This is a new feature of Swift 5 mode, so it deserves at least a
little bit of explanation right in the diagnostic. If you have an
otherwise-fully-covered switch but can't assume the enum is frozen,
you'll now get this message:

    switch covers known cases, but 'MusicGenre' may have additional
    unknown values

Furthermore, if the enum comes from a system header, it looks like
this:

    switch covers known cases, but 'NSMusicGenre' may have additional
    unknown values, possibly added in future versions

...to further suggest the idea that even though your switch is covered
/now/, it might not handle everything in the /future/. This extra bit
is limited to system headers to avoid showing up on C enums defined in
your own project, for which it sounds silly. (The main message is
still valid though, since you can cram whatever you want into a C
enum, and people use this pattern to implement "private cases".)

rdar://problem/39367045

Author: jrose-apple

include/swift/AST/DiagnosticsSema.def

@@ -4231,6 +4231,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

@@ -1015,7 +1015,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:
@@ -1042,16 +1043,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;
       }
 
@@ -1066,7 +1076,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());
       }
@@ -1083,7 +1093,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

@@ -802,7 +802,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
   }
@@ -834,7 +834,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
@@ -852,7 +852,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
@@ -865,7 +865,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
@@ -878,7 +878,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
   }
@@ -894,7 +894,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

test/stmt/nonexhaustive_switch_stmt_editor.swift

@@ -13,7 +13,7 @@ public func testNonExhaustive(_ value: NonExhaustive) {
   case .a: break
   }
 
-  switch value { // expected-warning {{switch must be exhaustive}}
+  switch value { // expected-warning {{switch covers known cases, but 'NonExhaustive' may have additional unknown values}}
   // expected-note@-1 {{handle unknown values using "@unknown default"}} {{3-3=@unknown default:\n<#fatalError()#>\n}}
   case .a: break
   case .b: break