Merge pull request #6863 from matthewcarroll/SR-3600-Better-recovery-for-naming-an-initializer-deinitializer-or-subscript

[DiagnosticsQoI] SR-3600: Better recovery for trying to name an initializer, deinitializer, or subscript

Author: jrose-apple

include/swift/AST/DiagnosticsParse.def

@@ -338,6 +338,8 @@ ERROR(subscript_decl_wrong_scope,none,
       "'subscript' functions may only be declared within a type", ())
 ERROR(expected_lparen_subscript,PointsToFirstBadToken,
       "expected '(' for subscript parameters", ())
+ERROR(subscript_has_name,PointsToFirstBadToken,
+      "subscripts cannot have a name", ())
 ERROR(expected_arrow_subscript,PointsToFirstBadToken,
       "expected '->' for subscript element type", ())
 ERROR(expected_type_subscript,PointsToFirstBadToken,
@@ -360,12 +362,17 @@ ERROR(initializer_decl_wrong_scope,none,
       "initializers may only be declared within a type", ())
 ERROR(expected_lparen_initializer,PointsToFirstBadToken,
       "expected '(' for initializer parameters", ())
+ERROR(initializer_has_name,PointsToFirstBadToken,
+      "initializers cannot have a name", ())
 
 // Destructor
 ERROR(destructor_decl_outside_class,none,
       "deinitializers may only be declared within a class", ())
 ERROR(expected_lbrace_destructor,PointsToFirstBadToken,
       "expected '{' for deinitializer", ())
+ERROR(destructor_has_name,PointsToFirstBadToken,
+      "deinitializers cannot have a name", ())
+
 ERROR(opened_destructor_expected_rparen,none,
       "expected ')' to close parameter list", ())
 ERROR(destructor_params,none,

lib/Parse/ParseDecl.cpp

@@ -5650,7 +5650,10 @@ parseDeclDeinit(ParseDeclOptions Flags, DeclAttributes &Attributes) {
   // '{'
   if (!Tok.is(tok::l_brace)) {
     if (!Tok.is(tok::l_brace) && !isInSILMode()) {
-      diagnose(Tok, diag::expected_lbrace_destructor);
+      if (Tok.is(tok::identifier)) {
+        diagnose(Tok, diag::destructor_has_name).fixItRemove(Tok.getLoc());
+      } else
+        diagnose(Tok, diag::expected_lbrace_destructor);
       return nullptr;
     }
   }

lib/Parse/ParsePattern.cpp

@@ -476,16 +476,23 @@ Parser::parseSingleParameterClause(ParameterContextKind paramContext,
   if (!Tok.is(tok::l_paren)) {
     // If we don't have the leading '(', complain.
     Diag<> diagID;
+    bool skipIdentifier = false;
     switch (paramContext) {
     case ParameterContextKind::Function:
     case ParameterContextKind::Operator:
       diagID = diag::func_decl_without_paren;
       break;
     case ParameterContextKind::Subscript:
-      diagID = diag::expected_lparen_subscript;
+      skipIdentifier = Tok.is(tok::identifier) &&
+                       peekToken().is(tok::l_paren);
+      diagID = skipIdentifier ? diag::subscript_has_name
+                              : diag::expected_lparen_subscript;
       break;
     case ParameterContextKind::Initializer:
-      diagID = diag::expected_lparen_initializer;
+      skipIdentifier = Tok.is(tok::identifier) &&
+                       peekToken().is(tok::l_paren);
+      diagID = skipIdentifier ? diag::initializer_has_name
+                              : diag::expected_lparen_initializer;
       break;
     case ParameterContextKind::Closure:
     case ParameterContextKind::Curried:
@@ -496,6 +503,12 @@ Parser::parseSingleParameterClause(ParameterContextKind paramContext,
     if (Tok.isAny(tok::l_brace, tok::arrow, tok::kw_throws, tok::kw_rethrows))
       diag.fixItInsertAfter(PreviousLoc, "()");
 
+    if (skipIdentifier) {
+      diag.fixItRemove(Tok.getLoc());
+      consumeToken();
+      skipSingle();
+    }
+    
     // Create an empty parameter list to recover.
     return makeParserErrorResult(
         ParameterList::createEmpty(Context, PreviousLoc, PreviousLoc));

test/Parse/init_deinit.swift

@@ -17,6 +17,8 @@ struct FooStructConstructorC {
 
 struct FooStructDeinitializerA {
   deinit // expected-error {{expected '{' for deinitializer}}
+  deinit x // expected-error {{deinitializers cannot have a name}} {{10-12=}}
+  deinit x() // expected-error {{deinitializers cannot have a name}} {{10-11=}}
 }
 
 struct FooStructDeinitializerB {

test/Parse/recovery.swift

@@ -537,6 +537,22 @@ func a(s: S[{{g) -> Int {}
 // expected-error@+1{{expected an identifier to name generic parameter}}
 func F() { init<( } )} // expected-note {{did you mean 'F'?}}
 
+struct InitializerWithName {
+  init x() {} // expected-error {{initializers cannot have a name}} {{8-9=}}
+}
+
+struct InitializerWithNameAndParam {
+  init a(b: Int) {} // expected-error {{initializers cannot have a name}} {{8-9=}}
+}
+
+struct InitializerWithLabels {
+  init c d: Int {}
+  // expected-error @-1 {{expected '(' for initializer parameters}}
+  // expected-error @-2 {{expected declaration}}
+  // expected-error @-3 {{consecutive declarations on a line must be separated by ';'}}
+  // expected-note @-5 {{in declaration of 'InitializerWithLabels'}}
+}
+
 // rdar://20337695
 func f1() {
 

test/Parse/subscripting.swift

@@ -80,9 +80,8 @@ let y2 = Y2() // expected-note{{change 'let' to 'var' to make it mutable}}{{1-4=
 _ = y2[0] // expected-error{{cannot use mutating getter on immutable value: 'y2' is a 'let' constant}}
 
 // Parsing errors
-// FIXME: Recovery here is horrible
 struct A0 {
-  subscript // expected-error{{expected '(' for subscript parameters}}
+  subscript // expected-error {{expected '(' for subscript parameters}}
     i : Int
      -> Int {
     get {
@@ -92,6 +91,10 @@ struct A0 {
       stored = value
     }
   }
+  
+  subscript -> Int { // expected-error {{expected '(' for subscript parameters}} {{12-12=()}}
+    return 1
+  }
 }
 
 struct A1 {
@@ -172,10 +175,23 @@ struct A8 {
       stored = value
     }
   }
-} // expected-error{{extraneous '}' at top level}} {{1-3=}}
 
 struct A9 {
-  subscript -> Int { // expected-error {{expected '(' for subscript parameters}} {{12-12=()}}
-    return 1
+  subscript x() -> Int { // expected-error {{subscripts cannot have a name}} {{13-14=}}
+    return 0
+  }
+}
+
+struct A10 {
+  subscript x(i: Int) -> Int { // expected-error {{subscripts cannot have a name}} {{13-14=}}
+    return 0
   }
 }
+
+struct A11 {
+  subscript x y : Int -> Int { // expected-error {{expected '(' for subscript parameters}}
+    return 0
+  }
+}
+
+} // expected-error{{extraneous '}' at top level}} {{1-3=}}