Merge pull request swiftlang#21238 from xedin/diag-property-as-member-via-fix

[CSFix] Call a function/member which is incorrectly used as a property

Author: xedin

lib/Sema/CSDiag.cpp

@@ -7392,26 +7392,9 @@ bool FailureDiagnosis::diagnoseMemberFailures(
   // Produce a specific diagnostic + fixit for this situation.
   if (auto baseFTy = baseObjTy->getAs<AnyFunctionType>()) {
     if (baseExpr && baseFTy->getParams().empty()) {
-      SourceLoc insertLoc = baseExpr->getEndLoc();
-
-      if (auto *DRE = dyn_cast<DeclRefExpr>(baseExpr)) {
-        diagnose(baseExpr->getLoc(), diag::did_not_call_function,
-                 DRE->getDecl()->getBaseName().getIdentifier())
-            .fixItInsertAfter(insertLoc, "()");
-        return true;
-      }
-
-      if (auto *DSCE = dyn_cast<DotSyntaxCallExpr>(baseExpr))
-        if (auto *DRE = dyn_cast<DeclRefExpr>(DSCE->getFn())) {
-          diagnose(baseExpr->getLoc(), diag::did_not_call_method,
-                   DRE->getDecl()->getBaseName().getIdentifier())
-              .fixItInsertAfter(insertLoc, "()");
-          return true;
-        }
-
-      diagnose(baseExpr->getLoc(), diag::did_not_call_function_value)
-          .fixItInsertAfter(insertLoc, "()");
-      return true;
+      auto failure =
+          MissingCallFailure(expr, CS, CS.getConstraintLocator(baseExpr));
+      return failure.diagnoseAsError();
     }
   }
 

lib/Sema/CSDiagnostics.cpp

@@ -1240,3 +1240,38 @@ bool NonOptionalUnwrapFailure::diagnoseAsError() {
 
   return true;
 }
+
+bool MissingCallFailure::diagnoseAsError() {
+  auto *baseExpr = getAnchor();
+  SourceLoc insertLoc = baseExpr->getEndLoc();
+
+  if (auto *FVE = dyn_cast<ForceValueExpr>(baseExpr))
+    baseExpr = FVE->getSubExpr();
+
+  if (auto *DRE = dyn_cast<DeclRefExpr>(baseExpr)) {
+    emitDiagnostic(baseExpr->getLoc(), diag::did_not_call_function,
+                   DRE->getDecl()->getBaseName().getIdentifier())
+        .fixItInsertAfter(insertLoc, "()");
+    return true;
+  }
+
+  if (auto *UDE = dyn_cast<UnresolvedDotExpr>(baseExpr)) {
+    emitDiagnostic(baseExpr->getLoc(), diag::did_not_call_method,
+                   UDE->getName().getBaseIdentifier())
+        .fixItInsertAfter(insertLoc, "()");
+    return true;
+  }
+
+  if (auto *DSCE = dyn_cast<DotSyntaxCallExpr>(baseExpr)) {
+    if (auto *DRE = dyn_cast<DeclRefExpr>(DSCE->getFn())) {
+      emitDiagnostic(baseExpr->getLoc(), diag::did_not_call_method,
+                     DRE->getDecl()->getBaseName().getIdentifier())
+          .fixItInsertAfter(insertLoc, "()");
+      return true;
+    }
+  }
+
+  emitDiagnostic(baseExpr->getLoc(), diag::did_not_call_function_value)
+      .fixItInsertAfter(insertLoc, "()");
+  return true;
+}

lib/Sema/CSDiagnostics.h

@@ -630,6 +630,15 @@ class NonOptionalUnwrapFailure final : public FailureDiagnostic {
   bool diagnoseAsError() override;
 };
 
+class MissingCallFailure final : public FailureDiagnostic {
+public:
+  MissingCallFailure(Expr *root, ConstraintSystem &cs,
+                     ConstraintLocator *locator)
+      : FailureDiagnostic(root, cs, locator) {}
+
+  bool diagnoseAsError() override;
+};
+
 } // end namespace constraints
 } // end namespace swift
 

lib/Sema/CSFix.cpp

@@ -22,6 +22,7 @@
 #include "ConstraintSystem.h"
 #include "swift/AST/Expr.h"
 #include "swift/AST/Type.h"
