Fix module definition jumping in Swift interface files #2233

aelam · 2025-08-11T11:51:19Z

Fix module definition jumping for submodules and Swift interface files

Problem Description

1. Submodule Import Navigation Issues

Foundation.NSArray: Clicking on NSArray in import Foundation.NSArray fails to jump to definition
Darwin Submodules: Clicking on sysdir in import Darwin.sysdir fails to jump to definition
Root Cause: SourceKit-LSP cannot properly handle module names containing dots (module.submodule)

2. Swift Interface File Navigation Problems

Generated Interface Navigation: Command+Click navigation functionality is broken within generated .swiftinterface files
Multiple Interface Caching: Repeatedly opening the same module interface generates multiple duplicate files
Language Service Mapping: Interface files cannot be properly mapped to corresponding language services

Solution

1. Enhanced Module Symbol Processing

File: Sources/SourceKitLSP/SourceKitLSPServer.swift

Module Name Parsing Logic

// Enhanced module symbol processing
if symbol.kind == .module {
  let moduleName: String
  let groupName: String?
  
  if let systemModule = symbol.systemModule {
    // Prefer using systemModule information
    moduleName = systemModule.moduleName
    groupName = systemModule.groupName
  } else if let name = symbol.name {
    // Handle module names containing dots like "Darwin.sysdir"
    if name.contains(".") {
      let components = name.split(separator: ".", maxSplits: 1)
      moduleName = String(components[0])
      groupName = components.count == 2 ? String(components[1]) : nil
    } else {
      moduleName = name
      groupName = nil
    }
  }
}

System Module Navigation

// System module navigation handling
if symbol.isSystem ?? false, let systemModule = symbol.systemModule {
  return try await self.definitionInInterface(
    moduleName: systemModule.moduleName,
    groupName: systemModule.groupName,
    symbolUSR: symbol.usr,
    originatorUri: uri,
    languageService: languageService
  )
}

2. Swift Interface File Support

Generated Interface Manager Enhancement

File: Sources/SourceKitLSP/Swift/GeneratedInterfaceManager.swift

// Improved interface file caching
private var interfaceDocuments: [InterfaceKey: GeneratedInterfaceDocument] = [:]

// Avoid duplicate generation of interface files for the same module
struct InterfaceKey: Hashable {
  let moduleName: String
  let groupName: String?
  let buildSettings: String
}

Language Service Mapping Fix

File: Sources/SourceKitLSP/Workspace.swift

// Fix language service mapping for interface files
func setDocumentService(_ service: DocumentService, for uri: DocumentURI) {
  let key = buildSettingsFile ?? uri
  documentService[key] = service
}

3. Interface File Recognition

File: Sources/SourceKitLSP/Swift/OpenInterface.swift

// Recognize .swiftinterface file extensions
private static let swiftInterfaceExtensions: Set<String> = ["swiftinterface"]

var isSwiftInterface: Bool {
  swiftInterfaceExtensions.contains(pathExtension.lowercased())
}

Test Cases

1. Foundation Submodules

✅ import Foundation.NSArray → Correctly jumps to NSArray interface
✅ import Foundation.NSURL → Correctly jumps to NSURL interface

2. Darwin Submodules

✅ import Darwin.sysdir → Correctly jumps to sysdir interface
✅ import Darwin.uuid → Correctly jumps to uuid interface
✅ import Darwin.POSIX → Correctly jumps to POSIX interface

3. Swift Interface Navigation

✅ Command+Click navigation works properly within generated .swiftinterface files
✅ Avoids duplicate generation of interface files for the same module
✅ Symbols in interface files can correctly jump to definitions

Technical Details

Key Improvements

Module Name Parsing: Properly handles module.submodule format import statements
System Module Support: Uses systemModule information for accurate module navigation
Interface File Caching: Avoids duplicate generation, improves performance
Language Service Mapping: Ensures interface files can be properly mapped to language services
File Type Recognition: Correctly identifies .swiftinterface file types

Backward Compatibility

✅ Maintains normal functionality of existing import statements
✅ Does not affect navigation behavior of non-system modules
✅ Compatible with existing interface generation mechanisms

Impact

Feature Improvements

Submodule Navigation: Full support for Foundation and Darwin submodule navigation
Interface File Navigation: Swift interface file navigation functionality restored
Performance: Reduced duplicate interface file generation, improved cache efficiency
Stability: Fixed interface file language service mapping issues

Related Issues

