Merge pull request #23500 from theblixguy/fix/SR-9267

[CSDiag] Improving the fix-it for defining computed variables

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -1522,6 +1522,8 @@ ERROR(extension_specialization,none,
       "type %0 with constraints specified by a 'where' clause", (Identifier))
 ERROR(extension_stored_property,none,
       "extensions must not contain stored properties", ())
+NOTE(extension_stored_property_fixit,none,
+     "Remove '=' to make %0 a computed property", (Identifier))
 ERROR(extension_nongeneric_trailing_where,none,
       "trailing 'where' clause for extension of non-generic type %0",
       (DeclName))

lib/Sema/CSDiag.cpp

@@ -2315,16 +2315,21 @@ bool FailureDiagnosis::diagnoseContextualConversionError(
       exprType->isEqual(contextualType)) {
     return false;
   }
-  
+
   // If we're trying to convert something of type "() -> T" to T, then we
   // probably meant to call the value.
   if (auto srcFT = exprType->getAs<AnyFunctionType>()) {
     if (srcFT->getParams().empty() &&
         !isUnresolvedOrTypeVarType(srcFT->getResult()) &&
         CS.TC.isConvertibleTo(srcFT->getResult(), contextualType, CS.DC)) {
-      diagnose(expr->getLoc(), diag::missing_nullary_call, srcFT->getResult())
-        .highlight(expr->getSourceRange())
-        .fixItInsertAfter(expr->getEndLoc(), "()");
+
+      auto locator =
+          CS.getConstraintLocator(expr, ConstraintLocator::ContextualType);
+      ContextualFailure failure =
+          ContextualFailure(nullptr, CS, srcFT, contextualType, locator);
+      auto diagnosed = failure.diagnoseAsError();
+      assert(diagnosed && "Failed to produce contextual failure diagnostic");
+      (void)diagnosed;
       return true;
     }
   }

lib/Sema/CSDiagnostics.cpp

@@ -22,6 +22,7 @@
 #include "swift/AST/Decl.h"
 #include "swift/AST/Expr.h"
 #include "swift/AST/GenericSignature.h"
+#include "swift/AST/Initializer.h"
 #include "swift/AST/ParameterList.h"
 #include "swift/AST/Pattern.h"
 #include "swift/AST/ProtocolConformance.h"
@@ -1348,6 +1349,9 @@ bool ContextualFailure::diagnoseMissingFunctionCall() const {
                  srcFT->getResult())
       .highlight(anchor->getSourceRange())
       .fixItInsertAfter(anchor->getEndLoc(), "()");
+
+  tryComputedPropertyFixIts(anchor);
+
   return true;
 }
 
@@ -1378,6 +1382,53 @@ bool ContextualFailure::trySequenceSubsequenceFixIts(InFlightDiagnostic &diag,
   return false;
 }
 
+void ContextualFailure::tryComputedPropertyFixIts(Expr *expr) const {
+  if (!isa<ClosureExpr>(expr))
+    return;
+
+  // It is possible that we're looking at a stored property being
+  // initialized with a closure. Something like:
+  //
+  // var foo: Int = { return 0 }
+  //
+  // Let's offer another fix-it to remove the '=' to turn the stored
+  // property into a computed property. If the variable is immutable, then
+  // replace the 'let' with a 'var'.
+
+  PatternBindingDecl *PBD = nullptr;
+
+  if (auto TLCD = dyn_cast<TopLevelCodeDecl>(getDC())) {
+    if (TLCD->getBody()->isImplicit()) {
+      if (auto decl = TLCD->getBody()->getElement(0).dyn_cast<Decl *>()) {
+        if (auto binding = dyn_cast<PatternBindingDecl>(decl)) {
+          PBD = binding;
+        }
+      }
+    }
+  } else if (auto PBI = dyn_cast<PatternBindingInitializer>(getDC())) {
+    PBD = PBI->getBinding();
+  }
+
+  if (PBD) {
+    if (auto VD = PBD->getSingleVar()) {
+      auto entry = PBD->getPatternEntryForVarDecl(VD);
+
+      if (!VD->isStatic() &&
+          !VD->getAttrs().getAttribute<DynamicReplacementAttr>() &&
+          entry.getInit() && isa<ClosureExpr>(entry.getInit())) {
+        auto diag = emitDiagnostic(expr->getLoc(),
+                                   diag::extension_stored_property_fixit,
+                                   VD->getName());
+        diag.fixItRemove(entry.getEqualLoc());
+
+        if (VD->isLet()) {
+          diag.fixItReplace(PBD->getStartLoc(), getTokenText(tok::kw_var));
+        }
+      }
+    }
+  }
+}
+
 bool AutoClosureForwardingFailure::diagnoseAsError() {
   auto path = getLocator()->getPath();
   assert(!path.empty());

lib/Sema/CSDiagnostics.h

@@ -681,6 +681,10 @@ class ContextualFailure final : public FailureDiagnostic {
     }
     return type;
   }
