Merge pull request swiftlang#34089 from xedin/nil-as-a-hole

[Diagnostics] Diagnose cases when it's impossible to infer type for `nil` literal

Author: xedin

lib/Sema/CSBindings.cpp

@@ -368,6 +368,15 @@ void ConstraintSystem::PotentialBindings::finalize(
       PotentiallyIncomplete = true;
     }
 
+    // Delay resolution of the `nil` literal to a hole until
+    // the very end to give it a change to be bound to some
+    // other type, just like code completion expression which
+    // relies solely on contextual information.
+    if (locator->directlyAt<NilLiteralExpr>()) {
+      FullyBound = true;
+      PotentiallyIncomplete = true;
+    }
+
     addPotentialBinding(PotentialBinding::forHole(TypeVar, locator));
   }
 
@@ -1230,6 +1239,8 @@ bool TypeVariableBinding::attempt(ConstraintSystem &cs) const {
       cs.increaseScore(SK_Hole);
 
       ConstraintFix *fix = nullptr;
+      unsigned fixImpact = 1;
+
       if (auto *GP = TypeVar->getImpl().getGenericParameter()) {
         // If it is represetative for a key path root, let's emit a more
         // specific diagnostic.
@@ -1268,9 +1279,14 @@ bool TypeVariableBinding::attempt(ConstraintSystem &cs) const {
           return true;
 
         fix = SpecifyKeyPathRootType::create(cs, dstLocator);
+      } else if (dstLocator->directlyAt<NilLiteralExpr>()) {
+        fix = SpecifyContextualTypeForNil::create(cs, dstLocator);
+        // This is a dramatic event, it means that there is absolutely
+        // no contextual information to resolve type of `nil`.
+        fixImpact = 10;
       }
 
-      if (fix && cs.recordFix(fix))
+      if (fix && cs.recordFix(fix, fixImpact))
         return true;
     }
   }

lib/Sema/CSDiagnostics.cpp

@@ -6969,3 +6969,43 @@ bool InvalidEmptyKeyPathFailure::diagnoseAsError() {
   emitDiagnostic(diag::expr_swift_keypath_empty);
   return true;
 }
+
+bool MissingContextualTypeForNil::diagnoseAsError() {
+  auto *expr = castToExpr<NilLiteralExpr>(getAnchor());
+
+  // If this is a standalone `nil` literal expression e.g.
+  // `_ = nil`, let's diagnose it here because solver can't
+  // attempt any types for it.
+  auto *parentExpr = findParentExpr(expr);
+
+  while (parentExpr && isa<IdentityExpr>(parentExpr))
+    parentExpr = findParentExpr(parentExpr);
+
+  // In cases like `_ = nil?` AST would have `nil`
+  // wrapped in `BindOptionalExpr`.
+  if (parentExpr && isa<BindOptionalExpr>(parentExpr))
+    parentExpr = findParentExpr(parentExpr);
+
+  if (parentExpr) {
+    // `_ = nil as? ...`
+    if (isa<ConditionalCheckedCastExpr>(parentExpr)) {
+      emitDiagnostic(diag::conditional_cast_from_nil);
+      return true;
+    }
+
+    // `_ = nil!`
+    if (isa<ForceValueExpr>(parentExpr)) {
+      emitDiagnostic(diag::cannot_force_unwrap_nil_literal);
+      return true;
+    }
+
+    // `_ = nil?`
+    if (isa<OptionalEvaluationExpr>(parentExpr)) {
+      emitDiagnostic(diag::unresolved_nil_literal);
+      return true;
+    }
+  }
+
+  emitDiagnostic(diag::unresolved_nil_literal);
+  return true;
+}

lib/Sema/CSDiagnostics.h

@@ -2268,6 +2268,23 @@ class InvalidEmptyKeyPathFailure final : public FailureDiagnostic {
   bool diagnoseAsError() override;
 };
 
