[CodeCompletion] Improve StmtCondition recovery for code-completion

In the condition expressions of if, while and guard statements we were
throwing away the AST if there was a parse error in the condition, or
the brace statement was missing.  This led to poor code-completion for
unresolved members (enums, options sets) because we couldn't find the
parent expression to type-check.

There are a few minor diagnostic changes because we now do more
type-checking of these conditions, particularly if they end up
containing an unused closure.

SR-2001

Author: benlangmuir

lib/Parse/ParseStmt.cpp

@@ -1390,7 +1390,23 @@ ParserResult<Stmt> Parser::parseStmtIf(LabeledStmtInfo LabelInfo) {
   // by a conditional binding. The else branch does *not* see these variables.
   {
     Scope S(this, ScopeKind::IfVars);
-  
+
+    auto recoverWithCond = [&](ParserStatus Status,
+                               StmtCondition Condition) -> ParserResult<Stmt> {
+      if (Condition.empty()) {
+        SmallVector<StmtConditionElement, 1> ConditionElems;
+        ConditionElems.emplace_back(new (Context) ErrorExpr(IfLoc));
+        Condition = Context.AllocateCopy(ConditionElems);
+      }
+      auto EndLoc = Condition.back().getEndLoc();
+      return makeParserResult(
+          Status,
+          new (Context) IfStmt(
+              LabelInfo, IfLoc, Condition,
+              BraceStmt::create(Context, EndLoc, {}, EndLoc, /*implicit=*/true),
+              SourceLoc(), nullptr));
+    };
+
     if (Tok.is(tok::l_brace)) {
       SourceLoc LBraceLoc = Tok.getLoc();
       diagnose(IfLoc, diag::missing_condition_after_if)
@@ -1401,17 +1417,14 @@ ParserResult<Stmt> Parser::parseStmtIf(LabeledStmtInfo LabelInfo) {
     } else {
       Status |= parseStmtCondition(Condition, diag::expected_condition_if,
                                    StmtKind::If);
-      if (Status.isError() || Status.hasCodeCompletion()) {
-        // FIXME: better recovery
-        return makeParserResult<Stmt>(Status, nullptr);
-      }
+      if (Status.isError() || Status.hasCodeCompletion())
+        return recoverWithCond(Status, Condition);
     }
 
     NormalBody = parseBraceItemList(diag::expected_lbrace_after_if);
-    if (NormalBody.isNull())
-      return nullptr; // FIXME: better recovery
-
     Status |= NormalBody;
+    if (NormalBody.isNull())
+      return recoverWithCond(Status, Condition);
   }
 
   // The else branch, if any, is outside of the scope of the condition.
@@ -1441,7 +1454,22 @@ ParserResult<Stmt> Parser::parseStmtGuard() {
   ParserStatus Status;
   StmtCondition Condition;
   ParserResult<BraceStmt> Body;
-  
+
+  auto recoverWithCond = [&](ParserStatus Status,
+                             StmtCondition Condition) -> ParserResult<Stmt> {
+    if (Condition.empty()) {
+      SmallVector<StmtConditionElement, 1> ConditionElems;
+      ConditionElems.emplace_back(new (Context) ErrorExpr(GuardLoc));
+      Condition = Context.AllocateCopy(ConditionElems);
+    }
+    auto EndLoc = Condition.back().getEndLoc();
+    return makeParserResult(
+        Status,
+        new (Context) GuardStmt(
+            GuardLoc, Condition,
+            BraceStmt::create(Context, EndLoc, {}, EndLoc, /*implicit=*/true)));
+  };
+
   if (Tok.is(tok::l_brace)) {
     SourceLoc LBraceLoc = Tok.getLoc();
     diagnose(GuardLoc, diag::missing_condition_after_guard)
@@ -1454,15 +1482,15 @@ ParserResult<Stmt> Parser::parseStmtGuard() {
                                  StmtKind::Guard);
     if (Status.isError() || Status.hasCodeCompletion()) {
       // FIXME: better recovery
-      return makeParserResult<Stmt>(Status, nullptr);
+      return recoverWithCond(Status, Condition);
     }
   }
 
   // Parse the 'else'.  If it is missing, and if the following token isn't a {
   // then the parser is hopelessly lost - just give up instead of spewing.
   if (parseToken(tok::kw_else, diag::expected_else_after_guard) &&
       Tok.isNot(tok::l_brace))
-    return makeParserError();
+    return recoverWithCond(Status, Condition);
 
   // Before parsing the body, disable all of the bound variables so that they
   // cannot be used unbound.
@@ -1479,8 +1507,8 @@ ParserResult<Stmt> Parser::parseStmtGuard() {
 
   Body = parseBraceItemList(diag::expected_lbrace_after_guard);
   if (Body.isNull())
-    return nullptr; // FIXME: better recovery
-  
+    return recoverWithCond(Status, Condition);
+
   Status |= Body;
 
   return makeParserResult(Status,
@@ -1810,6 +1838,21 @@ ParserResult<Stmt> Parser::parseStmtWhile(LabeledStmtInfo LabelInfo) {
   ParserStatus Status;
   StmtCondition Condition;
 
+  auto recoverWithCond = [&](ParserStatus Status,
+                             StmtCondition Condition) -> ParserResult<Stmt> {
+    if (Condition.empty()) {
+      SmallVector<StmtConditionElement, 1> ConditionElems;
+      ConditionElems.emplace_back(new (Context) ErrorExpr(WhileLoc));
+      Condition = Context.AllocateCopy(ConditionElems);
+    }
+    auto EndLoc = Condition.back().getEndLoc();
+    return makeParserResult(
+        Status,
+        new (Context) WhileStmt(
+            LabelInfo, WhileLoc, Condition,
+            BraceStmt::create(Context, EndLoc, {}, EndLoc, /*implicit=*/true)));
+  };
+
   if (Tok.is(tok::l_brace)) {
     SourceLoc LBraceLoc = Tok.getLoc();
     diagnose(WhileLoc, diag::missing_condition_after_while)
@@ -1820,18 +1863,15 @@ ParserResult<Stmt> Parser::parseStmtWhile(LabeledStmtInfo LabelInfo) {
   } else {
     Status |= parseStmtCondition(Condition, diag::expected_condition_while,
                                  StmtKind::While);
-    if (Status.isError() || Status.hasCodeCompletion()) {
-      // FIXME: better recovery
-      return makeParserResult<Stmt>(Status, nullptr);
-    }
+    if (Status.isError() || Status.hasCodeCompletion())
+      return recoverWithCond(Status, Condition);
   }
 
   ParserResult<BraceStmt> Body =
       parseBraceItemList(diag::expected_lbrace_after_while);
-  if (Body.isNull())
-    return nullptr; // FIXME: better recovery
-
   Status |= Body;
+  if (Body.isNull())
+    return recoverWithCond(Status, Condition);
 
   return makeParserResult(
       Status, new (Context) WhileStmt(LabelInfo, WhileLoc, Condition,

test/IDE/complete_stmt_controlling_expr.swift

@@ -116,6 +116,22 @@
 // RUN: FileCheck %s -check-prefix=WITH_I_INT_LOCAL < %t.where.txt
 // RUN: FileCheck %s -check-prefix=WITH_J_INT < %t.where.txt
 
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_IF_1 | FileCheck %s -check-prefix=UNRESOLVED_B
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_IF_2 | FileCheck %s -check-prefix=UNRESOLVED_B
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_IF_3 | FileCheck %s -check-prefix=UNRESOLVED_B
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_IF_4 | FileCheck %s -check-prefix=UNRESOLVED_B
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_WHILE_1 | FileCheck %s -check-prefix=UNRESOLVED_B
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_WHILE_2 | FileCheck %s -check-prefix=UNRESOLVED_B
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_WHILE_3 | FileCheck %s -check-prefix=UNRESOLVED_B
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_WHILE_4 | FileCheck %s -check-prefix=UNRESOLVED_B
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_GUARD_1 | FileCheck %s -check-prefix=UNRESOLVED_B
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_GUARD_2 | FileCheck %s -check-prefix=UNRESOLVED_B
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_GUARD_3 | FileCheck %s -check-prefix=UNRESOLVED_B
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_GUARD_4 | FileCheck %s -check-prefix=UNRESOLVED_B
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_GUARD_5 | FileCheck %s -check-prefix=UNRESOLVED_B
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_GUARD_6 | FileCheck %s -check-prefix=UNRESOLVED_B
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_GUARD_7 | FileCheck %s -check-prefix=UNRESOLVED_B
+
 
 struct FooStruct {
   var instanceVar : Int
@@ -496,3 +512,63 @@ func testSwitchCaseWhereExprIJ1(_ fooObject: FooStruct) {
 
 // WITH_I_E_EXPR_SPECIFIC: Decl[LocalVar]/ExprSpecific: i[#Int#]{{; name=.+$}}
 // WITH_I_E_EXPR_SPECIFIC: Decl[LocalVar]/Local:        e[#Int#]{{; name=.+$}}
+
+enum A { case aaa }
+enum B { case bbb }
+// UNRESOLVED_B-NOT: aaa
+// UNRESOLVED_B: Decl[EnumElement]/ExprSpecific:     bbb[#B#]; name=bbb
+// UNRESOLVED_B-NOT: aaa
+
+struct AA {
+  func takeEnum(_: A) {}
+}
+struct BB {
+  func takeEnum(_: B) {}
+}
+func testUnresolvedIF1(x: BB) {
+  if x.takeEnum(.#^UNRESOLVED_IF_1^#)
+}
+func testUnresolvedIF2(x: BB) {
+  if true, x.takeEnum(.#^UNRESOLVED_IF_2^#)
+}
+func testUnresolvedIF3(x: BB) {
+  if true, x.takeEnum(.#^UNRESOLVED_IF_3^#) {}
+}
+func testUnresolvedIF4(x: BB) {
+  if let x.takeEnum(.#^UNRESOLVED_IF_4^#)
+}
+
+func testUnresolvedWhile1(x: BB) {
+  while x.takeEnum(.#^UNRESOLVED_WHILE_1^#)
+}
+func testUnresolvedWhile2(x: BB) {
+  while true, x.takeEnum(.#^UNRESOLVED_WHILE_2^#)
+}
+func testUnresolvedWhile3(x: BB) {
+  while let x.takeEnum(.#^UNRESOLVED_WHILE_3^#)
+}
+func testUnresolvedWhile4(x: BB) {
+  while true, x.takeEnum(.#^UNRESOLVED_WHILE_4^#) {}
+}
+
+func testUnresolvedGuard1(x: BB) {
+  guard x.takeEnum(.#^UNRESOLVED_GUARD_1^#)
+}
+func testUnresolvedGuard2(x: BB) {
+  guard x.takeEnum(.#^UNRESOLVED_GUARD_2^#) {}
+}
+func testUnresolvedGuard3(x: BB) {
+  guard x.takeEnum(.#^UNRESOLVED_GUARD_3^#) else
+}
+func testUnresolvedGuard4(x: BB) {
+  guard x.takeEnum(.#^UNRESOLVED_GUARD_4^#) else {}
+}
+func testUnresolvedGuard5(x: BB) {
+  guard true, x.takeEnum(.#^UNRESOLVED_GUARD_5^#)
+}
+func testUnresolvedGuard6(x: BB) {
+  guard let x.takeEnum(.#^UNRESOLVED_GUARD_6^#)
+}
+func testUnresolvedGuard7(x: BB) {
+  guard let x.takeEnum(.#^UNRESOLVED_GUARD_7^#) else {}
+}

test/stmt/if_while_var.swift

@@ -25,8 +25,10 @@ class B {} // expected-note * {{did you mean 'B'?}}
 class D : B {}// expected-note * {{did you mean 'D'?}}
 
 // TODO poor recovery in these cases
-if let {} // expected-error {{expected '{' after 'if' condition}}
-if let x = {} // expected-error{{'{' after 'if'}} expected-error {{variable binding in a condition requires an initializer}}
+if let {} // expected-error {{expected '{' after 'if' condition}} expected-error {{pattern matching in a condition requires the 'case' keyword}}
+if let x = {} // expected-error{{'{' after 'if'}} expected-error {{variable binding in a condition requires an initializer}} expected-error{{initializer for conditional binding must have Optional type, not '() -> ()'}}
+
+/Users/blangmuir/src/s/swift/test/stmt/if_while_var.swift
 
 if let x = foo() {
 } else {
@@ -72,13 +74,13 @@ if 1 != 2, let x : Int? = opt {}
 
 // Test error recovery.
 // <rdar://problem/19939746> Improve error recovery for malformed if statements
-if 1 != 2, {  
+if 1 != 2, { // expected-error {{type '() -> ()' does not conform to protocol 'Boolean'}}
 } // expected-error {{expected '{' after 'if' condition}}
 if 1 != 2, 4 == 57 {}
 if 1 != 2, 4 == 57, let x = opt {}
 
 // Test that these don't cause the parser to crash.
-if true { if a == 0; {} }   // expected-error {{expected '{' after 'if' condition}} expected-error 2{{}}
+if true { if a == 0; {} }   // expected-error {{expected '{' after 'if' condition}} expected-error 3{{}}
 if a == 0, where b == 0 {}  // expected-error 4{{}} expected-note {{}} {{25-25=_ = }}
 
 

validation-test/IDE/crashers/047-swift-typechecker-typechecktoplevelcodedecl.swift

@@ -0,0 +1,2 @@
+// RUN: %target-swift-ide-test -code-completion -code-completion-token=A -source-filename=%s
+if c={func b:#^A^#