Merge pull request #35136 from nathawes/dont-allow-missing-args-if-trailing-and-not-function-type

Author: swift-ci

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

@@ -1697,20 +1697,34 @@ 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;
+    }
+    // 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;
+    }
+    return false;
+  };
+
   // If this was from a defaultable binding note that.
   if (Binding.isDefaultableBinding()) {
     cs.DefaultedConstraints.push_back(srcLocator);
 
-    if (type->isPlaceholder()) {
-      // 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->isPlaceholder() && 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