Improved diagnostics for misplaced AttributeList in variable declaration

Author: flashspys

CodeGeneration/Sources/generate-swift-syntax/templates/swiftsyntax/SyntaxRewriterFile.swift

@@ -31,11 +31,32 @@ let syntaxRewriterFile = SourceFileSyntax(leadingTrivia: copyrightHeader) {
     """
   ) {
     DeclSyntax("public let viewMode: SyntaxTreeViewMode")
+    DeclSyntax(
+      """
+      /// The arena in which the parents of rewritten nodes should be allocated.
+      /// 
+      /// The `SyntaxRewriter` subclass is responsible for generating the rewritten nodes. To incorporate them into the
+      /// tree, all of the rewritten node's parents also need to be re-created. This is the arena in which those 
+      /// intermediate nodes should be allocated.
+      private let arena: SyntaxArena?
+      """
+    )
 
     DeclSyntax(
       """
       public init(viewMode: SyntaxTreeViewMode = .sourceAccurate) {
         self.viewMode = viewMode
+        self.arena = nil
+      }
+      """
+    )
+
+    DeclSyntax(
+      """
+      @_spi(RawSyntax)
+      public init(viewMode: SyntaxTreeViewMode = .sourceAccurate, arena: SyntaxArena? = nil) {
+        self.viewMode = viewMode
+        self.arena = arena
       }
       """
     )
@@ -346,7 +367,7 @@ let syntaxRewriterFile = SourceFileSyntax(leadingTrivia: copyrightHeader) {
           // Sanity check, ensure the new children are the same length.
           precondition(newLayout.count == node.raw.layoutView!.children.count)
 
-          let arena = SyntaxArena()
+          let arena = self.arena ?? SyntaxArena()
           let newRaw = node.raw.layoutView!.replacingLayout(with: Array(newLayout), arena: arena)
           // 'withExtendedLifetime' to keep 'SyntaxArena's of them alive until here.
           return withExtendedLifetime(rewrittens) {

Sources/SwiftParser/Declarations.swift

@@ -1262,11 +1262,33 @@ extension Parser {
     let (unexpectedBeforeIntroducer, introducer) = self.eat(handle)
     let hasTryBeforeIntroducer = unexpectedBeforeIntroducer?.containsToken(where: { TokenSpec(.try) ~= $0 }) ?? false
 
+    var attrs = attrs
     var elements = [RawPatternBindingSyntax]()
     do {
       var keepGoing: RawTokenSyntax? = nil
       var loopProgress = LoopProgressCondition()
       repeat {
+        var unexpectedBeforePattern: RawUnexpectedNodesSyntax?
+
+        if self.at(.atSign), attrs.attributes.isEmpty {
+          let recoveredAttributes = self.parseAttributeList()
+          unexpectedBeforePattern = RawUnexpectedNodesSyntax(
+            [recoveredAttributes],
+            arena: self.arena
+          )
+
+          /// Create a version of the same attribute with all tokens missing.
+          class TokenMissingMaker: SyntaxRewriter {
+            override func visit(_ token: TokenSyntax) -> TokenSyntax {
+              return .init(token.tokenKind, presence: .missing)
+            }
+          }
+          let tokenMissingMaker = TokenMissingMaker(arena: self.arena)
+          let missingAttributes = tokenMissingMaker.rewrite(
+            Syntax(raw: RawSyntax(recoveredAttributes), rawNodeArena: arena)
+          ).raw
+          attrs.attributes = missingAttributes.cast(RawAttributeListSyntax.self)
+        }
 
         var (pattern, typeAnnotation) = self.parseTypedPattern()
 
@@ -1344,6 +1366,7 @@ extension Parser {
         keepGoing = self.consume(if: .comma)
         elements.append(
           RawPatternBindingSyntax(
+            unexpectedBeforePattern,
             pattern: pattern,
             typeAnnotation: typeAnnotation,
             initializer: initializer,

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

@@ -262,7 +262,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
       unexpectedTokenCondition: { EffectSpecifier(token: $0) != nil },
       correctTokens: [effectSpecifiers?.asyncSpecifier, effectSpecifiers?.throwsClause?.throwsSpecifier],
       message: { EffectsSpecifierAfterArrow(effectsSpecifiersAfterArrow: $0) },
-      moveFixIt: { MoveTokensInFrontOfFixIt(movedTokens: $0, inFrontOf: .arrow) },
+      moveFixIt: { MoveNodesInFrontOfFixIt(movedNodes: $0, inFrontOf: .arrow) },
       removeRedundantFixIt: { RemoveRedundantFixIt(removeTokens: $0) }
     )
   }
@@ -324,7 +324,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
         unexpectedTokenCondition: { AsyncEffectSpecifier(token: $0) != nil },
         correctTokens: [node.asyncSpecifier],
         message: { AsyncMustPrecedeThrows(asyncKeywords: $0, throwsKeyword: throwsClause.throwsSpecifier) },
-        moveFixIt: { MoveTokensInFrontOfFixIt(movedTokens: $0, inFrontOf: throwsClause.throwsSpecifier.tokenKind) },
+        moveFixIt: { MoveNodesInFrontOfFixIt(movedNodes: $0, inFrontOf: throwsClause.throwsSpecifier.tokenKind) },
         removeRedundantFixIt: { RemoveRedundantFixIt(removeTokens: $0) }
       )
     }
@@ -2052,6 +2052,27 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
       moveFixIt: { MoveTokensAfterFixIt(movedTokens: $0, after: .equal) },
       removeRedundantFixIt: { RemoveRedundantFixIt(removeTokens: $0) }
     )
+
+    if node.attributes.isMissingAllTokens,
+      let unexpected = node.bindings.compactMap({ $0.unexpectedBeforePattern }).first,
+      unexpected.only?.is(AttributeListSyntax.self) ?? false
+    {
+      let fixIt = FixIt(
+        message: MoveNodesInFrontOfFixIt(movedNodes: [unexpected], inFrontOf: node.bindingSpecifier.tokenKind),
+        changes: [
+          .makeMissing(unexpected),
+          .makePresent(node.attributes, trailingTrivia: .space),
+        ]
+      )
+
+      addDiagnostic(
+        unexpected,
+        .misplacedAttributeInVarDecl,
+        fixIts: [fixIt],
+        handledNodes: [node.attributes.id, unexpected.id]
+      )
+    }
+
     return .visitChildren
   }
 

Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

@@ -191,6 +191,9 @@ extension DiagnosticMessage where Self == StaticParserError {
   public static var maximumNestingLevelOverflow: Self {
     .init("parsing has exceeded the maximum nesting level")
   }
+  public static var misplacedAttributeInVarDecl: Self {
+    .init("misplaced attribute in variable declaration")
+  }
   public static var missingColonAndExprInTernaryExpr: Self {
     .init("expected ':' and expression after '? ...' in ternary expression")
   }
@@ -710,15 +713,15 @@ public struct MoveTokensAfterFixIt: ParserFixIt {
   }
 }
 
-public struct MoveTokensInFrontOfFixIt: ParserFixIt {
-  /// The token that should be moved
-  public let movedTokens: [TokenSyntax]
+public struct MoveNodesInFrontOfFixIt<Node: SyntaxProtocol>: ParserFixIt {
+  /// The nodes that should be moved
+  public let movedNodes: [Node]
 
-  /// The token before which 'movedTokens' should be moved
+  /// The token before which `movedNodes` should be moved
   public let inFrontOf: TokenKind
 
   public var message: String {
-    "move \(nodesDescription(movedTokens, format: false)) in front of '\(inFrontOf.nameForDiagnostics)'"
+    "move \(nodesDescription(movedNodes, format: false)) in front of '\(inFrontOf.nameForDiagnostics)'"
   }
 }
 

Sources/SwiftSyntax/generated/SyntaxRewriter.swift

@@ -24,8 +24,22 @@
 open class SyntaxRewriter {
   public let viewMode: SyntaxTreeViewMode
 
+  /// The arena in which the parents of rewritten nodes should be allocated.
+  /// 
+  /// The `SyntaxRewriter` subclass is responsible for generating the rewritten nodes. To incorporate them into the
+  /// tree, all of the rewritten node's parents also need to be re-created. This is the arena in which those 
+  /// intermediate nodes should be allocated.
+  private let arena: SyntaxArena?
+  
   public init(viewMode: SyntaxTreeViewMode = .sourceAccurate) {
     self.viewMode = viewMode
+    self.arena = nil
+  }
+  
+  @_spi(RawSyntax)
+  public init(viewMode: SyntaxTreeViewMode = .sourceAccurate, arena: SyntaxArena? = nil) {
+    self.viewMode = viewMode
+    self.arena = arena
   }
 
   /// Rewrite `node`, keeping its parent unless `detach` is `true`.
@@ -3966,7 +3980,7 @@ open class SyntaxRewriter {
       // Sanity check, ensure the new children are the same length.
       precondition(newLayout.count == node.raw.layoutView!.children.count)
 
-      let arena = SyntaxArena()
+      let arena = self.arena ?? SyntaxArena()
       let newRaw = node.raw.layoutView!.replacingLayout(with: Array(newLayout), arena: arena)
       // 'withExtendedLifetime' to keep 'SyntaxArena's of them alive until here.
       return withExtendedLifetime(rewrittens) {

Tests/SwiftParserTest/AttributeTests.swift

@@ -1022,4 +1022,26 @@ final class AttributeTests: ParserTestCase {
       ]
     )
   }
+
+  func testMisplacedAttributeInVariableDecl() {
+    assertParse(
+      """
+      struct A {
+        var 1️⃣@State name: String
+      }
+      """,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "misplaced attribute in variable declaration",
+          fixIts: ["move attributes in front of 'var'"]
+        )
+      ],
+      fixedSource:
+        """
+        struct A {
+          @State var name: String
+        }
+        """
+    )
+  }
 }

Tests/SwiftParserTest/DeclarationTests.swift

@@ -3308,4 +3308,21 @@ final class DeclarationTests: ParserTestCase {
       experimentalFeatures: .transferringArgsAndResults
     )
   }
+
+  func testMisplacedAttributeInVarDeclWithMultipleBindings() {
+    assertParse(
+      """
+      let a = "a", 1️⃣@foo b = "b"
+      """,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "misplaced attribute in variable declaration",
+          fixIts: ["move attributes in front of 'let'"]
+        )
+      ],
+      fixedSource: """
+        @foo let a = "a", b = "b"
+        """
+    )
+  }
 }