Merge pull request #2485 from github/koesie10/move-location-utils

Move functions for resolving locations

extensions/ql-vscode/src/compare/compare-view.ts

@@ -8,7 +8,7 @@ import {
 import { Logger } from "../common";
 import { CodeQLCliServer } from "../codeql-cli/cli";
 import { DatabaseManager } from "../databases/local-databases";
-import { jumpToLocation } from "../interface-utils";
+import { jumpToLocation } from "../databases/local-databases/locations";
 import {
   transformBqrsResultSet,
   RawResultSet,
@@ -130,7 +130,12 @@ export class CompareView extends AbstractWebview<
         break;
 
       case "viewSourceFile":
-        await jumpToLocation(msg, this.databaseManager, this.logger);
+        await jumpToLocation(
+          msg.databaseUri,
+          msg.loc,
+          this.databaseManager,
+          this.logger,
+        );
         break;
 
       case "openQuery":

extensions/ql-vscode/src/data-extensions-editor/data-extensions-editor-view.ts

@@ -31,7 +31,7 @@ import { generateFlowModel } from "./generate-flow-model";
 import { promptImportGithubDatabase } from "../databases/database-fetcher";
 import { App } from "../common/app";
 import { ResolvableLocationValue } from "../pure/bqrs-cli-types";
-import { showResolvableLocation } from "../interface-utils";
+import { showResolvableLocation } from "../databases/local-databases/locations";
 import { decodeBqrsToExternalApiUsages } from "./bqrs";
 import { redactableError } from "../pure/errors";
 import { readQueryResults, runQuery } from "./external-api-usage-query";

extensions/ql-vscode/src/databases/local-databases/locations.ts

@@ -0,0 +1,161 @@
+import {
+  Location,
+  Range,
+  Selection,
+  TextEditorRevealType,
+  ThemeColor,
+  Uri,
+  ViewColumn,
+  window as Window,
+  workspace,
+} from "vscode";
+import {
+  LineColumnLocation,
+  ResolvableLocationValue,
+  UrlValue,
+  WholeFileLocation,
+} from "../../pure/bqrs-cli-types";
+import {
+  isLineColumnLoc,
+  tryGetResolvableLocation,
+} from "../../pure/bqrs-utils";
+import { getErrorMessage } from "../../pure/helpers-pure";
+import { Logger } from "../../common";
+import { DatabaseItem } from "./database-item";
+import { DatabaseManager } from "./database-manager";
+
+const findMatchBackground = new ThemeColor("editor.findMatchBackground");
+const findRangeHighlightBackground = new ThemeColor(
+  "editor.findRangeHighlightBackground",
+);
+
+export const shownLocationDecoration = Window.createTextEditorDecorationType({
+  backgroundColor: findMatchBackground,
+});
+
+export const shownLocationLineDecoration =
+  Window.createTextEditorDecorationType({
+    backgroundColor: findRangeHighlightBackground,
+    isWholeLine: true,
+  });
+
+/**
+ * Resolves the specified CodeQL location to a URI into the source archive.
+ * @param loc CodeQL location to resolve. Must have a non-empty value for `loc.file`.
+ * @param databaseItem Database in which to resolve the file location.
+ */
+function resolveFivePartLocation(
+  loc: LineColumnLocation,
+  databaseItem: DatabaseItem,
+): Location {
+  // `Range` is a half-open interval, and is zero-based. CodeQL locations are closed intervals, and
+  // are one-based. Adjust accordingly.
+  const range = new Range(
+    Math.max(0, loc.startLine - 1),
+    Math.max(0, loc.startColumn - 1),
+    Math.max(0, loc.endLine - 1),
+    Math.max(1, loc.endColumn),
+  );
+
+  return new Location(databaseItem.resolveSourceFile(loc.uri), range);
+}
+
+/**
+ * Resolves the specified CodeQL filesystem resource location to a URI into the source archive.
+ * @param loc CodeQL location to resolve, corresponding to an entire filesystem resource. Must have a non-empty value for `loc.file`.
+ * @param databaseItem Database in which to resolve the filesystem resource location.
+ */
+function resolveWholeFileLocation(
+  loc: WholeFileLocation,
+  databaseItem: DatabaseItem,
+): Location {
+  // A location corresponding to the start of the file.
+  const range = new Range(0, 0, 0, 0);
+  return new Location(databaseItem.resolveSourceFile(loc.uri), range);
+}
+
+/**
+ * Try to resolve the specified CodeQL location to a URI into the source archive. If no exact location
+ * can be resolved, returns `undefined`.
+ * @param loc CodeQL location to resolve
+ * @param databaseItem Database in which to resolve the file location.
+ */
+export function tryResolveLocation(
+  loc: UrlValue | undefined,
+  databaseItem: DatabaseItem,
+): Location | undefined {
+  const resolvableLoc = tryGetResolvableLocation(loc);
+  if (!resolvableLoc || typeof resolvableLoc === "string") {
+    return;
+  } else if (isLineColumnLoc(resolvableLoc)) {
+    return resolveFivePartLocation(resolvableLoc, databaseItem);
+  } else {
+    return resolveWholeFileLocation(resolvableLoc, databaseItem);
+  }
+}
+
+export async function showResolvableLocation(
+  loc: ResolvableLocationValue,
+  databaseItem: DatabaseItem,
+): Promise<void> {
+  await showLocation(tryResolveLocation(loc, databaseItem));
+}
+
+export async function showLocation(location?: Location) {
+  if (!location) {
+    return;
+  }
+
+  const doc = await workspace.openTextDocument(location.uri);
+  const editorsWithDoc = Window.visibleTextEditors.filter(
+    (e) => e.document === doc,
+  );
+  const editor =
+    editorsWithDoc.length > 0
+      ? editorsWithDoc[0]
+      : await Window.showTextDocument(doc, {
+          // avoid preview mode so editor is sticky and will be added to navigation and search histories.
+          preview: false,
+          viewColumn: ViewColumn.One,
+        });
+
+  const range = location.range;
+  // When highlighting the range, vscode's occurrence-match and bracket-match highlighting will
+  // trigger based on where we place the cursor/selection, and will compete for the user's attention.
+  // For reference:
+  // - Occurences are highlighted when the cursor is next to or inside a word or a whole word is selected.
+  // - Brackets are highlighted when the cursor is next to a bracket and there is an empty selection.
+  // - Multi-line selections explicitly highlight line-break characters, but multi-line decorators do not.
+  //
+  // For single-line ranges, select the whole range, mainly to disable bracket highlighting.
+  // For multi-line ranges, place the cursor at the beginning to avoid visual artifacts from selected line-breaks.
+  // Multi-line ranges are usually large enough to overshadow the noise from bracket highlighting.
+  const selectionEnd =
+    range.start.line === range.end.line ? range.end : range.start;
+  editor.selection = new Selection(range.start, selectionEnd);
+  editor.revealRange(range, TextEditorRevealType.InCenter);
+  editor.setDecorations(shownLocationDecoration, [range]);
+  editor.setDecorations(shownLocationLineDecoration, [range]);
+}
+
+export async function jumpToLocation(
+  databaseUri: string,
+  loc: ResolvableLocationValue,
+  databaseManager: DatabaseManager,
+  logger: Logger,
+) {
+  const databaseItem = databaseManager.findDatabaseItem(Uri.parse(databaseUri));
+  if (databaseItem !== undefined) {
+    try {
+      await showResolvableLocation(loc, databaseItem);
+    } catch (e) {
+      if (e instanceof Error && e.message.match(/File not found/)) {
+        void Window.showErrorMessage(
+          "Original file of this result is not in the database's source archive.",
+        );
+      } else {
+        void logger.log(`Unable to jump to location: ${getErrorMessage(e)}`);
+      }
+    }
+  }
+}

extensions/ql-vscode/src/interface-utils.ts

@@ -1,25 +1,4 @@
-import {
-  Uri,
-  Location,
-  Range,
-  WebviewPanel,
-  workspace,
-  window as Window,
-  ViewColumn,
-  Selection,
-  TextEditorRevealType,
-  ThemeColor,
-} from "vscode";
-import { tryGetResolvableLocation, isLineColumnLoc } from "./pure/bqrs-utils";
-import { DatabaseItem, DatabaseManager } from "./databases/local-databases";
-import { ViewSourceFileMsg } from "./pure/interface-types";
-import { Logger } from "./common";
-import {
-  LineColumnLocation,
-  WholeFileLocation,
-  UrlValue,
-  ResolvableLocationValue,
-} from "./pure/bqrs-cli-types";
+import { Uri, WebviewPanel } from "vscode";
 
 /**
  * This module contains functions and types that are sharedd between
@@ -44,144 +23,3 @@ export function fileUriToWebviewUri(
 ): string {
   return panel.webview.asWebviewUri(fileUriOnDisk).toString();
 }
-
-/**
- * Resolves the specified CodeQL location to a URI into the source archive.
- * @param loc CodeQL location to resolve. Must have a non-empty value for `loc.file`.
- * @param databaseItem Database in which to resolve the file location.
- */
-function resolveFivePartLocation(
-  loc: LineColumnLocation,
-  databaseItem: DatabaseItem,
-): Location {
-  // `Range` is a half-open interval, and is zero-based. CodeQL locations are closed intervals, and
-  // are one-based. Adjust accordingly.
-  const range = new Range(
-    Math.max(0, loc.startLine - 1),
-    Math.max(0, loc.startColumn - 1),
-    Math.max(0, loc.endLine - 1),
-    Math.max(1, loc.endColumn),
-  );
-
-  return new Location(databaseItem.resolveSourceFile(loc.uri), range);
-}
-
-/**
- * Resolves the specified CodeQL filesystem resource location to a URI into the source archive.
- * @param loc CodeQL location to resolve, corresponding to an entire filesystem resource. Must have a non-empty value for `loc.file`.
- * @param databaseItem Database in which to resolve the filesystem resource location.
- */
-function resolveWholeFileLocation(
-  loc: WholeFileLocation,
-  databaseItem: DatabaseItem,
-): Location {
-  // A location corresponding to the start of the file.
-  const range = new Range(0, 0, 0, 0);
-  return new Location(databaseItem.resolveSourceFile(loc.uri), range);
-}
-
-/**
- * Try to resolve the specified CodeQL location to a URI into the source archive. If no exact location
- * can be resolved, returns `undefined`.
- * @param loc CodeQL location to resolve
- * @param databaseItem Database in which to resolve the file location.
- */
-export function tryResolveLocation(
-  loc: UrlValue | undefined,
-  databaseItem: DatabaseItem,
-): Location | undefined {
-  const resolvableLoc = tryGetResolvableLocation(loc);
-  if (!resolvableLoc || typeof resolvableLoc === "string") {
-    return;
-  } else if (isLineColumnLoc(resolvableLoc)) {
-    return resolveFivePartLocation(resolvableLoc, databaseItem);
-  } else {
-    return resolveWholeFileLocation(resolvableLoc, databaseItem);
-  }
-}
-
-export async function showResolvableLocation(
-  loc: ResolvableLocationValue,
-  databaseItem: DatabaseItem,
-): Promise<void> {
-  await showLocation(tryResolveLocation(loc, databaseItem));
-}
-
-export async function showLocation(location?: Location) {
-  if (!location) {
-    return;
-  }
-
-  const doc = await workspace.openTextDocument(location.uri);
-  const editorsWithDoc = Window.visibleTextEditors.filter(
-    (e) => e.document === doc,
-  );
-  const editor =
-    editorsWithDoc.length > 0
-      ? editorsWithDoc[0]
-      : await Window.showTextDocument(doc, {
-          // avoid preview mode so editor is sticky and will be added to navigation and search histories.
-          preview: false,
-          viewColumn: ViewColumn.One,
-        });
-
-  const range = location.range;
-  // When highlighting the range, vscode's occurrence-match and bracket-match highlighting will
-  // trigger based on where we place the cursor/selection, and will compete for the user's attention.
-  // For reference:
-  // - Occurences are highlighted when the cursor is next to or inside a word or a whole word is selected.
-  // - Brackets are highlighted when the cursor is next to a bracket and there is an empty selection.
-  // - Multi-line selections explicitly highlight line-break characters, but multi-line decorators do not.
-  //
-  // For single-line ranges, select the whole range, mainly to disable bracket highlighting.
-  // For multi-line ranges, place the cursor at the beginning to avoid visual artifacts from selected line-breaks.
-  // Multi-line ranges are usually large enough to overshadow the noise from bracket highlighting.
-  const selectionEnd =
-    range.start.line === range.end.line ? range.end : range.start;
-  editor.selection = new Selection(range.start, selectionEnd);
-  editor.revealRange(range, TextEditorRevealType.InCenter);
-  editor.setDecorations(shownLocationDecoration, [range]);
-  editor.setDecorations(shownLocationLineDecoration, [range]);
-}
-
-const findMatchBackground = new ThemeColor("editor.findMatchBackground");
-const findRangeHighlightBackground = new ThemeColor(
-  "editor.findRangeHighlightBackground",
-);
-
-export const shownLocationDecoration = Window.createTextEditorDecorationType({
-  backgroundColor: findMatchBackground,
-});
-
-export const shownLocationLineDecoration =
-  Window.createTextEditorDecorationType({
-    backgroundColor: findRangeHighlightBackground,
-    isWholeLine: true,
-  });
-
-export async function jumpToLocation(
-  msg: ViewSourceFileMsg,
-  databaseManager: DatabaseManager,
-  logger: Logger,
-) {
-  const databaseItem = databaseManager.findDatabaseItem(
-    Uri.parse(msg.databaseUri),
-  );
-  if (databaseItem !== undefined) {
-    try {
-      await showResolvableLocation(msg.loc, databaseItem);
-    } catch (e) {
-      if (e instanceof Error) {
-        if (e.message.match(/File not found/)) {
-          void Window.showErrorMessage(
-            "Original file of this result is not in the database's source archive.",
-          );
-        } else {
-          void logger.log(`Unable to handleMsgFromView: ${e.message}`);
-        }
-      } else {
-        void logger.log(`Unable to handleMsgFromView: ${e}`);
-      }
-    }
-  }
-}

extensions/ql-vscode/src/interface.ts

@@ -53,14 +53,13 @@ import {
   parseSarifLocation,
   parseSarifPlainTextMessage,
 } from "./pure/sarif-utils";
+import { WebviewReveal, fileUriToWebviewUri } from "./interface-utils";
 import {
-  WebviewReveal,
-  fileUriToWebviewUri,
   tryResolveLocation,
   shownLocationDecoration,
   shownLocationLineDecoration,
   jumpToLocation,
-} from "./interface-utils";
+} from "./databases/local-databases/locations";
 import {
   RawResultSet,
   transformBqrsResultSet,
@@ -259,7 +258,12 @@ export class ResultsView extends AbstractWebview<
           this.onWebViewLoaded();
           break;
         case "viewSourceFile": {
-          await jumpToLocation(msg, this.databaseManager, this.logger);
+          await jumpToLocation(
+            msg.databaseUri,
+            msg.loc,
+            this.databaseManager,
+            this.logger,
+          );
           break;
         }
         case "toggleDiagnostics": {

extensions/ql-vscode/src/language-support/ast-viewer/ast-viewer.ts

@@ -17,7 +17,7 @@ import { basename } from "path";
 
 import { DatabaseItem } from "../../databases/local-databases";
 import { UrlValue, BqrsId } from "../../pure/bqrs-cli-types";
-import { showLocation } from "../../interface-utils";
+import { showLocation } from "../../databases/local-databases/locations";
 import {
   isStringLoc,
   isWholeFileLoc,

extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases/locations.test.ts

@@ -0,0 +1,91 @@
+import { Location, Position, Range, Uri } from "vscode";
+import { mockDatabaseItem } from "../../../utils/mocking.helpers";
+import { tryResolveLocation } from "../../../../../src/databases/local-databases/locations";
+
+describe("tryResolveLocation", () => {
+  it("should resolve a whole file location", () => {
+    const databaseItem = mockDatabaseItem();
+    expect(tryResolveLocation("file://hucairz:0:0:0:0", databaseItem)).toEqual(
+      new Location(Uri.file("abc"), new Range(0, 0, 0, 0)),
+    );
+  });
+
+  it("should resolve a five-part location edge case", () => {
+    const databaseItem = mockDatabaseItem();
+    expect(tryResolveLocation("file://hucairz:1:1:1:1", databaseItem)).toEqual(
+      new Location(Uri.file("abc"), new Range(0, 0, 0, 1)),
+    );
+  });
+
+  it("should resolve a five-part location", () => {
+    const databaseItem = mockDatabaseItem();
+
+    expect(
+      tryResolveLocation(
+        {
+          startColumn: 1,
+          endColumn: 3,
+          startLine: 4,
+          endLine: 5,
+          uri: "hucairz",
+        },
+        databaseItem,
+      ),
+    ).toEqual(
+      new Location(
+        Uri.parse("abc"),
+        new Range(new Position(4, 3), new Position(3, 0)),
+      ),
+    );
+    expect(databaseItem.resolveSourceFile).toHaveBeenCalledTimes(1);
+    expect(databaseItem.resolveSourceFile).toHaveBeenCalledWith("hucairz");
+  });
+
+  it("should resolve a five-part location with an empty path", () => {
+    const databaseItem = mockDatabaseItem();
+
+    expect(
+      tryResolveLocation(
+        {
+          startColumn: 1,
+          endColumn: 3,
+          startLine: 4,
+          endLine: 5,
+          uri: "",
+        },
+        databaseItem,
+      ),
+    ).toBeUndefined();
+  });
+
+  it("should resolve a string location for whole file", () => {
+    const databaseItem = mockDatabaseItem();
+
+    expect(tryResolveLocation("file://hucairz:0:0:0:0", databaseItem)).toEqual(
+      new Location(Uri.parse("abc"), new Range(0, 0, 0, 0)),
+    );
+    expect(databaseItem.resolveSourceFile).toHaveBeenCalledTimes(1);
+    expect(databaseItem.resolveSourceFile).toHaveBeenCalledWith("hucairz");
+  });
+
+  it("should resolve a string location for five-part location", () => {
+    const databaseItem = mockDatabaseItem();
+
+    expect(tryResolveLocation("file://hucairz:5:4:3:2", databaseItem)).toEqual(
+      new Location(
+        Uri.parse("abc"),
+        new Range(new Position(4, 3), new Position(2, 2)),
+      ),
+    );
+    expect(databaseItem.resolveSourceFile).toHaveBeenCalledTimes(1);
+    expect(databaseItem.resolveSourceFile).toHaveBeenCalledWith("hucairz");
+  });
+
+  it("should resolve a string location for invalid string", () => {
+    const databaseItem = mockDatabaseItem();
+
+    expect(
+      tryResolveLocation("file://hucairz:x:y:z:a", databaseItem),
+    ).toBeUndefined();
+  });
+});

extensions/ql-vscode/test/vscode-tests/no-workspace/interface-utils.test.ts

@@ -1,20 +1,8 @@
-import {
-  Uri,
-  Location,
-  Range,
-  Position,
-  window,
-  ViewColumn,
-  WebviewPanel,
-} from "vscode";
+import { Uri, ViewColumn, WebviewPanel, window } from "vscode";
 import { basename } from "path";
-import { fileSync, FileResult } from "tmp";
-import {
-  fileUriToWebviewUri,
-  tryResolveLocation,
-} from "../../../src/interface-utils";
+import { FileResult, fileSync } from "tmp";
+import { fileUriToWebviewUri } from "../../../src/interface-utils";
 import { getDefaultResultSetName } from "../../../src/pure/interface-types";
-import { mockDatabaseItem } from "../utils/mocking.helpers";
 
 describe("interface-utils", () => {
   describe("webview uri conversion", () => {
@@ -81,94 +69,4 @@ describe("interface-utils", () => {
       expect(getDefaultResultSetName([])).toBeUndefined();
     });
   });
-
-  describe("resolveWholeFileLocation", () => {
-    it("should resolve a whole file location", () => {
-      const databaseItem = mockDatabaseItem();
-      expect(
-        tryResolveLocation("file://hucairz:0:0:0:0", databaseItem),
-      ).toEqual(new Location(Uri.file("abc"), new Range(0, 0, 0, 0)));
-    });
-
-    it("should resolve a five-part location edge case", () => {
-      const databaseItem = mockDatabaseItem();
-      expect(
-        tryResolveLocation("file://hucairz:1:1:1:1", databaseItem),
-      ).toEqual(new Location(Uri.file("abc"), new Range(0, 0, 0, 1)));
-    });
-
-    it("should resolve a five-part location", () => {
-      const databaseItem = mockDatabaseItem();
-
-      expect(
-        tryResolveLocation(
-          {
-            startColumn: 1,
-            endColumn: 3,
-            startLine: 4,
-            endLine: 5,
-            uri: "hucairz",
-          },
-          databaseItem,
-        ),
-      ).toEqual(
-        new Location(
-          Uri.parse("abc"),
-          new Range(new Position(4, 3), new Position(3, 0)),
-        ),
-      );
-      expect(databaseItem.resolveSourceFile).toHaveBeenCalledTimes(1);
-      expect(databaseItem.resolveSourceFile).toHaveBeenCalledWith("hucairz");
-    });
-
-    it("should resolve a five-part location with an empty path", () => {
-      const databaseItem = mockDatabaseItem();
-
-      expect(
-        tryResolveLocation(
-          {
-            startColumn: 1,
-            endColumn: 3,
-            startLine: 4,
-            endLine: 5,
-            uri: "",
-          },
-          databaseItem,
-        ),
-      ).toBeUndefined();
-    });
-
-    it("should resolve a string location for whole file", () => {
-      const databaseItem = mockDatabaseItem();
-
-      expect(
-        tryResolveLocation("file://hucairz:0:0:0:0", databaseItem),
-      ).toEqual(new Location(Uri.parse("abc"), new Range(0, 0, 0, 0)));
-      expect(databaseItem.resolveSourceFile).toHaveBeenCalledTimes(1);
-      expect(databaseItem.resolveSourceFile).toHaveBeenCalledWith("hucairz");
-    });
-
-    it("should resolve a string location for five-part location", () => {
-      const databaseItem = mockDatabaseItem();
-
-      expect(
-        tryResolveLocation("file://hucairz:5:4:3:2", databaseItem),
-      ).toEqual(
-        new Location(
-          Uri.parse("abc"),
-          new Range(new Position(4, 3), new Position(2, 2)),
-        ),
-      );
-      expect(databaseItem.resolveSourceFile).toHaveBeenCalledTimes(1);
-      expect(databaseItem.resolveSourceFile).toHaveBeenCalledWith("hucairz");
-    });
-
-    it("should resolve a string location for invalid string", () => {
-      const databaseItem = mockDatabaseItem();
-
-      expect(
-        tryResolveLocation("file://hucairz:x:y:z:a", databaseItem),
-      ).toBeUndefined();
-    });
-  });
 });