+#include "swift/AST/Types.h"
 #include "swift/Basic/SourceManager.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/raw_ostream.h"
@@ -224,3 +225,13 @@ RemoveUnwrap *RemoveUnwrap::create(ConstraintSystem &cs, Type baseType,
                                    ConstraintLocator *locator) {
   return new (cs.getAllocator()) RemoveUnwrap(cs, baseType, locator);
 }
+
+bool InsertExplicitCall::diagnose(Expr *root, bool asNote) const {
+  auto failure = MissingCallFailure(root, getConstraintSystem(), getLocator());
+  return failure.diagnose(asNote);
+}
+
+InsertExplicitCall *InsertExplicitCall::create(ConstraintSystem &cs,
+                                               ConstraintLocator *locator) {
+  return new (cs.getAllocator()) InsertExplicitCall(cs, locator);
+}

lib/Sema/CSFix.h

@@ -21,6 +21,7 @@
 #include "swift/AST/Expr.h"
 #include "swift/AST/Identifier.h"
 #include "swift/AST/Type.h"
+#include "swift/AST/Types.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/TrailingObjects.h"
@@ -92,6 +93,9 @@ enum class FixKind : uint8_t {
 
   /// Remove `!` or `?` because base is not an optional type.
   RemoveUnwrap,
+
+  /// Add explicit `()` at the end of function or member to call it.
+  InsertCall,
 };
 
 class ConstraintFix {
@@ -428,6 +432,21 @@ class RemoveUnwrap final : public ConstraintFix {
                               ConstraintLocator *locator);
 };
 
+class InsertExplicitCall final : public ConstraintFix {
+public:
+  InsertExplicitCall(ConstraintSystem &cs, ConstraintLocator *locator)
+      : ConstraintFix(cs, FixKind::InsertCall, locator) {}
+
+  std::string getName() const override {
+    return "insert explicit `()` to make a call";
+  }
+
+  bool diagnose(Expr *root, bool asNote = false) const override;
+
+  static InsertExplicitCall *create(ConstraintSystem &cs,
+                                    ConstraintLocator *locator);
+};
+
 } // end namespace constraints
 } // end namespace swift
 

lib/Sema/CSSimplify.cpp

@@ -3734,49 +3734,72 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
 
   // If the lookup found no hits at all (either viable or unviable), diagnose it
   // as such and try to recover in various ways.
