[Sema][NFC] Correcting test case for SR-11535 and adding an explanation comment

include/swift/Sema/CSFix.h

@@ -46,6 +46,7 @@ class ConstraintLocator;
 class ConstraintLocatorBuilder;
 enum class ConversionRestrictionKind;
 class Solution;
+struct MemberLookupResult;
 
 /// Describes the kind of fix to apply to the given constraint before
 /// visiting it.
@@ -289,6 +290,10 @@ enum class FixKind : uint8_t {
   /// Treat empty and single-element array literals as if they were incomplete
   /// dictionary literals when used as such.
   TreatArrayLiteralAsDictionary,
+
+  /// Explicitly specify the type to disambiguate between possible member base
+  /// types.
+  SpecifyBaseTypeForOptionalUnresolvedMember,
 };
 
 class ConstraintFix {
@@ -2143,6 +2148,34 @@ class AllowRefToInvalidDecl final : public ConstraintFix {
                                        ConstraintLocator *locator);
 };
 
+/// Diagnose if the base type is optional, we're referring to a nominal
+/// type member via the dot syntax and the member name matches
+/// Optional<T>.{member} or a .none member inferred as non-optional static
+/// member e.g. let _ : Foo? = .none where Foo has a static member none.
+class SpecifyBaseTypeForOptionalUnresolvedMember final : public ConstraintFix {
+  SpecifyBaseTypeForOptionalUnresolvedMember(ConstraintSystem &cs,
+                                             DeclNameRef memberName,
+                                             ConstraintLocator *locator)
+      : ConstraintFix(cs, FixKind::SpecifyBaseTypeForOptionalUnresolvedMember,
+                      locator, /*isWarning=*/true),
+        MemberName(memberName) {}
+  DeclNameRef MemberName;
+
+public:
+  std::string getName() const override {
+    const auto name = MemberName.getBaseName();
+    return "specify unresolved member optional base type explicitly '" +
+           name.userFacingName().str() + "'";
+  }
+
+  bool diagnose(const Solution &solution, bool asNote = false) const override;
+
+  static SpecifyBaseTypeForOptionalUnresolvedMember *
+  attempt(ConstraintSystem &cs, ConstraintKind kind, Type baseTy,
+          DeclNameRef memberName, FunctionRefKind functionRefKind,
+          MemberLookupResult result, ConstraintLocator *locator);
+};
+
 } // end namespace constraints
 } // end namespace swift
 

lib/Sema/CSApply.cpp

@@ -2862,105 +2862,9 @@ namespace {
       if (!result)
         return nullptr;
 
-      // Check for ambiguous member if the base is an Optional
-      if (baseTy->getOptionalObjectType()) {
-        diagnoseAmbiguousNominalMember(baseTy, result);
-      }
-
       return coerceToType(result, resultTy, cs.getConstraintLocator(expr));
     }
-
-    /// Diagnose if the base type is optional, we're referring to a nominal
-    /// type member via the dot syntax and the member name matches
-    /// Optional<T>.{member name}
-    void diagnoseAmbiguousNominalMember(Type baseTy, Expr *result) {
-      if (auto baseTyUnwrapped = baseTy->lookThroughAllOptionalTypes()) {
-        // Return if the base type doesn't have a nominal type decl
-        if (!baseTyUnwrapped->getNominalOrBoundGenericNominal()) {
-          return;
-        }
-
-        // Otherwise, continue digging
-        if (auto DSCE = dyn_cast<DotSyntaxCallExpr>(result)) {
-          auto calledValue = DSCE->getCalledValue();
-          auto isOptional = false;
-          Identifier memberName;
-
-          // Try cast the assigned value to an enum case
-          //
-          // This will always succeed if the base is Optional<T> & the
-          // assigned case comes from Optional<T>
-          if (auto EED = dyn_cast<EnumElementDecl>(calledValue)) {
-            isOptional = EED->getParentEnum()->isOptionalDecl();
-            memberName = EED->getBaseIdentifier();
-          }
-
-          // Return if the enum case doesn't come from Optional<T>
-          if (!isOptional) {
-            return;
-          }
-
-          // Look up the enum case in the unwrapped type to check if it exists
-          // as a member
-          auto baseTyNominalDecl = baseTyUnwrapped
-                                   ->getNominalOrBoundGenericNominal();
-          auto results = TypeChecker::lookupMember(
-              baseTyNominalDecl->getModuleContext(), baseTyUnwrapped,
-              DeclNameRef(memberName), defaultMemberLookupOptions);
-
-          // Filter out any functions, instance members, enum cases with
-          // associated values or variables whose type does not match the
-          // contextual type.
-          results.filter([&](const LookupResultEntry entry, bool isOuter) {
-            if (auto member = entry.getValueDecl()) {
-              if (isa<FuncDecl>(member))
-                return false;
-              if (member->isInstanceMember())
-                return false;
-              if (auto EED = dyn_cast<EnumElementDecl>(member)) {
-                return !EED->hasAssociatedValues();
-              }
-              if (auto VD = dyn_cast<VarDecl>(member)) {
-                auto baseType = DSCE->getType()->lookThroughAllOptionalTypes();
-                return VD->getInterfaceType()->isEqual(baseType);
-              }
-            }
-
-            // Filter out anything that's not one of the above. We don't care
-            // if we have a typealias named 'none' or a struct/class named
-            // 'none'.
-            return false;
-          });
-
-          if (results.empty()) {
-            return;
-          }
 
-          auto &de = cs.getASTContext().Diags;
-          if (auto member = results.front().getValueDecl()) {
-            // Emit a diagnostic with some fix-its
-            auto baseTyName = baseTy->getCanonicalType().getString();
-            auto baseTyUnwrappedName = baseTyUnwrapped->getString();
-            auto loc = DSCE->getLoc();
-            auto startLoc = DSCE->getStartLoc();
-            de.diagnoseWithNotes(
-                de.diagnose(loc, swift::diag::optional_ambiguous_case_ref,
-                            baseTyName, baseTyUnwrappedName, memberName.str()),
-                [&]() {
-                  de.diagnose(loc,
-                              swift::diag::optional_fixit_ambiguous_case_ref)
-                      .fixItInsert(startLoc, "Optional");
-                  de.diagnose(
-                        loc,
-                        swift::diag::type_fixit_optional_ambiguous_case_ref,
-                        baseTyUnwrappedName, memberName.str())
-                      .fixItInsert(startLoc, baseTyUnwrappedName);
-                });
-          }
-        }
-      }
-    }
-
   private:
     /// A list of "suspicious" optional injections that come from
     /// forced downcasts.