+/// Diagnose situations where there is no context to determine a
+/// type of `nil` literal e.g.
+///
+/// \code
+/// let _ = nil
+/// let _ = try nil
+/// let _ = nil!
+/// \endcode
+class MissingContextualTypeForNil final : public FailureDiagnostic {
+public:
+  MissingContextualTypeForNil(const Solution &solution,
+                              ConstraintLocator *locator)
+      : FailureDiagnostic(solution, locator) {}
+
+  bool diagnoseAsError() override;
+};
+
 } // end namespace constraints
 } // end namespace swift
 

lib/Sema/CSFix.cpp

@@ -1603,3 +1603,15 @@ IgnoreInvalidFunctionBuilderBody *IgnoreInvalidFunctionBuilderBody::create(
   return new (cs.getAllocator())
       IgnoreInvalidFunctionBuilderBody(cs, phase, locator);
 }
+
+bool SpecifyContextualTypeForNil::diagnose(const Solution &solution,
+                                           bool asNote) const {
+  MissingContextualTypeForNil failure(solution, getLocator());
+  return failure.diagnose(asNote);
+}
+
+SpecifyContextualTypeForNil *
+SpecifyContextualTypeForNil::create(ConstraintSystem &cs,
+                                    ConstraintLocator *locator) {
+  return new (cs.getAllocator()) SpecifyContextualTypeForNil(cs, locator);
+}

lib/Sema/CSFix.h

@@ -279,6 +279,9 @@ enum class FixKind : uint8_t {
 
   /// Ignore function builder body which fails `pre-check` call.
   IgnoreInvalidFunctionBuilderBody,
+
+  /// Resolve type of `nil` by providing a contextual type.
+  SpecifyContextualTypeForNil,
 };
 
 class ConstraintFix {
@@ -2026,6 +2029,26 @@ class IgnoreInvalidFunctionBuilderBody final : public ConstraintFix {
   create(ConstraintSystem &cs, ErrorInPhase phase, ConstraintLocator *locator);
 };
 
+class SpecifyContextualTypeForNil final : public ConstraintFix {
+  SpecifyContextualTypeForNil(ConstraintSystem &cs,
+                              ConstraintLocator *locator)
+    : ConstraintFix(cs, FixKind::SpecifyContextualTypeForNil, locator) {}
+
+public:
+  std::string getName() const override {
+    return "specify contextual type to resolve `nil` literal";
+  }
+
+  bool diagnose(const Solution &solution, bool asNote = false) const override;
+
+  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
+    return diagnose(*commonFixes.front().first);
+  }
+
+  static SpecifyContextualTypeForNil *create(ConstraintSystem & cs,
+                                             ConstraintLocator * locator);
+};
+
 } // end namespace constraints
 } // end namespace swift
 

lib/Sema/CSGen.cpp

