Skip to content

[SR-12583] Added checks to catch 'foo() {} {}' and 'foo {} () {}' #32000

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -1189,6 +1189,9 @@ WARNING(trailing_closure_after_newlines,none,
NOTE(trailing_closure_callee_here,none,
"callee is here", ())

ERROR(double_trailing_closure,none,
"double trailing closures", ())

ERROR(string_literal_no_atsign,none,
"string literals in Swift are not preceded by an '@' sign", ())

Expand Down
45 changes: 45 additions & 0 deletions lib/Parse/ParseExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1225,14 +1225,41 @@ Parser::parseExprPostfixSuffix(ParserResult<Expr> Result, bool isExprBasic,
if (trailingClosures.empty())
return nullptr;

// We are creating a call node
// with trailing closure
// if we are already in a call node with trailing closure we should diagnose
// (call_expr
// (call_expr
// (paren_expr trailing-closure))
// (paren_expr trailing-closure))
// this catches one problem
// let q1 = bar()
// {7 * $0}
// {5 * $0}
// but not the problem where the paren after bar() is
// after the first closure (with 7) instead
// TODO: is there any legitimate case for that parse?

// Trailing closure implicitly forms a call.

if (!trailingClosures.empty()
&& Result.get()->getKind() == ExprKind::Call) {
auto call = static_cast<CallExpr*>(Result.get());
if (call->hasTrailingClosure()) {
//diagnose and return bad result
diagnose(trailingClosures[0].ClosureExpr->getEndLoc(), diag::double_trailing_closure);
break;
}
}

Result = makeParserResult(
ParserStatus(Result) | trailingResult,
CallExpr::create(Context, Result.get(), SourceLoc(), {}, {}, {},
SourceLoc(), trailingClosures,
/*implicit=*/false));
SyntaxContext->createNodeInPlace(SyntaxKind::FunctionCallExpr);

// I think this is intended to handle the double-trailing-closures condition but does not catch it.
// We only allow a single trailing closure on a call. This could be
// generalized in the future, but needs further design.
if (Tok.is(tok::l_brace))
Expand Down Expand Up @@ -1599,6 +1626,7 @@ ParserResult<Expr> Parser::parseExprPrimary(Diag<> ID, bool isExprBasic) {
return makeParserErrorResult(new (Context) ErrorExpr(DotLoc));
SyntaxContext->createNodeInPlace(SyntaxKind::MemberAccessExpr);

// () suffix
// Check for a () suffix, which indicates a call when constructing
// this member. Note that this cannot be the start of a new line.
if (Tok.isFollowingLParen()) {
Expand Down Expand Up @@ -3448,7 +3476,24 @@ Parser::parseExprCallSuffix(ParserResult<Expr> fn, bool isExprBasic) {
trailingClosures,
SyntaxKind::TupleExprElementList);

// check for the pattern:
// let q2 =
// bar {7 * $0} ()
// {5 * $0}
// with the () after the first trailing closure

// Form the call.

if (!trailingClosures.empty()
&& fn.get()->getKind() == ExprKind::Call) {
auto call = static_cast<CallExpr*>(fn.get());
if (call->hasTrailingClosure()) {
diagnose(trailingClosures[0].ClosureExpr->getEndLoc(), diag::double_trailing_closure);
// return nullptr or error here?
return nullptr;
}
}

return makeParserResult(status | fn,
CallExpr::create(Context, fn.get(), lParenLoc, args,
argLabels, argLabelLocs, rParenLoc,
Expand Down
19 changes: 19 additions & 0 deletions test/Parse/double_trailing_closure.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// RUN: %target-typecheck-verify-swift

func bar(_ f: @escaping (Int) -> Int) -> ((Int) -> Int) -> Int {
let r : ((Int) -> Int) -> Int =
{(g: (_ r1 : Int) -> Int) in
g(f(2))
}
return r
}

// bad 1
let q1 = bar()
{7 * $0}
{5 * $0} // expected-error {{double trailing closures}}

// bad 2 (note that () can't be on its own line
let q2 =
bar {7 * $0} ()
{5 * $0} // expected-error {{double trailing closures}}