Refactor rename to support index-based discovery of rename locations

Author: ahoppen

Sources/SourceKitD/SKDRequestArray.swift

@@ -40,6 +40,12 @@ public final class SKDRequestArray {
   public func append(_ value: SKDRequestDictionary) {
     sourcekitd.api.request_array_set_value(array, -1, value.dict)
   }
+
+  public static func += (array: SKDRequestArray, other: some Sequence<SKDRequestDictionary>) {
+    for item in other {
+      array.append(item)
+    }
+  }
 }
 
 extension SKDRequestArray: CustomStringConvertible {

Sources/SourceKitLSP/Swift/Rename.swift

@@ -268,33 +268,47 @@ fileprivate struct SyntacticRenameName {
   }
 }
 
+struct RenameLocation {
+  /// The line of the identifier to be renamed (1-based).
+  let line: Int
+  /// The column of the identifier to be renamed in UTF-8 bytes (1-based).
+  let utf8Column: Int
+  let usage: RelatedIdentifier.Usage
+}
+
+private extension DocumentSnapshot {
+  init(_ url: URL, language: Language) throws {
+    let contents = try String(contentsOf: url)
+    self.init(uri: DocumentURI(url), language: language, version: 0, lineTable: LineTable(contents))
+  }
+}
+
 extension SwiftLanguageServer {
-  /// From a list of rename locations, provided by a related identifiers request, compute the list of
-  /// `SyntacticRenameName`s that define which ranges need to be edited to rename a compound decl name.
+  /// From a list of rename locations compute the list of `SyntacticRenameName`s that define which ranges need to be 
+  /// edited to rename a compound decl name.
+  ///  
+  /// - Parameters:
+  ///   - renameLocations: The locations to rename
+  ///   - oldName: The compound decl name that the declaration had before the rename. Used to verify that the rename 
+  ///     locations match that name. Eg. `myFunc(argLabel:otherLabel:)` or `myVar`
+  ///   - snapshot: If the document has been modified from the on-disk version, the current snapshot. `nil` to read the
+  ///     file contents from disk.
   private func getSyntacticRenameRanges(
-    relatedIdentifiers: RelatedIdentifiersResponse,
+    renameLocations: [RenameLocation],
+    oldName: String,
     in snapshot: DocumentSnapshot
   ) async throws -> [SyntacticRenameName] {
     let locations = SKDRequestArray(sourcekitd: sourcekitd)
-    for relatedIdentifier in relatedIdentifiers.relatedIdentifiers {
-      let position = relatedIdentifier.range.lowerBound
-      guard let utf8Column = snapshot.lineTable.utf8ColumnAt(line: position.line, utf16Column: position.utf16index)
-      else {
-        logger.fault("Unable to find UTF-8 column for \(position.line):\(position.utf16index)")
-        continue
-      }
-      let renameLocation = SKDRequestDictionary(sourcekitd: sourcekitd)
-      renameLocation[keys.line] = position.line + 1
-      renameLocation[keys.column] = utf8Column + 1
-      renameLocation[keys.nameType] = relatedIdentifier.usage.uid(keys: keys)
-      locations.append(renameLocation)
-    }
-    guard let name = relatedIdentifiers.name else {
-      throw ResponseError.unknown("Running sourcekit-lsp with a version of sourcekitd that does not support rename")
+    locations += renameLocations.map { renameLocation in
+      let skRenameLocation = SKDRequestDictionary(sourcekitd: sourcekitd)
+      skRenameLocation[keys.line] = renameLocation.line
+      skRenameLocation[keys.column] = renameLocation.utf8Column
+      skRenameLocation[keys.nameType] = renameLocation.usage.uid(keys: keys)
+      return skRenameLocation
     }
     let renameLocation = SKDRequestDictionary(sourcekitd: sourcekitd)
     renameLocation[keys.locations] = locations
-    renameLocation[keys.name] = name
+    renameLocation[keys.name] = oldName
 
     let renameLocations = SKDRequestArray(sourcekitd: sourcekitd)
     renameLocations.append(renameLocation)
@@ -323,22 +337,38 @@ extension SwiftLanguageServer {
       in: snapshot,
       includeNonEditableBaseNames: true
     )
+    guard let oldName = relatedIdentifiers.name else {
+      throw ResponseError.unknown("Running sourcekit-lsp with a version of sourcekitd that does not support rename")
+    }
+    
+    try Task.checkCancellation()
 
+    let renameLocations = relatedIdentifiers.relatedIdentifiers.compactMap { (relatedIdentifier) -> RenameLocation? in
+      let position = relatedIdentifier.range.lowerBound
+      guard let utf8Column = snapshot.lineTable.utf8ColumnAt(line: position.line, utf16Column: position.utf16index)
+      else {
+        logger.fault("Unable to find UTF-8 column for \(position.line):\(position.utf16index)")
+        return nil
+      }
+      return RenameLocation(line: position.line + 1, utf8Column: utf8Column + 1, usage: relatedIdentifier.usage)
+    }
+    
     try Task.checkCancellation()
 
-    let compoundRenameRanges = try await getSyntacticRenameRanges(relatedIdentifiers: relatedIdentifiers, in: snapshot)
+    let edits = try await renameRanges(from: renameLocations, in: snapshot, oldName: oldName, newName: try CompoundDeclName(request.newName))
 
-    try Task.checkCancellation()
+    return WorkspaceEdit(changes: [
+      snapshot.uri: edits
+    ])
+  }
 
-    let oldName =
-      if let name = relatedIdentifiers.name {
-        try CompoundDeclName(name)
-      } else {
-        throw ResponseError.unknown("Running sourcekit-lsp with a version of sourcekitd that does not support rename")
-      }
-    let newName = try CompoundDeclName(request.newName)
+  private func renameRanges(from renameLocations: [RenameLocation], in snapshot: DocumentSnapshot, oldName oldNameString: String, newName: CompoundDeclName) async throws -> [TextEdit] {
+    let compoundRenameRanges = try await getSyntacticRenameRanges(renameLocations: renameLocations, oldName: oldNameString, in: snapshot)
+    let oldName = try CompoundDeclName(oldNameString)
+
+    try Task.checkCancellation()
 
-    let edits = compoundRenameRanges.flatMap { (compoundRenameRange) -> [TextEdit] in
+    return compoundRenameRanges.flatMap { (compoundRenameRange) -> [TextEdit] in
       switch compoundRenameRange.category {
       case .unmatched, .mismatch:
         // The location didn't match. Don't rename it
@@ -428,9 +458,6 @@ extension SwiftLanguageServer {
         }
       }
     }
-    return WorkspaceEdit(changes: [
-      snapshot.uri: edits
-    ])
   }
 }
 

Tests/SourceKitLSPTests/RenameTests.swift

@@ -32,7 +32,7 @@ private func apply(edits: [TextEdit], to source: String) -> String {
   return lineTable.content
 }
 
-private func assertRename(
+private func assertSingleFileRename(
   _ markedSource: String,
   newName: String,
   expected: String,
@@ -57,7 +57,7 @@ private func assertRename(
 
 final class RenameTests: XCTestCase {
   func testRenameVariableBaseName() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       let 1️⃣foo = 1
       print(foo)
@@ -71,7 +71,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testRenameFunctionBaseName() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       func 1️⃣foo() {}
       foo()
@@ -85,7 +85,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testRenameFunctionParameter() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       func 1️⃣foo(x: Int) {}
       foo(x: 1)
@@ -99,7 +99,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testSecondParameterNameIfMatches() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       func 1️⃣foo(x y: Int) {}
       foo(x: 1)
@@ -113,7 +113,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testIntroduceLabel() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       func 1️⃣foo(_ y: Int) {}
       foo(1)
@@ -127,7 +127,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testRemoveLabel() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       func 1️⃣foo(x: Int) {}
       foo(x: 1)
@@ -141,7 +141,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testRemoveLabelWithExistingInternalName() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       func 1️⃣foo(x a: Int) {}
       foo(x: 1)
@@ -155,7 +155,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testRenameSubscript() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       struct Foo {
         1️⃣subscript(x x: Int) -> Int { x }
@@ -173,7 +173,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testRemoveExternalLabelFromSubscript() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       struct Foo {
         1️⃣subscript(x x: Int) -> Int { x }
@@ -191,7 +191,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testIntroduceExternalLabelFromSubscript() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       struct Foo {
         1️⃣subscript(x: Int) -> Int { x }
@@ -209,7 +209,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testIgnoreRenameSubscriptBaseName() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       struct Foo {
         1️⃣subscript(x: Int) -> Int { x }
@@ -227,7 +227,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testRenameInitializerLabels() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       struct Foo {
         1️⃣init(x: Int) {}
@@ -245,7 +245,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testIgnoreRenameOfInitBaseName() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       struct Foo {
         1️⃣init(x: Int) {}
@@ -263,7 +263,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testRenameCompoundFunctionName() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       func 1️⃣foo(a: Int) {}
       _ = foo(a:)
@@ -277,7 +277,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testRemoveLabelFromCompoundFunctionName() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       func 1️⃣foo(a: Int) {}
       _ = foo(a:)
@@ -291,7 +291,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testIntroduceLabelToCompoundFunctionName() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       func 1️⃣foo(_ a: Int) {}
       _ = foo(_:)
@@ -305,7 +305,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testRenameFromReference() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       func foo(_ a: Int) {}
       _ = 1️⃣foo(_:)
@@ -319,7 +319,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testRenameMultipleParameters() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       func 1️⃣foo(a: Int, b: Int) {}
       foo(a: 1, b: 1)
@@ -333,7 +333,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testDontRenameParametersOmittedFromNewName() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       func 1️⃣foo(a: Int, b: Int) {}
       foo(a: 1, b: 1)
@@ -347,7 +347,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testIgnoreAdditionalParametersInNewName() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       func 1️⃣foo(a: Int) {}
       foo(a: 1)
@@ -361,7 +361,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testOnlySpecifyBaseNameWhenRenamingFunction() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       func 1️⃣foo(a: Int) {}
       foo(a: 1)
@@ -375,7 +375,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testIgnoreParametersInNewNameWhenRenamingVariable() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       let 1️⃣foo = 1
       _ = foo
@@ -421,7 +421,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testSpacesInNewParameterNames() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       func 1️⃣foo(a: Int) {}
       foo(a: 1)
@@ -435,7 +435,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testRenameOperator() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       struct Foo {}
       func 1️⃣+(x: Foo, y: Foo) {}
@@ -451,7 +451,7 @@ final class RenameTests: XCTestCase {
   }
 
   func testRenameParameterToEmptyName() async throws {
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       func 1️⃣foo(x: Int) {}
       foo(x: 1)
@@ -468,7 +468,7 @@ final class RenameTests: XCTestCase {
     #if !canImport(Darwin)
     throw XCTSkip("#selector in test case doesn't compile without Objective-C runtime.")
     #endif
-    try await assertRename(
+    try await assertSingleFileRename(
       """
       import Foundation
       class Foo: NSObject {