-  if (shouldAttemptFixes() && baseObjTy->getOptionalObjectType()) {
-    // If the base type was an optional, look through it.
-
-    // If the base type is optional because we haven't chosen to force an
-    // implicit optional, don't try to fix it. The IUO will be forced instead.
-    if (auto dotExpr = dyn_cast<UnresolvedDotExpr>(locator->getAnchor())) {
-      auto baseExpr = dotExpr->getBase();
-      auto resolvedOverload = getResolvedOverloadSets();
-      while (resolvedOverload) {
-        if (resolvedOverload->Locator->getAnchor() == baseExpr) {
-          if (resolvedOverload->Choice.isImplicitlyUnwrappedValueOrReturnValue())
-            return SolutionKind::Error;
-          break;
+  if (shouldAttemptFixes()) {
+    if (baseObjTy->getOptionalObjectType()) {
+      // If the base type was an optional, look through it.
+
+      // If the base type is optional because we haven't chosen to force an
+      // implicit optional, don't try to fix it. The IUO will be forced instead.
+      if (auto dotExpr = dyn_cast<UnresolvedDotExpr>(locator->getAnchor())) {
+        auto baseExpr = dotExpr->getBase();
+        auto resolvedOverload = getResolvedOverloadSets();
+        while (resolvedOverload) {
+          if (resolvedOverload->Locator->getAnchor() == baseExpr) {
+            if (resolvedOverload->Choice
+                    .isImplicitlyUnwrappedValueOrReturnValue())
+              return SolutionKind::Error;
+            break;
+          }
+          resolvedOverload = resolvedOverload->Previous;
         }
-        resolvedOverload = resolvedOverload->Previous;
       }
+
+      // The result of the member access can either be the expected member type
+      // (for '!' or optional members with '?'), or the original member type
+      // with one extra level of optionality ('?' with non-optional members).
+      auto innerTV = createTypeVariable(locator, TVO_CanBindToLValue);
+      Type optTy = getTypeChecker().getOptionalType(
+          locator->getAnchor()->getSourceRange().Start, innerTV);
+      SmallVector<Constraint *, 2> optionalities;
+      auto nonoptionalResult = Constraint::createFixed(
+          *this, ConstraintKind::Bind,
+          UnwrapOptionalBase::create(*this, member, locator), innerTV, memberTy,
+          locator);
+      auto optionalResult = Constraint::createFixed(
+          *this, ConstraintKind::Bind,
+          UnwrapOptionalBase::createWithOptionalResult(*this, member, locator),
+          optTy, memberTy, locator);
+      optionalities.push_back(nonoptionalResult);
+      optionalities.push_back(optionalResult);
+      addDisjunctionConstraint(optionalities, locator);
+
+      // Look through one level of optional.
+      addValueMemberConstraint(baseObjTy->getOptionalObjectType(), member,
+                               innerTV, useDC, functionRefKind,
+                               outerAlternatives, locator);
+      return SolutionKind::Solved;
     }
 
-    // The result of the member access can either be the expected member type
-    // (for '!' or optional members with '?'), or the original member type with
-    // one extra level of optionality ('?' with non-optional members).
-    auto innerTV =
-        createTypeVariable(locator, TVO_CanBindToLValue);
-    Type optTy = getTypeChecker().getOptionalType(
-        locator->getAnchor()->getSourceRange().Start, innerTV);
-    SmallVector<Constraint *, 2> optionalities;
-    auto nonoptionalResult = Constraint::createFixed(
-        *this, ConstraintKind::Bind,
-        UnwrapOptionalBase::create(*this, member, locator), innerTV, memberTy,
-        locator);
-    auto optionalResult = Constraint::createFixed(
-        *this, ConstraintKind::Bind,
-        UnwrapOptionalBase::createWithOptionalResult(*this, member, locator),
-        optTy, memberTy, locator);
-    optionalities.push_back(nonoptionalResult);
-    optionalities.push_back(optionalResult);
-    addDisjunctionConstraint(optionalities, locator);
-
-    // Look through one level of optional.
-    addValueMemberConstraint(baseObjTy->getOptionalObjectType(), member,
-                             innerTV, useDC, functionRefKind, outerAlternatives,
-                             locator);
-    return SolutionKind::Solved;
+    if (auto *funcType = baseTy->getAs<FunctionType>()) {
+      // We can't really suggest anything useful unless
+      // function takes no arguments, otherwise it
+      // would make sense to report this a missing member.
+      if (funcType->getNumParams() == 0) {
+        auto *fix = InsertExplicitCall::create(*this, locator);
+        if (recordFix(fix))
+          return SolutionKind::Error;
+
+        auto result = simplifyMemberConstraint(
+            kind, funcType->getResult(), member, memberTy, useDC,
+            functionRefKind, outerAlternatives, flags, locatorB);
+
+        // If there is indeed a member with given name in result type
+        // let's return, otherwise let's fall-through and report
+        // this problem as a missing member.
+        if (result == SolutionKind::Solved)
+          return result;
+      }
+    }
   }
   return SolutionKind::Error;
 }
@@ -5439,6 +5462,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
     return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
   }
 
+  case FixKind::InsertCall:
   case FixKind::ExplicitlyEscaping:
   case FixKind::CoerceToCheckedCast:
   case FixKind::RelabelArguments:

test/Constraints/fixes.swift

@@ -104,9 +104,10 @@ class U {
 
 class T {
     func m1() {
-        // FIXME: should apply nullary function fixit here. {{function produces expected type 'U'; did you mean to call it with '()'?}}
-        // <rdar://problem/17741575>
-        let l = self.m2!.prop1 // expected-error{{cannot force unwrap value of non-optional type '() -> U?'}} {{24-25=}}
+      // <rdar://problem/17741575>
+      let l = self.m2!.prop1
+      // expected-error@-1 {{cannot force unwrap value of non-optional type '() -> U?'}} {{22-23=}}
+      // expected-error@-2 {{method 'm2' was used as a property; add () to call it}}  {{23-23=()}}
     }
 
     func m2() -> U! {