Merge pull request #1892 from ahoppen/6.1/merge-main-2024-12-17

Author: ahoppen

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -54,14 +54,21 @@ package struct SourceFileInfo: Sendable {
   /// from non-test targets or files that don't actually contain any tests.
   package var mayContainTests: Bool
 
+  /// Source files returned here fall into two categories:
+  ///  - Buildable source files are files that can be built by the build system and that make sense to background index
+  ///  - Non-buildable source files include eg. the SwiftPM package manifest or header files. We have sufficient
+  ///    compiler arguments for these files to provide semantic editor functionality but we can't build them.
+  package var isBuildable: Bool
+
   fileprivate func merging(_ other: SourceFileInfo?) -> SourceFileInfo {
     guard let other else {
       return self
     }
     return SourceFileInfo(
       targets: targets.union(other.targets),
       isPartOfRootProject: other.isPartOfRootProject || isPartOfRootProject,
-      mayContainTests: other.mayContainTests || mayContainTests
+      mayContainTests: other.mayContainTests || mayContainTests,
+      isBuildable: other.isBuildable || isBuildable
     )
   }
 }
@@ -327,11 +334,9 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
 
   private var cachedTargetSources = RequestCache<BuildTargetSourcesRequest>()
 
-  /// The parameters with which `SourceFilesAndDirectories` can be cached in `cachedSourceFilesAndDirectories`.
-  private struct SourceFilesAndDirectoriesKey: Hashable {
-    let includeNonBuildableFiles: Bool
-    let sourcesItems: [SourcesItem]
-  }
+  /// `SourceFilesAndDirectories` is a global property that only gets reset when the build targets change and thus
+  /// has no real key.
+  private struct SourceFilesAndDirectoriesKey: Hashable {}
 
   private struct SourceFilesAndDirectories {
     /// The source files in the workspace, ie. all `SourceItem`s that have `kind == .file`.
@@ -678,7 +683,7 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
   package func targets(for document: DocumentURI) async -> Set<BuildTargetIdentifier> {
     return await orLog("Getting targets for source file") {
       var result: Set<BuildTargetIdentifier> = []
-      let filesAndDirectories = try await sourceFilesAndDirectories(includeNonBuildableFiles: true)
+      let filesAndDirectories = try await sourceFilesAndDirectories()
       if let targets = filesAndDirectories.files[document]?.targets {
         result.formUnion(targets)
       }
@@ -1033,50 +1038,44 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
     return response.items
   }
 
-  /// Returns all source files in the project that can be built.
+  /// Returns all source files in the project.
   ///
   /// - SeeAlso: Comment in `sourceFilesAndDirectories` for a definition of what `buildable` means.
-  package func buildableSourceFiles() async throws -> [DocumentURI: SourceFileInfo] {
-    return try await sourceFilesAndDirectories(includeNonBuildableFiles: false).files
+  package func sourceFiles(includeNonBuildableFiles: Bool) async throws -> [DocumentURI: SourceFileInfo] {
+    let files = try await sourceFilesAndDirectories().files
+    if includeNonBuildableFiles {
+      return files
+    } else {
+      return files.filter(\.value.isBuildable)
+    }
   }
 
   /// Get all files and directories that are known to the build system, ie. that are returned by a `buildTarget/sources`
   /// request for any target in the project.
   ///
-  /// Source files returned here fall into two categories:
-  ///  - Buildable source files are files that can be built by the build system and that make sense to background index
-  ///  - Non-buildable source files include eg. the SwiftPM package manifest or header files. We have sufficient
-  ///    compiler arguments for these files to provide semantic editor functionality but we can't build them.
-  ///
-  /// `includeNonBuildableFiles` determines whether non-buildable files should be included.
-  private func sourceFilesAndDirectories(includeNonBuildableFiles: Bool) async throws -> SourceFilesAndDirectories {
-    let targets = try await self.buildTargets()
-    let sourcesItems = try await self.sourceFiles(in: Set(targets.keys))
-
-    let key = SourceFilesAndDirectoriesKey(
-      includeNonBuildableFiles: includeNonBuildableFiles,
-      sourcesItems: sourcesItems
-    )
+  /// - Important: This method returns both buildable and non-buildable source files. Callers need to check
+  /// `SourceFileInfo.isBuildable` if they are only interested in buildable source files.
+  private func sourceFilesAndDirectories() async throws -> SourceFilesAndDirectories {
+    return try await cachedSourceFilesAndDirectories.get(
+      SourceFilesAndDirectoriesKey(),
+      isolation: self
+    ) { key in
+      let targets = try await self.buildTargets()
+      let sourcesItems = try await self.sourceFiles(in: Set(targets.keys))
 
-    return try await cachedSourceFilesAndDirectories.get(key, isolation: self) { key in
       var files: [DocumentURI: SourceFileInfo] = [:]
       var directories: [DocumentURI: (pathComponents: [String]?, info: SourceFileInfo)] = [:]
-      for sourcesItem in key.sourcesItems {
+      for sourcesItem in sourcesItems {
         let target = targets[sourcesItem.target]?.target
         let isPartOfRootProject = !(target?.tags.contains(.dependency) ?? false)
         let mayContainTests = target?.tags.contains(.test) ?? true
-        if !key.includeNonBuildableFiles && (target?.tags.contains(.notBuildable) ?? false) {
-          continue
-        }
-
         for sourceItem in sourcesItem.sources {
-          if !key.includeNonBuildableFiles && sourceItem.sourceKitData?.isHeader ?? false {
-            continue
-          }
           let info = SourceFileInfo(
             targets: [sourcesItem.target],
             isPartOfRootProject: isPartOfRootProject,
-            mayContainTests: mayContainTests
+            mayContainTests: mayContainTests,
+            isBuildable: !(target?.tags.contains(.notBuildable) ?? false)
+              && !(sourceItem.sourceKitData?.isHeader ?? false)
           )
           switch sourceItem.kind {
           case .file:
@@ -1093,7 +1092,7 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
   }
 
   package func testFiles() async throws -> [DocumentURI] {
-    return try await buildableSourceFiles().compactMap { (uri, info) -> DocumentURI? in
+    return try await sourceFiles(includeNonBuildableFiles: false).compactMap { (uri, info) -> DocumentURI? in
       guard info.isPartOfRootProject, info.mayContainTests else {
         return nil
       }

Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift

@@ -99,7 +99,10 @@ package actor CompilationDatabaseBuildSystem: BuiltInBuildSystem {
           let args = command.commandLine
           for i in args.indices.reversed() {
             if args[i] == "-index-store-path" && i + 1 < args.count {
-              return URL(fileURLWithPath: args[i + 1])
+              return URL(
+                fileURLWithPath: args[i + 1],
+                relativeTo: URL(fileURLWithPath: command.directory, isDirectory: true)
+              )
             }
           }
         }

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

@@ -587,7 +587,7 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
         SourceItem(
           uri: DocumentURI($0),
           kind: $0.isDirectory ? .directory : .file,
-          generated: false,
+          generated: false
         )
       }
       result.append(SourcesItem(target: target, sources: sources))

Sources/LanguageServerProtocol/SupportTypes/DocumentURI.swift

@@ -56,12 +56,21 @@ public struct DocumentURI: Codable, Hashable, Sendable {
   /// fallback mode that drops semantic functionality.
   public var pseudoPath: String {
     if storage.isFileURL {
-      return storage.withUnsafeFileSystemRepresentation { filePath in
-        if let filePath {
-          String(cString: filePath)
-        } else {
-          ""
+      return storage.withUnsafeFileSystemRepresentation { filePathPtr in
+        guard let filePathPtr else {
+          return ""
         }
+        let filePath = String(cString: filePathPtr)
+        #if os(Windows)
+        // VS Code spells file paths with a lowercase drive letter, while the rest of Windows APIs use an uppercase
+        // drive letter. Normalize the drive letter spelling to be uppercase.
+        if filePath.first?.isASCII ?? false, filePath.first?.isLetter ?? false, filePath.first?.isLowercase ?? false,
+          filePath.count > 1, filePath[filePath.index(filePath.startIndex, offsetBy: 1)] == ":"
+        {
+          return filePath.first!.uppercased() + filePath.dropFirst()
+        }
+        #endif
+        return filePath
       }
     } else {
       return storage.absoluteString

Sources/SKTestSupport/MultiFileTestProject.swift

@@ -88,7 +88,9 @@ package class MultiFileTestProject {
   /// File contents can also contain `$TEST_DIR`, which gets replaced by the temporary directory.
   package init(
     files: [RelativeFileLocation: String],
-    workspaces: (URL) async throws -> [WorkspaceFolder] = { [WorkspaceFolder(uri: DocumentURI($0))] },
+    workspaces: (_ scratchDirectory: URL) async throws -> [WorkspaceFolder] = {
+      [WorkspaceFolder(uri: DocumentURI($0))]
+    },
     initializationOptions: LSPAny? = nil,
     capabilities: ClientCapabilities = ClientCapabilities(),
     options: SourceKitLSPOptions = .testDefault(),

Sources/SKTestSupport/SkipUnless.swift

@@ -418,6 +418,10 @@ package actor SkipUnless {
     try XCTSkipUnless(Platform.current == .darwin, message)
   }
 
+  package static func platformIsWindows(_ message: String) throws {
+    try XCTSkipUnless(Platform.current == .windows, message)
+  }
+
   package static func platformSupportsTaskPriorityElevation() throws {
     #if os(macOS)
     guard #available(macOS 14.0, *) else {

Sources/SKTestSupport/SwiftPMTestProject.swift

@@ -163,7 +163,9 @@ package class SwiftPMTestProject: MultiFileTestProject {
   package init(
     files: [RelativeFileLocation: String],
     manifest: String = SwiftPMTestProject.defaultPackageManifest,
-    workspaces: (URL) async throws -> [WorkspaceFolder] = { [WorkspaceFolder(uri: DocumentURI($0))] },
+    workspaces: (_ scratchDirectory: URL) async throws -> [WorkspaceFolder] = {
+      [WorkspaceFolder(uri: DocumentURI($0))]
+    },
     initializationOptions: LSPAny? = nil,
     capabilities: ClientCapabilities = ClientCapabilities(),
     options: SourceKitLSPOptions = .testDefault(),

Sources/SemanticIndex/SemanticIndexManager.swift

@@ -291,7 +291,8 @@ package final actor SemanticIndexManager {
             filesToIndex
           } else {
             await orLog("Getting files to index") {
-              try await self.buildSystemManager.buildableSourceFiles().keys.sorted { $0.stringValue < $1.stringValue }
+              try await self.buildSystemManager.sourceFiles(includeNonBuildableFiles: false).keys
+                .sorted { $0.stringValue < $1.stringValue }
             } ?? []
           }
         if !indexFilesWithUpToDateUnit {
@@ -408,7 +409,7 @@ package final actor SemanticIndexManager {
     toCover files: some Collection<DocumentURI> & Sendable
   ) async -> [FileToIndex] {
     let sourceFiles = await orLog("Getting source files in project") {
-      Set(try await buildSystemManager.buildableSourceFiles().keys)
+      Set(try await buildSystemManager.sourceFiles(includeNonBuildableFiles: false).keys)
     }
     guard let sourceFiles else {
       return []

Sources/SourceKitLSP/Workspace.swift

@@ -62,6 +62,51 @@ fileprivate func firstNonNil<T>(
   return try await defaultValue()
 }
 
+/// Actor that caches realpaths for `sourceFilesWithSameRealpath`.
+fileprivate actor SourceFilesWithSameRealpathInferrer {
+  private let buildSystemManager: BuildSystemManager
+  private var realpathCache: [DocumentURI: DocumentURI] = [:]
+
+  init(buildSystemManager: BuildSystemManager) {
+    self.buildSystemManager = buildSystemManager
+  }
+
+  private func realpath(of uri: DocumentURI) -> DocumentURI {
+    if let cached = realpathCache[uri] {
+      return cached
+    }
+    let value = uri.symlinkTarget ?? uri
+    realpathCache[uri] = value
+    return value
+  }
+
+  /// Returns the URIs of all source files in the project that have the same realpath as a document in `documents` but
+  /// are not in `documents`.
+  ///
+  /// This is useful in the following scenario: A project has target A containing A.swift an target B containing B.swift
+  /// B.swift is a symlink to A.swift. When A.swift is modified, both the dependencies of A and B need to be marked as
+  /// having an out-of-date preparation status, not just A.
+  package func sourceFilesWithSameRealpath(as documents: [DocumentURI]) async -> [DocumentURI] {
+    let realPaths = Set(documents.map { realpath(of: $0) })
+    return await orLog("Determining source files with same realpath") {
+      var result: [DocumentURI] = []
+      let filesAndDirectories = try await buildSystemManager.sourceFiles(includeNonBuildableFiles: true)
+      for file in filesAndDirectories.keys {
+        if realPaths.contains(realpath(of: file)) && !documents.contains(file) {
+          result.append(file)
+        }
+      }
+      return result
+    } ?? []
+  }
+
+  func filesDidChange(_ events: [FileEvent]) {
+    for event in events {
+      realpathCache[event.uri] = nil
+    }
+  }
+}
+
 /// Represents the configuration and state of a project or combination of projects being worked on
 /// together.
 ///
@@ -86,6 +131,8 @@ package final class Workspace: Sendable, BuildSystemManagerDelegate {
   /// The build system manager to use for documents in this workspace.
   package let buildSystemManager: BuildSystemManager
 
+  private let sourceFilesWithSameRealpathInferrer: SourceFilesWithSameRealpathInferrer
+
   let options: SourceKitLSPOptions
 
   /// The source code index, if available.
@@ -126,6 +173,9 @@ package final class Workspace: Sendable, BuildSystemManagerDelegate {
     self.options = options
     self._uncheckedIndex = ThreadSafeBox(initialValue: uncheckedIndex)
     self.buildSystemManager = buildSystemManager
+    self.sourceFilesWithSameRealpathInferrer = SourceFilesWithSameRealpathInferrer(
+      buildSystemManager: buildSystemManager
+    )
     if options.backgroundIndexingOrDefault, let uncheckedIndex,
       await buildSystemManager.initializationData?.prepareProvider ?? false
     {
@@ -316,6 +366,17 @@ package final class Workspace: Sendable, BuildSystemManagerDelegate {
   }
 
   package func filesDidChange(_ events: [FileEvent]) async {
+    // First clear any cached realpaths in `sourceFilesWithSameRealpathInferrer`.
+    await sourceFilesWithSameRealpathInferrer.filesDidChange(events)
+
+    // Now infer any edits for source files that share the same realpath as one of the modified files.
+    var events = events
+    events +=
+      await sourceFilesWithSameRealpathInferrer
+      .sourceFilesWithSameRealpath(as: events.filter { $0.type == .changed }.map(\.uri))
+      .map { FileEvent(uri: $0, type: .changed) }
+
+    // Notify all clients about the reported and inferred edits.
     await buildSystemManager.filesDidChange(events)
     await syntacticTestIndex.filesDidChange(events)
     await semanticIndexManager?.filesDidChange(events)

Sources/SwiftExtensions/URLExtensions.swift

@@ -67,11 +67,21 @@ extension URL {
       guard self.isFileURL else {
         throw FilePathError.noFileURL(self)
       }
-      return try self.withUnsafeFileSystemRepresentation { buffer in
-        guard let buffer else {
+      return try self.withUnsafeFileSystemRepresentation { filePathPtr in
+        guard let filePathPtr else {
           throw FilePathError.noFileSystemRepresentation(self)
         }
-        return String(cString: buffer)
+        let filePath = String(cString: filePathPtr)
+        #if os(Windows)
+        // VS Code spells file paths with a lowercase drive letter, while the rest of Windows APIs use an uppercase
+        // drive letter. Normalize the drive letter spelling to be uppercase.
+        if filePath.first?.isASCII ?? false, filePath.first?.isLetter ?? false, filePath.first?.isLowercase ?? false,
+          filePath.count > 1, filePath[filePath.index(filePath.startIndex, offsetBy: 1)] == ":"
+        {
+          return filePath.first!.uppercased() + filePath.dropFirst()
+        }
+        #endif
+        return filePath
       }
     }
   }

Sources/sourcekit-lsp/SourceKitLSP.swift

@@ -10,8 +10,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-#if compiler(>=6)
-public import ArgumentParser
 import BuildSystemIntegration
 import Csourcekitd  // Not needed here, but fixes debugging...
 import Diagnose
@@ -26,24 +24,16 @@ import SourceKitLSP
 import SwiftExtensions
 import ToolchainRegistry
 
-#if canImport(Android)
-import Android
-#endif
+import struct TSCBasic.AbsolutePath
+
+#if compiler(>=6)
+public import ArgumentParser
 #else
 import ArgumentParser
-import BuildSystemIntegration
-import Csourcekitd  // Not needed here, but fixes debugging...
-import Diagnose
-import Dispatch
-import Foundation
-import LanguageServerProtocol
-import LanguageServerProtocolExtensions
-import LanguageServerProtocolJSONRPC
-import SKLogging
-import SKOptions
-import SourceKitLSP
-import SwiftExtensions
-import ToolchainRegistry
+#endif
+
+#if canImport(Android)
+import Android
 #endif
 
 extension PathPrefixMapping {
@@ -279,6 +269,10 @@ struct SourceKitLSP: AsyncParsableCommand {
       outFD: realStdoutHandle
     )
 
+    // For reasons that are completely oblivious to me, `DispatchIO.write`, which is used to write LSP responses to
+    // stdout fails with error code 5 on Windows unless we call `AbsolutePath(validating:)` on some URL first.
+    _ = try AbsolutePath(validating: Bundle.main.bundlePath)
+
     var inputMirror: FileHandle? = nil
     if let inputMirrorDirectory = globalConfigurationOptions.loggingOrDefault.inputMirrorDirectory {
       orLog("Setting up input mirror") {