If 'throws' is written after the result type in a function decl, suggest

moving it to before the arrow.

<rdar://problem/20857518> QoI for "throws" in the wrong place

Swift SVN r28492

Author: cwillmor

include/swift/AST/DiagnosticEngine.h

@@ -381,7 +381,7 @@ namespace swift {
       return fixItReplace(R, {});
     }
 
-    /// \brief Add a token-based removal fix-it to the currently-active
+    /// \brief Add a character-based removal fix-it to the currently-active
     /// diagnostic.
     InFlightDiagnostic &fixItRemoveChars(SourceLoc Start, SourceLoc End) {
       return fixItReplaceChars(Start, End, {});

include/swift/AST/DiagnosticsParse.def

@@ -577,6 +577,10 @@ ERROR(throwing_non_function,type_parsing,PointsToFirstBadToken,
       "only function types may throw", ())
 ERROR(rethrowing_function_type,type_parsing,PointsToFirstBadToken,
       "only function declarations may be marked 'rethrows'", ())
+ERROR(throws_after_function_result,type_parsing,none,
+      "'throws' may only occur before '->'", ())
+ERROR(rethrows_after_function_result,type_parsing,none,
+      "'rethrows' may only occur before '->'", ())
 
 // Enum Types
 ERROR(expected_expr_enum_case_raw_value,type_parsing,PointsToFirstBadToken,

lib/Parse/ParsePattern.cpp

@@ -629,13 +629,14 @@ Parser::parseFunctionSignature(Identifier SimpleName,
     rethrows = true;
   }
 
+  SourceLoc arrowLoc;
   // If there's a trailing arrow, parse the rest as the result type.
   if (Tok.isAny(tok::arrow, tok::colon)) {
-    if (!consumeIf(tok::arrow)) {
+    if (!consumeIf(tok::arrow, arrowLoc)) {
       // FixIt ':' to '->'.
       diagnose(Tok, diag::func_decl_expected_arrow)
           .fixItReplace(SourceRange(Tok.getLoc()), "->");
-      consumeToken(tok::colon);
+      arrowLoc = consumeToken(tok::colon);
     }
 
     ParserResult<TypeRepr> ResultType =
@@ -652,6 +653,29 @@ Parser::parseFunctionSignature(Identifier SimpleName,
     retType = nullptr;
   }
 
+  // Check for 'throws' and 'rethrows' after the type and correct it.
+  if (!throwsLoc.isValid()) {
+    if (Tok.is(tok::kw_throws)) {
+      throwsLoc = consumeToken();
+    } else if (Tok.is(tok::kw_rethrows)) {
+      throwsLoc = consumeToken();
+      rethrows = true;
+    }
+
+    if (throwsLoc.isValid()) {
+      assert(arrowLoc.isValid());
+      assert(retType);
+      auto diag = rethrows ? diag::rethrows_after_function_result
+                           : diag::throws_after_function_result;
+      SourceLoc typeEndLoc = Lexer::getLocForEndOfToken(SourceMgr,
+                                                        retType->getEndLoc());
+      SourceLoc throwsEndLoc = Lexer::getLocForEndOfToken(SourceMgr, throwsLoc);
+      diagnose(Tok, diag)
+        .fixItInsert(arrowLoc, rethrows ? "rethrows " : "throws ")
+        .fixItRemoveChars(typeEndLoc, throwsEndLoc);
+    }
+  }
+
   return Status;
 }
 

lib/Parse/ParseType.cpp

@@ -163,12 +163,13 @@ ParserResult<TypeRepr> Parser::parseType(Diag<> MessageID) {
   if (ty.isNull())
     return nullptr;
 
+  ParserPosition beforeThrowsPos;
   SourceLoc throwsLoc;
-  if (consumeIf(tok::kw_throws, throwsLoc)) {
-    assert(throwsLoc.isValid());
-  } else if (consumeIf(tok::kw_rethrows, throwsLoc)) {
-    // 'rethrows' is only allowed on function declarations for now.
-    diagnose(throwsLoc, diag::rethrowing_function_type);
+  bool rethrows = false;
+  if (Tok.isAny(tok::kw_throws, tok::kw_rethrows)) {
+    beforeThrowsPos = getParserPosition();
+    rethrows = Tok.is(tok::kw_rethrows);
+    throwsLoc = consumeToken();
   }
 
   // Handle type-function if we have an arrow.
@@ -180,20 +181,22 @@ ParserResult<TypeRepr> Parser::parseType(Diag<> MessageID) {
       return makeParserCodeCompletionResult<TypeRepr>();
     if (SecondHalf.isNull())
       return nullptr;
+    if (rethrows) {
+      // 'rethrows' is only allowed on function declarations for now.
+      diagnose(throwsLoc, diag::rethrowing_function_type);
+    }
     auto fnTy = new (Context) FunctionTypeRepr(generics, ty.get(),
                                                throwsLoc,
                                                arrowLoc,
                                                SecondHalf.get());
     return makeParserResult(applyAttributeToType(fnTy, attrs));
+  } else if (throwsLoc.isValid()) {
+    // Don't consume 'throws', so we can emit a more useful diagnostic when
+    // parsing a function decl.
+    restoreParserPosition(beforeThrowsPos);
+    return ty;
   }
 
-  // Only function types may throw.
-  if (throwsLoc.isValid()) {
-    diagnose(throwsLoc, diag::throwing_non_function)
-      .highlight(ty.get()->getSourceRange());
-    return nullptr;
-  }
-
   // Only function types may be generic.
   if (generics) {
     auto brackets = generics->getSourceRange();

test/Parse/errors.swift

@@ -77,3 +77,11 @@ func illformed() throws {
     } catch MSV.CarriesInt(let i) where i == genError()) { // expected-error {{call can throw, but errors cannot be thrown out of a catch guard expression}} expected-error {{expected '{'}} expected-error {{braced block of statements is an unused closure}} expected-error {{type of expression is ambiguous without more context}}
     }
 }
+
+func postThrows() -> Int throws { // expected-error{{'throws' may only occur before '->'}}{{19-19=throws }}{{25-32=}}
+  return 5
+}
+
+func postRethrows(f: () throws -> Int) -> Int rethrows { // expected-error{{'rethrows' may only occur before '->'}}{{40-40=rethrows }}{{46-55=}}
+  return try f()
+}