[CodeCompletion][Sema] Refine logic that ignores missing arguments after the code completion position when solving

If the completion loc was in a trailing closure arg, it only makes sense to
ignore later missing args if they have function type, as there's no way for the
user to complete the call for overload being matched otherwise.

Also fix a bug where we were incorrectly penalizing solutions with holes that
were ultimately due to missing arguments after the completion position.

Resolves rdar://problem/72412302

Author: Nathan Hawes

include/swift/Sema/ConstraintLocator.h

@@ -511,12 +511,14 @@ class LocatorPathElt::ApplyArgToParam final : public StoredIntegerElement<3> {
   }
 };
 
-class LocatorPathElt::SynthesizedArgument final : public StoredIntegerElement<1> {
+class LocatorPathElt::SynthesizedArgument final : public StoredIntegerElement<2> {
 public:
-  SynthesizedArgument(unsigned index)
-      : StoredIntegerElement(ConstraintLocator::SynthesizedArgument, index) {}
+  SynthesizedArgument(unsigned index, bool afterCodeCompletionLoc = false)
+      : StoredIntegerElement(ConstraintLocator::SynthesizedArgument, index,
+                             afterCodeCompletionLoc) {}
 
-  unsigned getIndex() const { return getValue(); }
+  unsigned getIndex() const { return getValue<0>(); }
+  bool isAfterCodeCompletionLoc() const { return getValue<1>(); }
 
   static bool classof(const LocatorPathElt *elt) {
     return elt->getKind() == ConstraintLocator::SynthesizedArgument;

lib/Sema/CSBindings.cpp

@@ -1652,20 +1652,32 @@ bool TypeVariableBinding::attempt(ConstraintSystem &cs) const {
 
   cs.addConstraint(ConstraintKind::Bind, TypeVar, type, srcLocator);
 
+  auto reportHole = [&]() {
+    if (cs.isForCodeCompletion()) {
+      // Don't penalize solutions with unresolved generics.
+      if (TypeVar->getImpl().getGenericParameter())
+        return false;
+      // Don't penalize solutions with holes due to missing arguments after the
+      // code completion position.
+      auto argLoc = srcLocator->findLast<LocatorPathElt::SynthesizedArgument>();
+      if (argLoc && argLoc->isAfterCodeCompletionLoc())
+        return false;
+    }
+    cs.increaseScore(SK_Hole);
+
+    if (auto fix = fixForHole(cs)) {
+      if (cs.recordFix(/*fix=*/fix->first, /*impact=*/fix->second))
+        return true;
+    }
+    return false;
+  };
+
   // If this was from a defaultable binding note that.
   if (Binding.isDefaultableBinding()) {
     cs.DefaultedConstraints.push_back(srcLocator);
 
-    if (type->isHole()) {
-      // Reflect in the score that this type variable couldn't be
-      // resolved and had to be bound to a placeholder "hole" type.
-      cs.increaseScore(SK_Hole);
-
-      if (auto fix = fixForHole(cs)) {
-        if (cs.recordFix(/*fix=*/fix->first, /*impact=*/fix->second))
-          return true;
-      }
-    }
+    if (type->isHole() && reportHole())
+      return true;
   }
 
   return !cs.failedConstraint && !cs.simplify();

lib/Sema/CSSimplify.cpp

@@ -986,8 +986,35 @@ constraints::matchCallArguments(
   };
 }
 
-static Optional<unsigned>
-getCompletionArgIndex(ASTNode anchor, ConstraintSystem &CS) {
+struct CompletionArgInfo {
+  unsigned completionIdx;
+  Optional<unsigned> firstTrailingIdx;
+
+  bool isAllowableMissingArg(unsigned argInsertIdx,
+                             AnyFunctionType::Param param) {
+    // If the argument is before or at the index of the argument containing the
+    // completion, the user would likely have already written it if they
+    // intended this overload.
+    if (completionIdx >= argInsertIdx)
+      return false;
+
+    // If the argument is after the first trailing closure, the user can only
+    // continue on to write more trailing arguments, so only allow this overload
+    // if the missing argument is of function type.
+    if (firstTrailingIdx && argInsertIdx > *firstTrailingIdx) {
+      if (param.isInOut())
+        return false;
+
+      Type expectedTy = param.getPlainType()->lookThroughAllOptionalTypes();
+      return expectedTy->is<FunctionType>() || expectedTy->isAny() ||
+          expectedTy->isTypeVariableOrMember();
+    }
+    return true;
+  }
+};
+
+static Optional<CompletionArgInfo>
+getCompletionArgInfo(ASTNode anchor, ConstraintSystem &CS) {
   Expr *arg = nullptr;
   if (auto *CE = getAsExpr<CallExpr>(anchor))
     arg = CE->getArg();
@@ -999,16 +1026,15 @@ getCompletionArgIndex(ASTNode anchor, ConstraintSystem &CS) {
   if (!arg)
     return None;
 
-  if (auto *TE = dyn_cast<TupleExpr>(arg)) {
-    auto elems = TE->getElements();
-    auto idx = llvm::find_if(elems, [&](Expr *elem) {
-      return CS.containsCodeCompletionLoc(elem);
-    });
-    if (idx != elems.end())
-      return std::distance(elems.begin(), idx);
-  } else if (auto *PE = dyn_cast<ParenExpr>(arg)) {
+  auto trailingIdx = arg->getUnlabeledTrailingClosureIndexOfPackedArgument();
+  if (auto *PE = dyn_cast<ParenExpr>(arg)) {
     if (CS.containsCodeCompletionLoc(PE->getSubExpr()))
-      return 0;
+      return CompletionArgInfo{ 0, trailingIdx };
+  } else if (auto *TE = dyn_cast<TupleExpr>(arg)) {
+    for (unsigned i: indices(TE->getElements())) {
+      if (CS.containsCodeCompletionLoc(TE->getElement(i)))
+        return CompletionArgInfo{ i, trailingIdx };
+    }
   }
   return None;
 }
@@ -1021,7 +1047,7 @@ class ArgumentFailureTracker : public MatchCallArgumentListener {
 
   SmallVector<SynthesizedArg, 4> MissingArguments;
   SmallVector<std::pair<unsigned, AnyFunctionType::Param>, 4> ExtraArguments;
-  Optional<unsigned> CompletionArgIdx;
+  Optional<CompletionArgInfo> CompletionArgInfo;
 
 public:
   ArgumentFailureTracker(ConstraintSystem &cs,
@@ -1048,10 +1074,19 @@ class ArgumentFailureTracker : public MatchCallArgumentListener {
     const auto &param = Parameters[paramIdx];
 
     unsigned newArgIdx = Arguments.size();
+
+    bool isAfterCodeCompletionLoc = false;
+    if (CS.isForCodeCompletion()) {
+      if (!CompletionArgInfo)
+        CompletionArgInfo = getCompletionArgInfo(Locator.getAnchor(), CS);
+      isAfterCodeCompletionLoc = CompletionArgInfo &&
+        CompletionArgInfo->isAllowableMissingArg(argInsertIdx, param);
+    }
+
     auto *argLoc = CS.getConstraintLocator(
         Locator, {LocatorPathElt::ApplyArgToParam(newArgIdx, paramIdx,
                                                   param.getParameterFlags()),
-                  LocatorPathElt::SynthesizedArgument(newArgIdx)});
+                  LocatorPathElt::SynthesizedArgument(newArgIdx, isAfterCodeCompletionLoc)});
 
     auto *argType =
         CS.createTypeVariable(argLoc, TVO_CanBindToInOut | TVO_CanBindToLValue |
@@ -1060,16 +1095,12 @@ class ArgumentFailureTracker : public MatchCallArgumentListener {
     auto synthesizedArg = param.withType(argType);
     Arguments.push_back(synthesizedArg);
 
-    if (CS.isForCodeCompletion()) {
-      // When solving for code completion, if any argument contains the
-      // completion location, later arguments shouldn't be considered missing
-      // (causing the solution to have a worse score) as the user just hasn't
-      // written them yet. Early exit to avoid recording them in this case.
-      if (!CompletionArgIdx)
-        CompletionArgIdx = getCompletionArgIndex(Locator.getAnchor(), CS);
-      if (CompletionArgIdx && *CompletionArgIdx < argInsertIdx)
-        return newArgIdx;
-    }
+    // When solving for code completion, if any argument contains the
+    // completion location, later arguments shouldn't be considered missing
+    // (causing the solution to have a worse score) as the user just hasn't
+    // written them yet. Early exit to avoid recording them in this case.
+    if (isAfterCodeCompletionLoc)
+      return newArgIdx;
 
     MissingArguments.push_back(SynthesizedArg{paramIdx, synthesizedArg});
 

test/IDE/complete_ambiguous.swift

@@ -48,6 +48,8 @@
 // RUN: %swift-ide-test -code-completion  -source-filename %s -code-completion-token=REGULAR_MULTICLOSURE_APPLIED | %FileCheck %s --check-prefix=POINT_MEMBER
 // RUN: %swift-ide-test -code-completion  -source-filename %s -code-completion-token=BEST_SOLUTION_FILTER | %FileCheck %s --check-prefix=BEST_SOLUTION_FILTER
 // RUN: %swift-ide-test -code-completion  -source-filename %s -code-completion-token=BEST_SOLUTION_FILTER2 | %FileCheck %s --check-prefix=BEST_SOLUTION_FILTER
+// RUN: %swift-ide-test -code-completion  -source-filename %s -code-completion-token=MISSINGARG_INLINE | %FileCheck %s --check-prefix=MISSINGARG_INLINE
+// RUN: %swift-ide-test -code-completion  -source-filename %s -code-completion-token=MISSINGARG_TRAILING | %FileCheck %s --check-prefix=MISSINGARG_TRAILING
 
 
 struct A {
@@ -313,9 +315,55 @@ func testMissingArgs() {
 
   test3(after: Test.#^OVERLOADEDFUNC_MISSINGARG_BEFORE^#);
   test4(both: Test.#^OVERLOADEDFUNC_MISSINGARG_BEFOREANDAFTER^#)
-}
-
 
+  enum Bop { case bop }
+  enum Bix { case bix }
+  enum Blu { case blu }
+  enum Baz { case baz }
+  enum Boy { case boy }
+
+  func trailing(x: Int, _ y: () -> Foo, z: () -> ())  {}
+  func trailing(x: Int, _ y: () -> Bar, z: (() -> ())?)  {}
+  func trailing(x: Int, _ y: () -> Bop, z: Any)  {}
+  func trailing<T>(x: Int, _ y: () -> Bix, z: T) {}
+  func trailing<T>(x: Int, _ y: () -> Boy, z: T?) {}
+  func trailing<T>(x: Int, _ y: () -> Blu, z: [T]?) {}
+  func trailing(x: Int, _ y: () -> Blu, z: inout Any)  {}
+  func trailing(x: Int, _ y: () -> Baz, z: Int)  {}
+
+  trailing(x: 2, { .#^MISSINGARG_INLINE^# })
+  trailing(x: 2) { .#^MISSINGARG_TRAILING^# }
+
+  // MISSINGARG_INLINE: Begin completions, 14 items
+  // MISSINGARG_INLINE-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: foo[#Foo#]; name=foo
+  // MISSINGARG_INLINE-DAG: Decl[InstanceMethod]/CurrNominal:   hash({#(self): Foo#})[#(into: inout Hasher) -> Void#]; name=hash(self: Foo)
+  // MISSINGARG_INLINE-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: bar[#Bar#]; name=bar
+  // MISSINGARG_INLINE-DAG: Decl[InstanceMethod]/CurrNominal:   hash({#(self): Bar#})[#(into: inout Hasher) -> Void#]; name=hash(self: Bar)
+  // MISSINGARG_INLINE-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: bop[#Bop#]; name=bop
+  // MISSINGARG_INLINE-DAG: Decl[InstanceMethod]/CurrNominal:   hash({#(self): Bop#})[#(into: inout Hasher) -> Void#]; name=hash(self: Bop)
+  // MISSINGARG_INLINE-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: bix[#Bix#]; name=bix
+  // MISSINGARG_INLINE-DAG: Decl[InstanceMethod]/CurrNominal:   hash({#(self): Bix#})[#(into: inout Hasher) -> Void#]; name=hash(self: Bix)
+  // MISSINGARG_INLINE-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: boy[#Boy#]; name=boy
+  // MISSINGARG_INLINE-DAG: Decl[InstanceMethod]/CurrNominal:   hash({#(self): Boy#})[#(into: inout Hasher) -> Void#]; name=hash(self: Boy)
+  // MISSINGARG_INLINE-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: blu[#Blu#]; name=blu
+  // MISSINGARG_INLINE-DAG: Decl[InstanceMethod]/CurrNominal:   hash({#(self): Blu#})[#(into: inout Hasher) -> Void#]; name=hash(self: Blu)
+  // MISSINGARG_INLINE-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: baz[#Baz#]; name=baz
+  // MISSINGARG_INLINE-DAG: Decl[InstanceMethod]/CurrNominal:   hash({#(self): Baz#})[#(into: inout Hasher) -> Void#]; name=hash(self: Baz)
+  // MISSINGARG_INLINE: End completions
+
+  // MISSINGARG_TRAILING: Begin completions, 10 items
+  // MISSINGARG_TRAILING-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: foo[#Foo#]; name=foo
+  // MISSINGARG_TRAILING-DAG: Decl[InstanceMethod]/CurrNominal:   hash({#(self): Foo#})[#(into: inout Hasher) -> Void#]; name=hash(self: Foo)
+  // MISSINGARG_TRAILING-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: bar[#Bar#]; name=bar
+  // MISSINGARG_TRAILING-DAG: Decl[InstanceMethod]/CurrNominal:   hash({#(self): Bar#})[#(into: inout Hasher) -> Void#]; name=hash(self: Bar)
+  // MISSINGARG_TRAILING-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: bop[#Bop#]; name=bop
+  // MISSINGARG_TRAILING-DAG: Decl[InstanceMethod]/CurrNominal:   hash({#(self): Bop#})[#(into: inout Hasher) -> Void#]; name=hash(self: Bop)
+  // MISSINGARG_TRAILING-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: bix[#Bix#]; name=bix
+  // MISSINGARG_TRAILING-DAG: Decl[InstanceMethod]/CurrNominal:   hash({#(self): Bix#})[#(into: inout Hasher) -> Void#]; name=hash(self: Bix)
+  // MISSINGARG_TRAILING-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: boy[#Boy#]; name=boy
+  // MISSINGARG_TRAILING-DAG: Decl[InstanceMethod]/CurrNominal:   hash({#(self): Boy#})[#(into: inout Hasher) -> Void#]; name=hash(self: Boy)
+  // MISSINGARG_TRAILING: End completions
+}
 
 protocol C {
   associatedtype Element
@@ -450,7 +498,7 @@ struct Struct123: Equatable {
 func testNoBestSolutionFilter() {
   let a = Struct123();
   let b = [Struct123]().first(where: { $0 == a && 1 + 90 * 5 / 8 == 45 * -10 })?.structMem != .#^BEST_SOLUTION_FILTER^#
-  let c = min(10.3, 10 / 10.4) < 6 + 5 / (10 - 3) ? true : Optional(a)?.structMem != .#^BEST_SOLUTION_FILTER2^#
+  let c = min(10.3, 10 / 10.4) < 6 / 7 ? true : Optional(a)?.structMem != .#^BEST_SOLUTION_FILTER2^#
 }
 
 // BEST_SOLUTION_FILTER: Begin completions