[CodeComplete] Suggest single argument labels if code completion is invoked at start of function call with exiting parameters

This removes the distinction between argument completions and postfix expr paren completions, which was meaningless since solver-based completion.

It then determines whether to suggest the entire function call pattern (with all argument labels) or only a single argument based on whether there are any existing arguments in the call.

For this to work properly, we need to improve parser recovery a little bit so that it parsers arguments after the code completion token properly.

This should make call pattern heuristics obsolete.

rdar://84809503

Author: ahoppen

include/swift/AST/DiagnosticsParse.def

@@ -1364,6 +1364,8 @@ ERROR(expected_rparen_expr_list,none,
 ERROR(expected_rsquare_expr_list,none,
       "expected ']' in expression list", ())
 
+ERROR(expected_label_before_colon,none, "expected argument label before colon", ())
+
 // Array literal expressions
 ERROR(expected_rsquare_array_expr,PointsToFirstBadToken,
       "expected ']' in container literal expression", ())

include/swift/IDE/ArgumentCompletion.h

@@ -52,6 +52,14 @@ class ArgumentTypeCheckCompletionCallback : public TypeCheckCompletionCallback {
     /// True if the completion is a noninitial term in a variadic argument.
     bool IsNoninitialVariadic;
 
+    /// Whether to suggest the entire functions signature instead of a single
+    /// argument for this result.
+    ///
+    /// This is the case if there are no arguments or labels in the call yet and
+    /// causes us to add a completion result that completes `Point(|)` to
+    /// `Point(x: <Int>, y: <Int>)`.
+    bool IncludeSignature;
+
     /// The base type of the call/subscript (null for free functions).
     Type BaseType;
 
@@ -106,14 +114,10 @@ class ArgumentTypeCheckCompletionCallback : public TypeCheckCompletionCallback {
                                       DeclContext *DC)
       : CompletionExpr(CompletionExpr), DC(DC) {}
 
-  /// \param IncludeSignature Whether to include a suggestion for the entire
-  /// function signature instead of suggesting individual labels. Used when
-  /// completing after the opening '(' of a function call \param Loc The
-  /// location of the code completion token
   /// \param IsLabeledTrailingClosure Whether we are completing the label of a
   /// labeled trailing closure, ie. if the code completion location is outside
   /// the call after the first trailing closure of the call.
-  void collectResults(bool IncludeSignature, bool IsLabeledTrailingClosure,
+  void collectResults(bool IsLabeledTrailingClosure,
                       SourceLoc Loc, DeclContext *DC,
                       CodeCompletionContext &CompletionCtx);
 };

include/swift/IDE/CodeCompletionResult.h

@@ -195,7 +195,6 @@ enum class CompletionKind : uint8_t {
   StmtOrExpr,
   PostfixExprBeginning,
   PostfixExpr,
-  PostfixExprParen,
   KeyPathExprObjC,
   KeyPathExprSwift,
   TypeDeclResultBeginning,

include/swift/Parse/IDEInspectionCallbacks.h

@@ -170,10 +170,6 @@ class CodeCompletionCallbacks {
   /// is completing after set as its base.
   virtual void completePostfixExpr(CodeCompletionExpr *E, bool hasSpace){};
 
-  /// Complete a given expr-postfix, given that there is a following
-  /// left parenthesis.
-  virtual void completePostfixExprParen(Expr *E, Expr *CodeCompletionE) {};
-
   /// Complete the argument to an Objective-C #keyPath
   /// expression.
   ///

include/swift/Parse/Parser.h

@@ -1869,6 +1869,8 @@ class Parser {
   parseArgumentList(tok leftTok, tok rightTok, bool isExprBasic,
                     bool allowTrailingClosure = true);
 
+  ParserStatus parseExprListElement(tok rightTok, bool isArgumentList, SourceLoc leftLoc, SmallVectorImpl<ExprListElt> &elts);
+
   /// Parse one or more trailing closures after an argument list.
   ParserStatus parseTrailingClosures(bool isExprBasic, SourceRange calleeRange,
                                      SmallVectorImpl<Argument> &closures);

lib/IDE/ArgumentCompletion.cpp

@@ -278,10 +278,18 @@ void ArgumentTypeCheckCompletionCallback::sawSolutionImpl(const Solution &S) {
     }
   }
 
+  bool IncludeSignature = false;
+  if (ParentCall->getArgs()->getUnlabeledUnaryExpr() == CompletionExpr) {
+    // If the code completion expression is the only expression in the call
+    // and the code completion token doesn’t have a label, we have a case like
+    // `Point(|)`. Suggest the entire function signature.
+    IncludeSignature = true;
+  }
+
   Results.push_back(
-      {ExpectedTy, ExpectedCallType, isa<SubscriptExpr>(ParentCall),
+      {ExpectedTy,  ExpectedCallType, isa<SubscriptExpr>(ParentCall),
        Info.getValue(), FuncTy, ArgIdx, ParamIdx, std::move(ClaimedParams),
-       IsNoninitialVariadic, Info.BaseTy, HasLabel, FirstTrailingClosureIndex,
+       IsNoninitialVariadic, IncludeSignature, Info.BaseTy, HasLabel, FirstTrailingClosureIndex,
        IsAsync, DeclParamIsOptional, SolutionSpecificVarTypes});
 }
 
@@ -316,7 +324,7 @@ void ArgumentTypeCheckCompletionCallback::computeShadowedDecls(
 }
 
 void ArgumentTypeCheckCompletionCallback::collectResults(
-    bool IncludeSignature, bool IsLabeledTrailingClosure, SourceLoc Loc,
+    bool IsLabeledTrailingClosure, SourceLoc Loc,
     DeclContext *DC, ide::CodeCompletionContext &CompletionCtx) {
   ASTContext &Ctx = DC->getASTContext();
   CompletionLookup Lookup(CompletionCtx.getResultSink(), Ctx, DC,
@@ -333,13 +341,14 @@ void ArgumentTypeCheckCompletionCallback::collectResults(
   }
 
   SmallVector<Type, 8> ExpectedTypes;
+  SmallVector<PossibleParamInfo, 8> Params;
 
-  if (IncludeSignature && !Results.empty()) {
-    Lookup.setHaveLParen(true);
-    Lookup.setExpectedTypes(ExpectedCallTypes,
-                            /*isImplicitSingleExpressionReturn=*/false);
+  for (auto &Result : Results) {
+    if (Result.IncludeSignature) {
+      Lookup.setHaveLParen(true);
+      Lookup.setExpectedTypes(ExpectedCallTypes,
+                              /*isImplicitSingleExpressionReturn=*/false);
 
-    for (auto &Result : Results) {
       auto SemanticContext = SemanticContextKind::None;
       NominalTypeDecl *BaseNominal = nullptr;
       if (Result.BaseType) {
@@ -383,19 +392,16 @@ void ArgumentTypeCheckCompletionCallback::collectResults(
           }
         }
       }
-    }
-    Lookup.setHaveLParen(false);
-
-    shouldPerformGlobalCompletion |=
-        !Lookup.FoundFunctionCalls || Lookup.FoundFunctionsWithoutFirstKeyword;
-  } else if (!Results.empty()) {
-    SmallVector<PossibleParamInfo, 8> Params;
-    for (auto &Ret : Results) {
+      Lookup.setHaveLParen(false);
+      // We didn't find any function signatures. Perform global completion as a fallback.
+      shouldPerformGlobalCompletion |=
+          !Lookup.FoundFunctionCalls || Lookup.FoundFunctionsWithoutFirstKeyword;
+    } else {
       shouldPerformGlobalCompletion |=
-          addPossibleParams(Ret, Params, ExpectedTypes);
+          addPossibleParams(Result, Params, ExpectedTypes);
     }
-    Lookup.addCallArgumentCompletionResults(Params, IsLabeledTrailingClosure);
   }
+  Lookup.addCallArgumentCompletionResults(Params, IsLabeledTrailingClosure);
 
   if (shouldPerformGlobalCompletion) {
     llvm::SmallDenseMap<const VarDecl *, Type> SolutionSpecificVarTypes;

lib/IDE/CodeCompletion.cpp

@@ -259,7 +259,6 @@ class CodeCompletionCallbacksImpl : public CodeCompletionCallbacks,
   void completeForEachSequenceBeginning(CodeCompletionExpr *E) override;
   void completeForEachInKeyword() override;
   void completePostfixExpr(CodeCompletionExpr *E, bool hasSpace) override;
-  void completePostfixExprParen(Expr *E, Expr *CodeCompletionE) override;
   void completeExprKeyPath(KeyPathExpr *KPE, SourceLoc DotLoc) override;
 
   void completeTypeDeclResultBeginning() override;
@@ -419,32 +418,6 @@ void CodeCompletionCallbacksImpl::completePostfixExpr(CodeCompletionExpr *E,
   CurDeclContext = P.CurDeclContext;
 }
 
-void CodeCompletionCallbacksImpl::completePostfixExprParen(Expr *E,
-                                                           Expr *CodeCompletionE) {
-  assert(P.Tok.is(tok::code_complete));
-
-  // Don't produce any results in an enum element.
-  if (InEnumElementRawValue)
-    return;
-
-  Kind = CompletionKind::PostfixExprParen;
-  ParsedExpr = E;
-  CurDeclContext = P.CurDeclContext;
-  CodeCompleteTokenExpr = static_cast<CodeCompletionExpr*>(CodeCompletionE);
-
-  ShouldCompleteCallPatternAfterParen = true;
-  if (CompletionContext.getCallPatternHeuristics()) {
-    // Lookahead one token to decide what kind of call completions to provide.
-    // When it appears that there is already code for the call present, just
-    // complete values and/or argument labels.  Otherwise give the entire call
-    // pattern.
-    Token next = P.peekToken();
-    if (!next.isAtStartOfLine() && !next.is(tok::eof) && !next.is(tok::r_paren)) {
-      ShouldCompleteCallPatternAfterParen = false;
-    }
-  }
-}
-
 void CodeCompletionCallbacksImpl::completeExprKeyPath(KeyPathExpr *KPE,
                                                       SourceLoc DotLoc) {
   Kind = (!KPE || KPE->isObjC()) ? CompletionKind::KeyPathExprObjC
@@ -1075,7 +1048,6 @@ void CodeCompletionCallbacksImpl::addKeywords(CodeCompletionResultSink &Sink,
     break;
 
   case CompletionKind::CallArg:
-  case CompletionKind::PostfixExprParen:
     // Note that we don't add keywords here as the completion might be for
     // an argument list pattern. We instead add keywords later in
     // CodeCompletionCallbacksImpl::doneParsing when we know we're not
@@ -1554,8 +1526,7 @@ void CodeCompletionCallbacksImpl::postfixCompletion(SourceLoc CompletionLoc,
           Context.CompletionCallback, &Lookup);
       typeCheckContextAt(TypeCheckASTNodeAtLocContext::node(CurDeclContext, AE),
                          CompletionLoc);
-      Lookup.collectResults(/*IncludeSignature=*/false,
-                            /*IsLabeledTrailingClosure=*/true, CompletionLoc,
+      Lookup.collectResults(/*IsLabeledTrailingClosure=*/true, CompletionLoc,
                             CurDeclContext, CompletionContext);
     }
   }
@@ -1599,8 +1570,7 @@ void CodeCompletionCallbacksImpl::callCompletion(SourceLoc CompletionLoc,
                                              CurDeclContext);
   typeCheckWithLookup(Lookup, CompletionLoc);
 
-  Lookup.collectResults(ShouldCompleteCallPatternAfterParen,
-                        /*IsLabeledTrailingClosure=*/false, CompletionLoc,
+  Lookup.collectResults(/*IsLabeledTrailingClosure=*/false, CompletionLoc,
                         CurDeclContext, CompletionContext);
   Consumer.handleResults(CompletionContext);
 }
@@ -1725,7 +1695,6 @@ void CodeCompletionCallbacksImpl::doneParsing(SourceFile *SrcFile) {
   case CompletionKind::KeyPathExprSwift:
     keyPathExprCompletion(CompletionLoc, MaybeFuncBody);
     return;
-  case CompletionKind::PostfixExprParen:
   case CompletionKind::CallArg:
     callCompletion(CompletionLoc, MaybeFuncBody);
     return;
@@ -1773,8 +1742,7 @@ void CodeCompletionCallbacksImpl::doneParsing(SourceFile *SrcFile) {
       ParsedExpr->setType(*ExprType);
     }
 
-    if (!ExprType && Kind != CompletionKind::PostfixExprParen &&
-        Kind != CompletionKind::CallArg &&
+    if (!ExprType && Kind != CompletionKind::CallArg &&
         Kind != CompletionKind::KeyPathExprObjC)
       return;
   }
@@ -1810,7 +1778,6 @@ void CodeCompletionCallbacksImpl::doneParsing(SourceFile *SrcFile) {
   case CompletionKind::AfterPoundExpr:
   case CompletionKind::AccessorBeginning:
   case CompletionKind::CaseStmtBeginning:
-  case CompletionKind::PostfixExprParen:
   case CompletionKind::PostfixExpr:
   case CompletionKind::ReturnStmtExpr:
   case CompletionKind::YieldStmtExpr:

lib/Parse/ParseDecl.cpp

@@ -4148,44 +4148,24 @@ ParserResult<CustomAttr> Parser::parseCustomAttribute(
   ParserStatus status;
   ArgumentList *argList = nullptr;
   if (Tok.isFollowingLParen() && isCustomAttributeArgument()) {
-    if (peekToken().is(tok::code_complete)) {
-      auto lParenLoc = consumeToken(tok::l_paren);
-      auto typeE = new (Context) TypeExpr(type.get());
-      auto CCE = new (Context) CodeCompletionExpr(Tok.getLoc());
-      if (CodeCompletionCallbacks) {
-        CodeCompletionCallbacks->completePostfixExprParen(typeE, CCE);
-      }
-      consumeToken(tok::code_complete);
-      skipUntilDeclStmtRBrace(tok::r_paren);
-      auto rParenLoc = PreviousLoc;
-      if (Tok.is(tok::r_paren)) {
-        rParenLoc = consumeToken(tok::r_paren);
-      }
-
-      argList = ArgumentList::createParsed(
-          Context, lParenLoc, {Argument::unlabeled(CCE)}, rParenLoc,
-          /*trailingClosureIdx=*/llvm::None);
-      status.setHasCodeCompletionAndIsError();
-    } else {
-      // If we have no local context to parse the initial value into, create
-      // one for the PBD we'll eventually create.  This allows us to have
-      // reasonable DeclContexts for any closures that may live inside of
-      // initializers.
-      llvm::Optional<ParseFunctionBody> initParser;
-      if (!CurDeclContext->isLocalContext()) {
-        if (!initContext)
-          initContext = PatternBindingInitializer::create(CurDeclContext);
+    // If we have no local context to parse the initial value into, create
+    // one for the PBD we'll eventually create.  This allows us to have
+    // reasonable DeclContexts for any closures that may live inside of
+    // initializers.
+    llvm::Optional<ParseFunctionBody> initParser;
+    if (!CurDeclContext->isLocalContext()) {
+      if (!initContext)
+        initContext = PatternBindingInitializer::create(CurDeclContext);
 
-        initParser.emplace(*this, initContext);
-      }
-      auto result = parseArgumentList(tok::l_paren, tok::r_paren,
-                                      /*isExprBasic*/ true,
-                                      /*allowTrailingClosure*/ false);
-      status |= result;
-      argList = result.get();
-      assert(!argList->hasAnyTrailingClosures() &&
-             "Cannot parse a trailing closure here");
+      initParser.emplace(*this, initContext);
     }
+    auto result = parseArgumentList(tok::l_paren, tok::r_paren,
+                                    /*isExprBasic*/ true,
+                                    /*allowTrailingClosure*/ false);
+    status |= result;
+    argList = result.get();
+    assert(!argList->hasAnyTrailingClosures() &&
+           "Cannot parse a trailing closure here");
   }
 
   // Form the attribute.