Avoid blocking folder addition on package loading (#1422)

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

Author: plemarquand

src/FolderContext.ts

@@ -71,14 +71,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(
@@ -89,7 +87,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}`
@@ -99,7 +97,6 @@ export class FolderContext implements vscode.Disposable {
                 folderContext.name
             );
         }
-
         return folderContext;
     }
 
@@ -188,11 +185,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),

src/SwiftPackage.ts

@@ -22,7 +22,7 @@ import { BuildFlags } from "./toolchain/BuildFlags";
 import { SwiftOutputChannel } from "./ui/SwiftOutputChannel";
 
 /** Swift Package Manager contents */
-export interface PackageContents {
+interface PackageContents {
     name: string;
     products: Product[];
     dependencies: Dependency[];
@@ -185,8 +185,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
@@ -195,7 +197,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
     ) {}
@@ -209,10 +211,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;
+            }
+        });
     }
 
     /**
@@ -329,7 +355,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 */
@@ -346,31 +374,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 {
@@ -446,55 +469,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 ?? [];
+    private 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 {

src/TestExplorer/LSPTestDiscovery.ts

@@ -19,11 +19,9 @@ import {
     TextDocumentTestsRequest,
     WorkspaceTestsRequest,
 } from "../sourcekit-lsp/extensions";
-import { SwiftPackage, TargetType } from "../SwiftPackage";
-import {
-    checkExperimentalCapability,
-    LanguageClientManager,
-} from "../sourcekit-lsp/LanguageClientManager";
+import { checkExperimentalCapability } from "../sourcekit-lsp/LanguageClientManager";
+import { SwiftPackage } from "../SwiftPackage";
+import { LanguageClientManager } from "../sourcekit-lsp/LanguageClientManager";
 import { LanguageClient } from "vscode-languageclient/node";
 
 /**
@@ -71,7 +69,7 @@ export class LSPTestDiscovery {
             // workspace/tests method, and is at least version 2.
             if (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`);
             }
@@ -82,38 +80,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.

src/TestExplorer/TestDiscovery.ts

@@ -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(