Merge pull request #37432 from rintaro/ide-completion-rdar77867723

[CodeCompletion] Don't suggest argument labels from unapplicable overload

Author: rintaro

lib/IDE/ExprContextAnalysis.cpp

@@ -771,22 +771,22 @@ static Expr *getArgAtPosition(Expr *Args, unsigned Position) {
 /// 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) {
+/// \returns the position index number on success, \c None if \p CCExpr is not
+/// a part of \p Args.
+static Optional<unsigned>
+getPositionInParams(DeclContext &DC, Expr *Args, Expr *CCExpr,
+                    ArrayRef<AnyFunctionType::Param> Params, bool Lenient) {
   if (isa<ParenExpr>(Args)) {
-    PosInParams = 0;
-    return true;
+    return 0;
   }
 
   auto *tuple = dyn_cast<TupleExpr>(Args);
   if (!tuple) {
-    return false;
+    return None;
   }
 
   auto &SM = DC.getASTContext().SourceMgr;
-  PosInParams = 0;
+  unsigned PosInParams = 0;
   unsigned PosInArgs = 0;
   bool LastParamWasVariadic = false;
   // We advance PosInArgs until we find argument that is after the code
@@ -847,16 +847,22 @@ static bool getPositionInParams(DeclContext &DC, Expr *Args, Expr *CCExpr,
     }
 
     if (!AdvancedPosInParams) {
-      // We haven't performed any special advance logic. Assume the argument
-      // and parameter match, so advance PosInParams by 1.
-      ++PosInParams;
+      if (Lenient) {
+        // We haven't performed any special advance logic. Assume the argument
+        // and parameter match, so advance PosInParams by 1.
+        PosInParams += 1;
+      } else {
+        // If there is no matching argument label. These arguments can't be
+        // applied to the params.
+        return None;
+      }
     }
   }
   if (PosInArgs < tuple->getNumElements() && PosInParams < Params.size()) {
     // We didn't search until the end, so we found a position in Params. Success
-    return true;
+    return PosInParams;
   } else {
-    return false;
+    return None;
   }
 }
 
@@ -947,21 +953,45 @@ class ExprContextAnalyzer {
       }
       SmallPtrSet<CanType, 4> seenTypes;
       llvm::SmallSet<std::pair<Identifier, CanType>, 4> seenArgs;
