[SourceKit] Avoid expanding the last argument as trailing closure if there are other closures in the tuple. rdar://23428366 (#3408)

SourceKit invariantly expands the last argument in a function call as trailing closure,
if it is of function type. However, there are situations when inline closures
are preferred; for example, when the last argument is not the only closure in the function
call. This patch modifies SourceKit so that when the argument contains multiple closures,
the last argument is expanded as inline closure.

test/SourceKit/CodeExpand/code-expand.swift

@@ -54,3 +54,28 @@ func f1() {
   bar(<#T##d: () -> ()##() -> ()#>)
 }
 // CHECK-NOT: bar { () -> () in
+
+func f1() {
+  bar(<#T##d: () -> ()##() -> ()#>, <#T##d: () -> ()##() -> ()#>)
+}
+// CHECK:   bar({
+// CHECK-NEXT:	<#code#>
+// CHECK-NEXT:	}, {
+// CHECK-NEXT:	<#code#>
+// CHECK-NEXT:	})
+
+
+func f1() {
+  bar(a : <#T##d: () -> ()##() -> ()#>, b : <#T##d: () -> ()##() -> ()#>)
+}
+// CHECK: bar(a : {
+// CHECK-NEXT: <#code#>
+// CHECK-NEXT: }, b : {
+// CHECK-NEXT: <#code#>
+// CHECK-NEXT: })
+
+
+func f1() {
+  bar(a : {}}, <#T##d: () -> ()##() -> ()#>)
+}
+// CHECK: bar(a : {}}, <#T##d: () -> ()##() -> ()#>)

tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp

@@ -1351,6 +1351,7 @@ public:
 };
 
 class PlaceholderExpansionScanner {
+
 public:
   struct Param {
     CharSourceRange NameRange;
@@ -1360,9 +1361,14 @@ public:
   };
 
 private:
-  SourceManager &SM;
+
+  struct ClosureInfo {
     std::vector<Param> Params;
     CharSourceRange ReturnTypeRange;
+  };
+
+  SourceManager &SM;
+  ClosureInfo TargetClosureInfo;
   EditorPlaceholderExpr *PHE = nullptr;
 
   class PlaceholderFinder: public ASTWalker {
@@ -1384,20 +1390,13 @@ private:
     }
   };
 
-  bool scanClosureType(SourceFile &SF, SourceLoc PlaceholderLoc) {
-    Params.clear();
-    ReturnTypeRange = CharSourceRange();
-    PlaceholderFinder Finder(PlaceholderLoc, PHE);
-    SF.walk(Finder);
-    if (!PHE || !PHE->getTypeForExpansion())
-      return false;
-
   class ClosureTypeWalker: public ASTWalker {
+    SourceManager &SM;
+    ClosureInfo &Info;
   public:
-      PlaceholderExpansionScanner &S;
     bool FoundFunctionTypeRepr = false;
-      explicit ClosureTypeWalker(PlaceholderExpansionScanner &S)
-        :S(S) { }
+    explicit ClosureTypeWalker(SourceManager &SM, ClosureInfo &Info) : SM(SM),
+      Info(Info) { }
 
     bool walkToTypeReprPre(TypeRepr *T) override {
       if (auto *FTR = dyn_cast<FunctionTypeRepr>(T)) {
@@ -1412,21 +1411,21 @@ private:
                                    NTR->getName().getLength());
               ArgTR = NTR->getTypeRepr();
             }
-              SourceLoc SRE = Lexer::getLocForEndOfToken(S.SM,
+            SourceLoc SRE = Lexer::getLocForEndOfToken(SM,
                                                        ArgTR->getEndLoc());
-              TR = CharSourceRange(S.SM, ArgTR->getStartLoc(), SRE);
-              S.Params.emplace_back(NR, TR);
+            TR = CharSourceRange(SM, ArgTR->getStartLoc(), SRE);
+            Info.Params.emplace_back(NR, TR);
           }
         } else if (FTR->getArgsTypeRepr()) {
           CharSourceRange TR;
-            TR = CharSourceRange(S.SM, FTR->getArgsTypeRepr()->getStartLoc(),
-                                 Lexer::getLocForEndOfToken(S.SM,
+          TR = CharSourceRange(SM, FTR->getArgsTypeRepr()->getStartLoc(),
+                               Lexer::getLocForEndOfToken(SM,
                                         FTR->getArgsTypeRepr()->getEndLoc()));
-            S.Params.emplace_back(CharSourceRange(), TR);
+          Info.Params.emplace_back(CharSourceRange(), TR);
         }
         if (auto *RTR = FTR->getResultTypeRepr()) {
-            SourceLoc SRE = Lexer::getLocForEndOfToken(S.SM, RTR->getEndLoc());
-            S.ReturnTypeRange = CharSourceRange(S.SM, RTR->getStartLoc(), SRE);
+          SourceLoc SRE = Lexer::getLocForEndOfToken(SM, RTR->getEndLoc());
+          Info.ReturnTypeRange = CharSourceRange(SM, RTR->getStartLoc(), SRE);
         }
       }
       return !FoundFunctionTypeRepr;
@@ -1437,8 +1436,34 @@ private:
       return !FoundFunctionTypeRepr;
     }
 
-    } PW(*this);
+  };
 
+  bool containClosure(Expr *E) {
+    if (E->getStartLoc().isInvalid())
+      return false;
+    EditorPlaceholderExpr *Found;
+    ClosureInfo Info;
+    ClosureTypeWalker ClosureWalker(SM, Info);
+    PlaceholderFinder Finder(E->getStartLoc(), Found);
+    E->walk(Finder);
+    if (Found) {
+      if (auto TR = Found->getTypeLoc().getTypeRepr()) {
+        TR->walk(ClosureWalker);
+        return ClosureWalker.FoundFunctionTypeRepr;
+      }
+    }
+    E->walk(ClosureWalker);
+    return ClosureWalker.FoundFunctionTypeRepr;
+  }
+
+  bool scanClosureType(SourceFile &SF, SourceLoc PlaceholderLoc) {
+    TargetClosureInfo.Params.clear();
+    TargetClosureInfo.ReturnTypeRange = CharSourceRange();
+    PlaceholderFinder Finder(PlaceholderLoc, PHE);
+    SF.walk(Finder);
+    if (!PHE || !PHE->getTypeForExpansion())
+      return false;
+    ClosureTypeWalker PW(SM, TargetClosureInfo);
     PHE->getTypeForExpansion()->walk(PW);
     return PW.FoundFunctionTypeRepr;
   }
@@ -1512,6 +1537,22 @@ private:
     return std::make_pair(CE, true);
   }
 