@@ -1015,80 +1015,13 @@ namespace {
     }
 
     Type visitNilLiteralExpr(NilLiteralExpr *expr) {
-      auto &DE = CS.getASTContext().Diags;
-      // If this is a standalone `nil` literal expression e.g.
-      // `_ = nil`, let's diagnose it here because solver can't
-      // attempt any types for it.
-      auto *parentExpr = CS.getParentExpr(expr);
-      bool hasContextualType = bool(CS.getContextualType(expr));
-
-      while (parentExpr) {
-        if (!isa<IdentityExpr>(parentExpr))
-          break;
-
-        // If there is a parent, use it, otherwise we need
-        // to check whether the last parent node in the chain
-        // had a contextual type associated with it because
-        // in situations like:
-        //
-        // \code
-        // func foo() -> Int? {
-        //   return (nil)
-        // }
-        // \endcode
-        //
-        // parentheses around `nil` are significant.
-        if (auto *nextParent = CS.getParentExpr(parentExpr)) {
-          parentExpr = nextParent;
-        } else {
-          hasContextualType |= bool(CS.getContextualType(parentExpr));
-          // Since current expression is an identity expr
-          // and there are no more parents, let's pretend
-          // that `nil`  don't have a parent since parens
-          // are not semantically significant for further checks.
-          parentExpr = nullptr;
-        }
-      }
-
-      // In cases like `_ = nil?` AST would have `nil`
-      // wrapped in `BindOptionalExpr`.
-      if (parentExpr && isa<BindOptionalExpr>(parentExpr))
-        parentExpr = CS.getParentExpr(parentExpr);
-
-      if (parentExpr) {
-        // `_ = nil as? ...`
-        if (isa<ConditionalCheckedCastExpr>(parentExpr)) {
-          DE.diagnose(expr->getLoc(), diag::conditional_cast_from_nil);
-          return Type();
-        }
-
-        // `_ = nil!`
-        if (isa<ForceValueExpr>(parentExpr)) {
-          DE.diagnose(expr->getLoc(), diag::cannot_force_unwrap_nil_literal);
-          return Type();
-        }
-
-        // `_ = nil?`
-        if (isa<OptionalEvaluationExpr>(parentExpr)) {
-          DE.diagnose(expr->getLoc(), diag::unresolved_nil_literal);
-          return Type();
-        }
+      auto literalTy = visitLiteralExpr(expr);
+      // Allow `nil` to be a hole so we can diagnose it via a fix
+      // if it turns out that there is no contextual information.
+      if (auto *typeVar = literalTy->getAs<TypeVariableType>())
+        CS.recordPotentialHole(typeVar);
 
-        // `_ = nil`
-        if (auto *assignment = dyn_cast<AssignExpr>(parentExpr)) {
-          if (isa<DiscardAssignmentExpr>(assignment->getDest())) {
-            DE.diagnose(expr->getLoc(), diag::unresolved_nil_literal);
-            return Type();
-          }
-        }
-      }
-
-      if (!parentExpr && !hasContextualType) {
-        DE.diagnose(expr->getLoc(), diag::unresolved_nil_literal);
-        return Type();
-      }
-
-      return visitLiteralExpr(expr);
+      return literalTy;
     }
 
     Type visitFloatLiteralExpr(FloatLiteralExpr *expr) {
@@ -3042,11 +2975,19 @@ namespace {
                                             TVO_PrefersSubtypeBinding |
                                             TVO_CanBindToLValue |
                                             TVO_CanBindToNoEscape);
-      
+
+      auto *valueExpr = expr->getSubExpr();
+      // It's invalid to force unwrap `nil` literal e.g. `_ = nil!` or
+      // `_ = (try nil)!` and similar constructs.
+      if (auto *nilLiteral = dyn_cast<NilLiteralExpr>(
+              valueExpr->getSemanticsProvidingExpr())) {
+        CS.recordFix(SpecifyContextualTypeForNil::create(
+            CS, CS.getConstraintLocator(nilLiteral)));
+      }
+
       // The result is the object type of the optional subexpression.
-      CS.addConstraint(ConstraintKind::OptionalObject,
-                       CS.getType(expr->getSubExpr()), objectTy,
-                       locator);
+      CS.addConstraint(ConstraintKind::OptionalObject, CS.getType(valueExpr),
+                       objectTy, locator);
       return objectTy;
     }
 

lib/Sema/CSSimplify.cpp

@@ -5893,6 +5893,12 @@ ConstraintSystem::simplifyOptionalObjectConstraint(
   }
 
 
+  if (optTy->isHole()) {
+    if (auto *typeVar = second->getAs<TypeVariableType>())
+      recordPotentialHole(typeVar);
+    return SolutionKind::Solved;
+  }
+
   Type objectTy = optTy->getOptionalObjectType();
   // If the base type is not optional, let's attempt a fix (if possible)
   // and assume that `!` is just not there.
@@ -10072,7 +10078,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::SpecifyKeyPathRootType:
   case FixKind::SpecifyLabelToAssociateTrailingClosure:
   case FixKind::AllowKeyPathWithoutComponents:
-  case FixKind::IgnoreInvalidFunctionBuilderBody: {
+  case FixKind::IgnoreInvalidFunctionBuilderBody:
+  case FixKind::SpecifyContextualTypeForNil: {
     return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
   }
 

test/Constraints/function_builder_diags.swift

@@ -440,6 +440,7 @@ struct TestConstraintGenerationErrors {
 
   @TupleBuilder var nilWithoutContext: String {
     let a = nil // expected-error {{'nil' requires a contextual type}}
+    ""
   }
 
   func buildTupleClosure() {