-      for (auto &typeAndDecl : Candidates) {
-        DeclContext *memberDC = nullptr;
-        if (typeAndDecl.Decl)
-          memberDC = typeAndDecl.Decl->getInnermostDeclContext();
+      llvm::SmallVector<Optional<unsigned>, 2> posInParams;
+      {
+        bool found = false;
+        for (auto &typeAndDecl : Candidates) {
+          Optional<unsigned> pos = getPositionInParams(
+              *DC, Args, ParsedExpr, typeAndDecl.Type->getParams(),
+              /*lenient=*/false);
+          posInParams.push_back(pos);
+          found |= pos.hasValue();
+        }
+        if (!found) {
+          // If applicable overload is not found, retry with considering
+          // non-matching argument labels mis-typed.
+          for (auto i : indices(Candidates)) {
+            posInParams[i] = getPositionInParams(
+                *DC, Args, ParsedExpr, Candidates[i].Type->getParams(),
+                /*lenient=*/true);
+          }
+        }
+      }
+      assert(posInParams.size() == Candidates.size());
 
-        auto Params = typeAndDecl.Type->getParams();
-        unsigned PositionInParams;
-        if (!getPositionInParams(*DC, Args, ParsedExpr, Params,
-                                 PositionInParams)) {
+      for (auto i : indices(Candidates)) {
+        if (!posInParams[i].hasValue()) {
           // 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;
         }
+
+        auto &typeAndDecl = Candidates[i];
+        DeclContext *memberDC = nullptr;
+        if (typeAndDecl.Decl)
+          memberDC = typeAndDecl.Decl->getInnermostDeclContext();
+
+        auto Params = typeAndDecl.Type->getParams();
+        auto PositionInParams = *posInParams[i];
+
         ParameterList *paramList = nullptr;
         if (auto VD = typeAndDecl.Decl) {
           paramList = getParameterList(VD);

test/IDE/complete_call_arg.swift

@@ -933,3 +933,23 @@ func testGenericConstructor() {
 // GENERIC_INITIALIZER-DAG: Pattern/ExprSpecific:               {#text: String#}[#String#]
 // GENERIC_INITIALIZER: End completions
 }
+
+struct Rdar77867723 {
+  enum Horizontal { case east, west }
+  enum Vertical { case up, down }
+  func fn(aaa: Horizontal = .east, bbb: Vertical = .up) {}
+  func fn(ccc: Vertical = .up, ddd: Horizontal = .west) {}
+  func test1 {
+    self.fn(ccc: .up, #^OVERLOAD_LABEL1^#)
+// OVERLOAD_LABEL1: Begin completions, 1 items
+// OVERLOAD_LABEL1-DAG: Pattern/ExprSpecific:               {#ddd: Horizontal#}[#Horizontal#];
+// OVERLOAD_LABEL1: End completions
+  }
+  func test2 {
+    self.fn(eee: .up, #^OVERLOAD_LABEL2^#)
+// OVERLOAD_LABEL2: Begin completions, 2 items
+// OVERLOAD_LABEL2-DAG: Pattern/ExprSpecific:               {#bbb: Vertical#}[#Vertical#];
+// OVERLOAD_LABEL2-DAG: Pattern/ExprSpecific:               {#ddd: Horizontal#}[#Horizontal#];
+// OVERLOAD_LABEL2: End completions
+  }
+}

test/SourceKit/CodeComplete/complete_type_match.swift

@@ -14,32 +14,32 @@ func takeIntOpt(x: Int, y: Int?)
 func takeString(x: Int, y: String)
 func takeAny(x: Int, y: Any)
 
-takeInt(1, y: #^TOP_LEVEL_0^#)
+takeInt(x: 1, y: #^TOP_LEVEL_0^#)
 // TOP_LEVEL_0-NOT: nil
 // TOP_LEVEL_0: valueZ
 // TOP_LEVEL_0: Int
 // TOP_LEVEL_0: valueA
 // TOP_LEVEL_0: valueS
 
-takeString(1, y: #^TOP_LEVEL_1^#)
+takeString(x: 1, y: #^TOP_LEVEL_1^#)
 // TOP_LEVEL_1: valueS
 // TOP_LEVEL_1: String
 // TOP_LEVEL_1: valueA
 // TOP_LEVEL_1: valueZ
 
-takeAny(1, y: #^TOP_LEVEL_2^#)
+takeAny(x: 1, y: #^TOP_LEVEL_2^#)
 // TOP_LEVEL_2: valueA
 // TOP_LEVEL_2: valueS
 // TOP_LEVEL_2: valueZ
 
-takeIntOpt(1, y: #^TOP_LEVEL_3^#)
+takeIntOpt(x: 1, y: #^TOP_LEVEL_3^#)
 // TOP_LEVEL_3: nil
 // TOP_LEVEL_3: valueZ
 // TOP_LEVEL_3: valueA
 // TOP_LEVEL_3: valueS
 
 func testCrossContext(x: Int, y: String, z: Any) {
-  takeInt(1, y: #^CROSS_CONTEXT_0^#)
+  takeInt(x: 1, y: #^CROSS_CONTEXT_0^#)
 }
 // CROSS_CONTEXT_0: x
 // CROSS_CONTEXT_0: valueZ
@@ -56,7 +56,7 @@ struct FromMethod {
 }
 
 func testFromMethod(x: FromMethod) {
-  takeInt(1, y: x.#^FROM_METHOD_0^#)
+  takeInt(x: 1, y: x.#^FROM_METHOD_0^#)
 }
 // FROM_METHOD_0: valueZ()
 // FROM_METHOD_0: valueA()