Skip to content

Commit 97ec9bb

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 1637ecb commit 97ec9bb

28 files changed

+418
-326
lines changed

src/FolderContext.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,12 @@ export class FolderContext implements vscode.Disposable {
7070
): Promise<FolderContext> {
7171
const statusItemText = `Loading Package (${FolderContext.uriName(folder)})`;
7272
workspaceContext.statusItem.start(statusItemText);
73-
7473
const { linuxMain, swiftPackage } =
7574
await workspaceContext.statusItem.showStatusWhileRunning(statusItemText, async () => {
7675
const linuxMain = await LinuxMain.create(folder);
7776
const swiftPackage = await SwiftPackage.create(folder, workspaceContext.toolchain);
7877
return { linuxMain, swiftPackage };
7978
});
80-
8179
workspaceContext.statusItem.end(statusItemText);
8280

8381
const folderContext = new FolderContext(
@@ -88,7 +86,7 @@ export class FolderContext implements vscode.Disposable {
8886
workspaceContext
8987
);
9088

91-
const error = swiftPackage.error;
89+
const error = await swiftPackage.error;
9290
if (error) {
9391
vscode.window.showErrorMessage(
9492
`Failed to load ${folderContext.name}/Package.swift: ${error.message}`
@@ -98,7 +96,6 @@ export class FolderContext implements vscode.Disposable {
9896
folderContext.name
9997
);
10098
}
101-
10299
return folderContext;
103100
}
104101

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

src/SwiftPackage.ts

Lines changed: 77 additions & 39 deletions
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

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,9 @@ import {
1919
TextDocumentTestsRequest,
2020
WorkspaceTestsRequest,
2121
} from "../sourcekit-lsp/extensions";
22-
import { SwiftPackage, TargetType } from "../SwiftPackage";
23-
import {
24-
checkExperimentalCapability,
25-
LanguageClientManager,
26-
} from "../sourcekit-lsp/LanguageClientManager";
22+
import { checkExperimentalCapability } from "../sourcekit-lsp/LanguageClientManager";
23+
import { SwiftPackage } from "../SwiftPackage";
24+
import { LanguageClientManager } from "../sourcekit-lsp/LanguageClientManager";
2725
import { LanguageClient } from "vscode-languageclient/node";
2826

2927
/**
@@ -71,7 +69,7 @@ export class LSPTestDiscovery {
7169
// workspace/tests method, and is at least version 2.
7270
if (checkExperimentalCapability(client, WorkspaceTestsRequest.method, 2)) {
7371
const tests = await client.sendRequest(WorkspaceTestsRequest.type, token);
74-
return this.transformToTestClass(client, swiftPackage, tests);
72+
return await this.transformToTestClass(client, swiftPackage, tests);
7573
} else {
7674
throw new Error(`${WorkspaceTestsRequest.method} requests not supported`);
7775
}
@@ -82,38 +80,41 @@ export class LSPTestDiscovery {
8280
* Convert from `LSPTestItem[]` to `TestDiscovery.TestClass[]`,
8381
* updating the format of the location.
8482
*/
85-
private transformToTestClass(
83+
private async transformToTestClass(
8684
client: LanguageClient,
8785
swiftPackage: SwiftPackage,
8886
input: LSPTestItem[]
89-
): TestDiscovery.TestClass[] {
90-
return input.map(item => {
87+
): Promise<TestDiscovery.TestClass[]> {
88+
let result: TestDiscovery.TestClass[] = [];
89+
for (const item of input) {
9190
const location = client.protocol2CodeConverter.asLocation(item.location);
92-
return {
93-
...item,
94-
id: this.transformId(item, location, swiftPackage),
95-
children: this.transformToTestClass(client, swiftPackage, item.children),
96-
location,
97-
};
98-
});
91+
result = [
92+
...result,
93+
{
94+
...item,
95+
id: await this.transformId(item, location, swiftPackage),
96+
children: await this.transformToTestClass(client, swiftPackage, item.children),
97+
location,
98+
},
99+
];
100+
}
101+
return result;
99102
}
100103

101104
/**
102105
* If the test is an XCTest, transform the ID provided by the LSP from a
103106
* swift-testing style ID to one that XCTest can use. This allows the ID to
104107
* be used to tell to the test runner (xctest or swift-testing) which tests to run.
105108
*/
106-
private transformId(
109+
private async transformId(
107110
item: LSPTestItem,
108111
location: vscode.Location,
109112
swiftPackage: SwiftPackage
110-
): string {
113+
): Promise<string> {
111114
// XCTest: Target.TestClass/testCase
112115
// swift-testing: TestClass/testCase()
113116
// TestClassOrStruct/NestedTestSuite/testCase()
114-
const target = swiftPackage
115-
.getTargets(TargetType.test)
116-
.find(target => swiftPackage.getTarget(location.uri.fsPath) === target);
117+
const target = await swiftPackage.getTarget(location.uri.fsPath);
117118

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

src/TestExplorer/TestDiscovery.ts

Lines changed: 17 additions & 10 deletions
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)