Merge pull request #6948 from matthewcarroll/SR-3599-Better-Recovery-For-Decl-Names

slavapestov · web-flow · commit a67f4bdff42c · 2017-01-23T17:37:07.000-08:00
[DiagnosticsQoI] SR-3599: Better recovery for decls with consecutive identifiers
diff --git a/include/swift/Parse/Parser.h b/include/swift/Parse/Parser.h
@@ -463,6 +463,9 @@ class Parser {
 
   /// Parse an #endif.
   bool parseEndIfDirective(SourceLoc &Loc);
+  
+  /// True if Tok is the second consecutive identifier in a variable decl.
+  bool isSecondVarIdentifier();
 
 public:
   InFlightDiagnostic diagnose(SourceLoc Loc, Diagnostic Diag) {
@@ -490,6 +493,10 @@ class Parser {
   }
   
   void diagnoseRedefinition(ValueDecl *Prev, ValueDecl *New);
+  
+  /// Add a fix-it to remove the space in consecutive identifiers.
+  /// Add a camel-cased option if it is different than the first option.
+  void diagnoseConsecutiveIDs(Token First, Token Second);
      
   /// \brief Check whether the current token starts with '<'.
   bool startsWithLess(Token Tok) {
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
@@ -2705,6 +2705,22 @@ parseIdentifierDeclName(Parser &P, Identifier &Result, SourceLoc &L,
                                  ResyncP1, Diagnostic(ID, Args...));
 }
 
+/// Add a fix-it to remove the space in consecutive identifiers.
+/// Add a camel-cased option if it is different than the first option.
+void Parser::diagnoseConsecutiveIDs(Token First, Token Second) {
+  auto Joined = (First.getText() + Second.getText()).str();
+  SourceRange Range(First.getLoc(), Second.getLoc());
+  diagnose(Second.getLoc(), diag::join_identifiers).fixItReplace(Range, Joined);
+  
+  SmallString<8> Scratch;
+  auto SentenceCased = camel_case::toSentencecase(Tok.getText(), Scratch);
+  auto CamelCased = (First.getText() + SentenceCased).str();
+  if (Joined != CamelCased) {
+    diagnose(Second.getLoc(), diag::join_identifiers_camel_case)
+      .fixItReplace(Range, CamelCased);
+  }
+}
+
 /// Parse a Decl item in decl list.
 static ParserStatus parseDeclItem(Parser &P,
                                   bool &PreviousHadSemi,
@@ -2722,9 +2738,20 @@ static ParserStatus parseDeclItem(Parser &P,
   // If the previous declaration didn't have a semicolon and this new
   // declaration doesn't start a line, complain.
   if (!PreviousHadSemi && !P.Tok.isAtStartOfLine() && !P.Tok.is(tok::unknown)) {
-    auto endOfPrevious = P.getEndOfPreviousLoc();
-    P.diagnose(endOfPrevious, diag::declaration_same_line_without_semi)
-      .fixItInsert(endOfPrevious, ";");
+    // Add a fix-it to remove the space in consecutive identifiers
+    // in a property or enum case decl.
+    auto PreviousTok = P.L->getTokenAt(P.PreviousLoc);
+    if (P.Tok.is(tok::identifier) && PreviousTok.is(tok::identifier)) {
+      auto enumContext = P.CurDeclContext->getAsEnumOrEnumExtensionContext();
+      auto TypeName = enumContext ? "enum case" : "property";
+      P.diagnose(P.Tok.getLoc(), diag::repeated_identifier, TypeName);
+      P.diagnoseConsecutiveIDs(PreviousTok, P.Tok);
+      P.skipUntilDeclRBrace(tok::semi, tok::pound_endif);
+    } else {
+      auto endOfPrevious = P.getEndOfPreviousLoc();
+      P.diagnose(endOfPrevious, diag::declaration_same_line_without_semi)
+        .fixItInsert(endOfPrevious, ";");
+    }
   }
 
   auto Result = P.parseDecl(Options, handler);
@@ -4661,26 +4688,7 @@ Parser::parseDeclFunc(SourceLoc StaticLoc, StaticSpellingKind StaticSpelling,
     // space or newline accidentally.
     if (Tok.isIdentifierOrUnderscore() && SimpleName.str().back() != '<') {
       diagnose(Tok.getLoc(), diag::repeated_identifier, "function");
-
-      SourceRange DoubleIdentifierRange(NameLoc, Tok.getLoc());
-
-      // Provide two fix-its: a direct concatenation of the two identifiers
-      // and a camel-cased version.
-
-      auto DirectConcatenation = NameTok.getText().str() + Tok.getText().str();
-
-      diagnose(Tok.getLoc(), diag::join_identifiers)
-        .fixItReplace(DoubleIdentifierRange, DirectConcatenation);
-
-      SmallString<8> CapitalizedScratch;
-      auto Capitalized = camel_case::toSentencecase(Tok.getText(),
-                                                    CapitalizedScratch);
-      auto CamelCaseConcatenation = NameTok.getText().str() + Capitalized.str();
-
-      if (DirectConcatenation != CamelCaseConcatenation)
-        diagnose(Tok.getLoc(), diag::join_identifiers_camel_case)
-          .fixItReplace(DoubleIdentifierRange, CamelCaseConcatenation);
-
+      diagnoseConsecutiveIDs(NameTok, Tok);
       consumeToken();
     }
   }
@@ -4941,6 +4949,14 @@ ParserResult<EnumDecl> Parser::parseDeclEnum(ParseDeclOptions Flags,
     ED->setGenericParams(GenericParams);
   }
 
+  // Add a fix-it to remove the space in consecutive identifiers
+  // in an enum decl.
+  if (Tok.is(tok::identifier)) {
+    diagnose(Tok.getLoc(), diag::repeated_identifier, "enum");
+    diagnoseConsecutiveIDs(L->getTokenAt(EnumNameLoc), Tok);
+    consumeToken();
+  }
+
   SourceLoc LBLoc, RBLoc;
   if (parseToken(tok::l_brace, LBLoc, diag::expected_lbrace_enum)) {
     LBLoc = PreviousLoc;
@@ -5196,6 +5212,14 @@ ParserResult<StructDecl> Parser::parseDeclStruct(ParseDeclOptions Flags,
     SD->setGenericParams(GenericParams);
   }
 
+  // Add a fix-it to remove the space in consecutive identifiers
+  // in a struct decl.
+  if (Tok.is(tok::identifier)) {
+    diagnose(Tok.getLoc(), diag::repeated_identifier, "struct");
+    diagnoseConsecutiveIDs(L->getTokenAt(StructNameLoc), Tok);
+    consumeToken();
+  }
+  
   SourceLoc LBLoc, RBLoc;
   if (parseToken(tok::l_brace, LBLoc, diag::expected_lbrace_struct)) {
     LBLoc = PreviousLoc;
@@ -5279,6 +5303,14 @@ ParserResult<ClassDecl> Parser::parseDeclClass(SourceLoc ClassLoc,
     CD->setGenericParams(GenericParams);
   }
 
+  // Add a fix-it to remove the space in consecutive identifiers
+  // in a class decl.
+  if (Tok.is(tok::identifier)) {
+    diagnose(Tok.getLoc(), diag::repeated_identifier, "class");
+    diagnoseConsecutiveIDs(L->getTokenAt(ClassNameLoc), Tok);
+    consumeToken();
+  }
+
   SourceLoc LBLoc, RBLoc;
   if (parseToken(tok::l_brace, LBLoc, diag::expected_lbrace_class)) {
     LBLoc = PreviousLoc;
diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
@@ -283,10 +283,19 @@ ParserStatus Parser::parseBraceItems(SmallVectorImpl<ASTNode> &Entries,
     // If the previous statement didn't have a semicolon and this new
     // statement doesn't start a line, complain.
     if (!PreviousHadSemi && !Tok.isAtStartOfLine()) {
-      SourceLoc EndOfPreviousLoc = getEndOfPreviousLoc();
-      diagnose(EndOfPreviousLoc, diag::statement_same_line_without_semi)
-        .fixItInsert(EndOfPreviousLoc, ";");
-      // FIXME: Add semicolon to the AST?
+      // Add a fix-it to remove the space in consecutive identifiers
+      // in a variable decl.
+      if (isSecondVarIdentifier()) {
+        diagnose(Tok.getLoc(), diag::repeated_identifier, "variable");
+        auto Previous = L->getTokenAt(PreviousLoc);
+        diagnoseConsecutiveIDs(Previous, Tok);
+        skipUntilDeclRBrace(tok::semi, tok::pound_endif);
+      } else {
+        SourceLoc EndOfPreviousLoc = getEndOfPreviousLoc();
+        diagnose(EndOfPreviousLoc, diag::statement_same_line_without_semi)
+          .fixItInsert(EndOfPreviousLoc, ";");
+        // FIXME: Add semicolon to the AST?
+      }
     }
 
     ParserPosition BeginParserPosition;
diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp
@@ -737,6 +737,27 @@ void Parser::diagnoseRedefinition(ValueDecl *Prev, ValueDecl *New) {
              Prev->getName());
 }
 
+/// True if Tok is the second consecutive identifier in a variable decl.
+bool Parser::isSecondVarIdentifier() {
+  auto PreviousTok = L->getTokenAt(PreviousLoc);
+  if (Tok.isNot(tok::identifier) || PreviousTok.isNot(tok::identifier)) {
+    return false;
+  }
+  auto LineStart = L->getLocForStartOfLine(SourceMgr, Tok.getLoc());
+  auto FirstTok = L->getTokenAt(LineStart);
+  Lexer::State FirstTokState = L->getStateForBeginningOfToken(FirstTok);
+  auto StartPos = ParserPosition(FirstTokState, LineStart);
+  auto RestorePos = getParserPosition();
+  backtrackToPosition(StartPos);
+  while (Tok.isNot(tok::kw_let) && Tok.isNot(tok::kw_var)
+         && Tok.getLoc() != PreviousTok.getLoc()) {
+    consumeToken();
+  }
+  bool IsConsecutiveLoc = peekToken().getLoc() == PreviousTok.getLoc();
+  restoreParserPosition(RestorePos);
+  return IsConsecutiveLoc;
+}
+
 struct ParserUnit::Implementation {
   LangOptions LangOpts;
   SearchPathOptions SearchPathOpts;
diff --git a/test/Parse/recovery.swift b/test/Parse/recovery.swift
@@ -242,6 +242,58 @@ class NoBracesClass2 // expected-error {{expected '{' in class}}
 protocol NoBracesProtocol2 // expected-error {{expected '{' in protocol type}}
 extension NoBracesStruct2 // expected-error {{expected '{' in extension}}
 
+//===--- Recovery for multiple identifiers in decls
+
+enum EE EE {}
+// expected-error @-1 {{found an unexpected second identifier in enum declaration; is there an accidental break?}} 
+// expected-note @-2 {{join the identifiers together}} {{6-11=EEEE}}
+
+struct SS SS {}
+// expected-error @-1 {{found an unexpected second identifier in struct declaration; is there an accidental break?}}
+// expected-note @-2 {{join the identifiers together}} {{8-13=SSSS}}
+
+class CC CC {}
+// expected-error @-1 {{found an unexpected second identifier in class declaration; is there an accidental break?}}
+// expected-note @-2 {{join the identifiers together}} {{7-12=CCCC}}
+// expected-note @-3 {{did you mean 'CC'}}
+
+enum EEE {
+  
+  case a a
+  // expected-error @-1 {{found an unexpected second identifier in enum case declaration; is there an accidental break?}} 
+  // expected-note @-2 {{join the identifiers together}} {{8-11=aa}}
+  // expected-note @-3 {{join the identifiers together with camel-case}} {{8-11=aA}}
+  
+  case b
+}
+
+struct AA {
+  
+  private var a b = 0
+  // expected-error @-1 {{found an unexpected second identifier in property declaration; is there an accidental break?}}
+  // expected-note @-2 {{join the identifiers together}} {{15-18=ab}}
+  // expected-note @-3 {{join the identifiers together with camel-case}} {{15-18=aB}}
+  // expected-error @-4 {{type annotation missing in pattern}}
+  
+  func f() {
+    var c d = 5
+    // expected-error @-1 {{found an unexpected second identifier in variable declaration; is there an accidental break?}}
+    // expected-note @-2 {{join the identifiers together}} {{9-12=cd}}
+    // expected-note @-3 {{join the identifiers together with camel-case}} {{9-12=cD}}
+    // expected-error @-4 {{type annotation missing in pattern}}
+    
+    let _ = 0
+  }
+}
+
+let e f = 5
+// expected-error @-1 {{found an unexpected second identifier in variable declaration; is there an accidental break?}}
+// expected-note @-2 {{join the identifiers together}} {{5-8=ef}}
+// expected-note @-3 {{join the identifiers together with camel-case}} {{5-8=eF}}
+// expected-error @-4 {{type annotation missing in pattern}}
+// expected-note @-5 * {{did you mean 'e'?}}
+
+
 //===--- Recovery for parse errors in types.
 
 struct ErrorTypeInVarDecl1 {
diff --git a/validation-test/compiler_crashers_fixed/28619-basety-islvaluetype-basety-is-anymetatypetype.swift b/validation-test/compiler_crashers_fixed/28619-basety-islvaluetype-basety-is-anymetatypetype.swift
@@ -5,7 +5,7 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// RUN: not --crash %target-swift-frontend %s -emit-ir
+// RUN: not %target-swift-frontend %s -emit-ir
 // REQUIRES: asserts
 protocol a:RangeReplaceableCollection
 class a
