Skip to content

Commit

Permalink
[SourceKit] Avoid expanding the last argument as trailing closure if …
Browse files Browse the repository at this point in the history 
…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.
  • Loading branch information
@nkcsgexi
nkcsgexi committed Jul 8, 2016
1 parent 3691b48 commit 09a19bb
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 54 deletions.
25 changes: 25 additions & 0 deletions test/SourceKit/CodeExpand/code-expand.swift
View file
Expand Up @@ -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: () -> ()##() -> ()#>)
149 changes: 95 additions & 54 deletions tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp
View file
Expand Up @@ -1351,6 +1351,7 @@ class SwiftEditorSyntaxWalker: public ide::SyntaxModelWalker {
};

class PlaceholderExpansionScanner {

public:
struct Param {
CharSourceRange NameRange;
Expand All @@ -1360,9 +1361,14 @@ class PlaceholderExpansionScanner {
};

private:

struct ClosureInfo {
std::vector<Param> Params;
CharSourceRange ReturnTypeRange;
};

SourceManager &SM;
std::vector<Param> Params;
CharSourceRange ReturnTypeRange;
ClosureInfo TargetClosureInfo;
EditorPlaceholderExpr *PHE = nullptr;

class PlaceholderFinder: public ASTWalker {
Expand All @@ -1384,61 +1390,80 @@ class PlaceholderExpansionScanner {
}
};

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 {
public:
PlaceholderExpansionScanner &S;
bool FoundFunctionTypeRepr = false;
explicit ClosureTypeWalker(PlaceholderExpansionScanner &S)
:S(S) { }

bool walkToTypeReprPre(TypeRepr *T) override {
if (auto *FTR = dyn_cast<FunctionTypeRepr>(T)) {
FoundFunctionTypeRepr = true;
if (auto *TTR = dyn_cast_or_null<TupleTypeRepr>(FTR->getArgsTypeRepr())) {
for (auto *ArgTR : TTR->getElements()) {
CharSourceRange NR;
CharSourceRange TR;
auto *NTR = dyn_cast<NamedTypeRepr>(ArgTR);
if (NTR && NTR->hasName()) {
NR = CharSourceRange(NTR->getNameLoc(),
NTR->getName().getLength());
ArgTR = NTR->getTypeRepr();
}
SourceLoc SRE = Lexer::getLocForEndOfToken(S.SM,
ArgTR->getEndLoc());
TR = CharSourceRange(S.SM, ArgTR->getStartLoc(), SRE);
S.Params.emplace_back(NR, TR);
}
} else if (FTR->getArgsTypeRepr()) {
class ClosureTypeWalker: public ASTWalker {
SourceManager &SM;
ClosureInfo &Info;
public:
bool FoundFunctionTypeRepr = false;
explicit ClosureTypeWalker(SourceManager &SM, ClosureInfo &Info) : SM(SM),
Info(Info) { }

bool walkToTypeReprPre(TypeRepr *T) override {
if (auto *FTR = dyn_cast<FunctionTypeRepr>(T)) {
FoundFunctionTypeRepr = true;
if (auto *TTR = dyn_cast_or_null<TupleTypeRepr>(FTR->getArgsTypeRepr())) {
for (auto *ArgTR : TTR->getElements()) {
CharSourceRange NR;
CharSourceRange TR;
TR = CharSourceRange(S.SM, FTR->getArgsTypeRepr()->getStartLoc(),
Lexer::getLocForEndOfToken(S.SM,
FTR->getArgsTypeRepr()->getEndLoc()));
S.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);
auto *NTR = dyn_cast<NamedTypeRepr>(ArgTR);
if (NTR && NTR->hasName()) {
NR = CharSourceRange(NTR->getNameLoc(),
NTR->getName().getLength());
ArgTR = NTR->getTypeRepr();
}
SourceLoc SRE = Lexer::getLocForEndOfToken(SM,
ArgTR->getEndLoc());
TR = CharSourceRange(SM, ArgTR->getStartLoc(), SRE);
Info.Params.emplace_back(NR, TR);
}
} else if (FTR->getArgsTypeRepr()) {
CharSourceRange TR;
TR = CharSourceRange(SM, FTR->getArgsTypeRepr()->getStartLoc(),
Lexer::getLocForEndOfToken(SM,
FTR->getArgsTypeRepr()->getEndLoc()));
Info.Params.emplace_back(CharSourceRange(), TR);
}
if (auto *RTR = FTR->getResultTypeRepr()) {
SourceLoc SRE = Lexer::getLocForEndOfToken(SM, RTR->getEndLoc());
Info.ReturnTypeRange = CharSourceRange(SM, RTR->getStartLoc(), SRE);
}
return !FoundFunctionTypeRepr;
}
return !FoundFunctionTypeRepr;
}

bool walkToTypeReprPost(TypeRepr *T) override {
// If we just visited the FunctionTypeRepr, end traversal.
return !FoundFunctionTypeRepr;
}
bool walkToTypeReprPost(TypeRepr *T) override {
// If we just visited the FunctionTypeRepr, end traversal.
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;
}
Expand Down Expand Up @@ -1512,6 +1537,22 @@ class PlaceholderExpansionScanner {
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) { }

Expand Down Expand Up @@ -1543,13 +1584,13 @@ class PlaceholderExpansionScanner {
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;
}

Expand Down

0 comments on commit 09a19bb

Please sign in to comment.