Skip to content

Fix jump-to-definition to methods in swift interfaces that are in synthesized extensions #1185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,27 +23,24 @@ public struct OpenInterfaceRequest: TextDocumentRequest, Hashable {
public var moduleName: String

/// The module group name.
public var groupNames: [String]
public var groupName: String?

/// The symbol USR to search for in the generated module interface.
public var symbolUSR: String?

public init(textDocument: TextDocumentIdentifier, name: String, symbolUSR: String?) {
public init(textDocument: TextDocumentIdentifier, name: String, groupName: String?, symbolUSR: String?) {
self.textDocument = textDocument
self.symbolUSR = symbolUSR
// Stdlib Swift modules are all in the "Swift" module, but their symbols return a module name `Swift.***`.
let splitName = name.split(separator: ".")
self.moduleName = String(splitName[0])
self.groupNames = [String.SubSequence](splitName.dropFirst()).map(String.init)
self.moduleName = name
self.groupName = groupName
}

/// Name of interface module name with group names appended
public var name: String {
if groupNames.count > 0 {
return "\(self.moduleName).\(self.groupNames.joined(separator: "."))"
} else {
return self.moduleName
if let groupName {
return "\(self.moduleName).\(groupName.replacing("/", with: "."))"
}
return self.moduleName
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,18 @@ public struct SymbolInfoRequest: TextDocumentRequest, Hashable {
/// Detailed information about a symbol, such as the response to a `SymbolInfoRequest`
/// **(LSP Extension)**.
public struct SymbolDetails: ResponseType, Hashable {
public struct ModuleInfo: Codable, Hashable, Sendable {
/// The name of the module in which the symbol is defined.
public let moduleName: String

/// If the symbol is defined within a subgroup of a module, the name of the group. Otherwise `nil`.
public let groupName: String?

public init(moduleName: String, groupName: String? = nil) {
self.moduleName = moduleName
self.groupName = groupName
}
}

/// The name of the symbol, if any.
public var name: String?
Expand Down Expand Up @@ -87,6 +99,11 @@ public struct SymbolDetails: ResponseType, Hashable {
/// Optional because `clangd` does not return whether a symbol is dynamic.
public var isDynamic: Bool?

/// Whether this symbol is defined in the SDK or standard library.
///
/// This property only applies to Swift symbols.
public var isSystem: Bool?

/// If the symbol is dynamic, the USRs of the types that might be called.
///
/// This is relevant in the following cases
Expand All @@ -112,21 +129,31 @@ public struct SymbolDetails: ResponseType, Hashable {
/// `B` may be called dynamically.
public var receiverUsrs: [String]?

/// If the symbol is defined in a module that doesn't have source information associated with it, the name and group
/// and group name that defines this symbol.
///
/// This property only applies to Swift symbols.
public var systemModule: ModuleInfo?

public init(
name: String?,
containerName: String?,
usr: String?,
bestLocalDeclaration: Location?,
kind: SymbolKind?,
isDynamic: Bool?,
receiverUsrs: [String]?
isSystem: Bool?,
receiverUsrs: [String]?,
systemModule: ModuleInfo?
) {
self.name = name
self.containerName = containerName
self.usr = usr
self.bestLocalDeclaration = bestLocalDeclaration
self.kind = kind
self.isDynamic = isDynamic
self.isSystem = isSystem
self.receiverUsrs = receiverUsrs
self.systemModule = systemModule
}
}
28 changes: 15 additions & 13 deletions Sources/SourceKitLSP/SourceKitLSPServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1842,13 +1842,25 @@ extension SourceKitLSPServer {
if symbol.kind == .module, let name = symbol.name {
let interfaceLocation = try await self.definitionInInterface(
moduleName: name,
groupName: nil,
symbolUSR: nil,
originatorUri: uri,
languageService: languageService
)
return [interfaceLocation]
}

if symbol.isSystem ?? false, let systemModule = symbol.systemModule {
let location = try await self.definitionInInterface(
moduleName: systemModule.moduleName,
groupName: systemModule.groupName,
symbolUSR: symbol.usr,
originatorUri: uri,
languageService: languageService
)
return [location]
}

guard let index = await self.workspaceForDocument(uri: uri)?.index else {
if let bestLocalDeclaration = symbol.bestLocalDeclaration {
return [bestLocalDeclaration]
Expand Down Expand Up @@ -1891,19 +1903,7 @@ extension SourceKitLSPServer {
return [bestLocalDeclaration]
}

return try await occurrences.asyncCompactMap { occurrence in
if URL(fileURLWithPath: occurrence.location.path).pathExtension == "swiftinterface" {
// If the location is in `.swiftinterface` file, use moduleName to return textual interface.
return try await self.definitionInInterface(
moduleName: occurrence.location.moduleName,
symbolUSR: occurrence.symbol.usr,
originatorUri: uri,
languageService: languageService
)
}
return indexToLSPLocation(occurrence.location)
}
.sorted()
return occurrences.compactMap { indexToLSPLocation($0.location) }.sorted()
}

/// Returns the result of a `DefinitionRequest` by running a `SymbolInfoRequest`, inspecting
Expand Down Expand Up @@ -1968,13 +1968,15 @@ extension SourceKitLSPServer {
/// compiler arguments to generate the generated interface.
func definitionInInterface(
moduleName: String,
groupName: String?,
symbolUSR: String?,
originatorUri: DocumentURI,
languageService: LanguageService
) async throws -> Location {
let openInterface = OpenInterfaceRequest(
textDocument: TextDocumentIdentifier(originatorUri),
name: moduleName,
groupName: groupName,
symbolUSR: symbolUSR
)
guard let interfaceDetails = try await languageService.openInterface(openInterface) else {
Expand Down
16 changes: 14 additions & 2 deletions Sources/SourceKitLSP/Swift/CursorInfo.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ struct CursorInfo {
return nil
}

var location: Location? = nil
let location: Location?
if let filepath: String = dict[keys.filePath],
let line: Int = dict[keys.line],
let column: Int = dict[keys.column]
Expand All @@ -76,6 +76,16 @@ struct CursorInfo {
utf16index: column - 1
)
location = Location(uri: DocumentURI(URL(fileURLWithPath: filepath)), range: Range(position))
} else {
location = nil
}

let module: SymbolDetails.ModuleInfo?
if let moduleName: String = dict[keys.moduleName] {
let groupName: String? = dict[keys.groupName]
module = SymbolDetails.ModuleInfo(moduleName: moduleName, groupName: groupName)
} else {
module = nil
}

self.init(
Expand All @@ -86,7 +96,9 @@ struct CursorInfo {
bestLocalDeclaration: location,
kind: kind.asSymbolKind(sourcekitd.values),
isDynamic: dict[keys.isDynamic] ?? false,
receiverUsrs: dict[keys.receivers]?.compactMap { $0[keys.usr] as String? } ?? []
isSystem: dict[keys.isSystem] ?? false,
receiverUsrs: dict[keys.receivers]?.compactMap { $0[keys.usr] as String? } ?? [],
systemModule: module
),
annotatedDeclaration: dict[keys.annotatedDecl],
documentationXML: dict[keys.docFullAsXML],
Expand Down
2 changes: 1 addition & 1 deletion Sources/SourceKitLSP/Swift/OpenInterface.swift
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ extension SwiftLanguageService {
let skreq = sourcekitd.dictionary([
keys.request: requests.editorOpenInterface,
keys.moduleName: name,
keys.groupName: request.groupNames.isEmpty ? nil : request.groupNames as [SKDRequestValue],
keys.groupName: request.groupName,
keys.name: interfaceURI.pseudoPath,
keys.synthesizedExtension: 1,
keys.compilerArgs: await self.buildSettings(for: uri)?.compilerArgs as [SKDRequestValue]?,
Expand Down
117 changes: 78 additions & 39 deletions Tests/SourceKitLSPTests/SwiftInterfaceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ final class SwiftInterfaceTests: XCTestCase {
XCTFail("Unexpected response: \(resp)")
return
}
XCTAssertEqual(locations.count, 1)
let location = try XCTUnwrap(locations.first)
let location = try XCTUnwrap(locations.only)
XCTAssertTrue(location.uri.pseudoPath.hasSuffix("/Foundation.swiftinterface"))
let fileContents = try XCTUnwrap(location.uri.fileURL.flatMap({ try String(contentsOf: $0, encoding: .utf8) }))
// Sanity-check that the generated Swift Interface contains Swift code
Expand Down Expand Up @@ -87,6 +86,7 @@ final class SwiftInterfaceTests: XCTestCase {
let openInterface = OpenInterfaceRequest(
textDocument: TextDocumentIdentifier(mainUri),
name: "MyLibrary",
groupName: nil,
symbolUSR: nil
)
let interfaceDetails = try unwrap(await project.testClient.send(openInterface))
Expand All @@ -108,38 +108,6 @@ final class SwiftInterfaceTests: XCTestCase {
)
}

/// Used by testDefinitionInSystemModuleInterface
private func testSystemSwiftInterface(
uri: DocumentURI,
position: Position,
testClient: TestSourceKitLSPClient,
swiftInterfaceFile: String,
linePrefix: String,
line: UInt = #line
) async throws {
let definition = try await testClient.send(
DefinitionRequest(
textDocument: TextDocumentIdentifier(uri),
position: position
)
)
guard case .locations(let jump) = definition else {
XCTFail("Response is not locations", line: line)
return
}
let location = try XCTUnwrap(jump.first)
XCTAssertTrue(
location.uri.pseudoPath.hasSuffix(swiftInterfaceFile),
"Path was: '\(location.uri.pseudoPath)'",
line: line
)
// load contents of swiftinterface
let contents = try XCTUnwrap(location.uri.fileURL.flatMap({ try String(contentsOf: $0, encoding: .utf8) }))
let lineTable = LineTable(contents)
let destinationLine = lineTable[location.range.lowerBound.line]
XCTAssert(destinationLine.hasPrefix(linePrefix), "Full line was: '\(destinationLine)'", line: line)
}

func testDefinitionInSystemModuleInterface() async throws {
let project = try await IndexedSingleSwiftFileTestProject(
"""
Expand All @@ -158,23 +126,23 @@ final class SwiftInterfaceTests: XCTestCase {
)

// Test stdlib with one submodule
try await testSystemSwiftInterface(
try await assertSystemSwiftInterface(
uri: project.fileURI,
position: project.positions["1️⃣"],
testClient: project.testClient,
swiftInterfaceFile: "/Swift.String.swiftinterface",
linePrefix: "@frozen public struct String"
)
// Test stdlib with two submodules
try await testSystemSwiftInterface(
try await assertSystemSwiftInterface(
uri: project.fileURI,
position: project.positions["2️⃣"],
testClient: project.testClient,
swiftInterfaceFile: "/Swift.Math.Integers.swiftinterface",
linePrefix: "@frozen public struct Int"
)
// Test concurrency
try await testSystemSwiftInterface(
try await assertSystemSwiftInterface(
uri: project.fileURI,
position: project.positions["3️⃣"],
testClient: project.testClient,
Expand Down Expand Up @@ -224,8 +192,7 @@ final class SwiftInterfaceTests: XCTestCase {
XCTFail("Unexpected response: \(resp)")
return
}
XCTAssertEqual(locations.count, 1)
let location = try XCTUnwrap(locations.first)
let location = try XCTUnwrap(locations.only)
XCTAssertTrue(location.uri.pseudoPath.hasSuffix("/MyLibrary.swiftinterface"))
let fileContents = try XCTUnwrap(location.uri.fileURL.flatMap({ try String(contentsOf: $0, encoding: .utf8) }))
XCTAssertTrue(
Expand All @@ -242,4 +209,76 @@ final class SwiftInterfaceTests: XCTestCase {
"Generated interface did not contain expected text.\n\(fileContents)"
)
}

func testJumpToSynthesizedExtensionMethodInSystemModuleWithoutIndex() async throws {
let testClient = try await TestSourceKitLSPClient()
let uri = DocumentURI.for(.swift)

let positions = testClient.openDocument(
"""
func test(x: [String]) {
let rows = x.1️⃣filter { !$0.isEmpty }
}
""",
uri: uri
)

try await assertSystemSwiftInterface(
uri: uri,
position: positions["1️⃣"],
testClient: testClient,
swiftInterfaceFile: "/Swift.Collection.Array.swiftinterface",
linePrefix: "@inlinable public func filter(_ isIncluded: (Element) throws -> Bool) rethrows -> [Element]"
)
}

func testJumpToSynthesizedExtensionMethodInSystemModuleWithIndex() async throws {
let project = try await IndexedSingleSwiftFileTestProject(
"""
func test(x: [String]) {
let rows = x.1️⃣filter { !$0.isEmpty }
}
""",
indexSystemModules: true
)

try await assertSystemSwiftInterface(
uri: project.fileURI,
position: project.positions["1️⃣"],
testClient: project.testClient,
swiftInterfaceFile: "/Swift.Collection.Array.swiftinterface",
linePrefix: "@inlinable public func filter(_ isIncluded: (Element) throws -> Bool) rethrows -> [Element]"
)
}
}

private func assertSystemSwiftInterface(
uri: DocumentURI,
position: Position,
testClient: TestSourceKitLSPClient,
swiftInterfaceFile: String,
linePrefix: String,
line: UInt = #line
) async throws {
let definition = try await testClient.send(
DefinitionRequest(
textDocument: TextDocumentIdentifier(uri),
position: position
)
)
guard case .locations(let jump) = definition else {
XCTFail("Response is not locations", line: line)
return
}
let location = try XCTUnwrap(jump.only)
XCTAssertTrue(
location.uri.pseudoPath.hasSuffix(swiftInterfaceFile),
"Path was: '\(location.uri.pseudoPath)'",
line: line
)
// load contents of swiftinterface
let contents = try XCTUnwrap(location.uri.fileURL.flatMap({ try String(contentsOf: $0, encoding: .utf8) }))
let lineTable = LineTable(contents)
let destinationLine = lineTable[location.range.lowerBound.line].trimmingCharacters(in: .whitespaces)
XCTAssert(destinationLine.hasPrefix(linePrefix), "Full line was: '\(destinationLine)'", line: line)
}