Fix Foundation.NSArray and other submodule import jumping issues
Fix Darwin.sysdir and other system submodule navigation
Resolve navigation failures within generated Swift interface files
Optimize interface file caching mechanism to avoid duplicate generation

Testing Recommendations

It is recommended to test in the following scenarios:

Various Foundation submodule import jumping
Darwin system submodule import jumping
Navigation within generated Swift interface files
Caching behavior when opening the same module interface multiple times
Ensure normal module imports are not affected

* fixed second swiftinterface page won't response * fixed tap the same name of swiftinterface, it would open another swiftinterface with same content

aelam · 2025-08-11T14:00:49Z

@swift-ci Please test

ahoppen

Thanks for the PR @aelam. The screen recording looks great. 🚀 I left a few comments mostly about the subtleties between generated interfaces and build settings inline.

Could you also add a test case for the new functionality? You should be able to follow the structure in SwiftInterfaceTests.testSemanticFunctionalityInGeneratedInterface.

ahoppen · 2025-08-11T19:12:03Z

Sources/SourceKitLSP/Swift/GeneratedInterfaceManager.swift

-      guard let cachedIndex = openInterfaces.firstIndex(where: { $0.url == document }) else {
+      // Cache by module name and group name, not the full document data
+      // This allows reuse across different buildSettingsFrom URIs
+      guard let cachedIndex = openInterfaces.firstIndex(where: { 


I don’t think this is correct. Depending on the build settings used to retrieve the generated interface, the interface might look differently. I can’t come up with a great real-world example right now where the contents differ but a canonical example would be that we can’t load the generated interface if we are missing an include path to one of the module’s dependencies but we can generate the interface if all build settings are present.

But if we continue passing along the same buildSettingsFrom file, I don’t think we need this change. Or am I missing something?

you are right

but about same buildSettingsFrom file please check here #2233 (comment)

Sources/LanguageServerProtocolExtensions/Language+Inference.swift

ahoppen · 2025-08-11T19:17:08Z

Sources/SourceKitLSP/Swift/OpenInterface.swift

@@ -21,10 +21,18 @@ extension SwiftLanguageService {
    groupName: String?,
    symbolUSR symbol: String?
  ) async throws -> GeneratedInterfaceDetails? {
+    // Generate a deterministic document name based on module name and group name
+    // This ensures we reuse the same interface for the same module/group combination
+    let documentName = if let groupName {


Similar to my comment above, generated interfaces might look different depending on the build settings that were used to generate them, so module name and group name don’t uniquely identify its contents. That’s why we used the UUID-based filename below.

ahoppen · 2025-08-11T19:17:43Z

Sources/SourceKitLSP/Swift/ReferenceDocumentURL.swift

@@ -165,13 +165,14 @@ extension DocumentURI {
      return referenceDocument.buildSettingsFile
    }
    return self
+  }  /// Convert sourcekit-lsp:// URIs to actual file system paths when possible.


Missing newlines before ///?

ahoppen · 2025-08-11T19:20:29Z

Sources/SourceKitLSP/Swift/ReferenceDocumentURL.swift

+  }  /// Convert sourcekit-lsp:// URIs to actual file system paths when possible.
+  /// For generated Swift interfaces, this returns the path where the interface file
+  /// would be stored in the GeneratedInterfaces directory.
+  var actualFileSystemPath: DocumentURI {


I don’t think this is used anywhere, or am I missing something?

ahoppen · 2025-08-11T19:22:15Z

Sources/SourceKitLSP/SourceKitLSPServer.swift

@@ -553,7 +553,7 @@ package actor SourceKitLSPServer {
    }

    let toolchain = await workspace.buildServerManager.toolchain(
-      for: uri,
+      for: uri.buildSettingsFile,


Is this change necessary? I just checked and it looks like toolchain(for:in:language:) doesn’t even use uri right now.

Sources/SourceKitLSP/SourceKitLSPServer.swift

ahoppen · 2025-08-11T19:24:32Z

Sources/SourceKitLSP/SourceKitLSPServer.swift

+    // Check if we're already in the target interface with the same module/group/symbol
+    if let referenceDoc = try? ReferenceDocumentURL(from: originatorUri),
+       case .generatedInterface(let interfaceData) = referenceDoc,
+       interfaceData.moduleName == moduleName && interfaceData.groupName == groupName {


Similarly to my other comments about the build settings, we need to check here that we are using build settings from the same original file. But I think as long as we pass along the same build settings file, this shouldn’t cause any issues.

@ahoppen Thanks for the comment

without this change, here would happen

if I open defination of Foundation in Foundation.swiftInterace, it will open another one, repeat, it will open another new one
even if it's acceptable but it's quite slow (please check the video)

Cause:

originalUri is sourcekit-lsp://xxx?buildSettingsFrom=A A is sourcekit-lsp://xxx?buildSettingsFrom=B B is sourcekit-lsp://xxx?buildSettingsFrom=C C is file://

this makes the logic quite complicated

we have to recursively find the buildSettingsFrom

originalUri are different for same swiftinterface

I think we could assume the interface is the same if the interface was originally opened from a same buildSettingsFrom
because interface opened from A, B should always be consistent?

then we could simplify the originalUri as

originalUri is sourcekit-lsp://xxx?buildSettingsFrom=C

the benefits are

we don't need to resursively search

caching/creating of swiftinterface could be simplified and easy to hit the cache

much faster

the code change here is not needed any more.

Untitled.mov

Yeah, IMO when doing a jump to definition from within an interface, the buildSettingsFrom should be the original source file rather than the interface you're currently in. IIUC this is what you're saying in:

then we could simplify the originalUri as
originalUri is sourcekit-lsp://xxx?buildSettingsFrom=C
? If so, then 👍

Sources/SourceKitLSP/Workspace.swift

ahoppen · 2025-08-11T19:43:39Z

Sources/SourceKitLSP/SourceKitLSPServer.swift

+
+      // If we have a specific symbol USR, try to find its position in the current interface
+      if let symbolUSR = symbolUSR,
+         let swiftLanguageService = languageService as? SwiftLanguageService {


I’m currently trying to unravel the dependency of the SourceKitLSP module on SwiftLanguageService in #2234, so getting the SwiftLanguageService won’t work in the future.

What are the challenges of always going through openGeneratedInterface? I can see the following but I might be missing some:

Every time openGeneratedInterface is called, we get a new sourcekitdDocumentName. We could fix that by instead of appending the group name and a hash of document (or something of that sort).

If the issue is that we always write a new file if the client doesn’t support reference documents, we could just not write out the file if the contents haven’t changed

adding a group might be a good idea

i imagine there could be different versions of swiftinterface which are used by different modules/files

i think the challenge is how to determine it in the beginning instead of a uuid
then the cache logic could be simplified

aelam · 2025-08-12T07:58:55Z

Sources/SourceKitLSP/Swift/ReferenceDocumentURL.swift

+    case .generatedInterface(let data):
+      if let referenceDocumentURL = try? ReferenceDocumentURL(from: data.buildSettingsFrom) {
+        return referenceDocumentURL.buildSettingsFile
+      }
+      return data.buildSettingsFrom
    }
  }

  var primaryFile: DocumentURI? {
    switch self {
    case .macroExpansion(let data): return data.primaryFile
-    case .generatedInterface(let data): return data.buildSettingsFrom.primaryFile
+    case .generatedInterface(let data):
+      if let referenceDocumentURL = try? ReferenceDocumentURL(from: data.buildSettingsFrom) {
+        return referenceDocumentURL.primaryFile
+      }
+    return data.buildSettingsFrom.primaryFile


check https://github.com/swiftlang/sourcekit-lsp/pull/2233/files#r2269006407

aelam · 2025-08-12T11:47:10Z

Sources/SourceKitLSP/Swift/GeneratedInterfaceDocumentURLData.swift

+    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
+    }


@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

ahoppen

Thanks for your continued work on this PR and adding the tests, @aelam. I left some comments, mostly on the tests. Also, addressing the comments in SourceKitLSPServer.swift would be important to make the next review round meaningful.

ahoppen · 2025-08-13T06:49:43Z

Sources/SourceKitLSP/Swift/GeneratedInterfaceDocumentURLData.swift

-    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
+    }


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

ahoppen · 2025-08-13T06:50:42Z

Sources/SKTestSupport/Utils.swift

@@ -32,7 +32,7 @@ extension Language {
    case "cpp": self = .cpp
    case "m": self = .objective_c
    case "mm": self = .objective_cpp
-    case "swift": self = .swift
+    case "swift", "swiftinterface": self = .swift


Similar to my other comment about the .swiftinterface extension, I don’t think we need this here, do we? This only applies to tests and I don’t think we open any .swiftinterface files in tests.

I deleted but please check the tests about swiftinterface

Sources/SourceKitLSP/Swift/ReferenceDocumentURL.swift