Allow diagnostics to have Fix-Its

Author: ahoppen

Sources/SwiftDiagnostics/Diagnostic.swift

@@ -19,9 +19,14 @@ public struct Diagnostic: CustomDebugStringConvertible {
   /// The node at whose start location the message should be displayed.
   public let node: Syntax
 
-  public init(node: Syntax, message: DiagnosticMessage) {
+  /// Fix-Its that can be applied to resolve this diagnostic.
+  /// Each Fix-It offers a different way to resolve the diagnostic. Usually, there's only one.
+  public let fixIts: [FixIt]
+
+  public init(node: Syntax, message: DiagnosticMessage, fixIts: [FixIt] = []) {
     self.diagMessage = message
     self.node = node
+    self.fixIts = fixIts
   }
 
   /// The message that should be displayed to the user.

Sources/SwiftDiagnostics/FixIt.swift

@@ -0,0 +1,46 @@
+//===--- FixIt.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
+//
+//===----------------------------------------------------------------------===//
+
+import SwiftSyntax
+
+/// Types conforming to this protocol represent Fix-It messages that can be
+/// shown to the client.
+/// The messages should describe the change that the Fix-It will perform
+public protocol FixItMessage {
+  /// The diagnostic message that should be displayed in the client.
+  var message: String { get }
+
+  /// See ``MessageID``.
+  var fixItID: MessageID { get }
+}
+
+
+/// A Fix-It that can be applied to resolve a diagnostic.
+public struct FixIt {
+  public enum Change {
+    /// Replace `oldNode` by `newNode`.
+    case replace(oldNode: Syntax, newNode: Syntax)
+  }
+
+  /// 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 init(message: FixItMessage, changes: [Change]) {
+    assert(!changes.isEmpty, "A Fix-It must have at least one diagnostic associated with it")
+    self.message = message
+    self.changes = changes
+  }
+}
+

Sources/SwiftParser/Declarations.swift

@@ -1155,21 +1155,25 @@ extension Parser {
       arena: self.arena)
   }
 
+  /// If a `throws` keyword appears right in front of the `arrow`, it is returned as `misplacedThrowsKeyword` so it can be synthesized in front of the arrow.
   @_spi(RawSyntax)
