[CodeCompletion] Update for braceless multiple trailing closure

rintaro · rjmccall · commit 1cbb1e76d43d · 2020-05-06T01:56:41.000-04:00
diff --git a/include/swift/IDE/CodeCompletion.h b/include/swift/IDE/CodeCompletion.h
@@ -525,6 +525,7 @@ enum class CompletionKind {
   AttributeDeclParen,
   PoundAvailablePlatform,
   CallArg,
+  LabeledTrailingClosure,
   ReturnStmtExpr,
   YieldStmtExpr,
   ForEachSequence,
diff --git a/include/swift/Parse/CodeCompletionCallbacks.h b/include/swift/Parse/CodeCompletionCallbacks.h
@@ -198,6 +198,9 @@ class CodeCompletionCallbacks {
 
   virtual void completeCallArg(CodeCompletionExpr *E, bool isFirst) {};
 
+  virtual void completeLabeledTrailingClosure(CodeCompletionExpr *E,
+                                              bool isAtStartOfLine) {};
+
   virtual void completeReturnStmt(CodeCompletionExpr *E) {};
 
   /// Complete a yield statement.  A missing yield index means that the
diff --git a/lib/IDE/CodeCompletion.cpp b/lib/IDE/CodeCompletion.cpp
@@ -1446,6 +1446,7 @@ class CodeCompletionCallbacksImpl : public CodeCompletionCallbacks {
   bool ShouldCompleteCallPatternAfterParen = true;
   bool PreferFunctionReferencesToCalls = false;
   bool AttTargetIsIndependent = false;
+  bool IsAtStartOfLine = false;
   Optional<DeclKind> AttTargetDK;
   Optional<StmtKind> ParentStmtKind;
 
@@ -1572,6 +1573,9 @@ class CodeCompletionCallbacksImpl : public CodeCompletionCallbacks {
   void completeUnresolvedMember(CodeCompletionExpr *E,
                                 SourceLoc DotLoc) override;
   void completeCallArg(CodeCompletionExpr *E, bool isFirst) override;
+  void completeLabeledTrailingClosure(CodeCompletionExpr *E,
+                                      bool isAtStartOfLine) override;
+
   void completeReturnStmt(CodeCompletionExpr *E) override;
   void completeYieldStmt(CodeCompletionExpr *E,
                          Optional<unsigned> yieldIndex) override;
@@ -4155,12 +4159,15 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
   }
 
   void addCallArgumentCompletionResults(
-      ArrayRef<const AnyFunctionType::Param *> Args) {
+      ArrayRef<PossibleParamInfo> ParamInfos) {
     Type ContextType;
     if (auto typeContext = CurrDeclContext->getInnermostTypeContext())
       ContextType = typeContext->getDeclaredTypeInContext();
 
-    for (auto *Arg : Args) {
+    for (auto Info : ParamInfos) {
+      const auto *Arg = Info.Param;
+      if (!Arg)
+        continue;
       CodeCompletionResultBuilder Builder(
           Sink, CodeCompletionResult::ResultKind::Pattern,
           SemanticContextKind::ExpressionSpecific, {});
@@ -5150,6 +5157,14 @@ void CodeCompletionCallbacksImpl::completeCallArg(CodeCompletionExpr *E,
   }
 }
 
+void CodeCompletionCallbacksImpl::completeLabeledTrailingClosure(
+    CodeCompletionExpr *E, bool isAtStartOfLine) {
+  CurDeclContext = P.CurDeclContext;
+  CodeCompleteTokenExpr = E;
+  Kind = CompletionKind::LabeledTrailingClosure;
+  IsAtStartOfLine = isAtStartOfLine;
+}
+
 void CodeCompletionCallbacksImpl::completeReturnStmt(CodeCompletionExpr *E) {
   CurDeclContext = P.CurDeclContext;
   CodeCompleteTokenExpr = E;
@@ -5332,6 +5347,7 @@ void CodeCompletionCallbacksImpl::addKeywords(CodeCompletionResultSink &Sink,
   case CompletionKind::Import:
   case CompletionKind::UnresolvedMember:
   case CompletionKind::CallArg:
+  case CompletionKind::LabeledTrailingClosure:
   case CompletionKind::AfterPoundExpr:
   case CompletionKind::AfterPoundDirective:
   case CompletionKind::PlatformConditon:
@@ -5875,9 +5891,14 @@ void CodeCompletionCallbacksImpl::doneParsing() {
           (Lookup.FoundFunctionCalls &&
            Lookup.FoundFunctionsWithoutFirstKeyword);
     } else if (!ContextInfo.getPossibleParams().empty()) {
-      Lookup.addCallArgumentCompletionResults(ContextInfo.getPossibleParams());
+      auto params = ContextInfo.getPossibleParams();
+      Lookup.addCallArgumentCompletionResults(params);
 
       shouldPerformGlobalCompletion = !ContextInfo.getPossibleTypes().empty();
+      // Fallback to global completion if the position is out of number. It's
+      // better than suggest nothing.
+      shouldPerformGlobalCompletion |= llvm::all_of(
+          params, [](const PossibleParamInfo &P) { return !P.Param; });
     }
 
     if (shouldPerformGlobalCompletion) {
@@ -5888,6 +5909,52 @@ void CodeCompletionCallbacksImpl::doneParsing() {
     break;
   }
 
+  case CompletionKind::LabeledTrailingClosure: {
+    ExprContextInfo ContextInfo(CurDeclContext, CodeCompleteTokenExpr);
+
+    SmallVector<PossibleParamInfo, 2> params;
+    // Only complete function type parameters
+    llvm::copy_if(ContextInfo.getPossibleParams(), std::back_inserter(params),
+                  [](const PossibleParamInfo &P) {
+                    // nullptr indicates out of bounds.
+                    if (!P.Param)
+                      return true;
+                    return P.Param->getPlainType()
+                        ->lookThroughAllOptionalTypes()
+                        ->is<AnyFunctionType>();
+                  });
+
+    bool allRequired = false;
+    if (!params.empty()) {
+      Lookup.addCallArgumentCompletionResults(params);
+      allRequired = llvm::all_of(
+          params, [](const PossibleParamInfo &P) { return P.IsRequired; });
+    }
+
+    // If there're optional parameters, do global completion or member
+    // completion depending on the completion is happening at the start of line.
+    if (!allRequired) {
+      if (IsAtStartOfLine) {
+        //   foo() {}
+        //   <HERE>
+        // Global completion.
+        auto &Sink = CompletionContext.getResultSink();
+        addDeclKeywords(Sink);
+        addStmtKeywords(Sink, MaybeFuncBody);
+        addSuperKeyword(Sink);
+        addLetVarKeywords(Sink);
+        addExprKeywords(Sink);
+        addAnyTypeKeyword(Sink);
+        DoPostfixExprBeginning();
+      } else {
+        //   foo() {} <HERE>
+        // Member completion
+        // TODO: Member completion.
+      }
+    }
+    break;
+  }
+
   case CompletionKind::ReturnStmtExpr : {
     SourceLoc Loc = P.Context.SourceMgr.getCodeCompletionLoc();
     Lookup.setExpectedTypes(getReturnTypeFromContext(CurDeclContext),
diff --git a/lib/IDE/ExprContextAnalysis.cpp b/lib/IDE/ExprContextAnalysis.cpp
@@ -561,47 +561,6 @@ static bool getPositionInArgs(DeclContext &DC, Expr *Args, Expr *CCExpr,
   return false;
 }
 
-enum class ArgPositionKind {
-  /// The argument is in normal calling parenthesis.
-  /// i.e. foo(args..., <HERE>) { ... }
-  NormalArgument,
-  /// The argument is inside multiple trailing closure block.
-  /// i.e. foo(args...) { arg2: { ... } <HERE> }
-  InClosureBlock,
-  /// The argument is inside multiple trailing closure block, also it is the
-  /// sole element.
-  /// foo(args...) { <HERE> }
-  InEmptyClosureBlock,
-};
-
-static ArgPositionKind getArgPositionKind(DeclContext &DC, Expr *Args,
-                                          unsigned Position) {
-  SourceManager &SM = DC.getASTContext().SourceMgr;
-
-  if (auto tuple = dyn_cast<TupleExpr>(Args)) {
-    SourceLoc argPos = tuple->getElement(Position)->getStartLoc();
-    SourceLoc rParenLoc = tuple->getRParenLoc();
-    if (rParenLoc.isInvalid() || SM.isBeforeInBuffer(rParenLoc, argPos)) {
-      // Invariant: If the token is at the label position, the label location is
-      // invalid, and the value is a CodeCompletionExpr.
-      if (Position == tuple->getNumElements() - 1 &&
-          tuple->getNumTrailingElements() == 1 &&
-          tuple->getElementNameLoc(Position).isInvalid()) {
-        return ArgPositionKind::InEmptyClosureBlock;
-      }
-      return ArgPositionKind::InClosureBlock;
-    }
-  } else if (auto paren = dyn_cast<ParenExpr>(Args)) {
-    SourceLoc argLoc = paren->getSubExpr()->getStartLoc();
-    SourceLoc rParenLoc = paren->getRParenLoc();
-    // We don't have a way to distingish between 'foo { _: <here> }' and
-    // 'foo { <here> }'. For now, consider it latter one.
-    if (rParenLoc.isInvalid() || SM.isBeforeInBuffer(rParenLoc, argLoc))
-      return ArgPositionKind::InEmptyClosureBlock;
-  }
-  return ArgPositionKind::NormalArgument;
-}
-
 /// Given an expression and its context, the analyzer tries to figure out the
 /// expected type of the expression by analyzing its context.
 class ExprContextAnalyzer {
@@ -612,7 +571,7 @@ class ExprContextAnalyzer {
 
   // Results populated by Analyze()
   SmallVectorImpl<Type> &PossibleTypes;
-  SmallVectorImpl<const AnyFunctionType::Param *> &PossibleParams;
+  SmallVectorImpl<PossibleParamInfo> &PossibleParams;
   SmallVectorImpl<FunctionTypeAndDecl> &PossibleCallees;
   bool &singleExpressionBody;
 
@@ -623,8 +582,8 @@ class ExprContextAnalyzer {
     PossibleTypes.push_back(ty->getRValueType());
   }
 
-  void recordPossibleParam(const AnyFunctionType::Param &arg) {
-    PossibleParams.push_back(&arg);
+  void recordPossibleParam(const AnyFunctionType::Param *arg, bool isRequired) {
+    PossibleParams.emplace_back(arg, isRequired);
   }
 
   /// Collect context information at call argument position.
@@ -657,9 +616,9 @@ class ExprContextAnalyzer {
     if (!getPositionInArgs(*DC, Arg, ParsedExpr, Position, HasName))
       return false;
 
-    ArgPositionKind positionKind = getArgPositionKind(*DC, Arg, Position);
-
     // 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) ||
@@ -681,48 +640,18 @@ class ExprContextAnalyzer {
           if (paramList && paramList->size() != Params.size())
             paramList = nullptr;
         }
-
-        // Determine the index of the parameter that can be a single trailing
-        // closure.
-        unsigned singleTrailingClosureIdx = Params.size();
-        for (int idx = Params.size() - 1; idx >= 0; --idx) {
-          if (Params[idx].getPlainType()->is<AnyFunctionType>()) {
-            singleTrailingClosureIdx = idx;
-            break;
-          }
-          if (!paramList || !paramList->get(idx)->isDefaultArgument())
-            break;
-        }
-
         for (auto Pos = Position; Pos < Params.size(); ++Pos) {
           const auto &paramType = Params[Pos];
           Type ty = paramType.getPlainType();
           if (memberDC && ty->hasTypeParameter())
             ty = memberDC->mapTypeIntoContext(ty);
 
           if (paramType.hasLabel() && MayNeedName) {
-            // In trailing closure block, don't suggest non-closure arguments.
-            if (positionKind >= ArgPositionKind::InClosureBlock) {
-              Type argTy = ty;
-              if (paramType.isAutoClosure() && ty->is<AnyFunctionType>())
-                argTy = ty->castTo<AnyFunctionType>()->getResult();
-              if (!argTy->is<AnyFunctionType>())
-                continue;
-
-              // If the token is the only element in the closure block. It might
-              // be a single trailing closure. We should perform global
-              // completion as well.
-              if (positionKind == ArgPositionKind::InEmptyClosureBlock &&
-                  Pos == singleTrailingClosureIdx) {
-                auto resultTy = argTy->castTo<AnyFunctionType>()->getResult();
-                if (seenTypes.insert(resultTy.getPointer()).second)
-                  recordPossibleType(resultTy);
-              }
-            }
-
+            bool isDefaulted = paramList &&
+                               paramList->get(Pos)->isDefaultArgument();
             if (seenArgs.insert({paramType.getLabel(), ty.getPointer()}).second)
-              recordPossibleParam(paramType);
-            if (paramList && paramList->get(Position)->isDefaultArgument())
+              recordPossibleParam(&paramType, !isDefaulted);
+            if (isDefaulted)
               continue;
           } else {
             auto argTy = ty;
@@ -733,6 +662,12 @@ class ExprContextAnalyzer {
           }
           break;
         }
+        // If the argument position is out of expeceted number, indicate that
+        // with optional nullptr param.
+        if (Position >= Params.size()) {
+          if (seenArgs.insert({Identifier(), nullptr}).second)
+            recordPossibleParam(nullptr, /*isRequired=*/false);
+        }
       }
     }
     return !PossibleTypes.empty() || !PossibleParams.empty();
@@ -999,7 +934,7 @@ class ExprContextAnalyzer {
 public:
   ExprContextAnalyzer(
       DeclContext *DC, Expr *ParsedExpr, SmallVectorImpl<Type> &PossibleTypes,
-      SmallVectorImpl<const AnyFunctionType::Param *> &PossibleArgs,
+      SmallVectorImpl<PossibleParamInfo> &PossibleArgs,
       SmallVectorImpl<FunctionTypeAndDecl> &PossibleCallees,
       bool &singleExpressionBody)
       : DC(DC), ParsedExpr(ParsedExpr), SM(DC->getASTContext().SourceMgr),
diff --git a/lib/IDE/ExprContextAnalysis.h b/lib/IDE/ExprContextAnalysis.h
@@ -50,11 +50,28 @@ struct FunctionTypeAndDecl {
       : Type(Type), Decl(Decl), SemanticContext(SemanticContext) {}
 };
 
+struct PossibleParamInfo {
+  /// Expected parameter.
+  /// 
+  /// 'nullptr' indicates that the code completion position is at out of
+  /// expected argument position. E.g.
+  ///   func foo(x: Int) {}
+  ///   foo(x: 1, <HERE>)
+  const AnyFunctionType::Param *Param;
+  bool IsRequired;
+
+  PossibleParamInfo(const AnyFunctionType::Param *Param, bool IsRequired)
+      : Param(Param), IsRequired(IsRequired) {
+    assert((Param || !IsRequired) &&
+           "nullptr with required flag is not allowed");
+  };
+};
+
 /// Given an expression and its decl context, the analyzer tries to figure out
 /// the expected type of the expression by analyzing its context.
 class ExprContextInfo {
   SmallVector<Type, 2> PossibleTypes;
-  SmallVector<const AnyFunctionType::Param *, 2> PossibleParams;
+  SmallVector<PossibleParamInfo, 2> PossibleParams;
   SmallVector<FunctionTypeAndDecl, 2> PossibleCallees;
   bool singleExpressionBody = false;
 
@@ -73,7 +90,7 @@ class ExprContextInfo {
 
   // Returns a list of possible argument label names.
   // Valid only if \c getKind() is \c CallArgument.
-  ArrayRef<const AnyFunctionType::Param *> getPossibleParams() const {
+  ArrayRef<PossibleParamInfo> getPossibleParams() const {
     return PossibleParams;
   }
 
diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp
diff --git a/test/IDE/complete_multiple_trailingclosure.swift b/test/IDE/complete_multiple_trailingclosure.swift
