Fix <rdar://problem/23718816> QoI: "Extra argument" error when accidentally currying a method

lattner · lattner · commit 5dded3f3523e · 2015-12-08T23:01:44.000-08:00
It is a somewhat common case where folks are accidentally referring to an
instance member with the Type as the base.  This forms a curried member,
which then produced head scratching error messages downstream.

Now that the prep work has gone in, the first part of this is now
straight-forward to fix: simply check for this case and diagnose it
with a custom error, which makes it more clear what the mistake was.

The other half of this problem (tracked by 22108559) affects cases where
the method you're calling takes a single argument.  This isn't fixed
yet, but I'm adding a testcase for it anyway.
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -618,6 +618,10 @@ ERROR(argument_out_of_order,sema_tcc,none,
 ERROR(argument_out_of_order_named_unnamed,sema_tcc,none,
       "argument %0 must precede unnamed parameter #%1", (Identifier, unsigned))
 
+ERROR(instance_member_use_on_type,sema_tcc,none,
+      "use of instance member %1 on type %0; "
+      "did you mean to use a value of type %0 instead?", (Type, Identifier))
+
 ERROR(missing_argument_named,sema_tcc,none,
       "missing argument for parameter %0 in call", (Identifier))
 ERROR(missing_argument_positional,sema_tcc,none,
diff --git a/lib/Sema/CSDiag.cpp b/lib/Sema/CSDiag.cpp
@@ -1193,7 +1193,7 @@ namespace {
     /// If the candidate set has been narrowed down to a specific structural
     /// problem, e.g. that there are too few parameters specified or that
     /// argument labels don't match up, diagnose that error and return true.
-    bool diagnoseAnyStructuralArgumentError(Expr *argExpr);
+    bool diagnoseAnyStructuralArgumentError(Expr *fnExpr, Expr *argExpr);
     
     void dump() const LLVM_ATTRIBUTE_USED;
     
@@ -1694,7 +1694,8 @@ suggestPotentialOverloads(SourceLoc loc, bool isResult) {
 /// If the candidate set has been narrowed down to a specific structural
 /// problem, e.g. that there are too few parameters specified or that argument
 /// labels don't match up, diagnose that error and return true.
-bool CalleeCandidateInfo::diagnoseAnyStructuralArgumentError(Expr *argExpr) {
+bool CalleeCandidateInfo::diagnoseAnyStructuralArgumentError(Expr *fnExpr,
+                                                             Expr *argExpr) {
   // TODO: We only handle the situation where there is exactly one candidate
   // here.
   if (size() != 1) return false;
@@ -1705,6 +1706,31 @@ bool CalleeCandidateInfo::diagnoseAnyStructuralArgumentError(Expr *argExpr) {
     return false;
     
   auto args = decomposeArgParamType(argExpr->getType());
+  auto params = decomposeArgParamType(candidates[0].getArgumentType());
+
+  // It is a somewhat common error to try to access an instance method as a
+  // curried member on the type, instead of using an instance, e.g. the user
+  // wrote:
+  //
+  //   Foo.doThing(42, b: 19)
+  //
+  // instead of:
+  //
+  //   myFoo.doThing(42, b: 19)
+  //
+  // Check for this situation and handle it gracefully.
+  if (params.size() == 1 && candidates[0].decl->isInstanceMember() &&
+      candidates[0].level == 0) {
+    if (auto UDE = dyn_cast<UnresolvedDotExpr>(fnExpr))
+      if (isa<TypeExpr>(UDE->getBase())) {
+        auto baseType = candidates[0].getArgumentType();
+        CS->TC.diagnose(UDE->getLoc(), diag::instance_member_use_on_type,
+                        baseType, UDE->getName())
+          .highlight(UDE->getBase()->getSourceRange());
+        return true;
+      }
+  }
+  
   SmallVector<Identifier, 4> correctNames;
   unsigned OOOArgIdx = ~0U, OOOPrevArgIdx = ~0U;
   unsigned extraArgIdx = ~0U, missingParamIdx = ~0U;
@@ -1744,7 +1770,6 @@ bool CalleeCandidateInfo::diagnoseAnyStructuralArgumentError(Expr *argExpr) {
   // shape) to the specified candidates parameters.  This ignores the
   // concrete types of the arguments, looking only at the argument labels.
   SmallVector<ParamBinding, 4> paramBindings;
-  auto params = decomposeArgParamType(candidates[0].getArgumentType());
   if (!matchCallArguments(args, params, hasTrailingClosure,
                           /*allowFixes:*/true, listener, paramBindings))
     return false;
@@ -3645,7 +3670,8 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
   // Filter the candidate list based on the argument we may or may not have.
   calleeInfo.filterContextualMemberList(callExpr->getArg());
 
-  if (calleeInfo.diagnoseAnyStructuralArgumentError(callExpr->getArg()))
+  if (calleeInfo.diagnoseAnyStructuralArgumentError(callExpr->getFn(),
+                                                    callExpr->getArg()))
     return true;
   
   Type argType;  // Type of the argument list, if knowable.
@@ -3670,7 +3696,7 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
 
   calleeInfo.filterList(argExpr->getType());
 
-  if (calleeInfo.diagnoseAnyStructuralArgumentError(argExpr))
+  if (calleeInfo.diagnoseAnyStructuralArgumentError(callExpr->getFn(), argExpr))
     return true;
 
   // If we have a failure where the candidate set differs on exactly one
diff --git a/test/Constraints/diagnostics.swift b/test/Constraints/diagnostics.swift
@@ -166,7 +166,7 @@ func validateSaveButton(text: String) {
 // <rdar://problem/20201968> QoI: poor diagnostic when calling a class method via a metatype
 class r20201968C {
   func blah() {
-    r20201968C.blah()  // expected-error {{missing argument for parameter #1 in call}}
+    r20201968C.blah()  // expected-error {{use of instance member 'blah' on type 'r20201968C'; did you mean to use a value of type 'r20201968C' instead?}}
   }
 }
 
@@ -344,7 +344,7 @@ CurriedClass.method1(2.0)(1)       // expected-error {{cannot convert value of t
 CurriedClass.method2(c)(32)(b: 1)
 _ = CurriedClass.method2(c)
 _ = CurriedClass.method2(c)(32)
-_ = CurriedClass.method2(1,2)      // expected-error {{extra argument in call}}
+_ = CurriedClass.method2(1,2)      // expected-error {{use of instance member 'method2' on type 'CurriedClass'; did you mean to use a value of type 'CurriedClass' instead?}}
 CurriedClass.method2(c)(1.0)(b: 1) // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
 CurriedClass.method2(c)(1)(b: 1.0) // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
 CurriedClass.method2(c)(2)(c: 1.0) // expected-error {{incorrect argument label in call (have 'c:', expected 'b:')}}
@@ -353,7 +353,7 @@ CurriedClass.method3(c)(32, b: 1)
 _ = CurriedClass.method3(c)
 _ = CurriedClass.method3(c)(1, 2)        // expected-error {{missing argument label 'b:' in call}} {{32-32=b: }}
 _ = CurriedClass.method3(c)(1, b: 2)(32) // expected-error {{cannot call value of non-function type '()'}}
-_ = CurriedClass.method3(1, 2)           // expected-error {{extra argument in call}}
+_ = CurriedClass.method3(1, 2)           // expected-error {{use of instance member 'method3' on type 'CurriedClass'; did you mean to use a value of type 'CurriedClass' instead?}}
 CurriedClass.method3(c)(1.0, b: 1)       // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
 CurriedClass.method3(c)(1)               // expected-error {{missing argument for parameter 'b' in call}}
 
@@ -369,6 +369,22 @@ extension CurriedClass {
   }
 }
 
+extension CurriedClass {
+  func m1(a : Int, b : Int) {}
+  
+  func m2(a : Int) {}
+}
+
+// <rdar://problem/23718816> QoI: "Extra argument" error when accidentally currying a method
+CurriedClass.m1(2, b: 42)   // expected-error {{use of instance member 'm1' on type 'CurriedClass'; did you mean to use a value of type 'CurriedClass' instead?}}
+
+
+// <rdar://problem/22108559> QoI: Confusing error message when calling an instance method as a class method
+CurriedClass.m2(12)  // expected-error {{cannot convert value of type 'Int' to expected argument type 'CurriedClass'}}
+
+
+
+
 
 // <rdar://problem/19870975> Incorrect diagnostic for failed member lookups within closures passed as arguments ("(_) -> _")
 func ident<T>(t: T) -> T {}