lib/Sema/CSDiagnostics.cpp

@@ -7095,3 +7095,55 @@ bool InvalidReturnInResultBuilderBody::diagnoseAsError() {
 
   return true;
 }
+
+bool MemberMissingExplicitBaseTypeFailure::diagnoseAsError() {
+  auto UME = castToExpr<UnresolvedMemberExpr>(getAnchor());
+  auto memberName = UME->getName().getBaseIdentifier().str();
+  auto &DE = getASTContext().Diags;
+  auto &solution = getSolution();
+
+  auto selected = solution.getOverloadChoice(getLocator());
+  auto baseType =
+      resolveType(selected.choice.getBaseType()->getMetatypeInstanceType());
+
+  SmallVector<Type, 4> optionals;
+  auto baseTyUnwrapped = baseType->lookThroughAllOptionalTypes(optionals);
+
+  if (!optionals.empty()) {
+    auto baseTyName = baseType->getCanonicalType().getString();
+    auto baseTyUnwrappedName = baseTyUnwrapped->getString();
+    auto loc = UME->getLoc();
+    auto startLoc = UME->getStartLoc();
+
+    DE.diagnoseWithNotes(
+        DE.diagnose(loc, diag::optional_ambiguous_case_ref, baseTyName,
+                    baseTyUnwrappedName, memberName),
+        [&]() {
+          DE.diagnose(UME->getDotLoc(), diag::optional_fixit_ambiguous_case_ref)
+              .fixItInsert(startLoc, "Optional");
+          DE.diagnose(UME->getDotLoc(),
+                      diag::type_fixit_optional_ambiguous_case_ref,
+                      baseTyUnwrappedName, memberName)
+              .fixItInsert(startLoc, baseTyUnwrappedName);
+        });
+  } else {
+    auto baseTypeName = baseType->getCanonicalType().getString();
+    auto baseOptionalTypeName =
+        OptionalType::get(baseType)->getCanonicalType().getString();
+
+    DE.diagnoseWithNotes(
+        DE.diagnose(UME->getLoc(), diag::optional_ambiguous_case_ref,
+                    baseTypeName, baseOptionalTypeName, memberName),
+        [&]() {
+          DE.diagnose(UME->getDotLoc(),
+                      diag::type_fixit_optional_ambiguous_case_ref,
+                      baseOptionalTypeName, memberName)
+              .fixItInsert(UME->getDotLoc(), baseOptionalTypeName);
+          DE.diagnose(UME->getDotLoc(),
+                      diag::type_fixit_optional_ambiguous_case_ref,
+                      baseTypeName, memberName)
+              .fixItInsert(UME->getDotLoc(), baseTypeName);
+        });
+  }
+  return true;
+}

lib/Sema/CSDiagnostics.h

@@ -2352,6 +2352,35 @@ class InvalidReturnInResultBuilderBody final : public FailureDiagnostic {
   bool diagnoseAsError() override;
 };
 
+/// Diagnose if the base type is optional, we're referring to a nominal
+/// type member via the dot syntax and the member name matches
+/// Optional<T>.{member_name} or an unresolved `.none` inferred as a static
+/// non-optional member  base but could be an Optional<T>.none. So we enforce
+/// explicit type annotation to avoid ambiguity.
+///
+/// \code
+///   enum Enum<T> {
+///     case bar
+///     static var none: Enum<Int> { .bar }
+///   }
+///   let _: Enum<Int>? = .none // Base inferred as Optional.none, suggest
+///   // explicit type.
+///   let _: Enum? = .none // Base inferred as static member Enum<Int>.none,
+///   // emit warning suggesting explicit type.
+///   let _: Enum = .none // Ok
+/// \endcode
+class MemberMissingExplicitBaseTypeFailure final : public FailureDiagnostic {
+  DeclNameRef Member;
+
+public:
+  MemberMissingExplicitBaseTypeFailure(const Solution &solution,
+                                       DeclNameRef member,
+                                       ConstraintLocator *locator)
+      : FailureDiagnostic(solution, locator), Member(member) {}
+
+  bool diagnoseAsError() override;
+};
+
 } // end namespace constraints
 } // end namespace swift
 

