Merge pull request #881 from ahoppen/ahoppen/throws-after-arrow-expr

Add diagnostic to move 'async' and 'throws' in 'ArrowExpr' in front of '->'

Author: ahoppen

CodeGeneration/Sources/generate-swiftbasicformat/BasicFormatFile.swift

@@ -168,7 +168,7 @@ private func createTokenFormatFunction() -> FunctionDecl {
   ) {
     VariableDecl("var node = node")
     SwitchStmt(expression: MemberAccessExpr(base: "node", name: "tokenKind")) {
-      for token in SYNTAX_TOKENS {
+      for token in SYNTAX_TOKENS where token.name != "ContextualKeyword" {
         SwitchCase(label: SwitchCaseLabel(caseItems: CaseItem(pattern: ExpressionPattern(expression: MemberAccessExpr(name: token.swiftKind))))) {
           if token.requiresLeadingSpace {
             IfStmt(
@@ -196,6 +196,20 @@ private func createTokenFormatFunction() -> FunctionDecl {
       SwitchCase(label: SwitchCaseLabel(caseItems: CaseItem(pattern: ExpressionPattern(expression: MemberAccessExpr(name: "eof"))))) {
         BreakStmt("break")
       }
+      SwitchCase(label: SwitchCaseLabel(caseItems: CaseItem(pattern: ExpressionPattern(expression: MemberAccessExpr(name: "contextualKeyword"))))) {
+        SwitchStmt(
+          """
+          switch node.text {
+            case "async":
+              if node.trailingTrivia.isEmpty {
+                node.trailingTrivia += .space
+              }
+            default:
+              break
+          }
+          """
+        )
+      }
     }
     SequenceExpr("node.leadingTrivia = node.leadingTrivia.indented(indentation: indentation)")
     SequenceExpr("node.trailingTrivia = node.trailingTrivia.indented(indentation: indentation)")

Sources/SwiftBasicFormat/generated/BasicFormat.swift

@@ -3147,8 +3147,6 @@ open class BasicFormat: SyntaxRewriter {
       break
     case .dollarIdentifier: 
       break
-    case .contextualKeyword: 
-      break
     case .rawStringDelimiter: 
       break
     case .stringSegment: 
@@ -3159,6 +3157,15 @@ open class BasicFormat: SyntaxRewriter {
       break
     case .eof: 
       break
+    case .contextualKeyword: 
+      switch node.text {
+        case "async":
+          if node.trailingTrivia.isEmpty {
+            node.trailingTrivia += .space
+          }
+        default:
+          break
+      }
     }
     node.leadingTrivia = node.leadingTrivia.indented(indentation: indentation)
     node.trailingTrivia = node.trailingTrivia.indented(indentation: indentation)

Sources/SwiftDiagnostics/FixIt.swift

@@ -26,21 +26,39 @@ public protocol FixItMessage {
 
 /// A Fix-It that can be applied to resolve a diagnostic.
 public struct FixIt {
+  public struct Changes: ExpressibleByArrayLiteral {
+    public var changes: [Change]
+
+    public init(changes: [Change]) {
+      self.changes = changes
+    }
+
+    public init(arrayLiteral elements: FixIt.Change...) {
+      self.init(changes: elements)
+    }
+
+    public init(combining: [Changes]) {
+      self.init(changes: combining.flatMap(\.changes))
+    }
+  }
+
   public enum Change {
     /// Replace `oldNode` by `newNode`.
     case replace(oldNode: Syntax, newNode: Syntax)
-    /// Remove the trailing trivia of the given token
-    case removeTrailingTrivia(TokenSyntax)
+    /// Replace the leading trivia on the given token
+    case replaceLeadingTrivia(token: TokenSyntax, newTrivia: Trivia)
+    /// Replace the trailing trivia on the given token
+    case replaceTrailingTrivia(token: TokenSyntax, newTrivia: Trivia)
   }
 
   /// A description of what this Fix-It performs.
   public let message: FixItMessage
 
   /// The changes that need to be performed when the Fix-It is applied.
-  public let changes: [Change]
+  public let changes: Changes
 
-  public init(message: FixItMessage, changes: [Change]) {
-    assert(!changes.isEmpty, "A Fix-It must have at least one change associated with it")
+  public init(message: FixItMessage, changes: Changes) {
+    assert(!changes.changes.isEmpty, "A Fix-It must have at least one change associated with it")
     self.message = message
     self.changes = changes
   }

Sources/SwiftParser/Diagnostics/DiagnosticExtensions.swift

@@ -14,42 +14,72 @@ import SwiftDiagnostics
 import SwiftSyntax
 
 extension FixIt {
-  init(message: StaticParserFixIt, changes: [Change]) {
+  init(message: StaticParserFixIt, changes: Changes) {
     self.init(message: message as FixItMessage, changes: changes)
   }
+
+  public init(message: FixItMessage, changes: [Changes]) {
+    self.init(message: message, changes: FixIt.Changes(combining: changes))
+  }
+
+  init(message: StaticParserFixIt, changes: [Changes]) {
+    self.init(message: message as FixItMessage, changes: FixIt.Changes(combining: changes))
+  }
 }
 
-extension FixIt.Change {
+extension FixIt.Changes {
   /// Replaced a present node with a missing node
-  static func makeMissing(node: TokenSyntax) -> FixIt.Change {
+  static func makeMissing(node: TokenSyntax) -> Self {
     assert(node.presence == .present)
-    return .replace(
-      oldNode: Syntax(node),
-      newNode: Syntax(TokenSyntax(node.tokenKind, leadingTrivia: [], trailingTrivia: [], presence: .missing))
-    )
+    var changes = [
+      FixIt.Change.replace(
+        oldNode: Syntax(node),
+        newNode: Syntax(TokenSyntax(node.tokenKind, leadingTrivia: [], trailingTrivia: [], presence: .missing))
+      )
+    ]
+    if !node.leadingTrivia.isEmpty, let nextToken = node.nextToken(viewMode: .sourceAccurate) {
+      changes.append(.replaceLeadingTrivia(token: nextToken, newTrivia: node.leadingTrivia))
+    }
+    return FixIt.Changes(changes: changes)
   }
 
-  static func makePresent<T: SyntaxProtocol>(node: T) -> FixIt.Change {
-    return .replace(
+  static func makePresent<T: SyntaxProtocol>(node: T) -> Self {
+    return [.replace(
       oldNode: Syntax(node),
       newNode: PresentMaker().visit(Syntax(node))
-    )
+    )]
+  }
+
+  /// Make a token present. If `leadingTrivia` or `trailingTrivia` is specified,
+  /// override the default leading/trailing trivia inferred from `BasicFormat`.
+  static func makePresent(
+    node: TokenSyntax,
+    leadingTrivia: Trivia? = nil,
+    trailingTrivia: Trivia? = nil
+  ) -> Self {
+    var presentNode = PresentMaker().visit(Syntax(node)).as(TokenSyntax.self)!
+    if let leadingTrivia = leadingTrivia {
+      presentNode.leadingTrivia = leadingTrivia
+    }
+    if let trailingTrivia = trailingTrivia {
+      presentNode.trailingTrivia = trailingTrivia
+    }
+    return [.replace(
+      oldNode: Syntax(node),
+      newNode: Syntax(presentNode)
+    )]
   }
-}
 
-extension Array where Element == FixIt.Change {
   /// Makes the `token` present, moving it in front of the previous token's trivia.
-  static func makePresentBeforeTrivia(token: TokenSyntax) -> [FixIt.Change] {
+  static func makePresentBeforeTrivia(token: TokenSyntax) -> Self {
     if let previousToken = token.previousToken(viewMode: .sourceAccurate) {
       let presentToken = PresentMaker().visit(token).withTrailingTrivia(previousToken.trailingTrivia)
       return [
-        .removeTrailingTrivia(previousToken),
+        .replaceTrailingTrivia(token: previousToken, newTrivia: []),
         .replace(oldNode: Syntax(token), newNode: presentToken),
       ]
     } else {
-      return [
-        .makePresent(node: token)
-      ]
+      return .makePresent(node: token)
     }
   }
 }

Sources/SwiftParser/Diagnostics/MissingNodesError.swift

@@ -17,9 +17,11 @@ import SwiftBasicFormat
 // MARK: - Shared code
 
 /// Returns a string that describes `missingNodes`.
-/// `missingNodes` are expected to all be children of `commonParent`.
-private func missingNodesDescription(missingNodes: [Syntax], commonParent: Syntax?) -> String {
-  assert(missingNodes.allSatisfy({ $0.parent == commonParent }))
+/// If `commonParent` is not `nil`, `missingNodes` are expected to all be children of `commonParent`.
+func missingNodesDescription(missingNodes: [Syntax], commonParent: Syntax?) -> String {
+  if commonParent != nil {
+    assert(missingNodes.allSatisfy({ $0.parent == commonParent }))
+  }
 
   // If all tokens in the parent are missing, return the parent type name.
   if let commonParent = commonParent,
@@ -256,7 +258,7 @@ extension ParseDiagnosticsGenerator {
 
     let fixIt = FixIt(
       message: InsertTokenFixIt(missingNodes: missingNodes),
-      changes: missingNodes.map(FixIt.Change.makePresent)
+      changes: missingNodes.map(FixIt.Changes.makePresent)
     )
 
     var notes: [Note] = []

Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift

@@ -95,6 +95,51 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     return handledNodes.contains(node.id)
   }
 
+  /// Utility function to emit a diagnostic that removes a misplaced token and instead inserts an equivalent token at the corrected location.
+  ///
+  /// If `incorrectContainer` contains only tokens that satisfy `unexpectedTokenCondition`, emit a diagnostic with message `message` that marks this token as misplaced.
+  /// If `correctTokens` contains missing tokens, also emit a Fix-It with message `fixIt` that marks the unexpected token as missing and instead inserts `correctTokens`.
+  public func exchangeTokens(
+    unexpected: UnexpectedNodesSyntax?,
+    unexpectedTokenCondition: (TokenSyntax) -> Bool,
+    correctTokens: [TokenSyntax?],
+    message: (_ misplacedTokens: [TokenSyntax]) -> DiagnosticMessage,
+    fixIt: (_ misplacedTokens: [TokenSyntax]) -> FixItMessage
+  ) {
+    guard let incorrectContainer = unexpected,
+          let misplacedTokens = incorrectContainer.onlyTokens(satisfying: unexpectedTokenCondition) else {
+      // If there are no unexpected nodes or the unexpected contain multiple tokens, don't emit a diagnostic.
+      return
+    }
+
+    // Ignore `correctTokens` that are already present.
+    let correctTokens = correctTokens.compactMap({ $0 }).filter({ $0.presence == .missing })
+    var changes = misplacedTokens.map(FixIt.Changes.makeMissing)
+    for correctToken in correctTokens {
+      if misplacedTokens.count == 1, let misplacedToken = misplacedTokens.first,
+         misplacedToken.nextToken(viewMode: .all) == correctToken || misplacedToken.previousToken(viewMode: .all) == correctToken {
+        changes.append(FixIt.Changes.makePresent(
+          node: correctToken,
+          leadingTrivia: misplacedToken.leadingTrivia,
+          trailingTrivia: misplacedToken.trailingTrivia
+        ))
+      } else {
+        changes.append(FixIt.Changes.makePresent(
+          node: correctToken,
+          leadingTrivia: nil,
+          trailingTrivia: nil
+        ))
+      }
+    }
+    var fixIts: [FixIt] = []
+    if changes.count > 1 {
+      // Only emit a Fix-It if we are moving a token, i.e. also making a token present.
+      fixIts.append(FixIt(message: fixIt(misplacedTokens), changes: changes))
+    }
+
+    addDiagnostic(incorrectContainer, message(misplacedTokens), fixIts: fixIts, handledNodes: [incorrectContainer.id] + correctTokens.map(\.id))
+  }
+
   // MARK: - Generic diagnostic generation
 
   public override func visitAny(_ node: Syntax) -> SyntaxVisitorContinueKind {
@@ -180,6 +225,20 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     return .visitChildren
   }
 
+  public override func visit(_ node: ArrowExprSyntax) -> SyntaxVisitorContinueKind {
+    if shouldSkip(node) {
+      return .skipChildren
+    }
+    exchangeTokens(
+      unexpected: node.unexpectedAfterArrowToken,
+      unexpectedTokenCondition: { $0.tokenKind == .contextualKeyword("async") || $0.tokenKind == .throwsKeyword },
+      correctTokens: [node.asyncKeyword, node.throwsToken],
+      message: { EffectsSpecifierAfterArrow(effectsSpecifiersAfterArrow: $0) },
+      fixIt: { MoveTokensInFrontOfFixIt(movedTokens: $0, inFrontOf: .arrow) }
+    )
+    return .visitChildren
+  }
+
   public override func visit(_ node: ForInStmtSyntax) -> SyntaxVisitorContinueKind {
     if shouldSkip(node) {
       return .skipChildren
@@ -215,19 +274,13 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     if shouldSkip(node) {
       return .skipChildren
     }
-    if let output = node.output,
-       let missingThrowsKeyword = node.throwsOrRethrowsKeyword,
-       missingThrowsKeyword.presence == .missing,
-       let unexpectedBeforeReturnType = output.unexpectedBetweenArrowAndReturnType,
-       let throwsInReturnPosition = unexpectedBeforeReturnType.tokens(withKind: .throwsKeyword).first {
-      addDiagnostic(throwsInReturnPosition, .throwsInReturnPosition, fixIts: [
-        FixIt(message: .moveThrowBeforeArrow, changes: [
-          .makeMissing(node: throwsInReturnPosition),
-          .makePresent(node: missingThrowsKeyword),
-        ])
-      ], handledNodes: [unexpectedBeforeReturnType.id, missingThrowsKeyword.id, throwsInReturnPosition.id])
-      return .visitChildren
-    }
+    exchangeTokens(
+      unexpected: node.output?.unexpectedBetweenArrowAndReturnType,
+      unexpectedTokenCondition: { $0.tokenKind == .throwsKeyword },
+      correctTokens: [node.throwsOrRethrowsKeyword],
+      message: { _ in StaticParserError.throwsInReturnPosition },
+      fixIt: { MoveTokensInFrontOfFixIt(movedTokens: $0, inFrontOf: .arrow) }
+    )
     return .visitChildren
   }
 

Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift

@@ -83,6 +83,14 @@ public enum StaticParserError: String, DiagnosticMessage {
 
 // MARK: - Diagnostics (please sort alphabetically)
 
+public struct EffectsSpecifierAfterArrow: ParserError {
+  public let effectsSpecifiersAfterArrow: [TokenSyntax]
+
+  public var message: String {
+    "\(missingNodesDescription(missingNodes: effectsSpecifiersAfterArrow.map(Syntax.init), commonParent: nil)) may only occur before '->'"
+  }
+}
+
 public struct ExtaneousCodeAtTopLevel: ParserError {
   public let extraneousCode: UnexpectedNodesSyntax
 
@@ -141,11 +149,23 @@ public struct UnexpectedNodesError: ParserError {
 public enum StaticParserFixIt: String, FixItMessage {
   case insertSemicolon = "insert ';'"
   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 MoveTokensInFrontOfFixIt: ParserFixIt {
+  /// The token that should be moved
+  public let movedTokens: [TokenSyntax]
+
+  /// The token after which 'try' should be moved
+  public let inFrontOf: RawTokenKind
+
+  public var message: String {
+    "move \(missingNodesDescription(missingNodes: movedTokens.map(Syntax.init), commonParent: nil)) in front of '\(inFrontOf.nameForDiagnostics)'"
+  }
+}
+

Sources/SwiftParser/Diagnostics/SyntaxExtensions.swift

@@ -20,6 +20,30 @@ extension UnexpectedNodesSyntax {
   func tokens(withKind kind: TokenKind) -> [TokenSyntax] {
     return self.tokens(satisfying: { $0.tokenKind == kind })
   }
+
+  /// If this only contains a single item that is a token, return that token, otherwise return `nil`.
+  var onlyToken: TokenSyntax? {
+    return onlyToken(where: { _ in true })
+  }
+
+  /// If this only contains a single item, which is a token satisfying `condition`, return that token, otherwise return `nil`.
+  func onlyToken(where condition: (TokenSyntax) -> Bool) -> TokenSyntax? {
+    if self.count == 1, let token = self.first?.as(TokenSyntax.self), condition(token) {
+      return token
+    } else {
+      return nil
+    }
+  }
+
+  /// If this only contains tokens satisfying `condition`, return an array containing those tokens, otherwise return `nil`.
+  func onlyTokens(satisfying condition: (TokenSyntax) -> Bool) -> [TokenSyntax]? {
+    let tokens = tokens(satisfying: condition)
+    if tokens.count == self.count {
+      return tokens
+    } else {
+      return nil
+    }
+  }
 }
 
 extension Syntax {