Add Fix-Its for missing tokens and improve diagnostic in testMissingArgumentToAttribute

Author: ahoppen

Package.swift

@@ -115,7 +115,7 @@ let package = Package(
     ),
     .target(
       name: "SwiftParser",
-      dependencies: ["SwiftDiagnostics", "SwiftSyntax"],
+      dependencies: ["SwiftBasicFormat", "SwiftDiagnostics", "SwiftSyntax"],
       exclude: [
         "CMakeLists.txt",
         "README.md",

Sources/SwiftParser/Attributes.swift

@@ -685,11 +685,18 @@ extension Parser {
     let (unexpectedBeforeAtSign, atSign) = self.expect(.atSign)
     let (unexpectedBeforeDynamicReplacementToken, dynamicReplacementToken) = self.expectContextualKeyword("_dynamicReplacement")
     let (unexpectedBeforeLeftParen, leftParen) = self.expect(.leftParen)
-    let (unexpectedBeforeLabel, label) = self.expect(.forKeyword)
+    let (unexpectedBeforeLabel, label) = self.expect(.forKeyword, remapping: .identifier)
     let (unexpectedBeforeColon, colon) = self.expect(.colon)
-    let (base, args) = self.parseDeclNameRef([
-      .zeroArgCompoundNames, .keywordsUsingSpecialNames, .operators,
-    ])
+    let base: RawTokenSyntax
+    let args: RawDeclNameArgumentsSyntax?
+    if label.isMissing && colon.isMissing && self.currentToken.isAtStartOfLine {
+      base = RawTokenSyntax(missing: .identifier, arena: self.arena)
+      args = nil
+    } else {
+      (base, args) = self.parseDeclNameRef([
+        .zeroArgCompoundNames, .keywordsUsingSpecialNames, .operators,
+      ])
+    }
     let method = RawDeclNameSyntax(declBaseName: RawSyntax(base), declNameArguments: args, arena: self.arena)
     let (unexpectedBeforeRightParen, rightParen) = self.expect(.rightParen)
     return RawAttributeSyntax(

Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift

@@ -33,15 +33,20 @@ fileprivate extension FixIt.Change {
     )
   }
 
-  static func makePresent(node: TokenSyntax, leadingTrivia: Trivia = [], trailingTrivia: Trivia = []) -> FixIt.Change {
-    assert(node.presence == .missing)
+  static func makePresent<T: SyntaxProtocol>(node: T) -> FixIt.Change {
     return .replace(
       oldNode: Syntax(node),
-      newNode: Syntax(TokenSyntax(node.tokenKind, leadingTrivia: leadingTrivia, trailingTrivia: trailingTrivia, presence: .present))
+      newNode: PresentMaker().visit(Syntax(node))
     )
   }
 }
 
+fileprivate extension FixIt {
+  init(message: StaticParserFixIt, changes: [Change]) {
+    self.init(message: message as FixItMessage, changes: changes)
+  }
+}
+
 public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
   private var diagnostics: [Diagnostic] = []
 
@@ -112,7 +117,11 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
           let previousParent = invalidIdentifier.parent?.as(UnexpectedNodesSyntax.self) {
         addDiagnostic(invalidIdentifier, InvalidIdentifierError(invalidIdentifier: invalidIdentifier), handledNodes: [previousParent.id])
       } else {
-        addDiagnostic(node, MissingTokenError(missingToken: node))
+        addDiagnostic(node, MissingTokenError(missingToken: node), fixIts: [
+          FixIt(message: InsertTokenFixIt(missingToken: node), changes: [
+            .makePresent(node: node)
+          ])
+        ])
       }
     }
     return .skipChildren
@@ -152,6 +161,21 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     return handleMissingSyntax(node)
   }
 
+  public override func visit(_ node: AttributeSyntax) -> SyntaxVisitorContinueKind {
+    if shouldSkip(node) {
+      return .skipChildren
+    }
+    if let argument = node.argument, argument.isMissingAllTokens {
+      addDiagnostic(argument, MissingAttributeArgument(attributeName: node.attributeName), fixIts: [
+        FixIt(message: .insertAttributeArguments, changes: [
+          .makePresent(node: argument)
+        ])
+      ], handledNodes: [argument.id])
+      return .visitChildren
+    }
+    return .visitChildren
+  }
+
   public override func visit(_ node: ForInStmtSyntax) -> SyntaxVisitorContinueKind {
     if shouldSkip(node) {
       return .skipChildren
@@ -193,17 +217,17 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
        let unexpectedBeforeReturnType = output.unexpectedBetweenArrowAndReturnType,
        let throwsInReturnPosition = unexpectedBeforeReturnType.tokens(withKind: .throwsKeyword).first {
       addDiagnostic(throwsInReturnPosition, .throwsInReturnPosition, fixIts: [
-        FixIt(message: StaticParserFixIt.moveThrowBeforeArrow, changes: [
+        FixIt(message: .moveThrowBeforeArrow, changes: [
           .makeMissing(node: throwsInReturnPosition),
-          .makePresent(node: missingThrowsKeyword, trailingTrivia: .space),
+          .makePresent(node: missingThrowsKeyword),
         ])
       ], handledNodes: [unexpectedBeforeReturnType.id, missingThrowsKeyword.id, throwsInReturnPosition.id])
       return .visitChildren
     }
     return .visitChildren
   }
 
-  override public func visit(_ node: ParameterClauseSyntax) -> SyntaxVisitorContinueKind {
+  public override func visit(_ node: ParameterClauseSyntax) -> SyntaxVisitorContinueKind {
     if shouldSkip(node) {
       return .skipChildren
     }

Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift

@@ -86,7 +86,7 @@ public extension ParserFixIt {
   }
 }
 
-// MARK: - Static diagnostics
+// MARK: - Errors (please sort alphabetically)
 
 /// Please order the cases in this enum alphabetically by case name.
 public enum StaticParserError: String, DiagnosticMessage {
@@ -104,16 +104,6 @@ public enum StaticParserError: String, DiagnosticMessage {
   public var severity: DiagnosticSeverity { .error }
 }
 
-public enum StaticParserFixIt: String, FixItMessage {
-  case moveThrowBeforeArrow = "Move 'throws' before '->'"
-
-  public var message: String { self.rawValue }
-
-  public var fixItID: MessageID {
-    MessageID(domain: diagnosticDomain, id: "\(type(of: self)).\(self)")
-  }
-}
-
 // MARK: - Diagnostics (please sort alphabetically)
 
 public struct ExtaneousCodeAtTopLevel: ParserError {
@@ -148,7 +138,7 @@ public struct MissingNodeError: ParserError {
     var message: String
     var hasNamedParent = false
     if let parent = missingNode.parent,
-        let childName = parent.childNameForDiagnostics(missingNode.index) {
+       let childName = parent.childNameForDiagnostics(missingNode.index) {
       message = "Expected \(childName)"
       if let parentTypeName = parent.nodeTypeNameForDiagnostics(inherit: false) {
         message += " of \(parentTypeName)"
@@ -173,6 +163,15 @@ public struct MissingNodeError: ParserError {
   }
 }
 
+public struct MissingAttributeArgument: ParserError {
+  /// The name of the attribute that's missing the argument, without `@`.
+  public let attributeName: TokenSyntax
+
+  public var message: String {
+    return "Expected argument for '@\(attributeName)' attribute"
+  }
+}
+
 public struct MissingTokenError: ParserError {
   public let missingToken: TokenSyntax
 
@@ -226,3 +225,22 @@ public struct UnexpectedNodesError: ParserError {
     return message
   }
 }
+
+// MARK: - Fix-Its (please sort alphabetically)
+
+public enum StaticParserFixIt: String, FixItMessage {
+  case insertAttributeArguments = "Insert attribute argument"
+  case moveThrowBeforeArrow = "Move 'throws' before '->'"
+
+  public var message: String { self.rawValue }
+
+  public var fixItID: MessageID {
+    MessageID(domain: diagnosticDomain, id: "\(type(of: self)).\(self)")
+  }
+}
+
+public struct InsertTokenFixIt: ParserFixIt {
+  let missingToken: TokenSyntax
+
+  public var message: String { "Insert '\(missingToken.text)'" }
+}

Sources/SwiftParser/Diagnostics/PresenceUtils.swift

@@ -0,0 +1,63 @@
+//===--- PresenceUtils.swift ----------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2022 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+@_spi(RawSyntax) import SwiftSyntax
+import SwiftBasicFormat
+
+/// Walks a tree and checks whether the tree contained any present tokens.
+class PresentNodeChecker: SyntaxAnyVisitor {
+  var hasPresentToken: Bool = false
+
+  override func visitAny(_ node: Syntax) -> SyntaxVisitorContinueKind {
+    if hasPresentToken {
+      // If we already saw a present token, we don't need to continue.
+      return .skipChildren
+    } else {
+      return .visitChildren
+    }
+  }
+
+  override func visit(_ node: TokenSyntax) -> SyntaxVisitorContinueKind {
+    if node.presence == .present {
+      hasPresentToken = true
+    }
+    return .visitChildren
+  }
+}
+
+extension SyntaxProtocol {
+  /// Returns `true` if all tokens nodes in this tree are missing.
+  var isMissingAllTokens: Bool {
+    let checker = PresentNodeChecker(viewMode: .all)
+    checker.walk(Syntax(self))
+    return !checker.hasPresentToken
+  }
+}
+
+/// Transforms a syntax tree by making all missing tokens present.
+class PresentMaker: SyntaxRewriter {
+  override func visit(_ token: TokenSyntax) -> Syntax {
+    if token.presence == .missing {
+      let presentToken: TokenSyntax
+      let (rawKind, text) = token.tokenKind.decomposeToRaw()
+      if let text = text, !text.isEmpty {
+        presentToken = TokenSyntax(token.tokenKind, presence: .present)
+      } else {
+        let newKind = TokenKind.fromRaw(kind: rawKind, text: rawKind.defaultText.map(String.init) ?? "<#\(rawKind.nameForDiagnostics)#>")
+        presentToken = TokenSyntax(newKind, presence: .present)
+      }
+      return Syntax(Format().format(syntax: presentToken))
+    } else {
+      return Syntax(token)
+    }
+  }
+}

Sources/SwiftParser/Parser.swift

@@ -323,12 +323,19 @@ extension Parser {
   ///     a missing token of the requested kind.
   @_spi(RawSyntax)
   public mutating func expect(
-    _ kind: RawTokenKind
+    _ kind: RawTokenKind,
+    remapping: RawTokenKind? = nil
   ) -> (unexpected: RawUnexpectedNodesSyntax?, token: RawTokenSyntax) {
     return expectImpl(
-      consume: { $0.consume(if: kind) },
+      consume: { $0.consume(if: kind, remapping: remapping) },
       canRecoverTo: { $0.canRecoverTo([kind]) },
-      makeMissing: { $0.missingToken(kind, text: nil) }
+      makeMissing: {
+        if let remapping = remapping {
+          return $0.missingToken(remapping, text: kind.defaultText)
+        } else {
+          return $0.missingToken(kind, text: nil)
+        }
+      }
     )
   }
 

Sources/SwiftParser/TokenConsumer.swift

@@ -139,10 +139,15 @@ extension TokenConsumer {
   @_spi(RawSyntax)
   public mutating func consume(
     if kind: RawTokenKind,
+    remapping: RawTokenKind? = nil,
     where condition: (Lexer.Lexeme) -> Bool = { _ in true}
   ) -> Token? {
     if self.at(kind, where: condition) {
-      return self.consumeAnyToken()
+      if let remapping = remapping {
+        return self.consumeAnyToken(remapping: remapping)
+      } else {
+        return self.consumeAnyToken()
+      }
     }
     return nil
   }

Sources/SwiftSyntax/TokenKind.swift.gyb

@@ -149,7 +149,8 @@ public enum RawTokenKind: Equatable, Hashable {
   case ${token.swift_kind()}
 % end
 
-  var defaultText: SyntaxText? {
+  @_spi(RawSyntax)
+  public var defaultText: SyntaxText? {
     switch self {
     case .eof: return ""
 % for token in SYNTAX_TOKENS:

Sources/SwiftSyntax/gyb_generated/TokenKind.swift

@@ -1259,7 +1259,8 @@ public enum RawTokenKind: Equatable, Hashable {
   case stringInterpolationAnchor
   case yield
 
-  var defaultText: SyntaxText? {
+  @_spi(RawSyntax)
+  public var defaultText: SyntaxText? {
     switch self {
     case .eof: return ""
     case .associatedtypeKeyword: return "associatedtype"

Tests/SwiftParserTest/Attributes.swift

@@ -6,16 +6,19 @@ final class AttributeTests: XCTestCase {
   func testMissingArgumentToAttribute() {
     AssertParse(
       """
-      @_dynamicReplacement(#^DIAG_1^#
+      @_dynamicReplacement(#^DIAG^#
       func #^DIAG_2^#test_dynamic_replacement_for2() {
       }
       """,
       diagnostics: [
-        DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected declaration after attribute"),
-        DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected 'for' in attribute argument"),
-        DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected ':' in attribute argument"),
-        DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected ')' to end attribute"),
-      ]
+        DiagnosticSpec(message: "Expected argument for '@_dynamicReplacement' attribute", fixIts: ["Insert attribute argument"]),
+        DiagnosticSpec(message: "Expected ')' to end attribute", fixIts: ["Insert ')'"]),
+      ],
+      fixedSource: """
+      @_dynamicReplacement(for: <#identifier#>)
+      func test_dynamic_replacement_for2() {
+      }
+      """
     )
   }