lib/Sema/CSFix.cpp

@@ -1673,3 +1673,100 @@ IgnoreResultBuilderWithReturnStmts::create(ConstraintSystem &cs, Type builderTy,
   return new (cs.getAllocator())
       IgnoreResultBuilderWithReturnStmts(cs, builderTy, locator);
 }
+
+bool SpecifyBaseTypeForOptionalUnresolvedMember::diagnose(
+    const Solution &solution, bool asNote) const {
+  MemberMissingExplicitBaseTypeFailure failure(solution, MemberName,
+                                               getLocator());
+  return failure.diagnose(asNote);
+}
+
+SpecifyBaseTypeForOptionalUnresolvedMember *
+SpecifyBaseTypeForOptionalUnresolvedMember::attempt(
+    ConstraintSystem &cs, ConstraintKind kind, Type baseTy,
+    DeclNameRef memberName, FunctionRefKind functionRefKind,
+    MemberLookupResult result, ConstraintLocator *locator) {
+
+  if (kind != ConstraintKind::UnresolvedValueMember)
+    return nullptr;
+
+  // None or only one viable candidate, there is no ambiguity.
+  if (result.ViableCandidates.size() <= 1)
+    return nullptr;
+
+  // Only diagnose those situations for static members.
+  if (!baseTy->is<MetatypeType>())
+    return nullptr;
+
+  // Don't diagnose for function members e.g. Foo? = .none(0).
+  if (functionRefKind != FunctionRefKind::Unapplied)
+    return nullptr;
+
+  Type underlyingBaseType = baseTy->getMetatypeInstanceType();
+  if (!underlyingBaseType->getNominalOrBoundGenericNominal())
+    return nullptr;
+
+  if (!underlyingBaseType->getOptionalObjectType())
+    return nullptr;
+
+  auto unwrappedType = underlyingBaseType->lookThroughAllOptionalTypes();
+  bool allOptionalBaseCandidates = true;
+  auto filterViableCandidates =
+      [&](SmallVector<OverloadChoice, 4> &candidates,
+          SmallVector<OverloadChoice, 4> &viableCandidates,
+          bool &allOptionalBase) {
+        for (OverloadChoice choice : candidates) {
+          if (!choice.isDecl())
+            continue;
+
+          auto memberDecl = choice.getDecl();
+          if (isa<FuncDecl>(memberDecl))
+            continue;
+          if (memberDecl->isInstanceMember())
+            continue;
+
+          allOptionalBase &= bool(choice.getBaseType()
+                                      ->getMetatypeInstanceType()
+                                      ->getOptionalObjectType());
+
+          if (auto EED = dyn_cast<EnumElementDecl>(memberDecl)) {
+            if (!EED->hasAssociatedValues())
+              viableCandidates.push_back(choice);
+          } else if (auto VD = dyn_cast<VarDecl>(memberDecl)) {
+            if (unwrappedType->hasTypeVariable() ||
+                VD->getInterfaceType()->isEqual(unwrappedType))
+              viableCandidates.push_back(choice);
+          }
+        }
+      };
+
+  SmallVector<OverloadChoice, 4> viableCandidates;
+  filterViableCandidates(result.ViableCandidates, viableCandidates,
+                         allOptionalBaseCandidates);
+
+  // Also none or only one viable candidate after filtering candidates, there is
+  // no ambiguity.
+  if (viableCandidates.size() <= 1)
+    return nullptr;
+
+  // Right now, name lookup only unwraps a single layer of optionality, which
+  // for cases where base type is a multi-optional type e.g. Foo?? it only
+  // finds optional base candidates. To produce the correct warning we perform
+  // an extra lookup on unwrapped type.
+  if (!allOptionalBaseCandidates)
+    return new (cs.getAllocator())
+        SpecifyBaseTypeForOptionalUnresolvedMember(cs, memberName, locator);
+
+  MemberLookupResult unwrappedResult =
+      cs.performMemberLookup(kind, memberName, MetatypeType::get(unwrappedType),
+                             functionRefKind, locator,
+                             /*includeInaccessibleMembers*/ false);
+  SmallVector<OverloadChoice, 4> unwrappedViableCandidates;
+  filterViableCandidates(unwrappedResult.ViableCandidates,
+                         unwrappedViableCandidates, allOptionalBaseCandidates);
+  if (unwrappedViableCandidates.empty())
+    return nullptr;
+
+  return new (cs.getAllocator())
+      SpecifyBaseTypeForOptionalUnresolvedMember(cs, memberName, locator);
+}