Skip to content

Commit

Permalink
Avoid blocking folder addition on package loading
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@plemarquand
plemarquand committed Mar 4, 2025
1 parent d2990b3 commit e4cc8c9
Show file tree
Hide file tree
Showing 28 changed files with 396 additions and 309 deletions.
9 changes: 3 additions & 6 deletions src/FolderContext.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,12 @@ export class FolderContext implements vscode.Disposable {
): Promise<FolderContext> {
const statusItemText = `Loading Package (${FolderContext.uriName(folder)})`;
workspaceContext.statusItem.start(statusItemText);

const { linuxMain, swiftPackage } =
await workspaceContext.statusItem.showStatusWhileRunning(statusItemText, async () => {
const linuxMain = await LinuxMain.create(folder);
const swiftPackage = await SwiftPackage.create(folder, workspaceContext.toolchain);
return { linuxMain, swiftPackage };
});

workspaceContext.statusItem.end(statusItemText);

const folderContext = new FolderContext(
Expand All @@ -87,7 +85,7 @@ export class FolderContext implements vscode.Disposable {
workspaceContext
);

const error = swiftPackage.error;
const error = await swiftPackage.error;
if (error) {
vscode.window.showErrorMessage(
`Failed to load ${folderContext.name}/Package.swift: ${error.message}`
Expand All @@ -97,7 +95,6 @@ export class FolderContext implements vscode.Disposable {
folderContext.name
);
}

return folderContext;
}

Expand Down Expand Up @@ -186,11 +183,11 @@ export class FolderContext implements vscode.Disposable {
* @param uri URI to find target for
* @returns Target
*/
getTestTarget(uri: vscode.Uri, type?: TargetType): Target | undefined {
async getTestTarget(uri: vscode.Uri, type?: TargetType): Promise<Target | undefined> {
if (!isPathInsidePath(uri.fsPath, this.folder.fsPath)) {
return undefined;
}
const testTargets = this.swiftPackage.getTargets(type);
const testTargets = await this.swiftPackage.getTargets(type);
const target = testTargets.find(element => {
const relativeUri = path.relative(
path.join(this.folder.fsPath, element.path),
Expand Down
116 changes: 77 additions & 39 deletions src/SwiftPackage.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { SwiftToolchain } from "./toolchain/toolchain";
import { BuildFlags } from "./toolchain/BuildFlags";

/** Swift Package Manager contents */
export interface PackageContents {
interface PackageContents {
name: string;
products: Product[];
dependencies: Dependency[];
Expand Down Expand Up @@ -184,8 +184,10 @@ function isError(state: SwiftPackageState): state is Error {
/**
* Class holding Swift Package Manager Package
*/
export class SwiftPackage implements PackageContents {
export class SwiftPackage {
public plugins: PackagePlugin[] = [];
private _contents: SwiftPackageState | undefined;

/**
* SwiftPackage Constructor
* @param folder folder package is in
Expand All @@ -194,7 +196,7 @@ export class SwiftPackage implements PackageContents {
*/
private constructor(
readonly folder: vscode.Uri,
private contents: SwiftPackageState,
private contentsPromise: Promise<SwiftPackageState>,
public resolved: PackageResolved | undefined,
private workspaceState: WorkspaceState | undefined
) {}
Expand All @@ -208,10 +210,34 @@ export class SwiftPackage implements PackageContents {
folder: vscode.Uri,
toolchain: SwiftToolchain
): Promise<SwiftPackage> {
const contents = await SwiftPackage.loadPackage(folder, toolchain);
const resolved = await SwiftPackage.loadPackageResolved(folder);
const workspaceState = await SwiftPackage.loadWorkspaceState(folder);
return new SwiftPackage(folder, contents, resolved, workspaceState);
const [resolved, workspaceState] = await Promise.all([
SwiftPackage.loadPackageResolved(folder),
SwiftPackage.loadWorkspaceState(folder),
]);
return new SwiftPackage(
folder,
SwiftPackage.loadPackage(folder, toolchain),
resolved,
workspaceState
);
}

/**
* Returns the package state once it has loaded.
* A snapshot of the state is stored in `_contents` after initial resolution.
*/
private get contents(): Promise<SwiftPackageState> {
return this.contentsPromise.then(contents => {
// If `reload` is called immediately its possible for it to resolve
// before the initial contentsPromise resolution. In that case return
// the newer loaded `_contents`.
if (this._contents === undefined) {
this._contents = contents;
return contents;
} else {
return this._contents;
}
});
}

/**
Expand Down Expand Up @@ -326,7 +352,9 @@ export class SwiftPackage implements PackageContents {

/** Reload swift package */
public async reload(toolchain: SwiftToolchain) {
this.contents = await SwiftPackage.loadPackage(this.folder, toolchain);
const loadedContents = await SwiftPackage.loadPackage(this.folder, toolchain);
this._contents = loadedContents;
this.contentsPromise = Promise.resolve(loadedContents);
}

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

/** Return if has valid contents */
public get isValid(): boolean {
return isPackage(this.contents);
public get isValid(): Promise<boolean> {
return this.contents.then(contents => isPackage(contents));
}

/** Load error */
public get error(): Error | undefined {
if (isError(this.contents)) {
return this.contents;
} else {
return undefined;
}
public get error(): Promise<Error | undefined> {
return this.contents.then(contents => (isError(contents) ? contents : undefined));
}

/** Did we find a Package.swift */
public get foundPackage(): boolean {
return this.contents !== undefined;
public get foundPackage(): Promise<boolean> {
return this.contents.then(contents => contents !== undefined);
}

public rootDependencies(): ResolvedDependency[] {
public get rootDependencies(): Promise<ResolvedDependency[]> {
// Correlate the root dependencies found in the Package.swift with their
// checked out versions in the workspace-state.json.
const result = this.dependencies.map(dependency =>
this.resolveDependencyAgainstWorkspaceState(dependency)
return this.dependencies.then(dependencies =>
dependencies.map(dependency => this.resolveDependencyAgainstWorkspaceState(dependency))
);
return result;
}

private resolveDependencyAgainstWorkspaceState(dependency: Dependency): ResolvedDependency {
Expand Down Expand Up @@ -443,55 +466,70 @@ export class SwiftPackage implements PackageContents {
}
}

/** name of Swift Package */
get name(): string {
return (this.contents as PackageContents)?.name ?? "";
/** getName of Swift Package */
get name(): Promise<string> {
return this.contents.then(contents => (contents as PackageContents)?.name ?? "");
}

/** array of products in Swift Package */
get products(): Product[] {
return (this.contents as PackageContents)?.products ?? [];
get products(): Promise<Product[]> {
return this.contents.then(contents => (contents as PackageContents)?.products ?? []);
}

/** array of dependencies in Swift Package */
get dependencies(): Dependency[] {
return (this.contents as PackageContents)?.dependencies ?? [];
get dependencies(): Promise<Dependency[]> {
return this.contents.then(contents => (contents as PackageContents)?.dependencies ?? []);
}

/** array of targets in Swift Package */
get targets(): Target[] {
return (this.contents as PackageContents)?.targets ?? [];
get targets(): Promise<Target[]> {
return this.contents.then(contents => (contents as PackageContents)?.targets ?? []);
}

/** array of executable products in Swift Package */
get executableProducts(): Product[] {
return this.products.filter(product => product.type.executable !== undefined);
get executableProducts(): Promise<Product[]> {
return this.products.then(products =>
products.filter(product => product.type.executable !== undefined)
);
}

/** array of library products in Swift Package */
get libraryProducts(): Product[] {
return this.products.filter(product => product.type.library !== undefined);
get libraryProducts(): Promise<Product[]> {
return this.products.then(products =>
products.filter(product => product.type.library !== undefined)
);
}

/**
* Array of targets in Swift Package. The targets may not be loaded yet.
* It is preferable to use the `targets` property that returns a promise that
* returns the targets when they're guarenteed to be resolved.
**/
get currentTargets(): Target[] {
return (this._contents as unknown as { targets: Target[] })?.targets ?? [];
}

/**
* Return array of targets of a certain type
* @param type Type of target
* @returns Array of targets
*/
getTargets(type?: TargetType): Target[] {
async getTargets(type?: TargetType): Promise<Target[]> {
if (type === undefined) {
return this.targets;
} else {
return this.targets.filter(target => target.type === type);
return this.targets.then(targets => targets.filter(target => target.type === type));
}
}

/**
* Get target for file
*/
getTarget(file: string): Target | undefined {
async getTarget(file: string): Promise<Target | undefined> {
const filePath = path.relative(this.folder.fsPath, file);
return this.targets.find(target => isPathInsidePath(filePath, target.path));
return this.targets.then(targets =>
targets.find(target => isPathInsidePath(filePath, target.path))
);
}

private static trimStdout(stdout: string): string {
Expand Down
37 changes: 20 additions & 17 deletions src/TestExplorer/LSPTestDiscovery.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
TextDocumentTestsRequest,
WorkspaceTestsRequest,
} from "../sourcekit-lsp/extensions";
import { SwiftPackage, TargetType } from "../SwiftPackage";
import { SwiftPackage } from "../SwiftPackage";
import { LanguageClientManager } from "../sourcekit-lsp/LanguageClientManager";
import { LanguageClient } from "vscode-languageclient/node";

Expand Down Expand Up @@ -68,7 +68,7 @@ export class LSPTestDiscovery {
// workspace/tests method, and is at least version 2.
if (this.checkExperimentalCapability(client, WorkspaceTestsRequest.method, 2)) {
const tests = await client.sendRequest(WorkspaceTestsRequest.type, token);
return this.transformToTestClass(client, swiftPackage, tests);
return await this.transformToTestClass(client, swiftPackage, tests);
} else {
throw new Error(`${WorkspaceTestsRequest.method} requests not supported`);
}
Expand Down Expand Up @@ -96,38 +96,41 @@ export class LSPTestDiscovery {
* Convert from `LSPTestItem[]` to `TestDiscovery.TestClass[]`,
* updating the format of the location.
*/
private transformToTestClass(
private async transformToTestClass(
client: LanguageClient,
swiftPackage: SwiftPackage,
input: LSPTestItem[]
): TestDiscovery.TestClass[] {
return input.map(item => {
): Promise<TestDiscovery.TestClass[]> {
let result: TestDiscovery.TestClass[] = [];
for (const item of input) {
const location = client.protocol2CodeConverter.asLocation(item.location);
return {
...item,
id: this.transformId(item, location, swiftPackage),
children: this.transformToTestClass(client, swiftPackage, item.children),
location,
};
});
result = [
...result,
{
...item,
id: await this.transformId(item, location, swiftPackage),
children: await this.transformToTestClass(client, swiftPackage, item.children),
location,
},
];
}
return result;
}

/**
* If the test is an XCTest, transform the ID provided by the LSP from a
* swift-testing style ID to one that XCTest can use. This allows the ID to
* be used to tell to the test runner (xctest or swift-testing) which tests to run.
*/
private transformId(
private async transformId(
item: LSPTestItem,
location: vscode.Location,
swiftPackage: SwiftPackage
): string {
): Promise<string> {
// XCTest: Target.TestClass/testCase
// swift-testing: TestClass/testCase()
// TestClassOrStruct/NestedTestSuite/testCase()
const target = swiftPackage
.getTargets(TargetType.test)
.find(target => swiftPackage.getTarget(location.uri.fsPath) === target);
const target = await swiftPackage.getTarget(location.uri.fsPath);

// If we're using an older sourcekit-lsp it doesn't prepend the target name
// to the test item id.
Expand Down
27 changes: 17 additions & 10 deletions src/TestExplorer/TestDiscovery.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,27 +43,34 @@ const defaultTags = [runnableTag.id, "test-target", "XCTest", "swift-testing"];
* @param swiftPackage A swift package containing test targets
* @param testClasses Array of test classes
*/
export function updateTestsFromClasses(
export async function updateTestsFromClasses(
testController: vscode.TestController,
swiftPackage: SwiftPackage,
testItems: TestClass[]
) {
const targets = swiftPackage.getTargets(TargetType.test).map(target => {
const filteredItems = testItems.filter(
testItem =>
testItem.location && swiftPackage.getTarget(testItem.location.uri.fsPath) === target
);
return {
const targets = await swiftPackage.getTargets(TargetType.test);
const results: TestClass[] = [];
for (const target of targets) {
const filteredItems: TestClass[] = [];
for (const testItem of testItems) {
if (
testItem.location &&
(await swiftPackage.getTarget(testItem.location.uri.fsPath)) === target
) {
filteredItems.push(testItem);
}
}
results.push({
id: target.c99name,
label: target.name,
children: filteredItems,
location: undefined,
disabled: false,
style: "test-target",
tags: [],
} as TestClass;
});
updateTests(testController, targets);
});
}
updateTests(testController, results);
}

export function updateTestsForTarget(
Expand Down
Loading

0 comments on commit e4cc8c9

Please sign in to comment.