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

Add a diagnostic to remove the name of an initializer, deinitializer, or subscript. The identifier token is consumed and skipped to prevent the parser from emitting additional error messages.

Add tests to verify that the name is removed from initializers, deinitializers, and subscripts declared with a name.

Author: matthewcarroll

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

@@ -5515,7 +5515,12 @@ 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,21 @@ 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);
+      diagID = skipIdentifier ? diag::subscript_has_name
+                              : diag::expected_lparen_subscript;
       break;
     case ParameterContextKind::Initializer:
-      diagID = diag::expected_lparen_initializer;
+      skipIdentifier = Tok.is(tok::identifier);
+      diagID = skipIdentifier ? diag::initializer_has_name
+                              : diag::expected_lparen_initializer;
       break;
     case ParameterContextKind::Closure:
     case ParameterContextKind::Curried:
@@ -496,6 +501,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,10 @@ 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=}}
+}
+
 // 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 {{subscripts cannot have a name}} {{5-7=}}
     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 {
@@ -173,9 +176,3 @@ struct A8 {
     }
   }
 } // expected-error{{extraneous '}' at top level}} {{1-3=}}
-
-struct A9 {
-  subscript -> Int { // expected-error {{expected '(' for subscript parameters}} {{12-12=()}}
-    return 1
-  }
-}