-
Notifications
You must be signed in to change notification settings - Fork 332
Fallback to swiftmodule interface when index lookup fails #2407
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
Fallback to swiftmodule interface when index lookup fails #2407
Conversation
…efinition Currently, `indexBasedDefinition` relies heavily on IndexStoreDB. If a symbol belongs to a binary framework or a library that hasn't been indexed (but has module info provided by sourcekitd), the definition request fails or returns empty results. This change adds a fallback mechanism in `definitionLocations`. When no occurrences are found in the index, we check if `systemModule` information is available on the symbol. If so, we trigger `definitionInInterface` to generate the textual interface (via `editor.open.interface`) and return that location. This improves navigation for binary dependencies (XCFrameworks) and SDKs partially covered by the index.
ahoppen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have an example where this produces results where it previously doesn’t. A test case that wraps such an example would be ideal. If you have an example that requires a more complicated setup, I can help you reduce it to a test case.
Simplified Test Case ProposalI realized this can be tested in a much simpler way without relying on binary frameworks: by disabling background indexing. When background indexing is disabled (or when indexing has not run yet), IndexStoreDB does not contain cross-module symbol data, which causes the current implementation to fail. Proposed Scenario
Current BehaviorThe request fails (or returns no results) because Behavior with This PRThe request should succeed by falling back to generating the module interface for This demonstrates that the fallback path behaves as expected when the index is unavailable. Do you think this approach fits well with the existing test suite structure? |
|
I just tried replicating that setup you described and I don’t think we hit your fallback case that way. We need to generate the swiftmodule for My test casefunc testJumpToSwiftInterfaceIfIndexLookupFails() async throws {
let project = try await SwiftPMTestProject(
files: [
"Lib/Lib.swift": """
public func 1️⃣abc() {}
""",
"Executable/Executable.swift": """
import Lib
func test() {
2️⃣abc()
}
""",
],
manifest: """
let package = Package(
name: "MyLibrary",
targets: [
.target(name: "Lib"),
.executableTarget(name: "Executable", dependencies: ["Lib"])
]
)
"""
)
try await SwiftPMTestProject.build(at: project.scratchDirectory, extraArguments: ["--disable-index-store"])
let (uri, positions) = try project.openDocument("Executable.swift")
let definitions = try await project.testClient.send(
DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["2️⃣"])
)
XCTAssertEqual((definitions?.locations), [try project.location(from: "1️⃣", to: "1️⃣", in: "Lib.swift")])
} |
|
Good catch — a normal build still emits .swiftsourceinfo, so the fallback never triggers. |
|
Nice idea. Could you adjust the test case I proposed accordingly and add it to your PR? |
|
Sure — I’ve updated the test case accordingly and added it to the PR. |
ahoppen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just two nitpicks, otherwise looks good to me. Thank you!
| let project = try await SwiftPMTestProject( | ||
| files: [ | ||
| "Lib/Lib.swift": """ | ||
| public func 1️⃣abc() {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don’t need the 1 position marker anymore and 2 can then become 1 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Thanks for the review!
| ) | ||
| """ | ||
| ) | ||
| try await SwiftPMTestProject.build(at: project.scratchDirectory, extraArguments: ["--disable-index-store", "-Xswiftc", "-avoid-emit-module-source-info"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you run swift-format on your changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
ahoppen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Let’s get this in. Thanks for your contribution, @Kila2 ❤️
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
1 similar comment
|
@swift-ci Please test Windows |
Co-authored-by: Alex Hoppen <[email protected]>
Head branch was pushed to by a user without write access
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
Motivation
When working with third-party binary frameworks (XCFrameworks) or libraries that are not fully indexed by IndexStoreDB, "Jump to Definition" often fails silently or returns no results. However,
sourcekitdoften possesses knowledge that these symbols belong to a specific module (via thesystemModuleproperty inSymbolDetails), even if the specific USR location isn't in the index.Modification
Modified
definitionLocationsinSourceKitLSPServer.swift.When
indexBasedDefinitionis performed:The logic now checks if
symbol.systemModuleis present. If it is, it callsdefinitionInInterface(wrappingsource.request.editor.open.interface), allowing the editor to jump to the generated Swift Interface file (.swiftinterface/swiftmodule) instead of failing.Result
Users can now jump to definition for symbols in pre-compiled binary frameworks even without index data, provided the module is visible to the compiler.