[Type checker] Extend the diagnostics for unwrapping the base of a member access.

Introduce a new fix kind into the constraint solver to cover unwrapping the base
of a member access so we can refer to the a member of the unwrapped base.
Wire this fix kind to the just-added diagnostic that suggests either the
chaining ‘?’ or the force-unwrap ‘!’ via separate, descriptive Fix-Its.

Example:

error: value of optional type 'X?' must be unwrapped to refer to member 'f' of wrapped base type 'X'
  let _: Int = x.f()
               ^
note: chain the optional using '?' to access member 'f' only for non-'nil' base values
  let _: Int = x.f()
               ^
                ?
note: force-unwrap using '!' to abort execution if the optional value contains 'nil'
  let _: Int = x.f()
               ^
                !

Before this, we would sometimes get a Fix-It for just ‘?’ and sometimes get a Fix-It for the
coalescing ‘??’, neither of which is likely to be right.

More work on rdar://problem/42081852.

(cherry picked from commit e7eac0a)

Author: DougGregor

lib/Sema/CSApply.cpp

@@ -8107,7 +8107,7 @@ bool ConstraintSystem::applySolutionFix(Expr *expr,
       Type unwrappedType = type->getOptionalObjectType();
       if (!unwrappedType)
         return false;
-      
+
       TC.diagnose(affected->getLoc(), diag::optional_not_unwrapped, type,
                   unwrappedType);
 
@@ -8156,13 +8156,12 @@ bool ConstraintSystem::applySolutionFix(Expr *expr,
     return true;
   }
 
-  case FixKind::OptionalChaining: {
+  case FixKind::UnwrapOptionalBase: {
     auto type = solution.simplifyType(getType(affected))
                 ->getRValueObjectType();
-    auto diag = TC.diagnose(affected->getLoc(),
-                            diag::missing_unwrap_optional, type);
-    diag.fixItInsertAfter(affected->getEndLoc(), "?");
-    return true;
+    DeclName memberName = fix.first.getDeclNameArgument(*this);
+    return diagnoseBaseUnwrapForMemberAccess(affected, type, memberName,
+                                             SourceRange());
   }
 
   case FixKind::ForceDowncast: {

lib/Sema/CSDiag.cpp

@@ -2262,15 +2262,6 @@ Expr *FailureDiagnosis::typeCheckChildIndependently(
       allowFreeTypeVariables)
     TCEOptions |= TypeCheckExprFlags::AllowUnresolvedTypeVariables;
 
-  // If we're not passing down contextual type information this time, but the
-  // original failure had type info that wasn't an optional type,
-  // then set the flag to prefer fixits with force unwrapping.
-  if (!convertType) {
-    auto previousType = CS.getContextualType();
-    if (previousType && previousType->getOptionalObjectType().isNull())
-      TCEOptions |= TypeCheckExprFlags::PreferForceUnwrapToOptional;
-  }
-
   auto resultTy = CS.TC.typeCheckExpression(
       subExpr, CS.DC, TypeLoc::withoutLoc(convertType), convertTypePurpose,
       TCEOptions, listener, &CS);
@@ -7751,15 +7742,9 @@ bool FailureDiagnosis::diagnoseMemberFailures(
       }
 
       if (!optionalResult.ViableCandidates.empty()) {
-        diagnose(BaseLoc, diag::optional_base_not_unwrapped,
-                 baseObjTy, memberName, OT->getOptionalObjectType())
-          .highlight(memberRange);
-
-        diagnose(BaseLoc, diag::optional_base_chain, memberName)
-          .fixItInsertAfter(baseExpr->getEndLoc(), "?");
-        diagnose(BaseLoc, diag::unwrap_with_force_value)
-          .fixItInsertAfter(baseExpr->getEndLoc(), "!");
-        return true;
+        if (diagnoseBaseUnwrapForMemberAccess(baseExpr, baseObjTy, memberName,
+                                              memberRange))
+          return true;
       }
     }
 
@@ -8666,3 +8651,22 @@ bool ConstraintSystem::salvage(SmallVectorImpl<Solution> &viable, Expr *expr) {
   diagnoseFailureForExpr(expr);
   return true;
 }
+
+bool swift::diagnoseBaseUnwrapForMemberAccess(Expr *baseExpr, Type baseType,
+                                              DeclName memberName,
+                                              SourceRange memberRange) {
+  auto unwrappedBaseType = baseType->getOptionalObjectType();
+  if (!unwrappedBaseType)
+    return false;
+
+  ASTContext &ctx = baseType->getASTContext();
+  DiagnosticEngine &diags = ctx.Diags;
+  diags.diagnose(baseExpr->getLoc(), diag::optional_base_not_unwrapped,
+                 baseType, memberName, unwrappedBaseType);
+
+  diags.diagnose(baseExpr->getLoc(), diag::optional_base_chain, memberName)
+    .fixItInsertAfter(baseExpr->getEndLoc(), "?");
+  diags.diagnose(baseExpr->getLoc(), diag::unwrap_with_force_value)
+    .fixItInsertAfter(baseExpr->getEndLoc(), "!");
+  return true;
+}

lib/Sema/CSSimplify.cpp

@@ -3548,18 +3548,9 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
   // Value member lookup has some hacks too.
   if (shouldAttemptFixes() && baseObjTy->getOptionalObjectType()) {
     // If the base type was an optional, look through it.
-    
-    // Determine whether or not we want to provide an optional chaining fixit or
-    // a force unwrap fixit.
-    bool optionalChain;
-    if (!getContextualType())
-      optionalChain = !(Options & ConstraintSystemFlags::PreferForceUnwrapToOptional);
-    else
-      optionalChain = !getContextualType()->getOptionalObjectType().isNull();
-    auto fixKind = optionalChain ? FixKind::OptionalChaining : FixKind::ForceOptional;
-
-    // Note the fix.
-    if (recordFix(fixKind, locator))
+
+    // We're unwrapping the base to perform a member access.
+    if (recordFix(Fix::getUnwrapOptionalBase(*this, member), locator))
       return SolutionKind::Error;
 
     // Look through one level of optional.
@@ -4871,8 +4862,8 @@ ConstraintSystem::simplifyFixConstraint(Fix fix, Type type1, Type type2,
     getDefaultDecompositionOptions(flags) | TMF_ApplyingFix;
   switch (fix.getKind()) {
   case FixKind::ForceOptional:
-  case FixKind::OptionalChaining: {
-    // Assume that '!' was applied to the first type.
+  case FixKind::UnwrapOptionalBase: {
+    // Assume that we've unwrapped the first type.
     auto result =
         matchTypes(type1->getRValueObjectType()->getOptionalObjectType(), type2,
                    matchKind, subflags, locator);

lib/Sema/Constraint.cpp

@@ -487,17 +487,29 @@ Fix Fix::getForcedDowncast(ConstraintSystem &cs, Type toType) {
   return Fix(FixKind::ForceDowncast, index);
 }
 
+Fix Fix::getUnwrapOptionalBase(ConstraintSystem &cs, DeclName memberName) {
+  unsigned index = cs.FixedDeclNames.size();
+  cs.FixedDeclNames.push_back(memberName);
+  return Fix(FixKind::UnwrapOptionalBase, index);
+}
+
 Type Fix::getTypeArgument(ConstraintSystem &cs) const {
   assert(getKind() == FixKind::ForceDowncast);
   return cs.FixedTypes[Data];
 }
 
+/// If this fix has a name argument, retrieve it.
+DeclName Fix::getDeclNameArgument(ConstraintSystem &cs) const {
+  assert(getKind() == FixKind::UnwrapOptionalBase);
+  return cs.FixedDeclNames[Data];
+}
+
 StringRef Fix::getName(FixKind kind) {
   switch (kind) {
   case FixKind::ForceOptional:
     return "fix: force optional";
-  case FixKind::OptionalChaining:
-    return "fix: optional chaining";
+  case FixKind::UnwrapOptionalBase:
+    return "fix: unwrap optional base of member lookup";
   case FixKind::ForceDowncast:
     return "fix: force downcast";
   case FixKind::AddressOf:

lib/Sema/Constraint.h

@@ -233,8 +233,8 @@ enum class FixKind : uint8_t {
   /// Introduce a '!' to force an optional unwrap.
   ForceOptional,
 
-  /// Introduce a '?.' to begin optional chaining.
-  OptionalChaining,
+  /// Unwrap an optional base when we have a member access.
+  UnwrapOptionalBase,
 
   /// Append 'as! T' to force a downcast to the specified type.
   ForceDowncast,
@@ -265,17 +265,26 @@ class Fix {
 public:
   Fix(FixKind kind) : Kind(kind), Data(0) {
     assert(kind != FixKind::ForceDowncast && "Use getForceDowncast()");
+    assert(kind != FixKind::UnwrapOptionalBase &&
+           "Use getUnwrapOptionalBase()");
   }
 
   /// Produce a new fix that performs a forced downcast to the given type.
   static Fix getForcedDowncast(ConstraintSystem &cs, Type toType);
 
+  /// Produce a new fix that unwraps an optional base for an access to a member
+  /// with the given name.
+  static Fix getUnwrapOptionalBase(ConstraintSystem &cs, DeclName memberName);
+
   /// Retrieve the kind of fix.
   FixKind getKind() const { return Kind; }
 
   /// If this fix has a type argument, retrieve it.
   Type getTypeArgument(ConstraintSystem &cs) const;
 
+  /// If this fix has a name argument, retrieve it.
+  DeclName getDeclNameArgument(ConstraintSystem &cs) const;
+
   /// Return a string representation of a fix.
   static llvm::StringRef getName(FixKind kind);
 

lib/Sema/ConstraintSystem.h

@@ -796,10 +796,6 @@ enum class ConstraintSystemFlags {
   /// Whether we allow the solver to attempt fixes to the system.
   AllowFixes = 0x01,
 
-  /// Set if the client prefers fixits to be in the form of force unwrapping
-  /// or optional chaining to return an optional.
-  PreferForceUnwrapToOptional = 0x02,
-
   /// If set, this is going to prevent constraint system from erasing all
   /// discovered solutions except the best one.
   ReturnAllDiscoveredSolutions = 0x04,
@@ -1001,6 +997,9 @@ class ConstraintSystem {
   /// Types used in fixes.
   std::vector<Type> FixedTypes;
 
+  /// Declaration names used in fixes.
+  std::vector<DeclName> FixedDeclNames;
+
   /// \brief The set of remembered disjunction choices used to reach
   /// the current constraint system.
   SmallVector<std::pair<ConstraintLocator*, unsigned>, 32>
@@ -3508,6 +3507,15 @@ class OverloadSetCounter : public ASTWalker {
     return { true, expr };
   }
 };
+
+/// Diagnose an attempt to recover from a member access into a value of
+/// optional type which needed to be unwrapped for the member to be found.
+///
+/// \returns true if a diagnostic was produced.
+bool diagnoseBaseUnwrapForMemberAccess(Expr *baseExpr, Type baseType,
+                                       DeclName memberName,
+                                       SourceRange memberRange);
+
 } // end namespace swift
 
 #endif // LLVM_SWIFT_SEMA_CONSTRAINT_SYSTEM_H

lib/Sema/TypeCheckConstraints.cpp

@@ -1856,8 +1856,6 @@ Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
 
   // Construct a constraint system from this expression.
   ConstraintSystemOptions csOptions = ConstraintSystemFlags::AllowFixes;
-  if (options.contains(TypeCheckExprFlags::PreferForceUnwrapToOptional))
-    csOptions |= ConstraintSystemFlags::PreferForceUnwrapToOptional;
   ConstraintSystem cs(*this, dc, csOptions);
   cs.baseCS = baseCS;
   CleanupIllFormedExpressionRAII cleanup(Context, expr);

lib/Sema/TypeChecker.h

@@ -248,10 +248,6 @@ enum class TypeCheckExprFlags {
   /// and so we should not visit bodies of non-single expression closures.
   SkipMultiStmtClosures = 0x40,
 
-  /// Set if the client prefers fixits to be in the form of force unwrapping
-  /// or optional chaining to return an optional.
-  PreferForceUnwrapToOptional = 0x80,
-
   /// If set, don't apply a solution.
   SkipApplyingSolution = 0x100,
 };

test/Constraints/diagnostics.swift

@@ -603,15 +603,19 @@ func someFunction() -> () {
 // <rdar://problem/23560128> QoI: trying to mutate an optional dictionary result produces bogus diagnostic
 func r23560128() {
   var a : (Int,Int)?
-  a.0 = 42  // expected-error {{value of optional type '(Int, Int)?' not unwrapped; did you mean to use '!' or '?'?}} {{4-4=?}}
+  a.0 = 42 // expected-error{{value of optional type '(Int, Int)?' must be unwrapped to refer to member '0' of wrapped base type '(Int, Int)'}}
+  // expected-note@-1{{chain the optional }}
+  // expected-note@-2{{force-unwrap using '!'}}
 }
 
 // <rdar://problem/21890157> QoI: wrong error message when accessing properties on optional structs without unwrapping
 struct ExampleStruct21890157 {
   var property = "property"
 }
 var example21890157: ExampleStruct21890157?
-example21890157.property = "confusing"  // expected-error {{value of optional type 'ExampleStruct21890157?' not unwrapped; did you mean to use '!' or '?'?}} {{16-16=?}}
+example21890157.property = "confusing"  // expected-error {{value of optional type 'ExampleStruct21890157?' must be unwrapped to refer to member 'property' of wrapped base type 'ExampleStruct21890157'}}
+  // expected-note@-1{{chain the optional }}
+  // expected-note@-2{{force-unwrap using '!'}}
 
 
 struct UnaryOp {}

test/Constraints/fixes.swift

@@ -141,7 +141,9 @@ ciuo ? true : false // expected-error{{optional type 'C?' cannot be used as a bo
 !ciuo // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{2-2=(}} {{6-6= != nil)}}
 
 // Forgotten ! or ?
-var someInt = co.a // expected-error{{value of optional type 'C?' not unwrapped; did you mean to use '!' or '?'?}} {{17-17=?}}
+var someInt = co.a // expected-error{{value of optional type 'C?' must be unwrapped to refer to member 'a' of wrapped base type 'C'}}
+// expected-note@-1{{chain the optional using '?' to access member 'a' only for non-'nil' base values}}{{17-17=?}}
+// expected-note@-2{{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}{{17-17=!}}
 
 // SR-839
 struct Q {
@@ -157,7 +159,9 @@ let b: Int = q.s.utf8 // expected-error{{value of optional type 'String?' must b
 let d: Int! = q.s.utf8 // expected-error{{value of optional type 'String?' must be unwrapped to refer to member 'utf8' of wrapped base type 'String'}}
 // expected-note@-1{{chain the optional using '?'}}{{18-18=?}}
 // expected-note@-2{{force-unwrap using '!'}}{{18-18=!}}
-let c = q.s.utf8 // expected-error{{value of optional type 'String?' not unwrapped; did you mean to use '!' or '?'?}} {{12-12=?}}
+let c = q.s.utf8 // expected-error{{value of optional type 'String?' must be unwrapped to refer to member 'utf8' of wrapped base type 'String'}}
+// expected-note@-1{{chain the optional using '?' to access member 'utf8' only for non-'nil' base values}}{{12-12=?}}
+// expected-note@-2{{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}{{12-12=!}}
 
 // SR-1116
 struct S1116 {