Merge pull request #2233 from aelam/fix/group_of_module_definition

Fix module definition jumping in Swift interface files

Author: ahoppen

Sources/SourceKitLSP/GeneratedInterfaceDocumentURLData.swift

@@ -67,7 +67,7 @@ package struct GeneratedInterfaceDocumentURLData: Hashable, ReferenceURLData {
     self.moduleName = moduleName
     self.groupName = groupName
     self.sourcekitdDocumentName = sourcekitdDocumentName
-    self.buildSettingsFrom = primaryFile
+    self.buildSettingsFrom = primaryFile.buildSettingsFile
   }
 
   init(queryItems: [URLQueryItem]) throws {

Sources/SourceKitLSP/ReferenceDocumentURL.swift

@@ -170,8 +170,4 @@ extension DocumentURI {
 
 package struct ReferenceDocumentURLError: Error, CustomStringConvertible {
   package var description: String
-
-  init(description: String) {
-    self.description = description
-  }
 }

Sources/SourceKitLSP/SourceKitLSPServer.swift

@@ -1923,10 +1923,24 @@ extension SourceKitLSPServer {
     languageService: LanguageService
   ) async throws -> [Location] {
     // If this symbol is a module then generate a textual interface
-    if symbol.kind == .module, let name = symbol.name {
+    if symbol.kind == .module {
+      // For module symbols, prefer using systemModule information if available
+      let moduleName: String
+      let groupName: String?
+
+      if let systemModule = symbol.systemModule {
+        moduleName = systemModule.moduleName
+        groupName = systemModule.groupName
+      } else if let name = symbol.name {
+        moduleName = name
+        groupName = nil
+      } else {
+        return []
+      }
+
       let interfaceLocation = try await self.definitionInInterface(
-        moduleName: name,
-        groupName: nil,
+        moduleName: moduleName,
+        groupName: groupName,
         symbolUSR: nil,
         originatorUri: uri,
         languageService: languageService
@@ -2124,9 +2138,12 @@ extension SourceKitLSPServer {
     originatorUri: DocumentURI,
     languageService: LanguageService
   ) async throws -> Location {
+    // Let openGeneratedInterface handle all the logic, including checking if we're already in the right interface
+    let documentForBuildSettings = originatorUri.buildSettingsFile
+
     guard
       let interfaceDetails = try await languageService.openGeneratedInterface(
-        document: originatorUri,
+        document: documentForBuildSettings,
         moduleName: moduleName,
         groupName: groupName,
         symbolUSR: symbolUSR

Sources/SwiftLanguageService/OpenInterface.swift

@@ -22,10 +22,15 @@ extension SwiftLanguageService {
     groupName: String?,
     symbolUSR symbol: String?
   ) async throws -> GeneratedInterfaceDetails? {
+    // Include build settings context to distinguish different versions/configurations
+    let buildSettingsFileHash = "\(abs(document.buildSettingsFile.stringValue.hashValue))"
+    let sourcekitdDocumentName = [moduleName, groupName, buildSettingsFileHash].compactMap(\.self)
+      .joined(separator: ".")
+
     let urlData = GeneratedInterfaceDocumentURLData(
       moduleName: moduleName,
       groupName: groupName,
-      sourcekitdDocumentName: "\(moduleName)-\(UUID())",
+      sourcekitdDocumentName: sourcekitdDocumentName,
       primaryFile: document
     )
     let position: Position? =
@@ -40,7 +45,13 @@ extension SwiftLanguageService {
     if self.capabilityRegistry.clientHasExperimentalCapability(GetReferenceDocumentRequest.method) {
       return GeneratedInterfaceDetails(uri: try urlData.uri, position: position)
     }
-    let interfaceFilePath = self.generatedInterfacesPath.appendingPathComponent(urlData.displayName)
+    let interfaceFilePath = self.generatedInterfacesPath
+      .appendingPathComponent(buildSettingsFileHash)
+      .appendingPathComponent(urlData.displayName)
+    try FileManager.default.createDirectory(
+      at: interfaceFilePath.deletingLastPathComponent(),
+      withIntermediateDirectories: true
+    )
     try await generatedInterfaceManager.snapshot(of: urlData).text.write(
       to: interfaceFilePath,
       atomically: true,

Tests/SourceKitLSPTests/SwiftInterfaceTests.swift

@@ -319,6 +319,65 @@ final class SwiftInterfaceTests: XCTestCase {
     )
     XCTAssertEqual(diagnostics.fullReport?.items, [])
   }
+
+  func testFoundationImportNavigation() async throws {
+    let testClient = try await TestSourceKitLSPClient(
+      capabilities: ClientCapabilities(experimental: [
+        GetReferenceDocumentRequest.method: .dictionary(["supported": .bool(true)])
+      ])
+    )
+    let uri = DocumentURI(for: .swift)
+
+    let positions = testClient.openDocument(
+      """
+      import 1️⃣Foundation
+      """,
+      uri: uri,
+      language: .swift
+    )
+
+    // Test navigation to Foundation module
+    let foundationDefinition = try await testClient.send(
+      DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
+    )
+    let foundationLocation = try XCTUnwrap(foundationDefinition?.locations?.only)
+    XCTAssertEqual(foundationLocation.uri.scheme, "sourcekit-lsp")
+    assertContains(foundationLocation.uri.pseudoPath, "Foundation.swiftinterface")
+  }
+
+  func testFoundationSubmoduleNavigation() async throws {
+    try SkipUnless.platformIsDarwin("Non-Darwin platforms don't have Foundation submodules")
+
+    let testClient = try await TestSourceKitLSPClient(
+      capabilities: ClientCapabilities(experimental: [
+        GetReferenceDocumentRequest.method: .dictionary(["supported": .bool(true)])
+      ])
+    )
+    let uri = DocumentURI(for: .swift)
+
+    let positions = testClient.openDocument(
+      """
+      import 1️⃣Foundation.2️⃣NSAffineTransform
+      """,
+      uri: uri
+    )
+
+    let foundationDefinition = try await testClient.send(
+      DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
+    )
+    let foundationLocation = try XCTUnwrap(foundationDefinition?.locations?.only)
+    XCTAssertEqual(foundationLocation.uri.scheme, "sourcekit-lsp")
+    assertContains(foundationLocation.uri.pseudoPath, "Foundation.swiftinterface")
+
+    // Test navigation to NSAffineTransform
+    let transformDefinition = try await testClient.send(
+      DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["2️⃣"])
+    )
+    let transformLocation = try XCTUnwrap(transformDefinition?.locations?.only)
+    // Verify we can identify this as a swiftinterface file
+    XCTAssertEqual(transformLocation.uri.scheme, "sourcekit-lsp")
+    assertContains(transformLocation.uri.pseudoPath, "Foundation.NSAffineTransform.swiftinterface")
+  }
 }
 
 private func assertSystemSwiftInterface(