[Typechecker] Extended ambiguous none warning to cases as well (#29356)

Author: theblixguy

include/swift/AST/DiagnosticsSema.def

@@ -1583,8 +1583,12 @@ WARNING(optional_ambiguous_case_ref,none,
         (StringRef, StringRef, StringRef))
 NOTE(optional_fixit_ambiguous_case_ref,none,
      "explicitly specify 'Optional' to silence this warning", ())
+NOTE(optional_fixit_ambiguous_case_ref_switch,none,
+     "use 'nil' to silence this warning", ())
 NOTE(type_fixit_optional_ambiguous_case_ref,none,
      "use '%0.%1' instead", (StringRef, StringRef))
+NOTE(type_fixit_optional_ambiguous_case_ref_switch,none,
+     "use '%0' instead", (StringRef))
 
 ERROR(nscoding_unstable_mangled_name,none,
       "%select{private|fileprivate|nested|local}0 class %1 has an "

lib/Sema/TypeCheckPattern.cpp

@@ -1340,6 +1340,43 @@ Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern,
       if (!elt)
         return nullptr;
 
+      // Emit an ambiguous none diagnostic if:
+      // 1) We have an Optional<T> type.
+      // 2) We're matching a 'none' enum case.
+      // 3) The 'none' enum case exists in T too.
+      if (EEP->getName().isSimpleName("none") &&
+          type->getOptionalObjectType()) {
+        SmallVector<Type, 4> allOptionals;
+        auto baseTyUnwrapped = type->lookThroughAllOptionalTypes(allOptionals);
+        if (lookupEnumMemberElement(dc, baseTyUnwrapped, EEP->getName(),
+                                    EEP->getLoc())) {
+          auto baseTyName = type->getCanonicalType().getString();
+          auto baseTyUnwrappedName = baseTyUnwrapped->getString();
+          diags.diagnoseWithNotes(
+              diags.diagnose(EEP->getLoc(), diag::optional_ambiguous_case_ref,
+                             baseTyName, baseTyUnwrappedName, "none"),
+              [&]() {
+                // Emit a note to swap '.none' with 'nil' to match with
+                // the 'none' case in Optional<T>.
+                diags.diagnose(EEP->getLoc(),
+                              diag::optional_fixit_ambiguous_case_ref_switch)
+                    .fixItReplace(EEP->getSourceRange(), "nil");
+                // Emit a note to swap '.none' with 'none?' to match with the
+                // 'none' case in T. Add as many '?' as needed to look though
+                // all the optionals.
+                std::string fixItString = "none";
+                llvm::for_each(allOptionals,
+                               [&](const Type) { fixItString += "?"; });
+                diags.diagnose(
+                        EEP->getLoc(),
+                        diag::type_fixit_optional_ambiguous_case_ref_switch,
+                        fixItString)
+                    .fixItReplace(EEP->getNameLoc().getSourceRange(),
+                                  fixItString);
+              });
+        }
+      }
+
       enumTy = type;
     } else {
       // Check if the explicitly-written enum type matches the type we're

test/decl/enum/enumtest.swift

@@ -554,3 +554,65 @@ let _: EnumWithStructNone? = .none // Okay
 let _: EnumWithTypealiasNone? = .none // Okay
 let _: EnumWithBothStructAndComputedNone? = .none // Okay
 let _: EnumWithBothTypealiasAndComputedNone? = .none // Okay
+
+// SR-12063
+
+let foo1: Foo? = Foo.none
+let foo2: Foo?? = Foo.none
+
+switch foo1 {
+  case .none: break 
+  // expected-warning@-1 {{assuming you mean 'Optional<Foo>.none'; did you mean 'Foo.none' instead?}}
+  // expected-note@-2 {{use 'nil' to silence this warning}}{{8-13=nil}}
+  // expected-note@-3 {{use 'none?' instead}}{{9-13=none?}}
+  case .bar: break
+  default: break
+}
+
+switch foo2 {
+  case .none: break 
+  // expected-warning@-1 {{assuming you mean 'Optional<Optional<Foo>>.none'; did you mean 'Foo.none' instead?}}
+  // expected-note@-2 {{use 'nil' to silence this warning}}{{8-13=nil}}
+  // expected-note@-3 {{use 'none??' instead}}{{9-13=none??}}
+  case .bar: break
+  default: break
+}
+
+if case .none = foo1 {}
+// expected-warning@-1 {{assuming you mean 'Optional<Foo>.none'; did you mean 'Foo.none' instead?}}
+// expected-note@-2 {{use 'nil' to silence this warning}}{{9-14=nil}}
+// expected-note@-3 {{use 'none?' instead}}{{10-14=none?}}
+
+if case .none = foo2 {}
+// expected-warning@-1 {{assuming you mean 'Optional<Optional<Foo>>.none'; did you mean 'Foo.none' instead?}}
+// expected-note@-2 {{use 'nil' to silence this warning}}{{9-14=nil}}
+// expected-note@-3 {{use 'none??' instead}}{{10-14=none??}}
+
+switch foo1 {
+  case nil: break // Okay
+  case .bar: break
+  default: break
+}
+
+switch foo1 {
+  case .none?: break // Okay
+  case .bar: break
+  default: break
+}
+
+switch foo2 {
+  case nil: break // Okay
+  case .bar: break
+  default: break
+}
+
+switch foo2 {
+  case .none??: break // Okay
+  case .bar: break
+  default: break
+}
+
+if case nil = foo1 {} // Okay
+if case .none? = foo1 {} // Okay
+if case nil = foo2 {} // Okay
+if case .none?? = foo2 {} // Okay