Skip to content

Commit b069a94

Browse files
authored
Remove temp folder from WorkspaceContext (#1781)
* 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.
1 parent 8696cfd commit b069a94

File tree

7 files changed

+246
-78
lines changed

7 files changed

+246
-78
lines changed

src/TestExplorer/TestRunner.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -981,7 +981,8 @@ export class TestRunner {
981981
testBuildConfig: vscode.DebugConfiguration,
982982
runState: TestRunnerTestRunState
983983
) {
984-
await this.workspaceContext.tempFolder.withTemporaryFile("xml", async filename => {
984+
const tempFolder = await TemporaryFolder.create();
985+
await tempFolder.withTemporaryFile("xml", async filename => {
985986
const args = [...(testBuildConfig.args ?? []), "--xunit-output", filename];
986987

987988
try {

src/WorkspaceContext.ts

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import { StatusItem } from "./ui/StatusItem";
1919
import { swiftLibraryPathKey } from "./utilities/utilities";
2020
import { isExcluded, isPathInsidePath } from "./utilities/filesystem";
2121
import { LanguageClientToolchainCoordinator } from "./sourcekit-lsp/LanguageClientToolchainCoordinator";
22-
import { TemporaryFolder } from "./utilities/tempFolder";
2322
import { TaskManager } from "./tasks/TaskManager";
2423
import { makeDebugConfigurations } from "./debugger/launch";
2524
import configuration from "./configuration";
@@ -76,9 +75,8 @@ export class WorkspaceContext implements vscode.Disposable {
7675

7776
public loggerFactory: SwiftLoggerFactory;
7877

79-
private constructor(
78+
constructor(
8079
extensionContext: vscode.ExtensionContext,
81-
public tempFolder: TemporaryFolder,
8280
public logger: SwiftLogger,
8381
public globalToolchain: SwiftToolchain
8482
) {
@@ -230,16 +228,6 @@ export class WorkspaceContext implements vscode.Disposable {
230228
return this.globalToolchain.swiftVersion;
231229
}
232230

233-
/** Get swift version and create WorkspaceContext */
234-
static async create(
235-
extensionContext: vscode.ExtensionContext,
236-
logger: SwiftLogger,
237-
toolchain: SwiftToolchain
238-
): Promise<WorkspaceContext> {
239-
const tempFolder = await TemporaryFolder.create();
240-
return new WorkspaceContext(extensionContext, tempFolder, logger, toolchain);
241-
}
242-
243231
/**
244232
* Update context keys based on package contents
245233
*/

src/commands.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,15 @@ export function register(ctx: WorkspaceContext): vscode.Disposable[] {
213213
Commands.RESET_PACKAGE,
214214
async (_ /* Ignore context */, folder) => await resetPackage(ctx, folder)
215215
),
216-
vscode.commands.registerCommand("swift.runScript", async () => await runSwiftScript(ctx)),
216+
vscode.commands.registerCommand("swift.runScript", async () => {
217+
if (ctx.currentFolder && vscode.window.activeTextEditor?.document) {
218+
await runSwiftScript(
219+
vscode.window.activeTextEditor.document,
220+
ctx.tasks,
221+
ctx.currentFolder.toolchain
222+
);
223+
}
224+
}),
217225
vscode.commands.registerCommand("swift.openPackage", async () => {
218226
if (ctx.currentFolder) {
219227
return await openPackage(ctx.currentFolder.swiftVersion, ctx.currentFolder.folder);

src/commands/runSwiftScript.ts

Lines changed: 68 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -15,38 +15,58 @@
1515
import * as vscode from "vscode";
1616
import * as path from "path";
1717
import * as fs from "fs/promises";
18-
import { createSwiftTask } from "../tasks/SwiftTaskProvider";
19-
import { WorkspaceContext } from "../WorkspaceContext";
20-
import { Version } from "../utilities/version";
2118
import configuration from "../configuration";
19+
import { createSwiftTask } from "../tasks/SwiftTaskProvider";
20+
import { TemporaryFolder } from "../utilities/tempFolder";
21+
import { TaskManager } from "../tasks/TaskManager";
22+
import { SwiftToolchain } from "../toolchain/toolchain";
2223

2324
/**
24-
* Run the active document through the Swift REPL
25+
* Runs the Swift code in the supplied document.
26+
*
27+
* This function checks for a valid document and Swift version, then creates and executes
28+
* a Swift task to run the script file. The task is configured to always reveal its output
29+
* and clear previous output. The working directory is set to the script's location.
30+
*
31+
* @param document - The text document containing the Swift script to run. If undefined, the function returns early.
32+
* @param tasks - The TaskManager instance used to execute and manage the Swift task.
33+
* @param toolchain - The SwiftToolchain to use for running the script.
34+
* @returns A promise that resolves when the script has finished running, or returns early if the user is prompted
35+
* for which swift version to use and they exit the dialog without choosing one.
2536
*/
26-
export async function runSwiftScript(ctx: WorkspaceContext) {
27-
const document = vscode.window.activeTextEditor?.document;
28-
if (!document) {
29-
return;
30-
}
31-
32-
if (!ctx.currentFolder) {
37+
export async function runSwiftScript(
38+
document: vscode.TextDocument,
39+
tasks: TaskManager,
40+
toolchain: SwiftToolchain
41+
) {
42+
const targetVersion = await targetSwiftVersion();
43+
if (!targetVersion) {
3344
return;
3445
}
3546

36-
// Swift scripts require new swift driver to work on Windows. Swift driver is available
37-
// from v5.7 of Windows Swift
38-
if (
39-
process.platform === "win32" &&
40-
ctx.currentFolder.swiftVersion.isLessThan(new Version(5, 7, 0))
41-
) {
42-
void vscode.window.showErrorMessage(
43-
"Run Swift Script is unavailable with the legacy driver on Windows."
47+
await withDocumentFile(document, async filename => {
48+
const runTask = createSwiftTask(
49+
["-swift-version", targetVersion, filename],
50+
`Run ${filename}`,
51+
{
52+
scope: vscode.TaskScope.Global,
53+
cwd: vscode.Uri.file(path.dirname(filename)),
54+
presentationOptions: { reveal: vscode.TaskRevealKind.Always, clear: true },
55+
},
56+
toolchain
4457
);
45-
return;
46-
}
47-
48-
let target: string;
58+
await tasks.executeTaskAndWait(runTask);
59+
});
60+
}
4961

62+
/**
63+
* Determines the target Swift language version to use for script execution.
64+
* If the configuration is set to "Ask Every Run", prompts the user to select a version.
65+
* Otherwise, returns the default version from the user's settings.
66+
*
67+
* @returns {Promise<string | undefined>} The selected Swift version, or undefined if no selection was made.
68+
*/
69+
async function targetSwiftVersion() {
5070
const defaultVersion = configuration.scriptSwiftLanguageVersion;
5171
if (defaultVersion === "Ask Every Run") {
5272
const picked = await vscode.window.showQuickPick(
@@ -59,41 +79,36 @@ export async function runSwiftScript(ctx: WorkspaceContext) {
5979
placeHolder: "Select a target Swift version",
6080
}
6181
);
62-
63-
if (!picked) {
64-
return;
65-
}
66-
target = picked.value;
82+
return picked?.value;
6783
} else {
68-
target = defaultVersion;
84+
return defaultVersion;
6985
}
86+
}
7087

71-
let filename = document.fileName;
72-
let isTempFile = false;
88+
/**
89+
* Executes a callback with the filename of the given `vscode.TextDocument`.
90+
* If the document is untitled (not yet saved to disk), it creates a temporary file,
91+
* writes the document's content to it, and passes its filename to the callback.
92+
* Otherwise, it ensures the document is saved and passes its actual filename.
93+
*
94+
* The temporary file is automatically deleted when the callback completes.
95+
*
96+
* @param document - The VSCode text document to operate on.
97+
* @param callback - An async function that receives the filename of the document or temporary file.
98+
* @returns A promise that resolves when the callback has completed.
99+
*/
100+
async function withDocumentFile(
101+
document: vscode.TextDocument,
102+
callback: (filename: string) => Promise<void>
103+
) {
73104
if (document.isUntitled) {
74-
// if document hasn't been saved, save it to a temporary file
75-
isTempFile = true;
76-
filename = ctx.tempFolder.filename(document.fileName, "swift");
77-
const text = document.getText();
78-
await fs.writeFile(filename, text);
105+
const tmpFolder = await TemporaryFolder.create();
106+
await tmpFolder.withTemporaryFile("swift", async filename => {
107+
await fs.writeFile(filename, document.getText());
108+
await callback(filename);
109+
});
79110
} else {
80-
// otherwise save document
81111
await document.save();
82-
}
83-
const runTask = createSwiftTask(
84-
["-swift-version", target, filename],
85-
`Run ${filename}`,
86-
{
87-
scope: vscode.TaskScope.Global,
88-
cwd: vscode.Uri.file(path.dirname(filename)),
89-
presentationOptions: { reveal: vscode.TaskRevealKind.Always, clear: true },
90-
},
91-
ctx.currentFolder.toolchain
92-
);
93-
await ctx.tasks.executeTaskAndWait(runTask);
94-
95-
// delete file after running swift
96-
if (isTempFile) {
97-
await fs.rm(filename);
112+
await callback(document.fileName);
98113
}
99114
}

src/coverage/LcovResults.ts

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { FolderContext } from "../FolderContext";
2323
import { execFileStreamOutput } from "../utilities/utilities";
2424
import { BuildFlags } from "../toolchain/BuildFlags";
2525
import { TestLibrary } from "../TestExplorer/TestRunner";
26-
import { DisposableFileCollection } from "../utilities/tempFolder";
26+
import { DisposableFileCollection, TemporaryFolder } from "../utilities/tempFolder";
2727
import { TargetType } from "../SwiftPackage";
2828
import { TestingConfigurationFactory } from "../debugger/buildConfig";
2929
import { TestKind } from "../TestExplorer/TestKind";
@@ -35,13 +35,11 @@ interface CodeCovFile {
3535

3636
export class TestCoverage {
3737
private lcovFiles: CodeCovFile[] = [];
38-
private lcovTmpFiles: DisposableFileCollection;
38+
private _lcovTmpFiles?: DisposableFileCollection;
39+
private _lcovTmpFilesInit?: Promise<DisposableFileCollection>;
3940
private coverageDetails = new Map<vscode.Uri, vscode.FileCoverageDetail[]>();
4041

41-
constructor(private folderContext: FolderContext) {
42-
const tmpFolder = folderContext.workspaceContext.tempFolder;
43-
this.lcovTmpFiles = tmpFolder.createDisposableFileCollection();
44-
}
42+
constructor(private folderContext: FolderContext) {}
4543

4644
/**
4745
* Returns coverage information for the suppplied URI.
@@ -60,7 +58,7 @@ export class TestCoverage {
6058
true
6159
);
6260
const result = await asyncfs.readFile(`${buildDirectory}/debug/codecov/default.profdata`);
63-
const filename = this.lcovTmpFiles.file(testLibrary, "profdata");
61+
const filename = (await this.lcovTmpFiles()).file(testLibrary, "profdata");
6462
await asyncfs.writeFile(filename, result);
6563
this.lcovFiles.push({ testLibrary, path: filename });
6664
}
@@ -88,14 +86,14 @@ export class TestCoverage {
8886
this.coverageDetails.set(uri, detailedCoverage);
8987
}
9088
}
91-
await this.lcovTmpFiles.dispose();
89+
await this._lcovTmpFiles?.dispose();
9290
}
9391

9492
/**
9593
* Merges multiple `.profdata` files into a single `.profdata` file.
9694
*/
9795
private async mergeProfdata(profDataFiles: string[]) {
98-
const filename = this.lcovTmpFiles.file("merged", "profdata");
96+
const filename = (await this.lcovTmpFiles()).file("merged", "profdata");
9997
const toolchain = this.folderContext.toolchain;
10098
const llvmProfdata = toolchain.getToolchainExecutable("llvm-profdata");
10199
await execFileStreamOutput(
@@ -196,6 +194,27 @@ export class TestCoverage {
196194
return buffer;
197195
}
198196

197+
/**
198+
* Lazily creates (once) and returns the disposable file collection used for LCOV processing.
199+
* Safe against concurrent callers.
200+
*/
201+
private async lcovTmpFiles(): Promise<DisposableFileCollection> {
202+
if (this._lcovTmpFiles) {
203+
return this._lcovTmpFiles;
204+
}
205+
206+
// Use an internal promise to avoid duplicate folder creation in concurrent calls.
207+
if (!this._lcovTmpFilesInit) {
208+
this._lcovTmpFilesInit = (async () => {
209+
const tempFolder = await TemporaryFolder.create();
210+
this._lcovTmpFiles = tempFolder.createDisposableFileCollection();
211+
return this._lcovTmpFiles;
212+
})();
213+
}
214+
215+
return (await this._lcovTmpFilesInit)!;
216+
}
217+
199218
/**
200219
* Constructs a string containing all the paths to exclude from the code coverage report.
201220
* This should exclude everything in the `.build` folder as well as all the test targets.

src/extension.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ export async function activate(context: vscode.ExtensionContext): Promise<Api> {
8282
};
8383
}
8484

85-
const workspaceContext = await WorkspaceContext.create(context, logger, toolchain);
85+
const workspaceContext = new WorkspaceContext(context, logger, toolchain);
8686
context.subscriptions.push(workspaceContext);
8787

8888
context.subscriptions.push(new SwiftEnvironmentVariablesManager(context));

0 commit comments

Comments
 (0)