diff --git a/Sources/SKTestSupport/SwiftPMTestProject.swift b/Sources/SKTestSupport/SwiftPMTestProject.swift index 2c186b4ee..18de4f72f 100644 --- a/Sources/SKTestSupport/SwiftPMTestProject.swift +++ b/Sources/SKTestSupport/SwiftPMTestProject.swift @@ -177,7 +177,7 @@ public class SwiftPMTestProject: MultiFileTestProject { if !manifest.contains("swift-tools-version") { // Tests specify a shorthand package manifest that doesn't contain the tools version boilerplate. manifest = """ - // swift-tools-version: 5.7 + // swift-tools-version: 5.10 import PackageDescription diff --git a/Sources/SemanticIndex/CheckedIndex.swift b/Sources/SemanticIndex/CheckedIndex.swift index 55cf4d460..76d115eda 100644 --- a/Sources/SemanticIndex/CheckedIndex.swift +++ b/Sources/SemanticIndex/CheckedIndex.swift @@ -176,10 +176,6 @@ public struct UncheckedIndex: Sendable { return CheckedIndex(index: underlyingIndexStoreDB, checkLevel: checkLevel) } - public func symbolProvider(for sourceFilePath: String) -> SymbolProviderKind? { - return underlyingIndexStoreDB.symbolProvider(for: sourceFilePath) - } - /// Wait for IndexStoreDB to be updated based on new unit files written to disk. public func pollForUnitChangesAndWait() { self.underlyingIndexStoreDB.pollForUnitChangesAndWait() diff --git a/Sources/SourceKitLSP/Rename.swift b/Sources/SourceKitLSP/Rename.swift index f87741630..91df9c773 100644 --- a/Sources/SourceKitLSP/Rename.swift +++ b/Sources/SourceKitLSP/Rename.swift @@ -500,7 +500,7 @@ extension SourceKitLSPServer { ) async -> (swiftLanguageService: SwiftLanguageService, snapshot: DocumentSnapshot, location: SymbolLocation)? { var reference: SymbolOccurrence? = nil index.forEachSymbolOccurrence(byUSR: usr, roles: renameRoles) { - if index.unchecked.symbolProvider(for: $0.location.path) == .swift { + if $0.symbolProvider == .swift { reference = $0 // We have found a reference from Swift. Stop iteration. return false @@ -631,7 +631,7 @@ extension SourceKitLSPServer { // If we terminate early by returning `false` from the closure, `forEachSymbolOccurrence` returns `true`, // indicating that we have found a reference from clang. let hasReferenceFromClang = !index.forEachSymbolOccurrence(byUSR: usr, roles: renameRoles) { - return index.unchecked.symbolProvider(for: $0.location.path) != .clang + return $0.symbolProvider != .clang } let clangName: String? if hasReferenceFromClang { @@ -730,32 +730,7 @@ extension SourceKitLSPServer { // If we have a USR + old name, perform an index lookup to find workspace-wide symbols to rename. // First, group all occurrences of that USR by the files they occur in. - var locationsByFile: [DocumentURI: [RenameLocation]] = [:] - - actor LanguageServerTypesCache { - let index: UncheckedIndex - var languageServerTypesCache: [DocumentURI: LanguageServerType?] = [:] - - init(index: UncheckedIndex) { - self.index = index - } - - func languageServerType(for uri: DocumentURI) -> LanguageServerType? { - if let cachedValue = languageServerTypesCache[uri] { - return cachedValue - } - let serverType: LanguageServerType? = - if let fileURL = uri.fileURL { - LanguageServerType(symbolProvider: index.symbolProvider(for: fileURL.path)) - } else { - nil - } - languageServerTypesCache[uri] = serverType - return serverType - } - } - - let languageServerTypesCache = LanguageServerTypesCache(index: index.unchecked) + var locationsByFile: [DocumentURI: (renameLocations: [RenameLocation], symbolProvider: SymbolProviderKind)] = [:] let usrsToRename = overridingAndOverriddenUsrs(of: usr, index: index) let occurrencesToRename = usrsToRename.flatMap { index.occurrences(ofUSR: $0, roles: renameRoles) } @@ -770,19 +745,16 @@ extension SourceKitLSPServer { // perform an indexed rename for it. continue } - switch await languageServerTypesCache.languageServerType(for: uri) { + switch occurrence.symbolProvider { case .swift: // sourcekitd only produces AST-based results for the direct calls to this USR. This is because the Swift // AST only has upwards references to superclasses and overridden methods, not the other way round. It is // thus not possible to (easily) compute an up-down closure like described in `overridingAndOverriddenUsrs`. // We thus need to perform an indexed rename for other, related USRs. break - case .clangd: + case .clang: // clangd produces AST-based results for the entire class hierarchy, so nothing to do. continue - case nil: - // Unknown symbol provider - continue } } @@ -791,25 +763,39 @@ extension SourceKitLSPServer { utf8Column: occurrence.location.utf8Column, usage: RenameLocation.Usage(roles: occurrence.roles) ) - locationsByFile[uri, default: []].append(renameLocation) + if let existingLocations = locationsByFile[uri] { + if existingLocations.symbolProvider != occurrence.symbolProvider { + logger.fault( + """ + Found mismatching symbol providers for \(uri.forLogging): \ + \(String(describing: existingLocations.symbolProvider), privacy: .public) vs \ + \(String(describing: occurrence.symbolProvider), privacy: .public) + """ + ) + } + locationsByFile[uri] = (existingLocations.renameLocations + [renameLocation], occurrence.symbolProvider) + } else { + locationsByFile[uri] = ([renameLocation], occurrence.symbolProvider) + } } // Now, call `editsToRename(locations:in:oldName:newName:)` on the language service to convert these ranges into // edits. let urisAndEdits = await locationsByFile - .concurrentMap { (uri: DocumentURI, renameLocations: [RenameLocation]) -> (DocumentURI, [TextEdit])? in + .concurrentMap { + ( + uri: DocumentURI, + value: (renameLocations: [RenameLocation], symbolProvider: SymbolProviderKind) + ) -> (DocumentURI, [TextEdit])? in let language: Language - switch await languageServerTypesCache.languageServerType(for: uri) { - case .clangd: + switch value.symbolProvider { + case .clang: // Technically, we still don't know the language of the source file but defaulting to C is sufficient to // ensure we get the clang toolchain language server, which is all we care about. language = .c case .swift: language = .swift - case nil: - logger.error("Failed to determine symbol provider for \(uri.forLogging)") - return nil } // Create a document snapshot to operate on. If the document is open, load it from the document manager, // otherwise conjure one from the file on disk. We need the file in memory to perform UTF-8 to UTF-16 column @@ -825,13 +811,13 @@ extension SourceKitLSPServer { var edits: [TextEdit] = await orLog("Getting edits for rename location") { return try await languageService.editsToRename( - locations: renameLocations, + locations: value.renameLocations, in: snapshot, oldName: oldName, newName: newName ) } ?? [] - for location in renameLocations where location.usage == .definition { + for location in value.renameLocations where location.usage == .definition { edits += await languageService.editsToRenameParametersInFunctionBody( snapshot: snapshot, renameLocation: location, diff --git a/Tests/SourceKitLSPTests/CrossLanguageRenameTests.swift b/Tests/SourceKitLSPTests/CrossLanguageRenameTests.swift index e49225f32..ef6279ecc 100644 --- a/Tests/SourceKitLSPTests/CrossLanguageRenameTests.swift +++ b/Tests/SourceKitLSPTests/CrossLanguageRenameTests.swift @@ -23,6 +23,16 @@ private let libAlibBPackageManifest = """ ) """ +private let libAlibBCxxInteropPackageManifest = """ + let package = Package( + name: "MyLibrary", + targets: [ + .target(name: "LibA"), + .target(name: "LibB", dependencies: ["LibA"], swiftSettings: [.interoperabilityMode(.Cxx)]), + ] + ) + """ + final class CrossLanguageRenameTests: XCTestCase { func testZeroArgCFunction() async throws { try await SkipUnless.clangdSupportsIndexBasedRename() @@ -733,4 +743,140 @@ final class CrossLanguageRenameTests: XCTestCase { manifest: libAlibBPackageManifest ) } + + func testRenameCxxClassExposedToSwift() async throws { + try await SkipUnless.clangdSupportsIndexBasedRename() + try await assertMultiFileRename( + files: [ + "LibA/include/LibA.h": """ + struct 1️⃣Foo {}; + """, + "LibA/LibA.cpp": "", + "LibB/LibB.swift": """ + import LibA + + func test(foo: 2️⃣Foo) {} + """, + ], + headerFileLanguage: .cpp, + newName: "Bar", + expectedPrepareRenamePlaceholder: "Foo", + expected: [ + "LibA/include/LibA.h": """ + struct Bar {}; + """, + "LibA/LibA.cpp": "", + "LibB/LibB.swift": """ + import LibA + + func test(foo: Bar) {} + """, + ], + manifest: libAlibBCxxInteropPackageManifest + ) + } + + func testRenameCxxMethodExposedToSwift() async throws { + try await SkipUnless.clangdSupportsIndexBasedRename() + try await assertMultiFileRename( + files: [ + "LibA/include/LibA.h": """ + struct Foo { + int 1️⃣doStuff() const; + }; + """, + "LibA/LibA.cpp": """ + #include "LibA.h" + + int Foo::2️⃣doStuff() const { + return 42; + } + """, + "LibB/LibB.swift": """ + import LibA + + func test(foo: Foo) { + foo.3️⃣doStuff() + } + """, + ], + headerFileLanguage: .cpp, + newName: "doNewStuff", + expectedPrepareRenamePlaceholder: "doStuff", + expected: [ + "LibA/include/LibA.h": """ + struct Foo { + int doNewStuff() const; + }; + """, + "LibA/LibA.cpp": """ + #include "LibA.h" + + int Foo::doNewStuff() const { + return 42; + } + """, + "LibB/LibB.swift": """ + import LibA + + func test(foo: Foo) { + foo.doNewStuff() + } + """, + ], + manifest: libAlibBCxxInteropPackageManifest + ) + } + + func testRenameSwiftMethodExposedToSwift() async throws { + try await SkipUnless.clangdSupportsIndexBasedRename() + try await assertMultiFileRename( + files: [ + "LibA/include/LibA.h": """ + struct Foo { + int 1️⃣doStuff() const; + }; + """, + "LibA/LibA.cpp": """ + #include "LibA.h" + + int Foo::2️⃣doStuff() const { + return 42; + } + """, + "LibB/LibB.swift": """ + import LibA + + func test(foo: Foo) { + foo.3️⃣doStuff() + } + """, + ], + headerFileLanguage: .cpp, + newName: "doNewStuff", + expectedPrepareRenamePlaceholder: "doStuff", + expected: [ + "LibA/include/LibA.h": """ + struct Foo { + int doNewStuff() const; + }; + """, + "LibA/LibA.cpp": """ + #include "LibA.h" + + int Foo::doNewStuff() const { + return 42; + } + """, + "LibB/LibB.swift": """ + import LibA + + func test(foo: Foo) { + foo.doNewStuff() + } + """, + ], + manifest: libAlibBCxxInteropPackageManifest + ) + } }