[CSDiagnostics] Add missing arguments failure

Currently only supports closures, but could be easily expanded
to other types of situations e.g. function/member calls.

Author: xedin

lib/Sema/CSDiag.cpp

@@ -5775,8 +5775,8 @@ bool FailureDiagnosis::diagnoseClosureExpr(
   // If we have a contextual type available for this closure, apply it to the
   // ParamDecls in our parameter list.  This ensures that any uses of them get
   // appropriate types.
-  if (contextualType && contextualType->is<AnyFunctionType>()) {
-    auto fnType = contextualType->getAs<AnyFunctionType>();
+  if (contextualType && contextualType->is<FunctionType>()) {
+    auto fnType = contextualType->getAs<FunctionType>();
     auto *params = CE->getParameters();
     auto inferredArgs = fnType->getParams();
 
@@ -5791,36 +5791,6 @@ bool FailureDiagnosis::diagnoseClosureExpr(
     unsigned inferredArgCount = inferredArgs.size();
 
     if (actualArgCount != inferredArgCount) {
-      // If the closure didn't specify any arguments and it is in a context that
-      // needs some, produce a fixit to turn "{...}" into "{ _,_ in ...}".
-      if (actualArgCount == 0 && CE->getInLoc().isInvalid()) {
-        auto diag =
-          diagnose(CE->getStartLoc(), diag::closure_argument_list_missing,
-                   inferredArgCount);
-        std::string fixText; // Let's provide fixits for up to 10 args.
-
-        if (inferredArgCount <= 10) {
-          fixText += " _";
-          for (unsigned i = 0; i < inferredArgCount - 1; i ++) {
-            fixText += ",_";
-          }
-          fixText += " in ";
-        }
-
-        if (!fixText.empty()) {
-          // Determine if there is already a space after the { in the closure to
-          // make sure we introduce the right whitespace.
-          auto afterBrace = CE->getStartLoc().getAdvancedLoc(1);
-          auto text = CS.TC.Context.SourceMgr.extractText({afterBrace, 1});
-          if (text.size() == 1 && text == " ")
-            fixText = fixText.erase(fixText.size() - 1);
-          else
-            fixText = fixText.erase(0, 1);
-          diag.fixItInsertAfter(CE->getStartLoc(), fixText);
-        }
-        return true;
-      }
-
       if (inferredArgCount == 1 && actualArgCount > 1) {
         auto *argTupleTy = inferredArgs.front().getOldType()->getAs<TupleType>();
         // Let's see if inferred argument is actually a tuple inside of Paren.
@@ -5956,49 +5926,33 @@ bool FailureDiagnosis::diagnoseClosureExpr(
         }
       }
 
-      bool onlyAnonymousParams =
-      std::all_of(params->begin(), params->end(), [](ParamDecl *param) {
-        return !param->hasName();
-      });
-
-      // Okay, the wrong number of arguments was used, complain about that.
-      // Before doing so, strip attributes off the function type so that they
-      // don't confuse the issue.
-      fnType = FunctionType::get(fnType->getParams(), fnType->getResult(),
-                                 fnType->getExtInfo());
-      auto diag = diagnose(
-          params->getStartLoc(), diag::closure_argument_list_tuple, fnType,
-          inferredArgCount, actualArgCount, (actualArgCount == 1));
-
-      // If closure expects no parameters but N was given,
-      // and all of them are anonymous let's suggest removing them.
-      if (inferredArgCount == 0 && onlyAnonymousParams) {
-        auto inLoc = CE->getInLoc();
-        auto &sourceMgr = CS.getASTContext().SourceMgr;
-
-        if (inLoc.isValid())
-          diag.fixItRemoveChars(params->getStartLoc(),
-                                Lexer::getLocForEndOfToken(sourceMgr, inLoc));
+      // Extraneous arguments.
+      if (inferredArgCount < actualArgCount) {
+        auto diag = diagnose(
+            params->getStartLoc(), diag::closure_argument_list_tuple, fnType,
+            inferredArgCount, actualArgCount, (actualArgCount == 1));
+
+        bool onlyAnonymousParams =
+            std::all_of(params->begin(), params->end(),
+                        [](ParamDecl *param) { return !param->hasName(); });
+
+        // If closure expects no parameters but N was given,
+        // and all of them are anonymous let's suggest removing them.
+        if (inferredArgCount == 0 && onlyAnonymousParams) {
+          auto inLoc = CE->getInLoc();
+          auto &sourceMgr = CS.getASTContext().SourceMgr;
+
+          if (inLoc.isValid())
+            diag.fixItRemoveChars(params->getStartLoc(),
+                                  Lexer::getLocForEndOfToken(sourceMgr, inLoc));
+        }
         return true;
       }
 
-      // If the number of parameters is less than number of inferred
-      // and all of the parameters are anonymous, let's suggest a fix-it
-      // with the rest of the missing parameters.
-      if (actualArgCount < inferredArgCount) {
-        SmallString<32> fixIt;
-        llvm::raw_svector_ostream OS(fixIt);
-
-        OS << ",";
-        auto numMissing = inferredArgCount - actualArgCount;
-        for (unsigned i = 0; i != numMissing; ++i) {
-          OS << ((onlyAnonymousParams) ? "_" : "<#arg#>");
-          OS << ((i == numMissing - 1) ? " " : ",");
-        }
-
-        diag.fixItInsertAfter(params->getEndLoc(), OS.str());
-      }
-      return true;
+      MissingArgumentsFailure failure(
+          expr, CS, fnType, inferredArgCount - actualArgCount,
+          CS.getConstraintLocator(CE, ConstraintLocator::ContextualType));
+      return failure.diagnoseAsError();
     }
 
     // Coerce parameter types here only if there are no unresolved

lib/Sema/CSDiagnostics.cpp

@@ -1931,3 +1931,81 @@ bool ImplicitInitOnNonConstMetatypeFailure::diagnoseAsError() {
       .fixItInsert(loc, ".init");
   return true;
 }
+
+bool MissingArgumentsFailure::diagnoseAsError() {
+  auto *locator = getLocator();
+  auto path = locator->getPath();
+
+  // TODO: Currently this is only intended to diagnose contextual failures.
+  if (!(path.back().getKind() == ConstraintLocator::ApplyArgToParam ||
+        path.back().getKind() == ConstraintLocator::ContextualType))
+    return false;
+
+  if (auto *closure = dyn_cast<ClosureExpr>(getAnchor()))
+    return diagnoseTrailingClosure(closure);
+
+  return false;
+}
+
+bool MissingArgumentsFailure::diagnoseTrailingClosure(ClosureExpr *closure) {
+  auto diff = Fn->getNumParams() - NumSynthesized;
+
+  // If the closure didn't specify any arguments and it is in a context that
+  // needs some, produce a fixit to turn "{...}" into "{ _,_ in ...}".
+  if (diff == 0) {
+    auto diag =
+        emitDiagnostic(closure->getStartLoc(),
+                       diag::closure_argument_list_missing, NumSynthesized);
+
+    std::string fixText; // Let's provide fixits for up to 10 args.
+    if (Fn->getNumParams() <= 10) {
+      fixText += " ";
+      interleave(
+          Fn->getParams(),
+          [&fixText](const AnyFunctionType::Param &param) { fixText += '_'; },
+          [&fixText] { fixText += ','; });
+      fixText += " in ";
+    }
+
+    if (!fixText.empty()) {
+      // Determine if there is already a space after the { in the closure to
+      // make sure we introduce the right whitespace.
+      auto afterBrace = closure->getStartLoc().getAdvancedLoc(1);
+      auto text = getASTContext().SourceMgr.extractText({afterBrace, 1});
+      if (text.size() == 1 && text == " ")
+        fixText = fixText.erase(fixText.size() - 1);
+      else
+        fixText = fixText.erase(0, 1);
+      diag.fixItInsertAfter(closure->getStartLoc(), fixText);
+    }
+
+    return true;
+  }
+
+  auto params = closure->getParameters();
+  bool onlyAnonymousParams =
+      std::all_of(params->begin(), params->end(),
+                  [](ParamDecl *param) { return !param->hasName(); });
+
+  auto diag =
+      emitDiagnostic(params->getStartLoc(), diag::closure_argument_list_tuple,
+                     resolveType(Fn), Fn->getNumParams(), diff, diff == 1);
+
+  // If the number of parameters is less than number of inferred
+  // let's try to suggest a fix-it with the rest of the missing parameters.
+  if (!closure->hasExplicitResultType() &&
+      closure->getInLoc().isValid()) {
+    SmallString<32> fixIt;
+    llvm::raw_svector_ostream OS(fixIt);
+
+    OS << ",";
+    for (unsigned i = 0; i != NumSynthesized; ++i) {
+      OS << ((onlyAnonymousParams) ? "_" : "<#arg#>");
+      OS << ((i == NumSynthesized - 1) ? " " : ",");
+    }
+
+    diag.fixItInsertAfter(params->getEndLoc(), OS.str());
+  }
+
+  return true;
+}

lib/Sema/CSDiagnostics.h

@@ -877,6 +877,28 @@ class ImplicitInitOnNonConstMetatypeFailure final
   bool diagnoseAsError() override;
 };
 
+class MissingArgumentsFailure final : public FailureDiagnostic {
+  using Param = AnyFunctionType::Param;
+
+  FunctionType *Fn;
+  unsigned NumSynthesized;
+
+public:
+  MissingArgumentsFailure(Expr *root, ConstraintSystem &cs,
+                          FunctionType *funcType,
+                          unsigned numSynthesized,
+                          ConstraintLocator *locator)
+      : FailureDiagnostic(root, cs, locator), Fn(funcType),
+        NumSynthesized(numSynthesized) {}
+
+  bool diagnoseAsError() override;
+
+private:
+  /// If missing arguments come from trailing closure,
+  /// let's produce tailored diagnostics.
+  bool diagnoseTrailingClosure(ClosureExpr *closure);
+};
+
 } // end namespace constraints
 } // end namespace swift
 

lib/Sema/CSFix.cpp

@@ -344,7 +344,9 @@ AllowInvalidInitRef::create(RefKind kind, ConstraintSystem &cs, Type baseTy,
 }
 
 bool AddMissingArguments::diagnose(Expr *root, bool asNote) const {
-  return false;
+  MissingArgumentsFailure failure(root, getConstraintSystem(), Fn,
+                                  NumSynthesized, getLocator());
+  return failure.diagnose(asNote);
 }
 
 AddMissingArguments *

test/Constraints/closures.swift

@@ -875,3 +875,28 @@ struct rdar43866352<Options> {
     callback = { (options: Options) in } // expected-error {{cannot assign value of type '(inout Options) -> ()' to type '(inout _) -> Void'}}
   }
 }
+
+extension Hashable {
+  var self_: Self {
+    return self
+  }
+}
+
+do {
+  struct S<
+      C : Collection,
+      I : Hashable,
+      R : Numeric
+  > {
+    init(_ arr: C,
+         id: KeyPath<C.Element, I>,
+         content: @escaping (C.Element) -> R) {}
+  }
+
+  func foo(_ arr: [Int]) {
+    _ = S(arr, id: \.self_) {
+      // expected-error@-1 {{contextual type for closure argument list expects 1 argument, which cannot be implicitly ignored}} {{30-30=_ in }}
+      return 42
+    }
+  }
+}

test/Constraints/tuple-arguments-unsupported.swift

@@ -11,11 +11,11 @@ test3(.success()) // expected-error {{missing argument for parameter #1 in call}
 
 func toString(indexes: Int?...) -> String {
   let _ = indexes.reduce(0) { print($0); return $0.0 + ($0.1 ?? 0)}
-  // expected-error@-1 {{contextual closure type '(_, Int?) throws -> _' expects 2 arguments, but 1 was used in closure body}}
+  // expected-error@-1 {{contextual closure type '(Int, Int?) throws -> Int' expects 2 arguments, but 1 was used in closure body}}
   let _ = indexes.reduce(0) { (true ? $0 : (1, 2)).0 + ($0.1 ?? 0) }
-  // expected-error@-1 {{contextual closure type '(_, Int?) throws -> _' expects 2 arguments, but 1 was used in closure body}}
+  // expected-error@-1 {{contextual closure type '(Int, Int?) throws -> Int' expects 2 arguments, but 1 was used in closure body}}
   _ = ["Hello", "Foo"].sorted { print($0); return $0.0.count > ($0).1.count }
-  // expected-error@-1 {{argument passed to call that takes no arguments}}
+  // expected-error@-1 {{contextual closure type '(String, String) throws -> Bool' expects 2 arguments, but 1 was used in closure body}}
 }
 
 func doit(_ x: Int) -> Bool { return x > 0 }

test/decl/protocol/req/associated_type_inference.swift

@@ -200,6 +200,7 @@ public class CorePromise<U> : Thenable { // expected-error{{type 'CorePromise<U>
     public func then(_ success: @escaping (_ t: U, _: CorePromise<U>) -> U) -> Self {
         return self.then() { (t: U) -> U in // expected-error{{contextual closure type '(U, CorePromise<U>) -> U' expects 2 arguments, but 1 was used in closure body}}
             return success(t: t, self)
+            // expected-error@-1 {{extraneous argument label 't:' in call}}
         }
     }
 }