Merge pull request #216 from ahoppen/fixits

Provide Fix-Its through codeActions and PublishDiagnostics

Author: benlangmuir

Sources/LanguageServerProtocol/Requests/CodeActionRequest.swift

@@ -105,7 +105,7 @@ public struct CodeActionContext: Codable, Hashable {
   }
 }
 
-public struct CodeAction: Codable, Equatable {
+public struct CodeAction: Codable, Hashable {
 
   /// A short, human-readable, title for this code action.
   public var title: String

Sources/LanguageServerProtocol/SupportTypes/ClientCapabilities.swift

@@ -356,8 +356,14 @@ public struct TextDocumentClientCapabilities: Hashable, Codable {
     /// Whether the client accepts diagnostics with related information.
     public var relatedInformation: Bool? = nil
 
-    public init(relatedInformation: Bool? = nil) {
+    /// Requests that SourceKit-LSP send `Diagnostic.codeActions`.
+    /// **LSP Extension from clangd**.
+    public var codeActionsInline: Bool? = nil
+
+    public init(relatedInformation: Bool? = nil,
+                codeActionsInline: Bool? = nil) {
       self.relatedInformation = relatedInformation
+      self.codeActionsInline = codeActionsInline
     }
   }
 

Sources/LanguageServerProtocol/SupportTypes/Diagnostic.swift

@@ -47,20 +47,26 @@ public struct Diagnostic: Codable, Hashable {
   /// Related diagnostic notes.
   public var relatedInformation: [DiagnosticRelatedInformation]?
 
+  /// All the code actions that address this diagnostic.
+  /// **LSP Extension from clangd**.
+  public var codeActions: [CodeAction]?
+
   public init(
     range: Range<Position>,
     severity: DiagnosticSeverity?,
     code: DiagnosticCode? = nil,
     source: String?,
     message: String,
-    relatedInformation: [DiagnosticRelatedInformation]? = nil)
+    relatedInformation: [DiagnosticRelatedInformation]? = nil,
+    codeActions: [CodeAction]? = nil)
   {
     self._range = CustomCodable<PositionRange>(wrappedValue: range)
     self.severity = severity
     self.code = code
     self.source = source
     self.message = message
     self.relatedInformation = relatedInformation
+    self.codeActions = codeActions
   }
 }
 

Sources/SourceKit/sourcekitd/Diagnostic.swift

@@ -15,6 +15,29 @@ import LSPLogging
 import SKSupport
 import sourcekitd
 
+extension CodeAction {
+  init?(fixit: SKResponseDictionary, in snapshot: DocumentSnapshot) {
+    let keys = fixit.sourcekitd.keys
+
+    guard let utf8Offset: Int = fixit[keys.offset],
+        let length: Int = fixit[keys.length],
+        let replacement: String = fixit[keys.sourcetext] else {
+      return nil
+    }
+    guard let position = snapshot.positionOf(utf8Offset: utf8Offset),
+        let endPosition = snapshot.positionOf(utf8Offset: utf8Offset + length) else {
+      return nil
+    }
+    let textEdit = TextEdit(range: position..<endPosition, newText: replacement)
+
+    let workspaceEdit = WorkspaceEdit(changes: [snapshot.document.uri:[textEdit]])
+    self.init(title: "Fix",
+              kind: .quickFix,
+              diagnostics: nil,
+              edit: workspaceEdit)
+  }
+}
+
 extension Diagnostic {
 
   /// Creates a diagnostic from a sourcekitd response dictionary.
@@ -52,6 +75,17 @@ extension Diagnostic {
       }
     }
 
+    var fixits: [CodeAction]? = nil
+    if let skfixits: SKResponseArray = diag[keys.fixits] {
+      fixits = []
+      skfixits.forEach { (_, skfixit) -> Bool in
+        if let codeAction = CodeAction(fixit: skfixit, in: snapshot) {
+          fixits?.append(codeAction)
+        }
+        return true
+      }
+    }
+
     var notes: [DiagnosticRelatedInformation]? = nil
     if let sknotes: SKResponseArray = diag[keys.diagnostics] {
       notes = []
@@ -71,7 +105,8 @@ extension Diagnostic {
       code: nil,
       source: "sourcekitd",
       message: message,
-      relatedInformation: notes)
+      relatedInformation: notes,
+      codeActions: fixits)
   }
 }
 

Sources/SourceKit/sourcekitd/SwiftLanguageServer.swift

@@ -19,6 +19,24 @@ import SKSupport
 import sourcekitd
 import TSCBasic
 
