From 8f1bc8c940727af308d9b1492824f0a1625e7e20 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Wed, 13 Aug 2025 09:16:43 -0400 Subject: [PATCH 1/2] Remove temp folder from WorkspaceContext In an effort to pair down WorkspaceContext and the dependencies that need to be threaded through the application from it, remove the common tempFolder from it and grab a reference to it as needed. A large portion of this PR is refactoring the `runScript` commandand ensuring nothing broke by adding tests. --- src/TestExplorer/TestRunner.ts | 3 +- src/WorkspaceContext.ts | 14 +- src/commands.ts | 10 +- src/commands/runSwiftScript.ts | 121 +++++++++------- src/coverage/LcovResults.ts | 37 +++-- src/extension.ts | 2 +- .../unit-tests/commands/runSwiftSwift.test.ts | 137 ++++++++++++++++++ 7 files changed, 246 insertions(+), 78 deletions(-) create mode 100644 test/unit-tests/commands/runSwiftSwift.test.ts diff --git a/src/TestExplorer/TestRunner.ts b/src/TestExplorer/TestRunner.ts index 38e99b5c9..cc502f0bd 100644 --- a/src/TestExplorer/TestRunner.ts +++ b/src/TestExplorer/TestRunner.ts @@ -981,7 +981,8 @@ export class TestRunner { testBuildConfig: vscode.DebugConfiguration, runState: TestRunnerTestRunState ) { - await this.workspaceContext.tempFolder.withTemporaryFile("xml", async filename => { + const tempFolder = await TemporaryFolder.create(); + await tempFolder.withTemporaryFile("xml", async filename => { const args = [...(testBuildConfig.args ?? []), "--xunit-output", filename]; try { diff --git a/src/WorkspaceContext.ts b/src/WorkspaceContext.ts index 3a0a97f22..252f67f65 100644 --- a/src/WorkspaceContext.ts +++ b/src/WorkspaceContext.ts @@ -19,7 +19,6 @@ import { StatusItem } from "./ui/StatusItem"; import { swiftLibraryPathKey } from "./utilities/utilities"; import { isExcluded, isPathInsidePath } from "./utilities/filesystem"; import { LanguageClientToolchainCoordinator } from "./sourcekit-lsp/LanguageClientToolchainCoordinator"; -import { TemporaryFolder } from "./utilities/tempFolder"; import { TaskManager } from "./tasks/TaskManager"; import { makeDebugConfigurations } from "./debugger/launch"; import configuration from "./configuration"; @@ -76,9 +75,8 @@ export class WorkspaceContext implements vscode.Disposable { public loggerFactory: SwiftLoggerFactory; - private constructor( + constructor( extensionContext: vscode.ExtensionContext, - public tempFolder: TemporaryFolder, public logger: SwiftLogger, public globalToolchain: SwiftToolchain ) { @@ -230,16 +228,6 @@ export class WorkspaceContext implements vscode.Disposable { return this.globalToolchain.swiftVersion; } - /** Get swift version and create WorkspaceContext */ - static async create( - extensionContext: vscode.ExtensionContext, - logger: SwiftLogger, - toolchain: SwiftToolchain - ): Promise { - const tempFolder = await TemporaryFolder.create(); - return new WorkspaceContext(extensionContext, tempFolder, logger, toolchain); - } - /** * Update context keys based on package contents */ diff --git a/src/commands.ts b/src/commands.ts index a1873bb79..0a624597f 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -213,7 +213,15 @@ export function register(ctx: WorkspaceContext): vscode.Disposable[] { Commands.RESET_PACKAGE, async (_ /* Ignore context */, folder) => await resetPackage(ctx, folder) ), - vscode.commands.registerCommand("swift.runScript", async () => await runSwiftScript(ctx)), + vscode.commands.registerCommand("swift.runScript", async () => { + if (ctx.currentFolder && vscode.window.activeTextEditor?.document) { + await runSwiftScript( + vscode.window.activeTextEditor.document, + ctx.tasks, + ctx.currentFolder.toolchain + ); + } + }), vscode.commands.registerCommand("swift.openPackage", async () => { if (ctx.currentFolder) { return await openPackage(ctx.currentFolder.swiftVersion, ctx.currentFolder.folder); diff --git a/src/commands/runSwiftScript.ts b/src/commands/runSwiftScript.ts index e522c4cd2..009d5c6b8 100644 --- a/src/commands/runSwiftScript.ts +++ b/src/commands/runSwiftScript.ts @@ -15,38 +15,58 @@ import * as vscode from "vscode"; import * as path from "path"; import * as fs from "fs/promises"; -import { createSwiftTask } from "../tasks/SwiftTaskProvider"; -import { WorkspaceContext } from "../WorkspaceContext"; -import { Version } from "../utilities/version"; import configuration from "../configuration"; +import { createSwiftTask } from "../tasks/SwiftTaskProvider"; +import { TemporaryFolder } from "../utilities/tempFolder"; +import { TaskManager } from "../tasks/TaskManager"; +import { SwiftToolchain } from "../toolchain/toolchain"; /** - * Run the active document through the Swift REPL + * Runs the Swift code in the supplied document. + * + * This function checks for a valid document and Swift version, then creates and executes + * a Swift task to run the script file. The task is configured to always reveal its output + * and clear previous output. The working directory is set to the script's location. + * + * @param document - The text document containing the Swift script to run. If undefined, the function returns early. + * @param tasks - The TaskManager instance used to execute and manage the Swift task. + * @param toolchain - The SwiftToolchain to use for running the script. + * @returns A promise that resolves when the script has finished running, or returns early if the user is prompted + * for which swift version to use and they exit the dialog without choosing one. */ -export async function runSwiftScript(ctx: WorkspaceContext) { - const document = vscode.window.activeTextEditor?.document; - if (!document) { - return; - } - - if (!ctx.currentFolder) { +export async function runSwiftScript( + document: vscode.TextDocument, + tasks: TaskManager, + toolchain: SwiftToolchain +) { + const targetVersion = await targetSwiftVersion(); + if (!targetVersion) { return; } - // Swift scripts require new swift driver to work on Windows. Swift driver is available - // from v5.7 of Windows Swift - if ( - process.platform === "win32" && - ctx.currentFolder.swiftVersion.isLessThan(new Version(5, 7, 0)) - ) { - void vscode.window.showErrorMessage( - "Run Swift Script is unavailable with the legacy driver on Windows." + await withDocumentFile(document, async filename => { + const runTask = createSwiftTask( + ["-swift-version", targetVersion, filename], + `Run ${filename}`, + { + scope: vscode.TaskScope.Global, + cwd: vscode.Uri.file(path.dirname(filename)), + presentationOptions: { reveal: vscode.TaskRevealKind.Always, clear: true }, + }, + toolchain ); - return; - } - - let target: string; + await tasks.executeTaskAndWait(runTask); + }); +} +/** + * Determines the target Swift language version to use for script execution. + * If the configuration is set to "Ask Every Run", prompts the user to select a version. + * Otherwise, returns the default version from the user's settings. + * + * @returns {Promise} The selected Swift version, or undefined if no selection was made. + */ +async function targetSwiftVersion() { const defaultVersion = configuration.scriptSwiftLanguageVersion; if (defaultVersion === "Ask Every Run") { const picked = await vscode.window.showQuickPick( @@ -59,41 +79,36 @@ export async function runSwiftScript(ctx: WorkspaceContext) { placeHolder: "Select a target Swift version", } ); - - if (!picked) { - return; - } - target = picked.value; + return picked?.value; } else { - target = defaultVersion; + return defaultVersion; } +} - let filename = document.fileName; - let isTempFile = false; +/** + * Executes a callback with the filename of the given `vscode.TextDocument`. + * If the document is untitled (not yet saved to disk), it creates a temporary file, + * writes the document's content to it, and passes its filename to the callback. + * Otherwise, it ensures the document is saved and passes its actual filename. + * + * The temporary file is automatically deleted when the callback completes. + * + * @param document - The VSCode text document to operate on. + * @param callback - An async function that receives the filename of the document or temporary file. + * @returns A promise that resolves when the callback has completed. + */ +async function withDocumentFile( + document: vscode.TextDocument, + callback: (filename: string) => Promise +) { if (document.isUntitled) { - // if document hasn't been saved, save it to a temporary file - isTempFile = true; - filename = ctx.tempFolder.filename(document.fileName, "swift"); - const text = document.getText(); - await fs.writeFile(filename, text); + const tmpFolder = await TemporaryFolder.create(); + await tmpFolder.withTemporaryFile("swift", async filename => { + await fs.writeFile(filename, document.getText()); + await callback(filename); + }); } else { - // otherwise save document await document.save(); - } - const runTask = createSwiftTask( - ["-swift-version", target, filename], - `Run ${filename}`, - { - scope: vscode.TaskScope.Global, - cwd: vscode.Uri.file(path.dirname(filename)), - presentationOptions: { reveal: vscode.TaskRevealKind.Always, clear: true }, - }, - ctx.currentFolder.toolchain - ); - await ctx.tasks.executeTaskAndWait(runTask); - - // delete file after running swift - if (isTempFile) { - await fs.rm(filename); + await callback(document.fileName); } } diff --git a/src/coverage/LcovResults.ts b/src/coverage/LcovResults.ts index 4cad3fd74..786e3aacf 100644 --- a/src/coverage/LcovResults.ts +++ b/src/coverage/LcovResults.ts @@ -23,7 +23,7 @@ import { FolderContext } from "../FolderContext"; import { execFileStreamOutput } from "../utilities/utilities"; import { BuildFlags } from "../toolchain/BuildFlags"; import { TestLibrary } from "../TestExplorer/TestRunner"; -import { DisposableFileCollection } from "../utilities/tempFolder"; +import { DisposableFileCollection, TemporaryFolder } from "../utilities/tempFolder"; import { TargetType } from "../SwiftPackage"; import { TestingConfigurationFactory } from "../debugger/buildConfig"; import { TestKind } from "../TestExplorer/TestKind"; @@ -35,13 +35,11 @@ interface CodeCovFile { export class TestCoverage { private lcovFiles: CodeCovFile[] = []; - private lcovTmpFiles: DisposableFileCollection; + private _lcovTmpFiles?: DisposableFileCollection; + private _lcovTmpFilesInit?: Promise; private coverageDetails = new Map(); - constructor(private folderContext: FolderContext) { - const tmpFolder = folderContext.workspaceContext.tempFolder; - this.lcovTmpFiles = tmpFolder.createDisposableFileCollection(); - } + constructor(private folderContext: FolderContext) {} /** * Returns coverage information for the suppplied URI. @@ -60,7 +58,7 @@ export class TestCoverage { true ); const result = await asyncfs.readFile(`${buildDirectory}/debug/codecov/default.profdata`); - const filename = this.lcovTmpFiles.file(testLibrary, "profdata"); + const filename = (await this.lcovTmpFiles()).file(testLibrary, "profdata"); await asyncfs.writeFile(filename, result); this.lcovFiles.push({ testLibrary, path: filename }); } @@ -88,14 +86,14 @@ export class TestCoverage { this.coverageDetails.set(uri, detailedCoverage); } } - await this.lcovTmpFiles.dispose(); + await this._lcovTmpFiles?.dispose(); } /** * Merges multiple `.profdata` files into a single `.profdata` file. */ private async mergeProfdata(profDataFiles: string[]) { - const filename = this.lcovTmpFiles.file("merged", "profdata"); + const filename = (await this.lcovTmpFiles()).file("merged", "profdata"); const toolchain = this.folderContext.toolchain; const llvmProfdata = toolchain.getToolchainExecutable("llvm-profdata"); await execFileStreamOutput( @@ -196,6 +194,27 @@ export class TestCoverage { return buffer; } + /** + * Lazily creates (once) and returns the disposable file collection used for LCOV processing. + * Safe against concurrent callers. + */ + private async lcovTmpFiles(): Promise { + if (this._lcovTmpFiles) { + return this._lcovTmpFiles; + } + + // Use an internal promise to avoid duplicate folder creation in concurrent calls. + if (!this._lcovTmpFilesInit) { + this._lcovTmpFilesInit = (async () => { + const tempFolder = await TemporaryFolder.create(); + this._lcovTmpFiles = tempFolder.createDisposableFileCollection(); + return this._lcovTmpFiles; + })(); + } + + return (await this._lcovTmpFilesInit)!; + } + /** * Constructs a string containing all the paths to exclude from the code coverage report. * This should exclude everything in the `.build` folder as well as all the test targets. diff --git a/src/extension.ts b/src/extension.ts index abb42ba1a..6c1f35a2f 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -82,7 +82,7 @@ export async function activate(context: vscode.ExtensionContext): Promise { }; } - const workspaceContext = await WorkspaceContext.create(context, logger, toolchain); + const workspaceContext = new WorkspaceContext(context, logger, toolchain); context.subscriptions.push(workspaceContext); context.subscriptions.push(new SwiftEnvironmentVariablesManager(context)); diff --git a/test/unit-tests/commands/runSwiftSwift.test.ts b/test/unit-tests/commands/runSwiftSwift.test.ts new file mode 100644 index 000000000..9d7d0e945 --- /dev/null +++ b/test/unit-tests/commands/runSwiftSwift.test.ts @@ -0,0 +1,137 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the VS Code Swift open source project +// +// Copyright (c) 2025 the VS Code Swift project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of VS Code Swift project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +import * as vscode from "vscode"; +import { beforeEach } from "mocha"; +import { expect } from "chai"; +import { match, stub } from "sinon"; +import { instance, mockFn, mockGlobalObject, mockGlobalValue, mockObject } from "../../MockUtils"; +import { runSwiftScript } from "../../../src/commands/runSwiftScript"; +import { TaskManager } from "../../../src/tasks/TaskManager"; +import { SwiftToolchain } from "../../../src/toolchain/toolchain"; +import { BuildFlags } from "../../../src/toolchain/BuildFlags"; +import configuration from "../../../src/configuration"; + +suite("runSwiftScript Test Suite", () => { + const mockTaskManager = mockObject({ executeTaskAndWait: stub().resolves() }); + const mockToolchain = mockObject({ + getToolchainExecutable: () => "/usr/bin/swift", + buildFlags: instance( + mockObject({ + withAdditionalFlags: mockFn(s => s.callsFake(args => args)), + }) + ), + }); + + function createMockTextDocument( + options: { + isUntitled?: boolean; + } = {} + ) { + const isUntitled = options.isUntitled ?? false; + const baseDocument = { + getText: stub().returns('print("Hello, World!")'), + uri: vscode.Uri.file("/path/to/test.swift"), + languageId: "swift", + isUntitled, + }; + + // Add properties specific to saved documents + if (!isUntitled) { + return mockObject({ + ...baseDocument, + fileName: "test.swift", + save: stub().resolves(true), + }); + } + + return mockObject(baseDocument); + } + + beforeEach(() => { + mockTaskManager.executeTaskAndWait.resetHistory(); + }); + + test("Executes runTask command with a saved document", async () => { + await runSwiftScript( + instance(createMockTextDocument()), + instance(mockTaskManager), + instance(mockToolchain) + ); + + expect(mockTaskManager.executeTaskAndWait).to.have.been.calledOnceWith( + match.has("detail", "swift -swift-version 6 test.swift") + ); + }); + + test("Executes runTask command with an unsaved document", async () => { + await runSwiftScript( + instance(createMockTextDocument({ isUntitled: true })), + instance(mockTaskManager), + instance(mockToolchain) + ); + + expect(mockTaskManager.executeTaskAndWait).to.have.been.calledOnceWith( + match.has("detail", match(/^swift -swift-version 6 /)) + ); + }); + + suite("User Configuration", () => { + const config = mockGlobalValue(configuration, "scriptSwiftLanguageVersion"); + const mockWindow = mockGlobalObject(vscode, "window"); + + test("Executes run task with the users chosen swift version", async () => { + config.setValue("5"); + + await runSwiftScript( + instance(createMockTextDocument()), + instance(mockTaskManager), + instance(mockToolchain) + ); + + expect(mockTaskManager.executeTaskAndWait).to.have.been.calledOnceWith( + match.has("detail", "swift -swift-version 5 test.swift") + ); + }); + + test("Prompts for the users desired swift version", async () => { + config.setValue("Ask Every Run"); + const selectedItem = { value: "6", label: "Swift 6" }; + mockWindow.showQuickPick.resolves(selectedItem); + + await runSwiftScript( + instance(createMockTextDocument()), + instance(mockTaskManager), + instance(mockToolchain) + ); + + expect(mockTaskManager.executeTaskAndWait).to.have.been.calledOnceWith( + match.has("detail", "swift -swift-version 6 test.swift") + ); + }); + + test("Exists when the user cancels the prompt", async () => { + config.setValue("Ask Every Run"); + mockWindow.showQuickPick.resolves(undefined); + + await runSwiftScript( + instance(createMockTextDocument()), + instance(mockTaskManager), + instance(mockToolchain) + ); + + expect(mockTaskManager.executeTaskAndWait).to.not.have.been.called; + }); + }); +}); From f273e2f95e3f7ce3b83d4ae27f151ec6e77a4100 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Thu, 14 Aug 2025 10:40:40 -0400 Subject: [PATCH 2/2] Fix typo in test file name --- .../commands/{runSwiftSwift.test.ts => runSwiftScript.test.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/unit-tests/commands/{runSwiftSwift.test.ts => runSwiftScript.test.ts} (100%) diff --git a/test/unit-tests/commands/runSwiftSwift.test.ts b/test/unit-tests/commands/runSwiftScript.test.ts similarity index 100% rename from test/unit-tests/commands/runSwiftSwift.test.ts rename to test/unit-tests/commands/runSwiftScript.test.ts