Remove the update method from the Discovery class

See #2490 (comment)
for more explanation. This will make the class more useful for future usecases
where we don't want the behaviour of only calling update when there isn't
another refresh scheduled. I also think it doesn't negatively affect other
users such as the query test discovery. The effect should be that we'll see
more updates to the UI. These updates will get overwritten quickly, but they
are all genuine snapshots of the filesystem at the point the discovery process
ran, so they aren't incorrect, or aren't more incorrect than continuing to show
the old state before any discovery ran.

Author: robertbrignull

extensions/ql-vscode/src/common/discovery.ts

@@ -7,7 +7,7 @@ import { Logger } from "./logging";
  * files. This class automatically prevents more than one discovery operation from running at the
  * same time.
  */
-export abstract class Discovery<T> extends DisposableObject {
+export abstract class Discovery extends DisposableObject {
   private restartWhenFinished = false;
   private currentDiscoveryPromise: Promise<void> | undefined;
 
@@ -64,14 +64,12 @@ export abstract class Discovery<T> extends DisposableObject {
    * discovery.
    */
   private async launchDiscovery(): Promise<void> {
-    let results: T | undefined;
     try {
-      results = await this.discover();
+      await this.discover();
     } catch (err) {
       void this.logger.log(
         `${this.name} failed. Reason: ${getErrorMessage(err)}`,
       );
-      results = undefined;
     }
 
     if (this.restartWhenFinished) {
@@ -82,24 +80,11 @@ export abstract class Discovery<T> extends DisposableObject {
       // succeeded or failed.
       this.restartWhenFinished = false;
       await this.launchDiscovery();
-    } else {
-      // If the discovery was successful, then update any listeners with the results.
-      if (results !== undefined) {
-        this.update(results);
-      }
     }
   }
 
   /**
    * Overridden by the derived class to spawn the actual discovery operation, returning the results.
    */
-  protected abstract discover(): Promise<T>;
-
-  /**
-   * Overridden by the derived class to atomically update the `Discovery` object with the results of
-   * the discovery operation, and to notify any listeners that the discovery results may have
-   * changed.
-   * @param results The discovery results returned by the `discover` function.
-   */
-  protected abstract update(results: T): void;
+  protected abstract discover(): Promise<void>;
 }

extensions/ql-vscode/src/queries-panel/query-discovery.ts

@@ -37,11 +37,8 @@ export interface QueryDiscoveryResults {
 /**
  * Discovers all query files contained in the QL packs in a given workspace folder.
  */
-export class QueryDiscovery
-  extends Discovery<QueryDiscoveryResults>
-  implements QueryDiscoverer
-{
-  private results: QueryDiscoveryResults | undefined;
+export class QueryDiscovery extends Discovery implements QueryDiscoverer {
+  private results: Array<FileTreeDirectory<string>> | undefined;
 
   private readonly onDidChangeQueriesEmitter: AppEventEmitter<void>;
   private readonly watcher: MultiFileSystemWatcher = this.push(
@@ -60,7 +57,7 @@ export class QueryDiscovery
   }
 
   public get queries(): Array<FileTreeDirectory<string>> | undefined {
-    return this.results?.queries;
+    return this.results;
   }
 
   /**
@@ -70,28 +67,13 @@ export class QueryDiscovery
     return this.onDidChangeQueriesEmitter.event;
   }
 
-  protected async discover(): Promise<QueryDiscoveryResults> {
+  protected async discover() {
     const workspaceFolders = getOnDiskWorkspaceFoldersObjects();
-    if (workspaceFolders.length === 0) {
-      return {
-        queries: [],
-        watchPaths: [],
-      };
-    }
-
-    const queries = await this.discoverQueries(workspaceFolders);
-
-    return {
-      queries,
-      watchPaths: workspaceFolders.map((f) => f.uri),
-    };
-  }
 
-  protected update(results: QueryDiscoveryResults): void {
-    this.results = results;
+    this.results = await this.discoverQueries(workspaceFolders);
 
     this.watcher.clear();
-    for (const watchPath of results.watchPaths) {
+    for (const watchPath of workspaceFolders.map((f) => f.uri)) {
       // Watch for changes to any `.ql` file
       this.watcher.addWatch(new RelativePattern(watchPath, "**/*.{ql}"));
       // need to explicitly watch for changes to directories themselves.

extensions/ql-vscode/src/query-testing/qltest-discovery.ts

@@ -14,26 +14,10 @@ import { pathExists } from "fs-extra";
 import { FileTreeDirectory, FileTreeLeaf } from "../common/file-tree-nodes";
 import { extLogger } from "../common";
 
-/**
- * The results of discovering QL tests.
- */
-interface QLTestDiscoveryResults {
-  /**
-   * A directory that contains one or more QL Tests, or other QLTestDirectories.
-   */
-  testDirectory: FileTreeDirectory | undefined;
-
-  /**
-   * The file system path to a directory to watch. If any ql or qlref file changes in
-   * this directory, then this signifies a change in tests.
-   */
-  watchPath: string;
-}
-
 /**
  * Discovers all QL tests contained in the QL packs in a given workspace folder.
  */
-export class QLTestDiscovery extends Discovery<QLTestDiscoveryResults> {
+export class QLTestDiscovery extends Discovery {
   private readonly _onDidChangeTests = this.push(new EventEmitter<void>());
   private readonly watcher: MultiFileSystemWatcher = this.push(
     new MultiFileSystemWatcher(),
@@ -69,24 +53,18 @@ export class QLTestDiscovery extends Discovery<QLTestDiscoveryResults> {
       void this.refresh();
     }
   }
-  protected async discover(): Promise<QLTestDiscoveryResults> {
-    const testDirectory = await this.discoverTests();
-    return {
-      testDirectory,
-      watchPath: this.workspaceFolder.uri.fsPath,
-    };
-  }
-
-  protected update(results: QLTestDiscoveryResults): void {
-    this._testDirectory = results.testDirectory;
+  protected async discover() {
+    this._testDirectory = await this.discoverTests();
 
     this.watcher.clear();
     // Watch for changes to any `.ql` or `.qlref` file in any of the QL packs that contain tests.
     this.watcher.addWatch(
-      new RelativePattern(results.watchPath, "**/*.{ql,qlref}"),
+      new RelativePattern(this.workspaceFolder.uri.fsPath, "**/*.{ql,qlref}"),
     );
     // need to explicitly watch for changes to directories themselves.
-    this.watcher.addWatch(new RelativePattern(results.watchPath, "**/"));
+    this.watcher.addWatch(
+      new RelativePattern(this.workspaceFolder.uri.fsPath, "**/"),
+    );
     this._onDidChangeTests.fire(undefined);
   }