diff --git a/Sources/BuildSystemIntegration/BuildSystemManager.swift b/Sources/BuildSystemIntegration/BuildSystemManager.swift index d1a1fd3fa..bac299d6b 100644 --- a/Sources/BuildSystemIntegration/BuildSystemManager.swift +++ b/Sources/BuildSystemIntegration/BuildSystemManager.swift @@ -54,6 +54,12 @@ 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 @@ -61,7 +67,8 @@ package struct SourceFileInfo: Sendable { 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() - /// 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 { return await orLog("Getting targets for source file") { var result: Set = [] - 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 } diff --git a/Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift b/Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift index dffa9a720..6acfb815c 100644 --- a/Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift +++ b/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) + ) } } } diff --git a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift index adb790a74..50a8e963a 100644 --- a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift +++ b/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)) diff --git a/Sources/LanguageServerProtocol/SupportTypes/DocumentURI.swift b/Sources/LanguageServerProtocol/SupportTypes/DocumentURI.swift index 5f593d747..ad50fb849 100644 --- a/Sources/LanguageServerProtocol/SupportTypes/DocumentURI.swift +++ b/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 diff --git a/Sources/SKTestSupport/MultiFileTestProject.swift b/Sources/SKTestSupport/MultiFileTestProject.swift index 1d1d107f3..7f6b0b5dc 100644 --- a/Sources/SKTestSupport/MultiFileTestProject.swift +++ b/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(), diff --git a/Sources/SKTestSupport/SkipUnless.swift b/Sources/SKTestSupport/SkipUnless.swift index dbfe49777..69a9deb5b 100644 --- a/Sources/SKTestSupport/SkipUnless.swift +++ b/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 { diff --git a/Sources/SKTestSupport/SwiftPMTestProject.swift b/Sources/SKTestSupport/SwiftPMTestProject.swift index 5207eebd1..bf7d2a39f 100644 --- a/Sources/SKTestSupport/SwiftPMTestProject.swift +++ b/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(), diff --git a/Sources/SemanticIndex/SemanticIndexManager.swift b/Sources/SemanticIndex/SemanticIndexManager.swift index 797e68c2c..d40d79bbe 100644 --- a/Sources/SemanticIndex/SemanticIndexManager.swift +++ b/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 & 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 [] diff --git a/Sources/SourceKitLSP/Workspace.swift b/Sources/SourceKitLSP/Workspace.swift index 36f7cd78d..d31fe91c7 100644 --- a/Sources/SourceKitLSP/Workspace.swift +++ b/Sources/SourceKitLSP/Workspace.swift @@ -62,6 +62,51 @@ fileprivate func firstNonNil( 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) diff --git a/Sources/SwiftExtensions/URLExtensions.swift b/Sources/SwiftExtensions/URLExtensions.swift index 024e91f6f..cfac5ae81 100644 --- a/Sources/SwiftExtensions/URLExtensions.swift +++ b/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 } } } diff --git a/Sources/sourcekit-lsp/SourceKitLSP.swift b/Sources/sourcekit-lsp/SourceKitLSP.swift index c8c0fc0b7..993d0e171 100644 --- a/Sources/sourcekit-lsp/SourceKitLSP.swift +++ b/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") { diff --git a/Tests/BuildSystemIntegrationTests/CompilationDatabaseTests.swift b/Tests/BuildSystemIntegrationTests/CompilationDatabaseTests.swift index 6c136f947..2eaff2106 100644 --- a/Tests/BuildSystemIntegrationTests/CompilationDatabaseTests.swift +++ b/Tests/BuildSystemIntegrationTests/CompilationDatabaseTests.swift @@ -412,6 +412,25 @@ final class CompilationDatabaseTests: XCTestCase { ) } } + + func testIndexStorePathRelativeToWorkingDirectory() async throws { + try await checkCompilationDatabaseBuildSystem( + """ + [ + { + "file": "a.swift", + "directory": "/a", + "arguments": ["swift", "a.swift", "-index-store-path", "index-store"] + } + ] + """ + ) { buildSystem in + assertEqual( + try await buildSystem.indexStorePath?.filePath, + "\(pathSeparator)a\(pathSeparator)index-store" + ) + } + } } fileprivate var pathSeparator: String { diff --git a/Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift b/Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift index f800b0a2f..3944dbbaf 100644 --- a/Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift +++ b/Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift @@ -1156,7 +1156,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { files: [ "pkg/Sources/lib/a.swift": "", "pkg/Package.swift": """ - // swift-tools-version: 4.2 + // swift-tools-version:5.1 import PackageDescription """, ] @@ -1177,7 +1177,7 @@ final class SwiftPMBuildSystemTests: XCTestCase { fallbackAfterTimeout: false ) let compilerArgs = try XCTUnwrap(settings?.compilerArguments) - XCTAssert(compilerArgs.contains("-package-description-version")) + assertArgumentsContain("-package-description-version", "5.1.0", arguments: compilerArgs) XCTAssert(compilerArgs.contains(try manifestURL.filePath)) } } diff --git a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift index c47dff593..a0d3d0827 100644 --- a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift +++ b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift @@ -1620,6 +1620,63 @@ final class BackgroundIndexingTests: XCTestCase { return completionAfterEdit.items.map(\.label) == ["self", "test()"] } } + + func testSymlinkedTargetReferringToSameSourceFile() async throws { + let project = try await SwiftPMTestProject( + files: [ + "LibA/LibA.swift": """ + public let myVar: String + """, + "Client/Client.swift": """ + import LibASymlink + + func test() { + print(1️⃣myVar) + } + """, + ], + manifest: """ + let package = Package( + name: "MyLibrary", + targets: [ + .target(name: "LibA"), + .target(name: "LibASymlink"), + .target(name: "Client", dependencies: ["LibASymlink"]), + ] + ) + """, + workspaces: { scratchDirectory in + let sources = scratchDirectory.appendingPathComponent("Sources") + try FileManager.default.createSymbolicLink( + at: sources.appendingPathComponent("LibASymlink"), + withDestinationURL: sources.appendingPathComponent("LibA") + ) + return [WorkspaceFolder(uri: DocumentURI(scratchDirectory))] + }, + enableBackgroundIndexing: true + ) + + let (uri, positions) = try project.openDocument("Client.swift") + let preEditHover = try await project.testClient.send( + HoverRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"]) + ) + let preEditHoverContent = try XCTUnwrap(preEditHover?.contents.markupContent?.value) + XCTAssert( + preEditHoverContent.contains("String"), + "Pre edit hover content '\(preEditHoverContent)' does not contain 'String'" + ) + + let libAUri = try project.uri(for: "LibA.swift") + try "public let myVar: Int".write(to: try XCTUnwrap(libAUri.fileURL), atomically: true, encoding: .utf8) + project.testClient.send(DidChangeWatchedFilesNotification(changes: [FileEvent(uri: libAUri, type: .changed)])) + + try await repeatUntilExpectedResult { + let postEditHover = try await project.testClient.send( + HoverRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"]) + ) + return try XCTUnwrap(postEditHover?.contents.markupContent?.value).contains("Int") + } + } } extension HoverResponseContents { diff --git a/Tests/SourceKitLSPTests/PullDiagnosticsTests.swift b/Tests/SourceKitLSPTests/PullDiagnosticsTests.swift index 51ca9fb18..bcbcd2c11 100644 --- a/Tests/SourceKitLSPTests/PullDiagnosticsTests.swift +++ b/Tests/SourceKitLSPTests/PullDiagnosticsTests.swift @@ -48,6 +48,35 @@ final class PullDiagnosticsTests: XCTestCase { XCTAssertEqual(diagnostic.range, Position(line: 1, utf16index: 2)..