Merge pull request #2523 from github/robertbrignull/queries-loading

Display different message when queries are loading vs no queries found

Author: robertbrignull

extensions/ql-vscode/package.json

@@ -1699,7 +1699,7 @@
       },
       {
         "view": "codeQLQueries",
-        "contents": "This workspace doesn't contain any CodeQL queries at the moment."
+        "contents": "Looking for queries..."
       },
       {
         "view": "codeQLDatabases",

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

@@ -34,6 +34,12 @@ interface PathData {
  * relevant, and what extra data to compute for each file.
  */
 export abstract class FilePathDiscovery<T extends PathData> extends Discovery {
+  /**
+   * Has `discover` been called. This allows distinguishing between
+   * "no paths found" and not having scanned yet.
+   */
+  private discoverHasCompletedOnce = false;
+
   /** The set of known paths and associated data that we are tracking */
   private pathData: T[] = [];
 
@@ -73,7 +79,10 @@ export abstract class FilePathDiscovery<T extends PathData> extends Discovery {
     this.push(this.watcher.onDidChange(this.fileChanged.bind(this)));
   }
 
-  protected getPathData(): ReadonlyArray<Readonly<T>> {
+  protected getPathData(): ReadonlyArray<Readonly<T>> | undefined {
+    if (!this.discoverHasCompletedOnce) {
+      return undefined;
+    }
     return this.pathData;
   }
 
@@ -118,7 +127,8 @@ export abstract class FilePathDiscovery<T extends PathData> extends Discovery {
     });
 
     this.updateWatchers();
-    return this.refresh();
+    await this.refresh();
+    this.onDidChangePathDataEmitter.fire();
   }
 
   private workspaceFoldersChanged(event: WorkspaceFoldersChangeEvent) {
@@ -171,6 +181,7 @@ export abstract class FilePathDiscovery<T extends PathData> extends Discovery {
       }
     }
 
+    this.discoverHasCompletedOnce = true;
     if (pathsUpdated) {
       this.onDidChangePathDataEmitter.fire();
     }

extensions/ql-vscode/src/local-queries/local-queries.ts

@@ -132,15 +132,20 @@ export class LocalQueries extends DisposableObject {
   private async runQueryFromQueriesPanel(
     queryTreeViewItem: QueryTreeViewItem,
   ): Promise<void> {
-    await this.runQuery(Uri.file(queryTreeViewItem.path));
+    if (queryTreeViewItem.path !== undefined) {
+      await this.runQuery(Uri.file(queryTreeViewItem.path));
+    }
   }
 
   private async runQueriesFromQueriesPanel(
     queryTreeViewItem: QueryTreeViewItem,
   ): Promise<void> {
-    const uris = queryTreeViewItem.children.map((child) =>
-      Uri.file(child.path),
-    );
+    const uris = [];
+    for (const child of queryTreeViewItem.children) {
+      if (child.path !== undefined) {
+        uris.push(Uri.file(child.path));
+      }
+    }
     await this.runQueries(uris);
   }
 

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

@@ -56,10 +56,15 @@ export class QueryDiscovery
    *
    * Trivial directories where there is only one child will be collapsed into a single node.
    */
-  public buildQueryTree(): Array<FileTreeNode<string>> {
+  public buildQueryTree(): Array<FileTreeNode<string>> | undefined {
+    const pathData = this.getPathData();
+    if (pathData === undefined) {
+      return undefined;
+    }
+
     const roots = [];
     for (const workspaceFolder of getOnDiskWorkspaceFoldersObjects()) {
-      const queriesInRoot = this.getPathData().filter((query) =>
+      const queriesInRoot = pathData.filter((query) =>
         containsPath(workspaceFolder.uri.fsPath, query.path),
       );
       if (queriesInRoot.length === 0) {

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

@@ -36,8 +36,13 @@ export class QueryPackDiscovery extends FilePathDiscovery<QueryPack> {
    * or the pack's language is unknown.
    */
   public getLanguageForQueryFile(queryPath: string): QueryLanguage | undefined {
+    const pathData = this.getPathData();
+    if (pathData === undefined) {
+      return undefined;
+    }
+
     // Find all packs in a higher directory than the query
-    const packs = this.getPathData().filter((queryPack) =>
+    const packs = pathData.filter((queryPack) =>
       containsPath(dirname(queryPack.path), queryPath),
     );
 

extensions/ql-vscode/src/queries-panel/query-tree-data-provider.ts

@@ -1,10 +1,15 @@
 import { Event, EventEmitter, TreeDataProvider, TreeItem } from "vscode";
-import { QueryTreeViewItem } from "./query-tree-view-item";
+import {
+  QueryTreeViewItem,
+  createQueryTreeFileItem,
+  createQueryTreeFolderItem,
+  createQueryTreeTextItem,
+} from "./query-tree-view-item";
 import { DisposableObject } from "../common/disposable-object";
 import { FileTreeNode } from "../common/file-tree-nodes";
 
 export interface QueryDiscoverer {
-  readonly buildQueryTree: () => Array<FileTreeNode<string>>;
+  readonly buildQueryTree: () => Array<FileTreeNode<string>> | undefined;
   readonly onDidChangeQueries: Event<void>;
 }
 
@@ -34,19 +39,37 @@ export class QueryTreeDataProvider
   }
 
   private createTree(): QueryTreeViewItem[] {
-    return this.queryDiscoverer
-      .buildQueryTree()
-      .map(this.convertFileTreeNode.bind(this));
+    const queryTree = this.queryDiscoverer.buildQueryTree();
+    if (queryTree === undefined) {
+      return [];
+    } else if (queryTree.length === 0) {
+      return [this.noQueriesTreeViewItem()];
+    } else {
+      return queryTree.map(this.convertFileTreeNode.bind(this));
+    }
   }
 
   private convertFileTreeNode(
     fileTreeDirectory: FileTreeNode<string>,
   ): QueryTreeViewItem {
-    return new QueryTreeViewItem(
-      fileTreeDirectory.name,
-      fileTreeDirectory.path,
-      fileTreeDirectory.data,
-      fileTreeDirectory.children.map(this.convertFileTreeNode.bind(this)),
+    if (fileTreeDirectory.children.length === 0) {
+      return createQueryTreeFileItem(
+        fileTreeDirectory.name,
+        fileTreeDirectory.path,
+        fileTreeDirectory.data,
+      );
+    } else {
+      return createQueryTreeFolderItem(
+        fileTreeDirectory.name,
+        fileTreeDirectory.path,
+        fileTreeDirectory.children.map(this.convertFileTreeNode.bind(this)),
+      );
+    }
+  }
+
+  private noQueriesTreeViewItem(): QueryTreeViewItem {
+    return createQueryTreeTextItem(
+      "This workspace doesn't contain any CodeQL queries at the moment.",
     );
   }
 

extensions/ql-vscode/src/queries-panel/query-tree-view-item.ts

@@ -3,24 +3,43 @@ import * as vscode from "vscode";
 export class QueryTreeViewItem extends vscode.TreeItem {
   constructor(
     name: string,
-    public readonly path: string,
-    language: string | undefined,
+    public readonly path: string | undefined,
     public readonly children: QueryTreeViewItem[],
   ) {
     super(name);
-    this.tooltip = path;
-    if (this.children.length === 0) {
-      this.description = language;
-      this.collapsibleState = vscode.TreeItemCollapsibleState.None;
-      this.contextValue = "queryFile";
-      this.command = {
-        title: "Open",
-        command: "vscode.open",
-        arguments: [vscode.Uri.file(path)],
-      };
-    } else {
-      this.collapsibleState = vscode.TreeItemCollapsibleState.Collapsed;
-      this.contextValue = "queryFolder";
-    }
   }
 }
+
+export function createQueryTreeFolderItem(
+  name: string,
+  path: string,
+  children: QueryTreeViewItem[],
+): QueryTreeViewItem {
+  const item = new QueryTreeViewItem(name, path, children);
+  item.tooltip = path;
+  item.collapsibleState = vscode.TreeItemCollapsibleState.Collapsed;
+  item.contextValue = "queryFolder";
+  return item;
+}
+
+export function createQueryTreeFileItem(
+  name: string,
+  path: string,
+  language: string | undefined,
+): QueryTreeViewItem {
+  const item = new QueryTreeViewItem(name, path, []);
+  item.tooltip = path;
+  item.description = language;
+  item.collapsibleState = vscode.TreeItemCollapsibleState.None;
+  item.contextValue = "queryFile";
+  item.command = {
+    title: "Open",
+    command: "vscode.open",
+    arguments: [vscode.Uri.file(path)],
+  };
+  return item;
+}
+
+export function createQueryTreeTextItem(text: string): QueryTreeViewItem {
+  return new QueryTreeViewItem(text, undefined, []);
+}

extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts

@@ -76,7 +76,7 @@ import {
   showAndLogInformationMessage,
   showAndLogWarningMessage,
 } from "../common/logging";
-import type { QueryTreeViewItem } from "../queries-panel/query-tree-view-item";
+import { QueryTreeViewItem } from "../queries-panel/query-tree-view-item";
 
 const maxRetryCount = 3;
 
@@ -191,7 +191,11 @@ export class VariantAnalysisManager
   private async runVariantAnalysisFromQueriesPanel(
     queryTreeViewItem: QueryTreeViewItem,
   ): Promise<void> {
-    await this.runVariantAnalysisFromCommand(Uri.file(queryTreeViewItem.path));
+    if (queryTreeViewItem.path !== undefined) {
+      await this.runVariantAnalysisFromCommand(
+        Uri.file(queryTreeViewItem.path),
+      );
+    }
   }
 
   public async runVariantAnalysis(

extensions/ql-vscode/test/vscode-tests/minimal-workspace/common/vscode/file-path-discovery.test.ts

@@ -34,7 +34,7 @@ class TestFilePathDiscovery extends FilePathDiscovery<TestData> {
     return this.onDidChangePathData;
   }
 
-  public getPathData(): readonly TestData[] {
+  public getPathData(): readonly TestData[] | undefined {
     return super.getPathData();
   }
 
@@ -123,6 +123,10 @@ describe("FilePathDiscovery", () => {
   });
 
   describe("initialRefresh", () => {
+    it("should return undefined until initialRefresh is called", async () => {
+      expect(discovery.getPathData()).toEqual(undefined);
+    });
+
     it("should handle no files being present", async () => {
       await discovery.initialRefresh();
       expect(discovery.getPathData()).toEqual([]);
@@ -155,6 +159,26 @@ describe("FilePathDiscovery", () => {
         new Set([{ path: join(workspacePath, "1.test"), contents: "1" }]),
       );
     });
+
+    it("should trigger listener when paths are found", async () => {
+      makeTestFile(join(workspacePath, "123.test"));
+
+      const didChangePathsListener = jest.fn();
+      discovery.onDidChangePaths(didChangePathsListener);
+
+      await discovery.initialRefresh();
+
+      expect(didChangePathsListener).toHaveBeenCalled();
+    });
+
+    it("should trigger listener when no paths are found", async () => {
+      const didChangePathsListener = jest.fn();
+      discovery.onDidChangePaths(didChangePathsListener);
+
+      await discovery.initialRefresh();
+
+      expect(didChangePathsListener).toHaveBeenCalled();
+    });
   });
 
   describe("file added", () => {

extensions/ql-vscode/test/vscode-tests/minimal-workspace/queries-panel/query-discovery.test.ts

@@ -54,6 +54,10 @@ describe("Query pack discovery", () => {
   });
 
   describe("buildQueryTree", () => {
+    it("returns undefined before initial refresh has been done", async () => {
+      expect(discovery.buildQueryTree()).toEqual(undefined);
+    });
+
     it("returns an empty tree when there are no query files", async () => {
       await discovery.initialRefresh();