Add support for clangd commands + code actions (#429)

This adds support for clangd commands for clients which support
dynamic registration (including VS Code), as well as fixes an
issue which prevented clangd's code actions from working.

Also added a test to ensure the clangd code actions work, as well as regenerated the Linux test main (which was missing some other newly added tests).

Author: DavidGoldman

Sources/LanguageServerProtocol/Messages.swift

@@ -42,6 +42,9 @@ public let builtinRequests: [_RequestType.Type] = [
   ColorPresentationRequest.self,
   CodeActionRequest.self,
   ExecuteCommandRequest.self,
+  ApplyEditRequest.self,
+  RegisterCapabilityRequest.self,
+  UnregisterCapabilityRequest.self,
 
   // MARK: LSP Extension Requests
 

Sources/SKTestSupport/INPUTS/CodeActionCxx/main.cpp

@@ -0,0 +1,5 @@
+enum Color { RED, GREEN, BLUE };
+
+void someFunction(Color color) {
+  switch /*SwitchColor*/(color)/*SwitchColor:end*/ {}
+}

Sources/SKTestSupport/INPUTS/CodeActionCxx/project.json

@@ -0,0 +1 @@
+{ "sources": ["main.cpp"] }

Sources/SourceKitLSP/CapabilityRegistry.swift

@@ -29,6 +29,9 @@ public final class CapabilityRegistry {
   /// Dynamically registered semantic tokens options.
   private var semanticTokens: [CapabilityRegistration: SemanticTokensRegistrationOptions] = [:]
 
+  /// Dynamically registered command IDs.
+  private var commandIds: Set<String> = []
+
   public let clientCapabilities: ClientCapabilities
 
   public init(clientCapabilities: ClientCapabilities) {
@@ -47,6 +50,10 @@ public final class CapabilityRegistry {
     clientCapabilities.textDocument?.semanticTokens?.dynamicRegistration == true
   }
 
+  public var clientHasDynamicExecuteCommandRegistration: Bool {
+    clientCapabilities.workspace?.executeCommand?.dynamicRegistration == true
+  }
+
   /// Dynamically register completion capabilities if the client supports it and
   /// we haven't yet registered any completion capabilities for the given
   /// languages.
@@ -131,6 +138,30 @@ public final class CapabilityRegistry {
     registerOnClient(registration)
   }
 
+  /// Dynamically register executeCommand with the given IDs if the client supports
+  /// it and we haven't yet registered the given command IDs yet.
+  public func registerExecuteCommandIfNeeded(
+    commands: [String],
+    registerOnClient: ClientRegistrationHandler
+  ) {
+    guard clientHasDynamicExecuteCommandRegistration else { return }
+    var newCommands = Set(commands)
+    newCommands.subtract(self.commandIds)
+
+    // We only want to send the registration with unregistered command IDs since
+    // clients such as VS Code only allow a command to be registered once. We could
+    // unregister all our commandIds first but this is simpler.
+    guard !newCommands.isEmpty else { return }
+    self.commandIds.formUnion(newCommands)
+
+    let registrationOptions = ExecuteCommandRegistrationOptions(commands: Array(newCommands))
+    let registration = CapabilityRegistration(
+      method: ExecuteCommandRequest.method,
+      registerOptions: self.encode(registrationOptions))
+
+    registerOnClient(registration)
+  }
+
   /// Unregister a previously registered registration, e.g. if no longer needed
   /// or if registration fails.
   public func remove(registration: CapabilityRegistration) {

Sources/SourceKitLSP/Clang/ClangLanguageServer.swift

@@ -510,8 +510,7 @@ extension ClangLanguageServerShim {
   // MARK: - Other
 
   func executeCommand(_ req: Request<ExecuteCommandRequest>) {
-    //TODO: Implement commands.
-    return req.reply(nil)
+    forwardRequestToClangdOnQueue(req)
   }
 }
 

Sources/SourceKitLSP/SourceKitServer.swift

@@ -524,6 +524,12 @@ extension SourceKitServer {
         triggerCharacters: ["."]
       )
     }
+    let executeCommandOptions: ExecuteCommandOptions?
+    if registry.clientHasDynamicExecuteCommandRegistration {
+      executeCommandOptions = nil
+    } else {
+      executeCommandOptions = ExecuteCommandOptions(commands: builtinSwiftCommands)
+    }
     return ServerCapabilities(
       textDocumentSync: TextDocumentSyncOptions(
         openClose: true,
@@ -547,9 +553,7 @@ extension SourceKitServer {
       )),
       colorProvider: .bool(true),
       foldingRangeProvider: .bool(!registry.clientHasDynamicFoldingRangeRegistration),
-      executeCommandProvider: ExecuteCommandOptions(
-        commands: builtinSwiftCommands // FIXME: Clangd commands?
-      )
+      executeCommandProvider: executeCommandOptions
     )
   }
 
@@ -573,6 +577,11 @@ extension SourceKitServer {
         self.dynamicallyRegisterCapability($0, registry)
       }
     }
+    if let commandOptions = server.executeCommandProvider {
+      registry.registerExecuteCommandIfNeeded(commands: commandOptions.commands) {
+        self.dynamicallyRegisterCapability($0, registry)
+      }
+    }
   }
 
   private func dynamicallyRegisterCapability(

Tests/SKSwiftPMWorkspaceTests/XCTestManifests.swift

@@ -15,6 +15,7 @@ extension SwiftPMWorkspaceTests {
         ("testMultiTargetSwift", testMultiTargetSwift),
         ("testNoPackage", testNoPackage),
         ("testNoToolchain", testNoToolchain),
+        ("testSwiftDerivedSources", testSwiftDerivedSources),
         ("testSymlinkInWorkspaceCXX", testSymlinkInWorkspaceCXX),
         ("testSymlinkInWorkspaceSwift", testSymlinkInWorkspaceSwift),
         ("testUnknownFile", testUnknownFile),

Tests/SourceKitLSPTests/LocalClangTests.swift

@@ -174,6 +174,68 @@ final class LocalClangTests: XCTestCase {
     XCTAssertEqual(syms.first?.children?.first?.name, "foo")
   }
 
+  func testCodeAction() throws {
+    guard let ws = try staticSourceKitTibsWorkspace(name: "CodeActionCxx") else { return }
+    if ToolchainRegistry.shared.default?.clangd == nil { return }
+
+    let loc = ws.testLoc("SwitchColor")
+    let endLoc = ws.testLoc("SwitchColor:end")
+
+    let expectation = XCTestExpectation(description: "diagnostics")
+
+    ws.sk.handleNextNotification { (note: Notification<PublishDiagnosticsNotification>) in
+      let diagnostics = note.params.diagnostics
+      // It seems we either get no diagnostics or a `-Wswitch` warning. Either is fine
+      // as long as our code action works properly.
+      XCTAssert(diagnostics.isEmpty ||
+                  (diagnostics.count == 1 && diagnostics.first?.code == .string("-Wswitch")),
+                "Unexpected diagnostics \(diagnostics)")
+      expectation.fulfill()
+    }
+
+    try ws.openDocument(loc.url, language: .cpp)
+
+    let result = XCTWaiter.wait(for: [expectation], timeout: 15)
+    if result != .completed {
+      fatalError("error \(result) waiting for diagnostics notification")
+    }
+
+    let codeAction = CodeActionRequest(
+      range: Position(loc)..<Position(endLoc),
+      context: CodeActionContext(),
+      textDocument: loc.docIdentifier
+    )
+    guard let reply = try ws.sk.sendSync(codeAction) else {
+      XCTFail("CodeActionRequest had nil reply")
+      return
+    }
+    guard case let .commands(commands) = reply else {
+      XCTFail("Expected [Command] but got \(reply)")
+      return
+    }
+    guard let command = commands.first else {
+      XCTFail("Expected a non-empty [Command]")
+      return
+    }
+    XCTAssertEqual(command.command, "clangd.applyTweak")
+
+    let applyEdit = XCTestExpectation(description: "applyEdit")
+    ws.sk.handleNextRequest { (request: Request<ApplyEditRequest>) in
+      XCTAssertNotNil(request.params.edit.changes)
+      request.reply(ApplyEditResponse(applied: true, failureReason: nil))
+      applyEdit.fulfill()
+    }
+
+    let executeCommand = ExecuteCommandRequest(
+      command: command.command, arguments: command.arguments)
+    _ = try ws.sk.sendSync(executeCommand)
+
+    let editResult = XCTWaiter.wait(for: [applyEdit], timeout: 15)
+    if editResult != .completed {
+      fatalError("error \(editResult) waiting for applyEdit request")
+    }
+  }
+
   func testClangStdHeaderCanary() throws {
     guard let ws = try staticSourceKitTibsWorkspace(name: "ClangStdHeaderCanary") else { return }
     if ToolchainRegistry.shared.default?.clangd == nil { return }

Tests/SourceKitLSPTests/XCTestManifests.swift

@@ -110,6 +110,8 @@ extension LocalClangTests {
     static let __allTests__LocalClangTests = [
         ("testClangModules", testClangModules),
         ("testClangStdHeaderCanary", testClangStdHeaderCanary),
+        ("testCodeAction", testCodeAction),
+        ("testDocumentSymbols", testDocumentSymbols),
         ("testFoldingRange", testFoldingRange),
         ("testSemanticHighlighting", testSemanticHighlighting),
         ("testSymbolInfo", testSymbolInfo),
@@ -154,37 +156,6 @@ extension MainFilesProviderTests {
     ]
 }
 
-extension SemanticTokensTests {
-    // DO NOT MODIFY: This is autogenerated, use:
-    //   `swift test --generate-linuxmain`
-    // to regenerate.
-    static let __allTests__SemanticTokensTests = [
-      ("testEmpty", testEmpty),
-      ("testEmptyEdit", testEmptyEdit),
-      ("testInsertNewline", testInsertNewline),
-      ("testInsertSpaceAfterToken", testInsertSpaceAfterToken),
-      ("testInsertSpaceBeforeToken", testInsertSpaceBeforeToken),
-      ("testInsertTokens", testInsertTokens),
-      ("testIntArrayCoding", testIntArrayCoding),
-      ("testRanged", testRanged),
-      ("testRangeSplitting", testRangeSplitting),
-      ("testRemoveNewline", testRemoveNewline),
-      ("testReplaceUntilEndOfToken", testReplaceUntilEndOfToken),
-      ("testReplaceUntilMiddleOfToken", testReplaceUntilMiddleOfToken),
-      ("testSemanticMultiEdit", testSemanticMultiEdit),
-      ("testSemanticTokens", testSemanticTokens),
-      ("testSemanticTokensForEnumMembers", testSemanticTokensForEnumMembers),
-      ("testSemanticTokensForFunctionSignatures", testSemanticTokensForFunctionSignatures),
-      ("testSemanticTokensForFunctionSignaturesWithEmoji", testSemanticTokensForFunctionSignaturesWithEmoji),
-      ("testSemanticTokensForProtocols", testSemanticTokensForProtocols),
-      ("testSemanticTokensForStaticMethods", testSemanticTokensForStaticMethods),
-      ("testLexicalTokens", testLexicalTokens),
-      ("testLexicalTokensForBackticks", testLexicalTokensForBackticks),
-      ("testLexicalTokensForDocComments", testLexicalTokensForDocComments),
-      ("testLexicalTokensForMultiLineComments", testLexicalTokensForMultiLineComments),
-    ]
-}
-
 extension SKTests {
     // DO NOT MODIFY: This is autogenerated, use:
     //   `swift test --generate-linuxmain`
@@ -202,6 +173,37 @@ extension SKTests {
     ]
 }
 
+extension SemanticTokensTests {
+    // DO NOT MODIFY: This is autogenerated, use:
+    //   `swift test --generate-linuxmain`
+    // to regenerate.
+    static let __allTests__SemanticTokensTests = [
+        ("testEmpty", testEmpty),
+        ("testEmptyEdit", testEmptyEdit),
+        ("testInsertNewline", testInsertNewline),
+        ("testInsertSpaceAfterToken", testInsertSpaceAfterToken),
+        ("testInsertSpaceBeforeToken", testInsertSpaceBeforeToken),
+        ("testInsertTokens", testInsertTokens),
+        ("testIntArrayCoding", testIntArrayCoding),
+        ("testLexicalTokens", testLexicalTokens),
+        ("testLexicalTokensForBackticks", testLexicalTokensForBackticks),
+        ("testLexicalTokensForDocComments", testLexicalTokensForDocComments),
+        ("testLexicalTokensForMultiLineComments", testLexicalTokensForMultiLineComments),
+        ("testRanged", testRanged),
+        ("testRangeSplitting", testRangeSplitting),
+        ("testRemoveNewline", testRemoveNewline),
+        ("testReplaceUntilEndOfToken", testReplaceUntilEndOfToken),
+        ("testReplaceUntilMiddleOfToken", testReplaceUntilMiddleOfToken),
+        ("testSemanticMultiEdit", testSemanticMultiEdit),
+        ("testSemanticTokens", testSemanticTokens),
+        ("testSemanticTokensForEnumMembers", testSemanticTokensForEnumMembers),
+        ("testSemanticTokensForFunctionSignatures", testSemanticTokensForFunctionSignatures),
+        ("testSemanticTokensForFunctionSignaturesWithEmoji", testSemanticTokensForFunctionSignaturesWithEmoji),
+        ("testSemanticTokensForProtocols", testSemanticTokensForProtocols),
+        ("testSemanticTokensForStaticMethods", testSemanticTokensForStaticMethods),
+    ]
+}
+
 extension SwiftCompileCommandsTest {
     // DO NOT MODIFY: This is autogenerated, use:
     //   `swift test --generate-linuxmain`
@@ -256,8 +258,8 @@ public func __allTests() -> [XCTestCaseEntry] {
         testCase(LocalClangTests.__allTests__LocalClangTests),
         testCase(LocalSwiftTests.__allTests__LocalSwiftTests),
         testCase(MainFilesProviderTests.__allTests__MainFilesProviderTests),
-        testCase(SemanticTokensTests.__allTests__SemanticTokensTests),
         testCase(SKTests.__allTests__SKTests),
+        testCase(SemanticTokensTests.__allTests__SemanticTokensTests),
         testCase(SwiftCompileCommandsTest.__allTests__SwiftCompileCommandsTest),
         testCase(SwiftCompletionTests.__allTests__SwiftCompletionTests),
         testCase(SwiftPMIntegrationTests.__allTests__SwiftPMIntegrationTests),