Fix <rdar://23036383> QoI: Invalid trailing closures in stmt-conditions produce lowsy diagnostics

It is a common problem that people use a call to a function with a
trailing closure in a if condition, foreach loop, etc.  These don't allow
trailing closures, because they are ambiguous with the brace-enclosed body
of the conditional statement.

In an effort to improve QoI on this, perform lookahead to disambiguate the most common
form of this, in a very narrow situation.  This allows us to produce a nice error
with fixit hints like:

t.swift:26:25: error: trailing closure requires parentheses for disambiguation in this context
for _ in numbers.filter {$0 > 4} {
                        ^

instead of spewing out this garbage:

t.swift:26:26: error: anonymous closure argument not contained in a closure
for _ in numbers.filter {$0 > 4} {
                         ^
t.swift:26:33: error: consecutive statements on a line must be separated by ';'
for _ in numbers.filter {$0 > 4} {
                                ^
                                ;
t.swift:26:34: error: statement cannot begin with a closure expression
for _ in numbers.filter {$0 > 4} {
                                 ^
t.swift:26:34: note: explicitly discard the result of the closure by assigning to '_'
for _ in numbers.filter {$0 > 4} {
                                 ^
                                 _ =
t.swift:26:34: error: braced block of statements is an unused closure
for _ in numbers.filter {$0 > 4} {
                                 ^
t.swift:26:18: error: type '(@NoEscape (Int) throws -> Bool) throws -> [Int]' does not conform to protocol 'Sequence'
for _ in numbers.filter {$0 > 4} {
                 ^
t.swift:26:34: error: expression resolves to an unused function
for _ in numbers.filter {$0 > 4} {
                                 ^

Author: lattner

Diff for: include/swift/AST/DiagnosticsParse.def

@@ -987,6 +987,10 @@ ERROR(unexpected_tokens_before_closure_in,none,
 ERROR(expected_closure_rbrace,none,
       "expected '}' at end of closure", ())
 
+ERROR(trailing_closure_requires_parens,none,
+      "trailing closure requires parentheses for disambiguation in this"
+      " context", ())
+
 WARNING(trailing_closure_excess_newlines,none,
         "trailing closure is separated from call site by multiple newlines", ())
 NOTE(trailing_closure_call_here,none,

Diff for: lib/Parse/ParseExpr.cpp

@@ -746,9 +746,66 @@ static bool isStartOfGetSetAccessor(Parser &P) {
          P.Tok.isContextualKeyword("willSet");
 }
 
+/// Disambiguate and diagnose invalid uses of trailing closures in a situation
+/// where the parser requires an expr-basic (which does not allow them).  We
+/// handle this by doing some lookahead in common situations and emitting a
+/// diagnostic with a fixit to add wrapping parens.
+static bool isValidTrailingClosure(bool isExprBasic, Expr *baseExpr, Parser &P){
+  assert(P.Tok.is(tok::l_brace) && "Couldn't be a trailing closure");
+  
+  // If this is the start of a get/set accessor, then it isn't a trailing
+  // closure.
+  if (isStartOfGetSetAccessor(P))
+    return false;
+
+  // If this is a normal expression (not an expr-basic) then trailing closures
+  // are allowed, so this is obviously one.
+  // TODO: We could handle try to diambiguate cases like:
+  //   let x = foo
+  //   {...}()
+  // by looking ahead for the ()'s, but this has been replaced by do{}, so this
+  // probably isn't worthwhile.
+  //
+  if (!isExprBasic)
+    return true;
+  
+  // If this is an expr-basic, then a trailing closure is not allowed.  However,
+  // it is very common for someone to write something like:
+  //
+  //    for _ in numbers.filter {$0 > 4} {
+  //
+  // and we want to recover from this very well.   We need to perform arbitrary
+  // look-ahead to disambiguate this case, so we only do this in the case where
+  // the token after the { is on the same line as the {.
+  if (P.peekToken().isAtStartOfLine())
+    return false;
+  
+  
+  // Determine if the {} goes with the expression by eating it, and looking
+  // to see if it is immediately followed by another {.  If so, we consider it
+  // to be part of the proceeding expression.
+  Parser::BacktrackingScope backtrack(P);
+  auto startLoc = P.consumeToken(tok::l_brace);
+  P.skipUntil(tok::r_brace);
+  SourceLoc endLoc;
+  if (!P.consumeIf(tok::r_brace, endLoc) ||
+      P.Tok.isNot(tok::l_brace))
+    return false;
+  
+  // Diagnose the bad case and return true so that the caller parses this as a
+  // trailing closure.
+  P.diagnose(startLoc, diag::trailing_closure_requires_parens)
+    .fixItInsert(baseExpr->getStartLoc(), "(")
+    .fixItInsertAfter(endLoc, ")");
+  return true;
+}
+
+
+
 /// Map magic literal tokens such as #file to their
 /// MagicIdentifierLiteralExpr kind.
-MagicIdentifierLiteralExpr::Kind getMagicIdentifierLiteralKind(tok Kind) {
+static MagicIdentifierLiteralExpr::Kind
+getMagicIdentifierLiteralKind(tok Kind) {
   switch (Kind) {
   case tok::kw___COLUMN__:
   case tok::pound_column:
@@ -771,6 +828,7 @@ MagicIdentifierLiteralExpr::Kind getMagicIdentifierLiteralKind(tok Kind) {
   }
 }
 
+
 /// parseExprPostfix
 ///
 ///   expr-literal:
@@ -1260,8 +1318,8 @@ ParserResult<Expr> Parser::parseExprPostfix(Diag<> ID, bool isExprBasic) {
     }
 
     // Check for a trailing closure, if allowed.
-    if (!isExprBasic && Tok.is(tok::l_brace) &&
-        !isStartOfGetSetAccessor(*this)) {
+    if (Tok.is(tok::l_brace) &&
+        isValidTrailingClosure(isExprBasic, Result.get(), *this)) {
       SourceLoc braceLoc = Tok.getLoc();
       // Parse the closure.
       ParserResult<Expr> closure = parseExprClosure();

Diff for: test/Parse/recovery.swift

@@ -673,3 +673,14 @@ func postfixDot(a : String) {
   _ = a.       // expected-error {{expected member name following '.'}}
     a.         // expected-error {{expected member name following '.'}}
 }
+
+// <rdar://problem/23036383> QoI: Invalid trailing closures in stmt-conditions produce lowsy diagnostics
+func r23036383(arr : [Int]?) {
+  if let _ = arr?.map {$0+1} {  // expected-error {{trailing closure requires parentheses for disambiguation in this context}} {{14-14=(}} {{29-29=)}}
+  }
+
+  let numbers = [1, 2]
+  for _ in numbers.filter {$0 > 4} {  // expected-error {{trailing closure requires parentheses for disambiguation in this context}} {{12-12=(}} {{35-35=)}}
+  }
+}
+