Skip to content

Commit 09a19bb

Browse files
nkcsgexinkcsgexi
authored
[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.
1 parent 3691b48 commit 09a19bb

File tree

2 files changed

+120
-54
lines changed

2 files changed

+120
-54
lines changed

‎test/SourceKit/CodeExpand/code-expand.swift

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,3 +54,28 @@ func f1() {
5454
bar(<#T##d: () -> ()##() -> ()#>)
5555
}
5656
// CHECK-NOT: bar { () -> () in
57+
58+
func f1() {
59+
bar(<#T##d: () -> ()##() -> ()#>, <#T##d: () -> ()##() -> ()#>)
60+
}
61+
// CHECK: bar({
62+
// CHECK-NEXT: <#code#>
63+
// CHECK-NEXT: }, {
64+
// CHECK-NEXT: <#code#>
65+
// CHECK-NEXT: })
66+
67+
68+
func f1() {
69+
bar(a : <#T##d: () -> ()##() -> ()#>, b : <#T##d: () -> ()##() -> ()#>)
70+
}
71+
// CHECK: bar(a : {
72+
// CHECK-NEXT: <#code#>
73+
// CHECK-NEXT: }, b : {
74+
// CHECK-NEXT: <#code#>
75+
// CHECK-NEXT: })
76+
77+
78+
func f1() {
79+
bar(a : {}}, <#T##d: () -> ()##() -> ()#>)
80+
}
81+
// CHECK: bar(a : {}}, <#T##d: () -> ()##() -> ()#>)

‎tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp

Lines changed: 95 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1351,6 +1351,7 @@ class SwiftEditorSyntaxWalker: public ide::SyntaxModelWalker {
13511351
};
13521352

13531353
class PlaceholderExpansionScanner {
1354+
13541355
public:
13551356
struct Param {
13561357
CharSourceRange NameRange;
@@ -1360,9 +1361,14 @@ class PlaceholderExpansionScanner {
13601361
};
13611362

13621363
private:
1364+
1365+
struct ClosureInfo {
1366+
std::vector<Param> Params;
1367+
CharSourceRange ReturnTypeRange;
1368+
};
1369+
13631370
SourceManager &SM;
1364-
std::vector<Param> Params;
1365-
CharSourceRange ReturnTypeRange;
1371+
ClosureInfo TargetClosureInfo;
13661372
EditorPlaceholderExpr *PHE = nullptr;
13671373

13681374
class PlaceholderFinder: public ASTWalker {
@@ -1384,61 +1390,80 @@ class PlaceholderExpansionScanner {
13841390
}
13851391
};
13861392

1387-
bool scanClosureType(SourceFile &SF, SourceLoc PlaceholderLoc) {
1388-
Params.clear();
1389-
ReturnTypeRange = CharSourceRange();
1390-
PlaceholderFinder Finder(PlaceholderLoc, PHE);
1391-
SF.walk(Finder);
1392-
if (!PHE || !PHE->getTypeForExpansion())
1393-
return false;
1394-
1395-
class ClosureTypeWalker: public ASTWalker {
1396-
public:
1397-
PlaceholderExpansionScanner &S;
1398-
bool FoundFunctionTypeRepr = false;
1399-
explicit ClosureTypeWalker(PlaceholderExpansionScanner &S)
1400-
:S(S) { }
1401-
1402-
bool walkToTypeReprPre(TypeRepr *T) override {
1403-
if (auto *FTR = dyn_cast<FunctionTypeRepr>(T)) {
1404-
FoundFunctionTypeRepr = true;
1405-
if (auto *TTR = dyn_cast_or_null<TupleTypeRepr>(FTR->getArgsTypeRepr())) {
1406-
for (auto *ArgTR : TTR->getElements()) {
1407-
CharSourceRange NR;
1408-
CharSourceRange TR;
1409-
auto *NTR = dyn_cast<NamedTypeRepr>(ArgTR);
1410-
if (NTR && NTR->hasName()) {
1411-
NR = CharSourceRange(NTR->getNameLoc(),
1412-
NTR->getName().getLength());
1413-
ArgTR = NTR->getTypeRepr();
1414-
}
1415-
SourceLoc SRE = Lexer::getLocForEndOfToken(S.SM,
1416-
ArgTR->getEndLoc());
1417-
TR = CharSourceRange(S.SM, ArgTR->getStartLoc(), SRE);
1418-
S.Params.emplace_back(NR, TR);
1419-
}
1420-
} else if (FTR->getArgsTypeRepr()) {
1393+
class ClosureTypeWalker: public ASTWalker {
1394+
SourceManager &SM;
1395+
ClosureInfo &Info;
1396+
public:
1397+
bool FoundFunctionTypeRepr = false;
1398+
explicit ClosureTypeWalker(SourceManager &SM, ClosureInfo &Info) : SM(SM),
1399+
Info(Info) { }
1400+
1401+
bool walkToTypeReprPre(TypeRepr *T) override {
1402+
if (auto *FTR = dyn_cast<FunctionTypeRepr>(T)) {
1403+
FoundFunctionTypeRepr = true;
1404+
if (auto *TTR = dyn_cast_or_null<TupleTypeRepr>(FTR->getArgsTypeRepr())) {
1405+
for (auto *ArgTR : TTR->getElements()) {
1406+
CharSourceRange NR;
14211407
CharSourceRange TR;
1422-
TR = CharSourceRange(S.SM, FTR->getArgsTypeRepr()->getStartLoc(),
1423-
Lexer::getLocForEndOfToken(S.SM,
1424-
FTR->getArgsTypeRepr()->getEndLoc()));
1425-
S.Params.emplace_back(CharSourceRange(), TR);
1426-
}
1427-
if (auto *RTR = FTR->getResultTypeRepr()) {
1428-
SourceLoc SRE = Lexer::getLocForEndOfToken(S.SM, RTR->getEndLoc());
1429-
S.ReturnTypeRange = CharSourceRange(S.SM, RTR->getStartLoc(), SRE);
1408+
auto *NTR = dyn_cast<NamedTypeRepr>(ArgTR);
1409+
if (NTR && NTR->hasName()) {
1410+
NR = CharSourceRange(NTR->getNameLoc(),
1411+
NTR->getName().getLength());
1412+
ArgTR = NTR->getTypeRepr();
1413+
}
1414+
SourceLoc SRE = Lexer::getLocForEndOfToken(SM,
1415+
ArgTR->getEndLoc());
1416+
TR = CharSourceRange(SM, ArgTR->getStartLoc(), SRE);
1417+
Info.Params.emplace_back(NR, TR);
14301418
}
1419+
} else if (FTR->getArgsTypeRepr()) {
1420+
CharSourceRange TR;
1421+
TR = CharSourceRange(SM, FTR->getArgsTypeRepr()->getStartLoc(),
1422+
Lexer::getLocForEndOfToken(SM,
1423+
FTR->getArgsTypeRepr()->getEndLoc()));
1424+
Info.Params.emplace_back(CharSourceRange(), TR);
1425+
}
1426+
if (auto *RTR = FTR->getResultTypeRepr()) {
1427+
SourceLoc SRE = Lexer::getLocForEndOfToken(SM, RTR->getEndLoc());
1428+
Info.ReturnTypeRange = CharSourceRange(SM, RTR->getStartLoc(), SRE);
14311429
}
1432-
return !FoundFunctionTypeRepr;
14331430
}
1431+
return !FoundFunctionTypeRepr;
1432+
}
14341433

1435-
bool walkToTypeReprPost(TypeRepr *T) override {
1436-
// If we just visited the FunctionTypeRepr, end traversal.
1437-
return !FoundFunctionTypeRepr;
1438-
}
1434+
bool walkToTypeReprPost(TypeRepr *T) override {
1435+
// If we just visited the FunctionTypeRepr, end traversal.
1436+
return !FoundFunctionTypeRepr;
1437+
}
14391438

1440-
} PW(*this);
1439+
};
14411440

1441+
bool containClosure(Expr *E) {
1442+
if (E->getStartLoc().isInvalid())
1443+
return false;
1444+
EditorPlaceholderExpr *Found;
1445+
ClosureInfo Info;
1446+
ClosureTypeWalker ClosureWalker(SM, Info);
1447+
PlaceholderFinder Finder(E->getStartLoc(), Found);
1448+
E->walk(Finder);
1449+
if (Found) {
1450+
if (auto TR = Found->getTypeLoc().getTypeRepr()) {
1451+
TR->walk(ClosureWalker);
1452+
return ClosureWalker.FoundFunctionTypeRepr;
1453+
}
1454+
}
1455+
E->walk(ClosureWalker);
1456+
return ClosureWalker.FoundFunctionTypeRepr;
1457+
}
1458+
1459+
bool scanClosureType(SourceFile &SF, SourceLoc PlaceholderLoc) {
1460+
TargetClosureInfo.Params.clear();
1461+
TargetClosureInfo.ReturnTypeRange = CharSourceRange();
1462+
PlaceholderFinder Finder(PlaceholderLoc, PHE);
1463+
SF.walk(Finder);
1464+
if (!PHE || !PHE->getTypeForExpansion())
1465+
return false;
1466+
ClosureTypeWalker PW(SM, TargetClosureInfo);
14421467
PHE->getTypeForExpansion()->walk(PW);
14431468
return PW.FoundFunctionTypeRepr;
14441469
}
@@ -1512,6 +1537,22 @@ class PlaceholderExpansionScanner {
15121537
return std::make_pair(CE, true);
15131538
}
15141539

1540+
bool shouldUseTrailingClosureInTuple(TupleExpr *TE,
1541+
SourceLoc PlaceHolderStartLoc) {
1542+
if (!TE->getElements().empty()) {
1543+
for (unsigned I = 0, N = TE->getNumElements(); I < N; ++ I) {
1544+
bool IsLast = I == N - 1;
1545+
Expr *E = TE->getElement(I);
1546+
if (IsLast) {
1547+
return E->getStartLoc() == PlaceHolderStartLoc;
1548+
} else if (containClosure(E)) {
1549+
return false;
1550+
}
1551+
}
1552+
}
1553+
return false;
1554+
}
1555+
15151556
public:
15161557
explicit PlaceholderExpansionScanner(SourceManager &SM) : SM(SM) { }
15171558

@@ -1543,13 +1584,13 @@ class PlaceholderExpansionScanner {
15431584
if (isa<ParenExpr>(Args)) {
15441585
UseTrailingClosure = true;
15451586
} else if (auto *TE = dyn_cast<TupleExpr>(Args)) {
1546-
if (!TE->getElements().empty())
1547-
UseTrailingClosure =
1548-
TE->getElements().back()->getStartLoc() == PlaceholderStartLoc;
1587+
UseTrailingClosure = shouldUseTrailingClosureInTuple(TE,
1588+
PlaceholderStartLoc);
15491589
}
15501590
}
15511591

1552-
Callback(Args, UseTrailingClosure, Params, ReturnTypeRange);
1592+
Callback(Args, UseTrailingClosure, TargetClosureInfo.Params,
1593+
TargetClosureInfo.ReturnTypeRange);
15531594
return true;
15541595
}
15551596

0 commit comments

Comments
 (0)