Skip to content

Fix module definition jumping in Swift interface files #2233

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

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 3 additions & 3 deletions Sources/SourceKitLSP/SourceKitLSPServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2086,10 +2086,10 @@ extension SourceKitLSPServer {
return Location(uri: originatorUri, range: Range(Position(line: 0, utf16index: 0)))
}
}

// If the originator URI is already a generated interface, use its primary file for build settings
let documentForBuildSettings = originatorUri.primaryFile ?? originatorUri
let documentForBuildSettings = originatorUri.buildSettingsFile

guard
let interfaceDetails = try await languageService.openGeneratedInterface(
document: documentForBuildSettings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,14 @@ package struct GeneratedInterfaceDocumentURLData: Hashable, ReferenceURLData {
self.moduleName = moduleName
self.groupName = groupName
self.sourcekitdDocumentName = sourcekitdDocumentName
self.buildSettingsFrom = primaryFile
if let referenceDocumentURL = try? ReferenceDocumentURL(from: primaryFile),
let fileURL = referenceDocumentURL.buildSettingsFile.fileURL {
self.buildSettingsFrom = DocumentURI(fileURL)
} else if let fileURL = primaryFile.fileURL {
self.buildSettingsFrom = DocumentURI(fileURL)
} else {
self.buildSettingsFrom = primaryFile
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahoppen

make sure buildSettingsFrom can be only an actual file otherwise

it could be
sourcekit-lsp://xxx?buildSettingsFrom=A
A=sourcekit-lsp://xxx?buildSettingsFrom=B

NOTE: this happens if we enable navigation inside swiftinterface in this PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have an accessor on DocumentURI that should do exactly the same, so you should be able to simplify this to

Suggested change
self.buildSettingsFrom = primaryFile
if let referenceDocumentURL = try? ReferenceDocumentURL(from: primaryFile),
let fileURL = referenceDocumentURL.buildSettingsFile.fileURL {
self.buildSettingsFrom = DocumentURI(fileURL)
} else if let fileURL = primaryFile.fileURL {
self.buildSettingsFrom = DocumentURI(fileURL)
} else {
self.buildSettingsFrom = primaryFile
}
self.buildSettingsFrom = primaryFile.buildSettingsFile

}

init(queryItems: [URLQueryItem]) throws {
Expand Down
13 changes: 8 additions & 5 deletions Sources/SourceKitLSP/Swift/ReferenceDocumentURL.swift
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,9 @@ package enum ReferenceDocumentURL {
switch self {
case .macroExpansion(let data): return data.primaryFile
case .generatedInterface(let data):
if let referenceDocumentURL = try? ReferenceDocumentURL(from: data.buildSettingsFrom) {
return referenceDocumentURL.buildSettingsFile
if let referenceDocumentURL = try? ReferenceDocumentURL(from: data.buildSettingsFrom),
let fileURL = referenceDocumentURL.buildSettingsFile.fileURL {
return DocumentURI(fileURL)
}
return data.buildSettingsFrom
}
Expand All @@ -134,10 +135,12 @@ package enum ReferenceDocumentURL {
switch self {
case .macroExpansion(let data): return data.primaryFile
case .generatedInterface(let data):
if let referenceDocumentURL = try? ReferenceDocumentURL(from: data.buildSettingsFrom) {
return referenceDocumentURL.primaryFile
if let referenceDocumentURL = try? ReferenceDocumentURL(from: data.buildSettingsFrom),
let primaryFile = referenceDocumentURL.primaryFile,
let fileURL = primaryFile.fileURL {
return DocumentURI(fileURL)
}
return data.buildSettingsFrom.primaryFile
return data.buildSettingsFrom.primaryFile
}
}
}
Expand Down