Produce more useful diagnostics for bogus specifier before arrow and leftbrace

[StevenWong12]

Purpose

To produce more useful diagnostics for bogus specifier before arrow and leftbrace as the first step to solve issue #1262

Implementation details

Replace at with recovery function and a new testcase.

Additionally, to pass the testMacroDecl testcase, I made an edit about TokenPrecedence to get the correct precedence of the macro keyword.

I also modified some of the other testcases but I am not sure if they're reasonable.

If there is anything I can do for this PR, please tell me. Many thanks!

[ahoppen]

Thank you for your contribution @StevenWong12. Your changes look good to me, I've got a few small comments to make sure we don't accidentally parse valid code as invalid with the new recovery.

[StevenWong12]

Thanks for your detailed suggestions! @ahoppen Although the code snippet above was correctly parsed, I think it is indeed a little risky to recover to a left brace on an arbitrary line so I will make changes as your suggestions about checking -> and {.

As for the modification about init function of TokenPrecedence, my original intention was to solve the problems when parsing codes containing multi-line macro declaration with function like signatures e.g.

macro m2(_: Int) = A.M2
macro m3(a b: Int) -> Int = A.M3

Since the marco signature parsing method reuses the logic of function signature parsing, the tokens from = to ) in the second line will be recovered to -> . Specifically, the second macro cannot be recognized as a keyword because it is considered contextual and will be initialized with exprKeyword precedence.

About this issue, I came up with two solutions:

1. Reduce the recovery precedence of -> to weakPunctuator or exprKeyword inside function signature parsing;
2. Add a case for RawTokenKind to specify this kind of "contextual keyword" and assign to them when initializing lexeme.

Solution 1 is pretty straightforward but, from my perspective, not pretty fit the context, since the precedence of macro is supposed to be declKeyword
Solution 2 may bring extra wordload but we can explicitly assign a token precedence to these " contextual keywords" in TokenPrecedence.swift

For now, I simply implement solution 1. But it seems solution 2 is a more generic solution, maybe I can implement it in the future if needed. Also, if there is anything I missed or misunderstood, please let me know.

Thanks in advance!

[ahoppen]

I just debugged this myself and the problem seems to be that canRecoveryTo is not respecting allowAtStartOfLine when skipping over tokens. The following changes should fix that an remove the need to change the recovery precedence of the arrow. I think one other test case needs to be updated but that’s fine.

diff --git a/Sources/SwiftParser/Declarations.swift b/Sources/SwiftParser/Declarations.swift
index b3368e3c..0d56ef09 100644
--- a/Sources/SwiftParser/Declarations.swift
+++ b/Sources/SwiftParser/Declarations.swift
@@ -1376,7 +1376,7 @@ extension Parser {
 
     let output: RawReturnClauseSyntax?
 
-    if self.at(.arrow) || self.canRecoverTo(TokenSpec(.arrow, recoveryPrecedence: .weakPunctuator, allowAtStartOfLine: false)) != nil {
+    if self.at(.arrow) || self.canRecoverTo(TokenSpec(.arrow, allowAtStartOfLine: false)) != nil {
       output = self.parseFunctionReturnClause(effectSpecifiers: &effectSpecifiers, allowNamedOpaqueResultType: true)
     } else {
       output = nil
diff --git a/Sources/SwiftParser/Recovery.swift b/Sources/SwiftParser/Recovery.swift
index 21b4f7f7..4e48f45d 100644
--- a/Sources/SwiftParser/Recovery.swift
+++ b/Sources/SwiftParser/Recovery.swift
@@ -70,9 +70,10 @@ extension Parser.Lookahead {
     let initialTokensConsumed = self.tokensConsumed
 
     let recoveryPrecedence = min(spec1.recoveryPrecedence, spec2.recoveryPrecedence, spec3.recoveryPrecedence)
+    let shouldSkipOverNewlines = recoveryPrecedence.shouldSkipOverNewlines && spec1.allowAtStartOfLine && spec2.allowAtStartOfLine && spec3.allowAtStartOfLine
 
     while !self.at(.eof) {
-      if !recoveryPrecedence.shouldSkipOverNewlines, self.currentToken.isAtStartOfLine {
+      if !shouldSkipOverNewlines, self.currentToken.isAtStartOfLine {
         break
       }
       let matchedSpec: TokenSpec?

[StevenWong12]

Cool! Thanks for your help @ahoppen ! The changes have been committed.😀

[ahoppen]

Thank you for your contribution @StevenWong12!

[ahoppen]

@swift-ci Please test