+
+  /// Try to add a fix-it to convert a stored property into a computed
+  /// property
+  void tryComputedPropertyFixIts(Expr *expr) const;
 };
 
 /// Diagnose situations when @autoclosure argument is passed to @autoclosure

lib/Sema/CSSimplify.cpp

@@ -1989,6 +1989,17 @@ repairFailures(ConstraintSystem &cs, Type lhs, Type rhs,
     break;
   }
 
+  case ConstraintLocator::ContextualType: {
+    if (lhs->is<FunctionType>() && !rhs->is<AnyFunctionType>() &&
+        isa<ClosureExpr>(anchor)) {
+      auto *fix = ContextualMismatch::create(cs, lhs, rhs,
+                                             cs.getConstraintLocator(locator));
+      conversionsOrFixes.push_back(fix);
+    }
+
+    break;
+  }
+
   default:
     return;
   }

test/decl/var/properties.swift

@@ -1277,3 +1277,32 @@ let sr8811b: Never = fatalError() // Ok
 let sr8811c = (16, fatalError()) // expected-warning {{constant 'sr8811c' inferred to have type '(Int, Never)', which contains an enum with no cases}} expected-note {{add an explicit type annotation to silence this warning}}
 
 let sr8811d: (Int, Never) = (16, fatalError()) // Ok
+
+// SR-9267
+
+class SR_9267 {}
+extension SR_9267 {
+  var foo: String = { // expected-error {{extensions must not contain stored properties}} // expected-error {{function produces expected type 'String'; did you mean to call it with '()'?}} // expected-note {{Remove '=' to make 'foo' a computed property}}{{19-21=}}
+    return "Hello"
+  }
+}
+
+enum SR_9267_E {
+  var SR_9267_prop: String = { // expected-error {{enums must not contain stored properties}} // expected-error {{function produces expected type 'String'; did you mean to call it with '()'?}} // expected-note {{Remove '=' to make 'SR_9267_prop' a computed property}}{{28-30=}}
+    return "Hello"
+  }
+}
+
+var SR_9267_prop_1: Int = { // expected-error {{function produces expected type 'Int'; did you mean to call it with '()'?}} // expected-note {{Remove '=' to make 'SR_9267_prop_1' a computed property}}{{25-27=}}
+  return 0
+}
+
+class SR_9267_C {
+  var SR_9267_prop_2: String = { // expected-error {{function produces expected type 'String'; did you mean to call it with '()'?}} // expected-note {{Remove '=' to make 'SR_9267_prop_2' a computed property}}{{30-32=}}
+    return "Hello"
+  }
+}
+
+class SR_9267_C2 {
+  let SR_9267_prop_3: Int = { return 0 } // expected-error {{function produces expected type 'Int'; did you mean to call it with '()'?}} // expected-note {{Remove '=' to make 'SR_9267_prop_3' a computed property}}{{3-6=var}}{{27-29=}}
+}

test/expr/closure/closures.swift

@@ -20,8 +20,7 @@ var closure5 : (Double) -> Int = {
 
 var closure6 = $0  // expected-error {{anonymous closure argument not contained in a closure}}
 
-var closure7 : Int =
-   { 4 }  // expected-error {{function produces expected type 'Int'; did you mean to call it with '()'?}} {{9-9=()}}
+var closure7 : Int = { 4 }  // expected-error {{function produces expected type 'Int'; did you mean to call it with '()'?}} {{27-27=()}} // expected-note {{Remove '=' to make 'closure7' a computed property}}{{20-22=}}
 
 var capturedVariable = 1
 var closure8 = { [capturedVariable] in