+fileprivate extension Range {
+  /// Checks if this range overlaps with the other range, counting an overlap with an empty range as a valid overlap.
+  /// The standard library implementation makes `1..<3.overlaps(2..<2)` return false because the second range is empty and thus the overlap is also empty.
+  /// This implementation over overlap considers such an inclusion of an empty range as a valid overlap.
+  func overlapsIncludingEmptyRanges(other: Range<Bound>) -> Bool {
+    switch (self.isEmpty, other.isEmpty) {
+    case (true, true):
+      return self.lowerBound == other.lowerBound
+    case (true, false):
+      return other.contains(self.lowerBound)
+    case (false, true):
+      return self.contains(other.lowerBound)
+    case (false, false):
+      return self.overlaps(other)
+    }
+  }
+}
+
 public final class SwiftLanguageServer: ToolchainLanguageServer {
 
   /// The server's request queue, used to serialize requests and responses to `sourcekitd`.
@@ -155,7 +173,7 @@ extension SwiftLanguageServer {
       documentSymbolProvider: true,
       codeActionProvider: .value(CodeActionServerCapabilities(
         clientCapabilities: initialize.capabilities.textDocument?.codeAction,
-        codeActionOptions: CodeActionOptions(codeActionKinds: nil),
+        codeActionOptions: CodeActionOptions(codeActionKinds: [.quickFix, .refactor]),
         supportsCodeActions: true)),
       colorProvider: .bool(true),
       foldingRangeProvider: .bool(true),
@@ -981,9 +999,8 @@ extension SwiftLanguageServer {
 
   public func codeAction(_ req: Request<CodeActionRequest>) {
     let providersAndKinds: [(provider: CodeActionProvider, kind: CodeActionKind)] = [
-      (retrieveRefactorCodeActions, .refactor)
-      //TODO: Implement the providers.
-      //(retrieveQuickFixCodeActions, .quickFix)
+      (retrieveRefactorCodeActions, .refactor),
+      (retrieveQuickFixCodeActions, .quickFix)
     ]
     let wantedActionKinds = req.params.context.only
     let providers = providersAndKinds.filter { wantedActionKinds?.contains($0.1) != false }
@@ -1061,6 +1078,51 @@ extension SwiftLanguageServer {
     }
   }
 
+  func retrieveQuickFixCodeActions(_ params: CodeActionRequest, completion: @escaping CodeActionProviderCompletion) {
+    guard let cachedDiags = currentDiagnostics[params.textDocument.uri] else {
+      completion(.success([]))
+      return
+    }
+
+    let codeActions = cachedDiags.flatMap { (cachedDiag) -> [CodeAction] in
+      let diag = cachedDiag.diagnostic
+
+      guard let codeActions = diag.codeActions else {
+        // The diagnostic doesn't have fix-its. Don't return anything.
+        return []
+      }
+
+      // Check if the diagnostic overlaps with the selected range.
+      guard params.range.overlapsIncludingEmptyRanges(other: diag.range) else {
+        return []
+      }
+
+      // Check if the set of diagnostics provided by the request contains this diagnostic.
+      // For this, only compare the 'basic' properties of the diagnostics, excluding related information and code actions since
+      // code actions are only defined in an LSP extension and might not be sent back to us.
+      guard params.context.diagnostics.contains(where: { (contextDiag) -> Bool in
+        return contextDiag.range == diag.range &&
+          contextDiag.severity == diag.severity &&
+          contextDiag.code == diag.code &&
+          contextDiag.source == diag.source &&
+          contextDiag.message == diag.message
+      }) else {
+        return []
+      }
+
+      // Flip the attachment of diagnostic to code action instead of the code action being attached to the diagnostic
+      return codeActions.map({
+        var codeAction = $0
+        var diagnosticWithoutCodeActions = diag
+        diagnosticWithoutCodeActions.codeActions = nil
+        codeAction.diagnostics = [diagnosticWithoutCodeActions]
+        return codeAction
+      })
+    }
+
+    completion(.success(codeActions))
+  }
+
   public func executeCommand(_ req: Request<ExecuteCommandRequest>) {
     let params = req.params
     //TODO: If there's support for several types of commands, we might need to structure this similarly to the code actions request.

Sources/SourceKit/sourcekitd/SwiftSourceKitFramework.swift

@@ -215,6 +215,7 @@ struct sourcekitd_keys {
   let name: sourcekitd_uid_t
   let kind: sourcekitd_uid_t
   let notification: sourcekitd_uid_t
+  let fixits: sourcekitd_uid_t
   let diagnostics: sourcekitd_uid_t
   let diagnostic_stage: sourcekitd_uid_t
   let severity: sourcekitd_uid_t
@@ -263,6 +264,7 @@ struct sourcekitd_keys {
     name = api.uid_get_from_cstr("key.name")!
     kind = api.uid_get_from_cstr("key.kind")!
     notification = api.uid_get_from_cstr("key.notification")!
+    fixits = api.uid_get_from_cstr("key.fixits")!
     diagnostics = api.uid_get_from_cstr("key.diagnostics")!
     diagnostic_stage = api.uid_get_from_cstr("key.diagnostic_stage")!
     severity = api.uid_get_from_cstr("key.severity")!

Tests/SourceKitTests/LocalSwiftTests.swift

@@ -41,7 +41,12 @@ final class LocalSwiftTests: XCTestCase {
         rootPath: nil,
         rootURI: nil,
         initializationOptions: nil,
-        capabilities: ClientCapabilities(workspace: nil, textDocument: nil),
+        capabilities: ClientCapabilities(workspace: nil,
+                                         textDocument: TextDocumentClientCapabilities(
+                                          codeAction: .init(
+                                            codeActionLiteralSupport: .init(
+                                              codeActionKind: .init(valueSet: [.quickFix])
+                                          )))),
         trace: .off,
         workspaceFolders: nil))
   }
@@ -413,6 +418,89 @@ final class LocalSwiftTests: XCTestCase {
     })
   }
 
+  func testFixitsAreIncludedInPublishDiagnostics() {
+    let url = URL(fileURLWithPath: "/a.swift")
+    let uri = DocumentURI(url)
+
+    sk.allowUnexpectedNotification = false
+
+    sk.sendNoteSync(DidOpenTextDocumentNotification(textDocument: TextDocumentItem(
+      uri: uri, language: .swift, version: 12,
+      text: """
+      func foo() {
+        let a = 2
+      }
+      """
+    )), { (note: Notification<PublishDiagnosticsNotification>) in
+      log("Received diagnostics for open - syntactic")
+    }, { (note: Notification<PublishDiagnosticsNotification>) in
+      log("Received diagnostics for open - semantic")
+      XCTAssertEqual(note.params.diagnostics.count, 1)
+      let diag = note.params.diagnostics.first!
+      XCTAssertNotNil(diag.codeActions)
+      XCTAssertEqual(diag.codeActions!.count, 1)
+      let fixit = diag.codeActions!.first!
+
+      // Expected Fix-it: Replace `let a` with `_` because it's never used
+      let expectedTextEdit = TextEdit(range: Position(line: 1, utf16index: 2)..<Position(line: 1, utf16index: 7), newText: "_")
+      XCTAssertEqual(fixit, CodeAction(title: "Fix",
+                                       kind: .quickFix,
+                                       diagnostics: nil,
+                                       edit: WorkspaceEdit(changes: [uri: [expectedTextEdit]], documentChanges: nil),
+                                       command: nil))
+    })
+  }
+
+  func testFixitsAreReturnedFromCodeActions() {
+    let url = URL(fileURLWithPath: "/a.swift")
+    let uri = DocumentURI(url)
+
+    var diagnostic: Diagnostic! = nil
+    sk.sendNoteSync(DidOpenTextDocumentNotification(textDocument: TextDocumentItem(
+      uri: uri, language: .swift, version: 12,
+      text: """
+      func foo() {
+        let a = 2
+      }
+      """
+    )), { (note: Notification<PublishDiagnosticsNotification>) in
+      log("Received diagnostics for open - syntactic")
+    }, { (note: Notification<PublishDiagnosticsNotification>) in
+      log("Received diagnostics for open - semantic")
+      XCTAssertEqual(note.params.diagnostics.count, 1)
+      diagnostic = note.params.diagnostics.first!
+    })
+
+    let request = CodeActionRequest(
+      range: Position(line: 1, utf16index: 0)..<Position(line: 1, utf16index: 11),
+      context: CodeActionContext(diagnostics: [diagnostic], only: nil),
+      textDocument: TextDocumentIdentifier(uri)
+    )
+    let response = try! sk.sendSync(request)
+
+    XCTAssertNotNil(response)
+    guard case .codeActions(let codeActions) = response else {
+      XCTFail("Expected code actions as response")
+      return
+    }
+    XCTAssertEqual(codeActions.count, 1)
+    let fixit = codeActions.first!
+
+    // Expected Fix-it: Replace `let a` with `_` because it's never used
+    let expectedTextEdit = TextEdit(range: Position(line: 1, utf16index: 2)..<Position(line: 1, utf16index: 7), newText: "_")
+    XCTAssertEqual(fixit, CodeAction(title: "Fix",
+                                     kind: .quickFix,
+                                     diagnostics: [Diagnostic(range: Position(line: 1, utf16index: 6)..<Position(line: 1, utf16index: 6),
+                                                              severity: .warning,
+                                                              code: nil,
+                                                              source: "sourcekitd",
+                                                              message: "initialization of immutable value \'a\' was never used; consider replacing with assignment to \'_\' or removing it",
+                                                              relatedInformation: [],
+                                                              codeActions: nil)],
+                                     edit: WorkspaceEdit(changes: [uri: [expectedTextEdit]], documentChanges: nil),
+                                     command: nil))
+  }
+
   func testXMLToMarkdownDeclaration() {
     XCTAssertEqual(try! xmlDocumentationToMarkdown("""
       <Declaration>func foo(_ bar: <Type usr="fake">Baz</Type>)</Declaration>

Tests/SourceKitTests/XCTestManifests.swift

@@ -108,6 +108,8 @@ extension LocalSwiftTests {
         ("testEditing", testEditing),
         ("testEditingNonURL", testEditingNonURL),
         ("testEditorPlaceholderParsing", testEditorPlaceholderParsing),
+        ("testFixitsAreIncludedInPublishDiagnostics", testFixitsAreIncludedInPublishDiagnostics),
+        ("testFixitsAreReturnedFromCodeActions", testFixitsAreReturnedFromCodeActions),
         ("testHover", testHover),
         ("testHoverNameEscaping", testHoverNameEscaping),
         ("testSymbolInfo", testSymbolInfo),