-  public mutating func parseFunctionReturnClause() -> RawReturnClauseSyntax {
+  public mutating func parseFunctionReturnClause() -> (returnClause: RawReturnClauseSyntax, misplacedThrowsKeyword: RawTokenSyntax?) {
     let arrow = self.eat(.arrow)
+    var misplacedThrowsKeyword: RawTokenSyntax? = nil
     let unexpectedBeforeReturnType: RawUnexpectedNodesSyntax?
-    if let unexpectedToken = self.consume(ifAny: .tryKeyword, .throwKeyword, .throwsKeyword) {
-      unexpectedBeforeReturnType = RawUnexpectedNodesSyntax(elements: [RawSyntax(unexpectedToken)], arena: self.arena)
+    if let throwsKeyword = self.consume(ifAny: .rethrowsKeyword, .throwsKeyword) {
+      misplacedThrowsKeyword = throwsKeyword
+      unexpectedBeforeReturnType = RawUnexpectedNodesSyntax(elements: [RawSyntax(throwsKeyword)], arena: self.arena)
     } else {
       unexpectedBeforeReturnType = nil
     }
     let result = self.parseType()
-    return RawReturnClauseSyntax(
+    let returnClause = RawReturnClauseSyntax(
       arrow: arrow,
       unexpectedBeforeReturnType,
       returnType: result,
       arena: self.arena)
+    return (returnClause, misplacedThrowsKeyword)
   }
 }
 
@@ -1227,7 +1231,7 @@ extension Parser {
       async = nil
     }
 
-    let throwsKeyword: RawTokenSyntax?
+    var throwsKeyword: RawTokenSyntax?
     if self.at(.throwsKeyword) || self.at(.rethrowsKeyword) {
       throwsKeyword = self.consumeAnyToken()
     } else {
@@ -1236,7 +1240,11 @@ extension Parser {
 
     let output: RawReturnClauseSyntax?
     if self.at(.arrow) {
-      output = self.parseFunctionReturnClause()
+      let result = self.parseFunctionReturnClause()
+      output = result.returnClause
+      if let misplacedThrowsKeyword = result.misplacedThrowsKeyword, throwsKeyword == nil {
+        throwsKeyword = RawTokenSyntax(missing: misplacedThrowsKeyword.tokenKind, arena: self.arena)
+      }
     } else {
       output = nil
     }
@@ -1275,7 +1283,7 @@ extension Parser {
 
     let result: RawReturnClauseSyntax
     if self.at(.arrow) {
-      result = self.parseFunctionReturnClause()
+      result = self.parseFunctionReturnClause().returnClause
     } else {
       result = RawReturnClauseSyntax(
         arrow: RawTokenSyntax(missing: .arrow, arena: self.arena),

Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift

@@ -23,6 +23,25 @@ extension UnexpectedNodesSyntax {
   }
 }
 
+fileprivate extension FixIt.Change {
+  /// Replaced a present node with a missing node
+  static func makeMissing(node: TokenSyntax) -> FixIt.Change {
+    assert(node.presence == .present)
+    return .replace(
+      oldNode: Syntax(node),
+      newNode: Syntax(TokenSyntax(node.tokenKind, leadingTrivia: [], trailingTrivia: [], presence: .missing))
+    )
+  }
+
+  static func makePresent(node: TokenSyntax, leadingTrivia: Trivia = [], trailingTrivia: Trivia = []) -> FixIt.Change {
+    assert(node.presence == .missing)
+    return .replace(
+      oldNode: Syntax(node),
+      newNode: Syntax(TokenSyntax(node.tokenKind, leadingTrivia: leadingTrivia, trailingTrivia: trailingTrivia, presence: .present))
+    )
+  }
+}
+
 public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
   private var diagnostics: [Diagnostic] = []
 
@@ -43,13 +62,13 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
   // MARK: - Private helper functions
 
   /// Produce a diagnostic.
-  private func addDiagnostic<T: SyntaxProtocol>(_ node: T, _ message: DiagnosticMessage) {
-    diagnostics.append(Diagnostic(node: Syntax(node), message: message))
+  private func addDiagnostic<T: SyntaxProtocol>(_ node: T, _ message: DiagnosticMessage, fixIts: [FixIt] = []) {
+    diagnostics.append(Diagnostic(node: Syntax(node), message: message, fixIts: fixIts))
   }
 
   /// Produce a diagnostic.
-  private func addDiagnostic<T: SyntaxProtocol>(_ node: T, _ message: StaticParserError) {
-    addDiagnostic(node, message as DiagnosticMessage)
+  private func addDiagnostic<T: SyntaxProtocol>(_ node: T, _ message: StaticParserError, fixIts: [FixIt] = []) {
+    addDiagnostic(node, message as DiagnosticMessage, fixIts: fixIts)
   }
 
   /// If a diagnostic is generated that covers multiple syntax nodes, mark them as handles so they don't produce the generic diagnostics anymore.
@@ -111,11 +130,18 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
       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)
-        markNodesAsHandled(unexpectedBeforeReturnType.id, throwsInReturnPosition.id)
-        return .visitChildren
+      addDiagnostic(throwsInReturnPosition, .throwsInReturnPosition, fixIts: [
+        FixIt(message: StaticParserFixIt.moveThrowBeforeArrow, changes: [
+          .makeMissing(node: throwsInReturnPosition),
+          .makePresent(node: missingThrowsKeyword, trailingTrivia: .space),
+        ])
+      ])
+      markNodesAsHandled(unexpectedBeforeReturnType.id, missingThrowsKeyword.id, throwsInReturnPosition.id)
+      return .visitChildren
     }
     return .visitChildren
   }

Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift

@@ -57,6 +57,20 @@ public extension ParserError {
   }
 }
 
+public protocol ParserFixIt: FixItMessage {
+  var fixItID: MessageID { get }
+}
+
+public extension ParserFixIt {
+  static var fixItID: MessageID {
+    return MessageID(domain: diagnosticDomain, id: "\(self)")
+  }
+
+  var fixItID: MessageID {
+    return Self.fixItID
+  }
+}
+
 // MARK: - Static diagnostics
 
 /// Please order the cases in this enum alphabetically by case name.
@@ -73,6 +87,16 @@ public enum StaticParserError: String, DiagnosticMessage {
   public var severity: DiagnosticSeverity { .error }
 }
 
+public enum StaticParserFixIt: String, FixItMessage {
+  case moveThrowBeforeArrow = "Move 'throws' in before of '->'"
+
+  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 MissingTokenError: ParserError {

Tests/SwiftParserTest/DiagnosticAssertions.swift

@@ -4,6 +4,32 @@ import SwiftParser
 import XCTest
 import _SwiftSyntaxTestSupport
 
+public class FixItApplier: SyntaxRewriter {
+  var changes: [FixIt.Change]
+
+  init(diagnostics: [Diagnostic]) {
+    self.changes = diagnostics.flatMap({ $0.fixIts }).flatMap({ $0.changes })
+  }
+
+  public override func visitAny(_ node: Syntax) -> Syntax? {
+    for change in changes {
+      switch change {
+      case .replace(oldNode: let oldNode, newNode: let newNode) where oldNode.id == node.id:
+        return newNode
+      default:
+        break
+      }
+    }
+    return nil
+  }
+
+  /// Applies all Fix-Its in `diagnostics` to `tree` and returns the fixed syntax tree.
+  public static func applyFixes<T: SyntaxProtocol>(in diagnostics: [Diagnostic], to tree: T) -> Syntax {
+    let applier = FixItApplier(diagnostics: diagnostics)
+    return applier.visit(Syntax(tree))
+  }
+}
+
 /// Asserts that the diagnostics `diag` inside `tree` occurs at `line` and
 /// `column`.
 /// If `message` is not `nil`, assert that the diagnostic has the given message.
@@ -34,12 +60,14 @@ func XCTAssertDiagnostic<T: SyntaxProtocol>(
 /// at `line` and `column`.
 /// If `message` is not `nil`, assert that the diagnostic has the given message.
 /// If `id` is not `nil`, assert that the diagnostic has the given message.
+/// If `expectedFixedSource` is not `nil`, assert that the source code that it is source code that result from applying all Fix-Its.
 func XCTAssertSingleDiagnostic<T: SyntaxProtocol>(
   in tree: T,
   line: Int,
   column: Int,
   id: MessageID? = nil,
   message: String? = nil,
+  expectedFixedSource: String? = nil,
   testFile: StaticString = #filePath,
   testLine: UInt = #line
 ) {
@@ -49,4 +77,8 @@ func XCTAssertSingleDiagnostic<T: SyntaxProtocol>(
     return
   }
   XCTAssertDiagnostic(diags.first!, in: tree, line: line, column: column, id: id, message: message, testFile: testFile, testLine: testLine)
+  if let expectedFixedSource = expectedFixedSource {
+    let fixedSource = FixItApplier.applyFixes(in: diags, to: tree).description
+    AssertStringsEqualWithDiff(fixedSource, expectedFixedSource, file: testFile, line: testLine)
+  }
 }

Tests/SwiftParserTest/DiagnosticTests.swift

@@ -65,6 +65,11 @@ public class DiagnosticTests: XCTestCase {
       Syntax(raw: $0.parseFunctionSignature().raw).as(FunctionSignatureSyntax.self)!
     }
 
-    XCTAssertSingleDiagnostic(in: signature, line: 1, column: 7, message: "'throws' may only occur before '->'")
+    XCTAssertSingleDiagnostic(
+      in: signature,
+      line: 1, column: 7,
+      message: "'throws' may only occur before '->'",
+      expectedFixedSource: "() throws -> Int"
+    )
   }
 }