+  bool shouldUseTrailingClosureInTuple(TupleExpr *TE,
+                                       SourceLoc PlaceHolderStartLoc) {
+    if (!TE->getElements().empty()) {
+      for (unsigned I = 0, N = TE->getNumElements(); I < N; ++ I) {
+        bool IsLast = I == N - 1;
+        Expr *E = TE->getElement(I);
+        if (IsLast) {
+          return E->getStartLoc() == PlaceHolderStartLoc;
+        } else if (containClosure(E)) {
+          return false;
+        }
+      }
+    }
+    return false;
+  }
+
 public:
   explicit PlaceholderExpansionScanner(SourceManager &SM) : SM(SM) { }
 
@@ -1543,13 +1584,13 @@ public:
       if (isa<ParenExpr>(Args)) {
         UseTrailingClosure = true;
       } else if (auto *TE = dyn_cast<TupleExpr>(Args)) {
-        if (!TE->getElements().empty())
-          UseTrailingClosure =
-            TE->getElements().back()->getStartLoc() == PlaceholderStartLoc;
+        UseTrailingClosure = shouldUseTrailingClosureInTuple(TE,
+                                                          PlaceholderStartLoc);
       }
     }
 
-    Callback(Args, UseTrailingClosure, Params, ReturnTypeRange);
+    Callback(Args, UseTrailingClosure, TargetClosureInfo.Params,
+             TargetClosureInfo.ReturnTypeRange);
     return true;
   }