Skip to content

Commit fa56bbf

Browse files
committed
Avoid blocking folder addition on package loading
When a folder is added to a VSCode workspace we perform a `swift package describe` to load information about the package. This operation must complete before the folder is added to the `workspaceContext`, which prevents the user from taking many actions in the extension that could be made available right away. A `swift package describe` operation can potentially take a good amount of time if package resolution has not been performed. Work around this limitation by wrapping the data pulled from this `package describe` behind promises, allowing clients to wait for package resolution to complete only if they need information that is gated by it. Issue: #1250
1 parent d2990b3 commit fa56bbf

28 files changed

+416
-322
lines changed

src/FolderContext.ts

+3-6
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,12 @@ export class FolderContext implements vscode.Disposable {
6969
): Promise<FolderContext> {
7070
const statusItemText = `Loading Package (${FolderContext.uriName(folder)})`;
7171
workspaceContext.statusItem.start(statusItemText);
72-
7372
const { linuxMain, swiftPackage } =
7473
await workspaceContext.statusItem.showStatusWhileRunning(statusItemText, async () => {
7574
const linuxMain = await LinuxMain.create(folder);
7675
const swiftPackage = await SwiftPackage.create(folder, workspaceContext.toolchain);
7776
return { linuxMain, swiftPackage };
7877
});
79-
8078
workspaceContext.statusItem.end(statusItemText);
8179

8280
const folderContext = new FolderContext(
@@ -87,7 +85,7 @@ export class FolderContext implements vscode.Disposable {
8785
workspaceContext
8886
);
8987

90-
const error = swiftPackage.error;
88+
const error = await swiftPackage.error;
9189
if (error) {
9290
vscode.window.showErrorMessage(
9391
`Failed to load ${folderContext.name}/Package.swift: ${error.message}`
@@ -97,7 +95,6 @@ export class FolderContext implements vscode.Disposable {
9795
folderContext.name
9896
);
9997
}
100-
10198
return folderContext;
10299
}
103100

@@ -186,11 +183,11 @@ export class FolderContext implements vscode.Disposable {
186183
* @param uri URI to find target for
187184
* @returns Target
188185
*/
189-
getTestTarget(uri: vscode.Uri, type?: TargetType): Target | undefined {
186+
async getTestTarget(uri: vscode.Uri, type?: TargetType): Promise<Target | undefined> {
190187
if (!isPathInsidePath(uri.fsPath, this.folder.fsPath)) {
191188
return undefined;
192189
}
193-
const testTargets = this.swiftPackage.getTargets(type);
190+
const testTargets = await this.swiftPackage.getTargets(type);
194191
const target = testTargets.find(element => {
195192
const relativeUri = path.relative(
196193
path.join(this.folder.fsPath, element.path),

src/SwiftPackage.ts

+77-39
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { SwiftToolchain } from "./toolchain/toolchain";
2121
import { BuildFlags } from "./toolchain/BuildFlags";
2222

2323
/** Swift Package Manager contents */
24-
export interface PackageContents {
24+
interface PackageContents {
2525
name: string;
2626
products: Product[];
2727
dependencies: Dependency[];
@@ -184,8 +184,10 @@ function isError(state: SwiftPackageState): state is Error {
184184
/**
185185
* Class holding Swift Package Manager Package
186186
*/
187-
export class SwiftPackage implements PackageContents {
187+
export class SwiftPackage {
188188
public plugins: PackagePlugin[] = [];
189+
private _contents: SwiftPackageState | undefined;
190+
189191
/**
190192
* SwiftPackage Constructor
191193
* @param folder folder package is in
@@ -194,7 +196,7 @@ export class SwiftPackage implements PackageContents {
194196
*/
195197
private constructor(
196198
readonly folder: vscode.Uri,
197-
private contents: SwiftPackageState,
199+
private contentsPromise: Promise<SwiftPackageState>,
198200
public resolved: PackageResolved | undefined,
199201
private workspaceState: WorkspaceState | undefined
200202
) {}
@@ -208,10 +210,34 @@ export class SwiftPackage implements PackageContents {
208210
folder: vscode.Uri,
209211
toolchain: SwiftToolchain
210212
): Promise<SwiftPackage> {
211-
const contents = await SwiftPackage.loadPackage(folder, toolchain);
212-
const resolved = await SwiftPackage.loadPackageResolved(folder);
213-
const workspaceState = await SwiftPackage.loadWorkspaceState(folder);
214-
return new SwiftPackage(folder, contents, resolved, workspaceState);
213+
const [resolved, workspaceState] = await Promise.all([
214+
SwiftPackage.loadPackageResolved(folder),
215+
SwiftPackage.loadWorkspaceState(folder),
216+
]);
217+
return new SwiftPackage(
218+
folder,
219+
SwiftPackage.loadPackage(folder, toolchain),
220+
resolved,
221+
workspaceState
222+
);
223+
}
224+
225+
/**
226+
* Returns the package state once it has loaded.
227+
* A snapshot of the state is stored in `_contents` after initial resolution.
228+
*/
229+
private get contents(): Promise<SwiftPackageState> {
230+
return this.contentsPromise.then(contents => {
231+
// If `reload` is called immediately its possible for it to resolve
232+
// before the initial contentsPromise resolution. In that case return
233+
// the newer loaded `_contents`.
234+
if (this._contents === undefined) {
235+
this._contents = contents;
236+
return contents;
237+
} else {
238+
return this._contents;
239+
}
240+
});
215241
}
216242

217243
/**
@@ -326,7 +352,9 @@ export class SwiftPackage implements PackageContents {
326352

327353
/** Reload swift package */
328354
public async reload(toolchain: SwiftToolchain) {
329-
this.contents = await SwiftPackage.loadPackage(this.folder, toolchain);
355+
const loadedContents = await SwiftPackage.loadPackage(this.folder, toolchain);
356+
this._contents = loadedContents;
357+
this.contentsPromise = Promise.resolve(loadedContents);
330358
}
331359

332360
/** Reload Package.resolved file */
@@ -343,31 +371,26 @@ export class SwiftPackage implements PackageContents {
343371
}
344372

345373
/** Return if has valid contents */
346-
public get isValid(): boolean {
347-
return isPackage(this.contents);
374+
public get isValid(): Promise<boolean> {
375+
return this.contents.then(contents => isPackage(contents));
348376
}
349377

350378
/** Load error */
351-
public get error(): Error | undefined {
352-
if (isError(this.contents)) {
353-
return this.contents;
354-
} else {
355-
return undefined;
356-
}
379+
public get error(): Promise<Error | undefined> {
380+
return this.contents.then(contents => (isError(contents) ? contents : undefined));
357381
}
358382

359383
/** Did we find a Package.swift */
360-
public get foundPackage(): boolean {
361-
return this.contents !== undefined;
384+
public get foundPackage(): Promise<boolean> {
385+
return this.contents.then(contents => contents !== undefined);
362386
}
363387

364-
public rootDependencies(): ResolvedDependency[] {
388+
public get rootDependencies(): Promise<ResolvedDependency[]> {
365389
// Correlate the root dependencies found in the Package.swift with their
366390
// checked out versions in the workspace-state.json.
367-
const result = this.dependencies.map(dependency =>
368-
this.resolveDependencyAgainstWorkspaceState(dependency)
391+
return this.dependencies.then(dependencies =>
392+
dependencies.map(dependency => this.resolveDependencyAgainstWorkspaceState(dependency))
369393
);
370-
return result;
371394
}
372395

373396
private resolveDependencyAgainstWorkspaceState(dependency: Dependency): ResolvedDependency {
@@ -443,55 +466,70 @@ export class SwiftPackage implements PackageContents {
443466
}
444467
}
445468

446-
/** name of Swift Package */
447-
get name(): string {
448-
return (this.contents as PackageContents)?.name ?? "";
469+
/** getName of Swift Package */
470+
get name(): Promise<string> {
471+
return this.contents.then(contents => (contents as PackageContents)?.name ?? "");
449472
}
450473

451474
/** array of products in Swift Package */
452-
get products(): Product[] {
453-
return (this.contents as PackageContents)?.products ?? [];
475+
private get products(): Promise<Product[]> {
476+
return this.contents.then(contents => (contents as PackageContents)?.products ?? []);
454477
}
455478

456479
/** array of dependencies in Swift Package */
457-
get dependencies(): Dependency[] {
458-
return (this.contents as PackageContents)?.dependencies ?? [];
480+
get dependencies(): Promise<Dependency[]> {
481+
return this.contents.then(contents => (contents as PackageContents)?.dependencies ?? []);
459482
}
460483

461484
/** array of targets in Swift Package */
462-
get targets(): Target[] {
463-
return (this.contents as PackageContents)?.targets ?? [];
485+
get targets(): Promise<Target[]> {
486+
return this.contents.then(contents => (contents as PackageContents)?.targets ?? []);
464487
}
465488

466489
/** array of executable products in Swift Package */
467-
get executableProducts(): Product[] {
468-
return this.products.filter(product => product.type.executable !== undefined);
490+
get executableProducts(): Promise<Product[]> {
491+
return this.products.then(products =>
492+
products.filter(product => product.type.executable !== undefined)
493+
);
469494
}
470495

471496
/** array of library products in Swift Package */
472-
get libraryProducts(): Product[] {
473-
return this.products.filter(product => product.type.library !== undefined);
497+
get libraryProducts(): Promise<Product[]> {
498+
return this.products.then(products =>
499+
products.filter(product => product.type.library !== undefined)
500+
);
501+
}
502+
503+
/**
504+
* Array of targets in Swift Package. The targets may not be loaded yet.
505+
* It is preferable to use the `targets` property that returns a promise that
506+
* returns the targets when they're guarenteed to be resolved.
507+
**/
508+
get currentTargets(): Target[] {
509+
return (this._contents as unknown as { targets: Target[] })?.targets ?? [];
474510
}
475511

476512
/**
477513
* Return array of targets of a certain type
478514
* @param type Type of target
479515
* @returns Array of targets
480516
*/
481-
getTargets(type?: TargetType): Target[] {
517+
async getTargets(type?: TargetType): Promise<Target[]> {
482518
if (type === undefined) {
483519
return this.targets;
484520
} else {
485-
return this.targets.filter(target => target.type === type);
521+
return this.targets.then(targets => targets.filter(target => target.type === type));
486522
}
487523
}
488524

489525
/**
490526
* Get target for file
491527
*/
492-
getTarget(file: string): Target | undefined {
528+
async getTarget(file: string): Promise<Target | undefined> {
493529
const filePath = path.relative(this.folder.fsPath, file);
494-
return this.targets.find(target => isPathInsidePath(filePath, target.path));
530+
return this.targets.then(targets =>
531+
targets.find(target => isPathInsidePath(filePath, target.path))
532+
);
495533
}
496534

497535
private static trimStdout(stdout: string): string {

src/TestExplorer/LSPTestDiscovery.ts

+20-17
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
TextDocumentTestsRequest,
2020
WorkspaceTestsRequest,
2121
} from "../sourcekit-lsp/extensions";
22-
import { SwiftPackage, TargetType } from "../SwiftPackage";
22+
import { SwiftPackage } from "../SwiftPackage";
2323
import { LanguageClientManager } from "../sourcekit-lsp/LanguageClientManager";
2424
import { LanguageClient } from "vscode-languageclient/node";
2525

@@ -68,7 +68,7 @@ export class LSPTestDiscovery {
6868
// workspace/tests method, and is at least version 2.
6969
if (this.checkExperimentalCapability(client, WorkspaceTestsRequest.method, 2)) {
7070
const tests = await client.sendRequest(WorkspaceTestsRequest.type, token);
71-
return this.transformToTestClass(client, swiftPackage, tests);
71+
return await this.transformToTestClass(client, swiftPackage, tests);
7272
} else {
7373
throw new Error(`${WorkspaceTestsRequest.method} requests not supported`);
7474
}
@@ -96,38 +96,41 @@ export class LSPTestDiscovery {
9696
* Convert from `LSPTestItem[]` to `TestDiscovery.TestClass[]`,
9797
* updating the format of the location.
9898
*/
99-
private transformToTestClass(
99+
private async transformToTestClass(
100100
client: LanguageClient,
101101
swiftPackage: SwiftPackage,
102102
input: LSPTestItem[]
103-
): TestDiscovery.TestClass[] {
104-
return input.map(item => {
103+
): Promise<TestDiscovery.TestClass[]> {
104+
let result: TestDiscovery.TestClass[] = [];
105+
for (const item of input) {
105106
const location = client.protocol2CodeConverter.asLocation(item.location);
106-
return {
107-
...item,
108-
id: this.transformId(item, location, swiftPackage),
109-
children: this.transformToTestClass(client, swiftPackage, item.children),
110-
location,
111-
};
112-
});
107+
result = [
108+
...result,
109+
{
110+
...item,
111+
id: await this.transformId(item, location, swiftPackage),
112+
children: await this.transformToTestClass(client, swiftPackage, item.children),
113+
location,
114+
},
115+
];
116+
}
117+
return result;
113118
}
114119

115120
/**
116121
* If the test is an XCTest, transform the ID provided by the LSP from a
117122
* swift-testing style ID to one that XCTest can use. This allows the ID to
118123
* be used to tell to the test runner (xctest or swift-testing) which tests to run.
119124
*/
120-
private transformId(
125+
private async transformId(
121126
item: LSPTestItem,
122127
location: vscode.Location,
123128
swiftPackage: SwiftPackage
124-
): string {
129+
): Promise<string> {
125130
// XCTest: Target.TestClass/testCase
126131
// swift-testing: TestClass/testCase()
127132
// TestClassOrStruct/NestedTestSuite/testCase()
128-
const target = swiftPackage
129-
.getTargets(TargetType.test)
130-
.find(target => swiftPackage.getTarget(location.uri.fsPath) === target);
133+
const target = await swiftPackage.getTarget(location.uri.fsPath);
131134

132135
// If we're using an older sourcekit-lsp it doesn't prepend the target name
133136
// to the test item id.

src/TestExplorer/TestDiscovery.ts

+17-10
Original file line numberDiff line numberDiff line change
@@ -43,27 +43,34 @@ const defaultTags = [runnableTag.id, "test-target", "XCTest", "swift-testing"];
4343
* @param swiftPackage A swift package containing test targets
4444
* @param testClasses Array of test classes
4545
*/
46-
export function updateTestsFromClasses(
46+
export async function updateTestsFromClasses(
4747
testController: vscode.TestController,
4848
swiftPackage: SwiftPackage,
4949
testItems: TestClass[]
5050
) {
51-
const targets = swiftPackage.getTargets(TargetType.test).map(target => {
52-
const filteredItems = testItems.filter(
53-
testItem =>
54-
testItem.location && swiftPackage.getTarget(testItem.location.uri.fsPath) === target
55-
);
56-
return {
51+
const targets = await swiftPackage.getTargets(TargetType.test);
52+
const results: TestClass[] = [];
53+
for (const target of targets) {
54+
const filteredItems: TestClass[] = [];
55+
for (const testItem of testItems) {
56+
if (
57+
testItem.location &&
58+
(await swiftPackage.getTarget(testItem.location.uri.fsPath)) === target
59+
) {
60+
filteredItems.push(testItem);
61+
}
62+
}
63+
results.push({
5764
id: target.c99name,
5865
label: target.name,
5966
children: filteredItems,
6067
location: undefined,
6168
disabled: false,
6269
style: "test-target",
6370
tags: [],
64-
} as TestClass;
65-
});
66-
updateTests(testController, targets);
71+
});
72+
}
73+
updateTests(testController, results);
6774
}
6875

6976
export function updateTestsForTarget(

0 commit comments

Comments
 (0)