Merge pull request #1031 from ahoppen/ahoppen/clang-rename

Support cross-file rename for clang languages

Author: ahoppen

Sources/LanguageServerProtocol/CMakeLists.txt

@@ -56,6 +56,7 @@ add_library(LanguageServerProtocol STATIC
   Requests/FormattingRequests.swift
   Requests/HoverRequest.swift
   Requests/ImplementationRequest.swift
+  Requests/IndexedRenameRequest.swift
   Requests/InitializeRequest.swift
   Requests/InlayHintRefreshRequest.swift
   Requests/InlayHintRequest.swift

Sources/LanguageServerProtocol/Requests/IndexedRenameRequest.swift

@@ -0,0 +1,93 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2023 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
+//
+//===----------------------------------------------------------------------===//
+
+/// Rename all occurrences of a symbol named `oldName` to `newName` at the
+/// given `positions`.
+///
+/// The use case of this method is for when the positions to rename are already
+/// known, eg. from an index lookup outside of clangd's built-in index. In
+/// particular, it determines the edits necessary to rename multi-piece
+/// Objective-C selector names.
+///
+/// `textDocument` is used to determine the language options for the symbol to
+/// rename, eg. to decide whether `oldName` and `newName` are Objective-C
+/// selectors or normal identifiers.
+///
+/// This is a clangd extension.
+public struct IndexedRenameRequest: TextDocumentRequest, Hashable {
+  public static let method: String = "workspace/indexedRename"
+  public typealias Response = WorkspaceEdit?
+
+  /// The document in which the declaration to rename is declared. Its compiler
+  /// arguments are used to infer language settings for the rename.
+  public var textDocument: TextDocumentIdentifier
+
+  /// The old name of the symbol.
+  public var oldName: String
+
+  /// The new name of the symbol.
+  public var newName: String
+
+  /// The positions at which the symbol is known to appear and that should be
+  /// renamed. The key is a document URI
+  public var positions: [DocumentURI: [Position]]
+
+  public init(
+    textDocument: TextDocumentIdentifier,
+    oldName: String,
+    newName: String,
+    positions: [DocumentURI: [Position]]
+  ) {
+    self.textDocument = textDocument
+    self.oldName = oldName
+    self.newName = newName
+    self.positions = positions
+  }
+}
+
+// Workaround for Codable not correctly encoding dictionaries whose keys aren't strings.
+extension IndexedRenameRequest: Codable {
+  private enum CodingKeys: CodingKey {
+    case textDocument
+    case oldName
+    case newName
+    case positions
+  }
+
+  public init(from decoder: Decoder) throws {
+    let container = try decoder.container(keyedBy: CodingKeys.self)
+
+    self.textDocument = try container.decode(
+      TextDocumentIdentifier.self,
+      forKey: IndexedRenameRequest.CodingKeys.textDocument
+    )
+    self.oldName = try container.decode(String.self, forKey: IndexedRenameRequest.CodingKeys.oldName)
+    self.newName = try container.decode(String.self, forKey: IndexedRenameRequest.CodingKeys.newName)
+    self.positions = try container.decode([String: [Position]].self, forKey: .positions).mapKeys(DocumentURI.init)
+  }
+
+  public func encode(to encoder: Encoder) throws {
+    var container = encoder.container(keyedBy: CodingKeys.self)
+
+    try container.encode(self.textDocument, forKey: IndexedRenameRequest.CodingKeys.textDocument)
+    try container.encode(self.oldName, forKey: IndexedRenameRequest.CodingKeys.oldName)
+    try container.encode(self.newName, forKey: IndexedRenameRequest.CodingKeys.newName)
+    try container.encode(self.positions.mapKeys(\.stringValue), forKey: IndexedRenameRequest.CodingKeys.positions)
+
+  }
+}
+
+fileprivate extension Dictionary {
+  func mapKeys<NewKeyType: Hashable>(_ transform: (Key) -> NewKeyType) -> [NewKeyType: Value] {
+    return [NewKeyType: Value](uniqueKeysWithValues: self.map { (transform($0.key), $0.value) })
+  }
+}

Sources/SourceKitLSP/Rename.swift

@@ -608,18 +608,40 @@ extension SwiftLanguageServer {
 // MARK: - Clang
 
 extension ClangLanguageServerShim {
-  func rename(_ request: RenameRequest) async throws -> (edits: WorkspaceEdit, usr: String?, oldName: String?) {
-    let edits = try await forwardRequestToClangd(request)
-    return (edits ?? WorkspaceEdit(), nil, nil)
+  func rename(_ renameRequest: RenameRequest) async throws -> (edits: WorkspaceEdit, usr: String?, oldName: String?) {
+    async let edits = forwardRequestToClangd(renameRequest)
+    let symbolInfoRequest = SymbolInfoRequest(
+      textDocument: renameRequest.textDocument,
+      position: renameRequest.position
+    )
+    let symbolDetail = try await forwardRequestToClangd(symbolInfoRequest).only
+    return (try await edits ?? WorkspaceEdit(), symbolDetail?.usr, symbolDetail?.name)
   }
 
   func editsToRename(
     locations renameLocations: [RenameLocation],
     in snapshot: DocumentSnapshot,
-    oldName oldNameString: String,
+    oldName: String,
     newName: String
   ) async throws -> [TextEdit] {
-    throw ResponseError.internalError("Global rename not implemented for clangd")
+    let positions = [
+      snapshot.uri: renameLocations.compactMap {
+        snapshot.positionOf(zeroBasedLine: $0.line - 1, utf8Column: $0.utf8Column - 1)
+      }
+    ]
+    let request = IndexedRenameRequest(
+      textDocument: TextDocumentIdentifier(snapshot.uri),
+      oldName: oldName,
+      newName: newName,
+      positions: positions
+    )
+    do {
+      let edits = try await forwardRequestToClangd(request)
+      return edits?.changes?[snapshot.uri] ?? []
+    } catch {
+      logger.error("Failed to get indexed rename edits: \(error.forLogging)")
+      return []
+    }
   }
 
   public func prepareRename(_ request: PrepareRenameRequest) async throws -> PrepareRenameResponse? {

Tests/SourceKitLSPTests/RenameTests.swift

@@ -36,6 +36,7 @@ private func apply(edits: [TextEdit], to source: String) -> String {
 /// Test that applying the edits returned from the requests always result in `expected`.
 private func assertSingleFileRename(
   _ markedSource: String,
+  language: Language? = nil,
   newName: String,
   expected: String,
   testName: String = #function,
@@ -44,7 +45,7 @@ private func assertSingleFileRename(
 ) async throws {
   let testClient = try await TestSourceKitLSPClient()
   let uri = DocumentURI.for(.swift, testName: testName)
-  let positions = testClient.openDocument(markedSource, uri: uri)
+  let positions = testClient.openDocument(markedSource, uri: uri, language: language)
   for marker in positions.allMarkers {
     let response: WorkspaceEdit?
     do {
@@ -111,6 +112,7 @@ private func assertRenamedSourceMatches(
 /// to be placed in a state where there are in-memory changes that haven't been written to disk yet.
 private func assertMultiFileRename(
   files: [RelativeFileLocation: String],
+  language: Language? = nil,
   newName: String,
   expected: [RelativeFileLocation: String],
   manifest: String = SwiftPMTestWorkspace.defaultPackageManifest,
@@ -131,7 +133,7 @@ private func assertMultiFileRename(
     if markers.isEmpty {
       continue
     }
-    let (uri, positions) = try ws.openDocument(fileLocation.fileName)
+    let (uri, positions) = try ws.openDocument(fileLocation.fileName, language: language)
     defer {
       ws.testClient.send(DidCloseTextDocumentNotification(textDocument: TextDocumentIdentifier(uri)))
     }
@@ -759,4 +761,74 @@ final class RenameTests: XCTestCase {
     XCTAssertEqual(range, positions["1️⃣"]..<positions["2️⃣"])
     XCTAssertEqual(placeholder, "foo(a:b:)")
   }
+
+  func testGlobalRenameC() async throws {
+    try await assertMultiFileRename(
+      files: [
+        "Sources/MyLibrary/include/lib.h": """
+        void 1️⃣do2️⃣Stuff();
+        """,
+        "lib.c": """
+        #include "lib.h"
+
+        void 3️⃣doStuff() {
+          4️⃣doStuff();
+        }
+        """,
+      ],
+      language: .c,
+      newName: "doRecursiveStuff",
+      expected: [
+        "Sources/MyLibrary/include/lib.h": """
+        void doRecursiveStuff();
+        """,
+        "lib.c": """
+        #include "lib.h"
+
+        void doRecursiveStuff() {
+          doRecursiveStuff();
+        }
+        """,
+      ]
+    )
+  }
+
+  func testGlobalRenameObjC() async throws {
+    try await assertMultiFileRename(
+      files: [
+        "Sources/MyLibrary/include/lib.h": """
+        @interface Foo
+        - (int)1️⃣perform2️⃣Action:(int)action 3️⃣wi4️⃣th:(int)value;
+        @end
+        """,
+        "lib.m": """
+        #include "lib.h"
+
+        @implementation Foo
+        - (int)5️⃣performAction:(int)action 6️⃣with:(int)value {
+          return [self 7️⃣performAction:action 8️⃣with:value];
+        }
+        @end
+        """,
+      ],
+      language: .objective_c,
+      newName: "performNewAction:by:",
+      expected: [
+        "Sources/MyLibrary/include/lib.h": """
+        @interface Foo
+        - (int)performNewAction:(int)action by:(int)value;
+        @end
+        """,
+        "lib.m": """
+        #include "lib.h"
+
+        @implementation Foo
+        - (int)performNewAction:(int)action by:(int)value {
+          return [self performNewAction:action by:value];
+        }
+        @end
+        """,
+      ]
+    )
+  }
 }