Merge pull request #36538 from ahoppen/pr/complete-defaulted-labels

[CodeComplete] Match argument labels when completing function arguments

Author: ahoppen

lib/IDE/ExprContextAnalysis.cpp

@@ -745,6 +745,79 @@ static bool getPositionInArgs(DeclContext &DC, Expr *Args, Expr *CCExpr,
   return false;
 }
 
+/// Get index of \p CCExpr in \p Params. Note that the position in \p Params may
+/// be different than the position in \p Args if there are defaulted arguments
+/// in \p Params which don't occur in \p Args.
+///
+/// \returns \c true if success, \c false if \p CCExpr is not a part of \p Args.
+static bool getPositionInParams(DeclContext &DC, Expr *Args, Expr *CCExpr,
+                                ArrayRef<AnyFunctionType::Param> Params,
+                                unsigned &PosInParams) {
+  if (isa<ParenExpr>(Args)) {
+    PosInParams = 0;
+    return true;
+  }
+
+  auto *tuple = dyn_cast<TupleExpr>(Args);
+  if (!tuple) {
+    return false;
+  }
+
+  auto &SM = DC.getASTContext().SourceMgr;
+  PosInParams = 0;
+  unsigned PosInArgs = 0;
+  bool LastParamWasVariadic = false;
+  // We advance PosInArgs until we find argument that is after the code
+  // completion token, which is when we stop.
+  // For each argument, we try to find a matching parameter either by matching
+  // argument labels, in which case PosInParams may be advanced by more than 1,
+  // or by advancing PosInParams and PosInArgs both by 1.
+  for (; PosInArgs < tuple->getNumElements(); ++PosInArgs) {
+    if (!SM.isBeforeInBuffer(tuple->getElement(PosInArgs)->getEndLoc(),
+                             CCExpr->getStartLoc())) {
+      // The arg is after the code completion position. Stop.
+      break;
+    }
+
+    auto ArgName = tuple->getElementName(PosInArgs);
+    // If the last parameter we matched was variadic, we claim all following
+    // unlabeled arguments for that variadic parameter -> advance PosInArgs but
+    // not PosInParams.
+    if (LastParamWasVariadic && ArgName.empty()) {
+      continue;
+    } else {
+      LastParamWasVariadic = false;
+    }
+
+    // Look for a matching parameter label.
+    bool FoundLabelMatch = false;
+    for (unsigned i = PosInParams; i < Params.size(); ++i) {
+      if (Params[i].getLabel() == ArgName) {
+        // We have found a label match. Advance the position in the params
+        // to point to the param after the one with this label.
+        PosInParams = i + 1;
+        FoundLabelMatch = true;
+        if (Params[i].isVariadic()) {
+          LastParamWasVariadic = true;
+        }
+        break;
+      }
+    }
+
+    if (!FoundLabelMatch) {
+      // We haven't found a matching argument label. Assume the current one is
+      // named incorrectly and advance by one.
+      ++PosInParams;
+    }
+  }
+  if (PosInArgs < tuple->getNumElements() && PosInParams < Params.size()) {
+    // We didn't search until the end, so we found a position in Params. Success
+    return true;
+  } else {
+    return false;
+  }
+}
+
 /// Given an expression and its context, the analyzer tries to figure out the
 /// expected type of the expression by analyzing its context.
 class ExprContextAnalyzer {
@@ -791,14 +864,12 @@ class ExprContextAnalyzer {
     PossibleCallees.assign(Candidates.begin(), Candidates.end());
 
     // Determine the position of code completion token in call argument.
-    unsigned Position;
+    unsigned PositionInArgs;
     bool HasName;
-    if (!getPositionInArgs(*DC, Arg, ParsedExpr, Position, HasName))
+    if (!getPositionInArgs(*DC, Arg, ParsedExpr, PositionInArgs, HasName))
       return false;
 
     // Collect possible types (or labels) at the position.
-    // FIXME: Take variadic and optional parameters into account. We need to do
-    //        something equivalent to 'constraints::matchCallArguments'
     {
       bool MayNeedName = !HasName && !E->isImplicit() &&
                          (isa<CallExpr>(E) | isa<SubscriptExpr>(E) ||
@@ -811,13 +882,22 @@ class ExprContextAnalyzer {
           memberDC = typeAndDecl.Decl->getInnermostDeclContext();
 
         auto Params = typeAndDecl.Type->getParams();
+        unsigned PositionInParams;
+        if (!getPositionInParams(*DC, Arg, ParsedExpr, Params,
+                                 PositionInParams)) {
+          // If the argument doesn't have a matching position in the parameters,
+          // indicate that with optional nullptr param.
+          if (seenArgs.insert({Identifier(), CanType()}).second)
+            recordPossibleParam(nullptr, /*isRequired=*/false);
+          continue;
+        }
         ParameterList *paramList = nullptr;
         if (auto VD = typeAndDecl.Decl) {
           paramList = getParameterList(VD);
           if (paramList && paramList->size() != Params.size())
             paramList = nullptr;
         }
-        for (auto Pos = Position; Pos < Params.size(); ++Pos) {
+        for (auto Pos = PositionInParams; Pos < Params.size(); ++Pos) {
           const auto &paramType = Params[Pos];
           Type ty = paramType.getPlainType();
           if (memberDC && ty->hasTypeParameter())
@@ -843,12 +923,6 @@ class ExprContextAnalyzer {
           if (!canSkip)
             break;
         }
-        // If the argument position is out of expeceted number, indicate that
-        // with optional nullptr param.
-        if (Position >= Params.size()) {
-          if (seenArgs.insert({Identifier(), CanType()}).second)
-            recordPossibleParam(nullptr, /*isRequired=*/false);
-        }
       }
     }
     return !PossibleTypes.empty() || !PossibleParams.empty();

test/IDE/complete_call_arg.swift

@@ -116,6 +116,10 @@
 
 // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=LABEL_IN_SELF_DOT_INIT | %FileCheck %s -check-prefix=LABEL_IN_SELF_DOT_INIT
 
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=MISSING_REQUIRED_PARAM | %FileCheck %s -check-prefix=MISSING_REQUIRED_PARAM
+
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=NAMED_PARAMETER_WITH_LEADING_VARIADIC | %FileCheck %s -check-prefix=NAMED_PARAMETER_WITH_LEADING_VARIADIC
+
 var i1 = 1
 var i2 = 2
 var oi1 : Int?
@@ -700,9 +704,7 @@ func testStaticMemberCall() {
 // STATIC_METHOD_SECOND: End completions
 
   let _ = TestStaticMemberCall.create2(1, arg3: 2, #^STATIC_METHOD_SKIPPED^#)
-// STATIC_METHOD_SKIPPED: Begin completions, 2 items
-// FIXME: 'arg3' shouldn't be suggested.
-// STATIC_METHOD_SKIPPED: Pattern/ExprSpecific: {#arg3: Int#}[#Int#];
+// STATIC_METHOD_SKIPPED: Begin completions, 1 item
 // STATIC_METHOD_SKIPPED: Pattern/ExprSpecific: {#arg4: Int#}[#Int#];
 // STATIC_METHOD_SKIPPED: End completions
 
@@ -739,9 +741,7 @@ func testImplicitMember() {
 // IMPLICIT_MEMBER_SECOND: End completions
 
   let _: TestStaticMemberCall = .create2(1, arg3: 2, #^IMPLICIT_MEMBER_SKIPPED^#)
-// IMPLICIT_MEMBER_SKIPPED: Begin completions, 2 items
-// FIXME: 'arg3' shouldn't be suggested.
-// IMPLICIT_MEMBER_SKIPPED: Pattern/ExprSpecific: {#arg3: Int#}[#Int#];
+// IMPLICIT_MEMBER_SKIPPED: Begin completions, 1 item
 // IMPLICIT_MEMBER_SKIPPED: Pattern/ExprSpecific: {#arg4: Int#}[#Int#];
 // IMPLICIT_MEMBER_SKIPPED: End completions
 
@@ -923,3 +923,27 @@ func testLabelsInSelfDotInit() {
     }
   }
 }
+
+func testMissingRequiredParameter() {
+  class C {
+    func foo(x: Int, y: Int, z: Int)  {}
+  }
+  func test(c: C) {
+    c.foo(y: 1, #^MISSING_REQUIRED_PARAM^#)
+// MISSING_REQUIRED_PARAM: Begin completions, 1 item
+// MISSING_REQUIRED_PARAM-DAG: Pattern/ExprSpecific:               {#z: Int#}[#Int#]
+// MISSING_REQUIRED_PARAM: End completions
+  }
+}
+
+func testAfterVariadic() {
+  class C {
+    func foo(x: Int..., y: Int, z: Int)  {}
+  }
+  func test(c: C) {
+    c.foo(x: 10, 20, 30, y: 40, #^NAMED_PARAMETER_WITH_LEADING_VARIADIC^#)
+// NAMED_PARAMETER_WITH_LEADING_VARIADIC: Begin completions, 1 item
+// NAMED_PARAMETER_WITH_LEADING_VARIADIC-DAG: Pattern/ExprSpecific:               {#z: Int#}[#Int#]
+// NAMED_PARAMETER_WITH_LEADING_VARIADIC: End completions
+  }
+}

test/IDE/complete_call_as_function.swift

@@ -103,10 +103,8 @@ func testCallAsFunctionOverloaded(fn: Functor) {
 //OVERLOADED_PAREN: End completions
 
   fn(h: .left, #^OVERLOADED_ARG2_LABEL^#)
-// FIXME: Should only suggest 'v:' (rdar://problem/60346573).
-//OVERLOADED_ARG2_LABEL: Begin completions, 2 items
+//OVERLOADED_ARG2_LABEL: Begin completions, 1 item
 //OVERLOADED_ARG2_LABEL-DAG: Pattern/ExprSpecific:               {#v: Functor.Vertical#}[#Functor.Vertical#];
-//OVERLOADED_ARG2_LABEL-DAG: Pattern/ExprSpecific:               {#h: Functor.Horizontal#}[#Functor.Horizontal#];
 //OVERLOADED_ARG2_LABEL: End completions
 
   fn(h: .left, v: .#^OVERLOADED_ARG2_VALUE^#)