Improved diagnostics for misplaced AttributeList in variable declaration

Author: flashspys

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

@@ -30,7 +30,19 @@ let syntaxRewriterFile = SourceFileSyntax(leadingTrivia: generateCopyrightHeader
     open class SyntaxRewriter
     """
   ) {
-    DeclSyntax("public init() {}")
+    DeclSyntax(
+      """
+      let arena: SyntaxArena?
+      """
+    )
+
+    DeclSyntax(
+      """
+      public init(arena: SyntaxArena? = nil) {
+        self.arena = arena
+      }
+      """
+    )
 
     for node in SYNTAX_NODES where node.isVisitable {
       if node.baseType.baseName == "Syntax" && node.name != "MissingSyntax" {
@@ -322,7 +334,7 @@ let syntaxRewriterFile = SourceFileSyntax(leadingTrivia: generateCopyrightHeader
           // Sanity check, ensure the new children are the same length.
           assert(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((arena, rewrittens)) {

Sources/SwiftDiagnostics/DiagnosticsFormatter.swift

@@ -301,10 +301,17 @@ public struct DiagnosticsFormatter {
           }
         }
 
-        for diag in diags.dropLast(1) {
-          annotatedSource.append("\(preMessage)├─ \(colorizeIfRequested(diag.diagMessage))\n")
+        for (index, diag) in diags.enumerated() {
+          if index < (diags.count - 1) {
+            annotatedSource.append("\(preMessage)├─ \(colorizeIfRequested(diag.diagMessage))\n")
+          } else {
+            annotatedSource.append("\(preMessage)╰─ \(colorizeIfRequested(diag.diagMessage))\n")
+          }
+          for fixIt in diag.fixIts {
+            annotatedSource.append("\(preMessage)  🔨 \(fixIt.message.message)\n")
+          }
         }
-        annotatedSource.append("\(preMessage)╰─ \(colorizeIfRequested(diags.last!.diagMessage))\n")
+
       }
 
       // Add suffix text.

Sources/SwiftParser/Declarations.swift

@@ -1521,12 +1521,31 @@ extension Parser {
     let (unexpectedBeforeIntroducer, introducer) = self.eat(handle)
     let hasTryBeforeIntroducer = unexpectedBeforeIntroducer?.containsToken(where: { TokenSpec(.try) ~= $0 }) ?? false
 
+    var unexpectedBetweenBindingKeywordAndBindings: RawUnexpectedNodesSyntax?
+    var attrs = attrs
     var elements = [RawPatternBindingSyntax]()
     do {
       var keepGoing: RawTokenSyntax? = nil
       var loopProgress = LoopProgressCondition()
       repeat {
 
+        if self.at(.atSign),
+          attrs.attributes?.isEmpty ?? true,
+          let recoveredAttributes = self.parseAttributeList()
+        {
+          unexpectedBetweenBindingKeywordAndBindings = RawUnexpectedNodesSyntax([recoveredAttributes], arena: self.arena)
+
+          class TokenMissingMaker: SyntaxRewriter {
+            override func visit(_ token: TokenSyntax) -> TokenSyntax {
+              return .init(token.tokenKind, presence: .missing)
+            }
+          }
+          let tokenMissingMaker = TokenMissingMaker(arena: self.arena)
+          let missingAttributes = tokenMissingMaker.visit(Syntax(raw: recoveredAttributes.raw)).raw
+          attrs.attributes = RawAttributeListSyntax(missingAttributes)
+
+        }
+
         var (pattern, typeAnnotation) = self.parseTypedPattern()
 
         // Parse an initializer if present.
@@ -1614,6 +1633,7 @@ extension Parser {
       modifiers: attrs.modifiers,
       unexpectedBeforeIntroducer,
       bindingKeyword: introducer,
+      unexpectedBetweenBindingKeywordAndBindings,
       bindings: RawPatternBindingListSyntax(elements: elements, arena: self.arena),
       arena: self.arena
     )

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

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

Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

@@ -212,6 +212,9 @@ extension DiagnosticMessage where Self == StaticParserError {
   public static var unexpectedSemicolon: Self {
     .init("unexpected ';' separator")
   }
+  public static var misplacedAttributeInVarDecl: Self {
+    .init("misplaced attribute in variable declaration")
+  }
 }
 
 // MARK: - Diagnostics (please sort alphabetically)
@@ -509,15 +512,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 after 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/Raw/RawSyntaxNodeProtocol.swift

@@ -11,7 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 /// All typed raw syntax nodes conform to this protocol.
-/// `RawXXXSyntax` is a typed wrappeer of `RawSyntax`.
+/// `RawXXXSyntax` is a typed wrapper of `RawSyntax`.
 @_spi(RawSyntax)
 public protocol RawSyntaxNodeProtocol: CustomStringConvertible, TextOutputStreamable {
   /// Returns `true` if `raw` can be cast to this concrete raw syntax type.

Sources/SwiftSyntax/generated/SyntaxRewriter.swift

@@ -22,7 +22,10 @@
 //===----------------------------------------------------------------------===//
 
 open class SyntaxRewriter {
-  public init() {
+  let arena: SyntaxArena?
+  
+  public init(arena: SyntaxArena? = nil) {
+    self.arena = arena
   }
 
   /// Visit a `AccessPathComponentSyntax`.
@@ -6755,7 +6758,7 @@ open class SyntaxRewriter {
       // Sanity check, ensure the new children are the same length.
       assert(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((arena, rewrittens)) {

Sources/swift-parser-cli/swift-parser-cli.swift

@@ -217,7 +217,7 @@ class PrintDiags: ParsableCommand {
       let annotatedSource = DiagnosticsFormatter.annotatedSource(
         tree: tree,
         diags: diags,
-        colorize: colorize || TerminalHelper.isConnectedToTerminal
+        colorize: (colorize || TerminalHelper.isConnectedToTerminal && ProcessInfo.processInfo.environment["__XCODE_BUILT_PRODUCTS_DIR_PATHS"] == nil)
       )
 
       print(annotatedSource)

Tests/SwiftParserTest/AttributeTests.swift

@@ -570,4 +570,23 @@ final class AttributeTests: XCTestCase {
       """
     )
   }
+
+  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
+        }
+        """
+    )
+  }
 }