Save dirty documents before evaluating queries

Author: Dave Bartolomeo

extensions/ql-vscode/package.json

@@ -71,7 +71,8 @@
   "contributes": {
     "configurationDefaults": {
       "[ql]": {
-        "editor.wordBasedSuggestions": false
+        "editor.wordBasedSuggestions": false,
+        "debug.saveBeforeStart": "nonUntitledEditorsInActiveGroup"
       },
       "[dbscheme]": {
         "editor.wordBasedSuggestions": false
@@ -246,8 +247,8 @@
         },
         "codeQL.runningQueries.autoSave": {
           "type": "boolean",
-          "default": false,
-          "description": "Enable automatically saving a modified query file when running a query."
+          "description": "Enable automatically saving a modified query file when running a query.",
+          "markdownDeprecationMessage": "This property is deprecated and no longer has any effect. To control automatic saving of documents before running queries, use the `debug.saveBeforeStart` setting."
         },
         "codeQL.runningQueries.maxQueries": {
           "type": "integer",

extensions/ql-vscode/src/config.ts

@@ -5,6 +5,7 @@ import {
   EventEmitter,
   ConfigurationChangeEvent,
   ConfigurationTarget,
+  ConfigurationScope,
 } from "vscode";
 import { DistributionManager } from "./codeql-cli/distribution";
 import { extLogger } from "./common/logging/vscode";
@@ -23,7 +24,11 @@ export class Setting {
   parent?: Setting;
   private _hasChildren = false;
 
-  constructor(name: string, parent?: Setting) {
+  constructor(
+    name: string,
+    parent?: Setting,
+    private readonly languageId?: string,
+  ) {
     this.name = name;
     this.parent = parent;
     if (parent !== undefined) {
@@ -44,12 +49,22 @@ export class Setting {
     }
   }
 
+  get scope(): ConfigurationScope | undefined {
+    if (this.languageId !== undefined) {
+      return {
+        languageId: this.languageId,
+      };
+    } else {
+      return this.parent?.scope;
+    }
+  }
+
   getValue<T>(): T {
     if (this.parent === undefined) {
       throw new Error("Cannot get the value of a root setting.");
     }
     return workspace
-      .getConfiguration(this.parent.qualifiedName)
+      .getConfiguration(this.parent.qualifiedName, this.parent.scope)
       .get<T>(this.name)!;
   }
 
@@ -58,7 +73,7 @@ export class Setting {
       throw new Error("Cannot update the value of a root setting.");
     }
     return workspace
-      .getConfiguration(this.parent.qualifiedName)
+      .getConfiguration(this.parent.qualifiedName, this.parent.scope)
       .update(this.name, value, target);
   }
 }
@@ -69,6 +84,12 @@ export interface InspectionResult<T> {
   workspaceFolderValue?: T;
 }
 
+const VSCODE_DEBUG_SETTING = new Setting("debug", undefined, "ql");
+export const VSCODE_SAVE_BEFORE_START_SETTING = new Setting(
+  "saveBeforeStart",
+  VSCODE_DEBUG_SETTING,
+);
+
 const ROOT_SETTING = new Setting("codeQL");
 
 // Global configuration
@@ -160,10 +181,6 @@ export const NUMBER_OF_TEST_THREADS_SETTING = new Setting(
   RUNNING_TESTS_SETTING,
 );
 export const MAX_QUERIES = new Setting("maxQueries", RUNNING_QUERIES_SETTING);
-export const AUTOSAVE_SETTING = new Setting(
-  "autoSave",
-  RUNNING_QUERIES_SETTING,
-);
 export const PAGE_SIZE = new Setting("pageSize", RESULTS_DISPLAY_SETTING);
 const CUSTOM_LOG_DIRECTORY_SETTING = new Setting(
   "customLogDirectory",

extensions/ql-vscode/src/debugger/debugger-ui.ts

@@ -14,6 +14,7 @@ import { CoreQueryResults } from "../query-server";
 import {
   getQuickEvalContext,
   QueryOutputDir,
+  saveBeforeStart,
   validateQueryUri,
 } from "../run-queries-shared";
 import { QLResolvedDebugConfiguration } from "./debug-configuration";
@@ -73,6 +74,9 @@ class QLDebugAdapterTracker
   }
 
   public async quickEval(): Promise<void> {
+    // Since we're not going through VS Code's launch path, we need to save dirty files ourselves.
+    await saveBeforeStart();
+
     const args: CodeQLProtocol.QuickEvalRequest["arguments"] = {
       quickEvalContext: await getQuickEvalContext(undefined, false),
     };

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

@@ -10,7 +10,6 @@ import {
   Range,
   Uri,
   window,
-  workspace,
 } from "vscode";
 import {
   TeeLogger,
@@ -32,8 +31,8 @@ import {
   createInitialQueryInfo,
   createTimestampFile,
   getQuickEvalContext,
-  promptUserToSaveChanges,
   QueryOutputDir,
+  saveBeforeStart,
   SelectedQuery,
   validateQueryUri,
 } from "../run-queries-shared";
@@ -53,25 +52,6 @@ interface DatabaseQuickPickItem extends QuickPickItem {
   databaseItem: DatabaseItem;
 }
 
-/**
- * If either the query file or the quickeval file is dirty, give the user the chance to save them.
- */
-async function promptToSaveQueryIfNeeded(query: SelectedQuery): Promise<void> {
-  // There seems to be no way to ask VS Code to find an existing text document by name, without
-  // automatically opening the document if it is not found.
-  const queryUri = Uri.file(query.queryPath).toString();
-  const quickEvalUri =
-    query.quickEval !== undefined
-      ? Uri.file(query.quickEval.quickEvalPosition.fileName).toString()
-      : undefined;
-  for (const openDocument of workspace.textDocuments) {
-    const documentUri = openDocument.uri.toString();
-    if (documentUri === queryUri || documentUri === quickEvalUri) {
-      await promptUserToSaveChanges(openDocument);
-    }
-  }
-}
-
 export enum QuickEvalType {
   None,
   QuickEval,
@@ -408,7 +388,7 @@ export class LocalQueries extends DisposableObject {
     const additionalPacks = getOnDiskWorkspaceFolders();
     const extensionPacks = await this.getDefaultExtensionPacks(additionalPacks);
 
-    await promptToSaveQueryIfNeeded(selectedQuery);
+    await saveBeforeStart();
 
     const coreQueryRun = this.queryRunner.createQueryRun(
       databaseItem.databaseUri.fsPath,

extensions/ql-vscode/src/run-queries-shared.ts

@@ -3,15 +3,14 @@ import * as legacyMessages from "./pure/legacy-messages";
 import { DatabaseInfo, QueryMetadata } from "./common/interface-types";
 import { join, parse, dirname, basename } from "path";
 import {
-  ConfigurationTarget,
   Range,
   TextDocument,
   TextEditor,
   Uri,
   window,
+  workspace,
 } from "vscode";
-import { isCanary, AUTOSAVE_SETTING } from "./config";
-import { UserCancellationException } from "./common/vscode/progress";
+import { isCanary, VSCODE_SAVE_BEFORE_START_SETTING } from "./config";
 import {
   pathExists,
   readFile,
@@ -491,55 +490,78 @@ async function getSelectedPosition(
   };
 }
 
+type SaveBeforeStartMode =
+  | "nonUntitledEditorsInActiveGroup"
+  | "allEditorsInActiveGroup"
+  | "none";
+
 /**
- * Prompts the user to save `document` if it has unsaved changes.
- *
- * @param document The document to save.
- *
- * @returns true if we should save changes and false if we should continue without saving changes.
- * @throws UserCancellationException if we should abort whatever operation triggered this prompt
+ * Saves dirty files before running queries, based on the user's settings.
  */
-export async function promptUserToSaveChanges(
-  document: TextDocument,
-): Promise<boolean> {
-  if (document.isDirty) {
-    if (AUTOSAVE_SETTING.getValue()) {
-      return true;
-    } else {
-      const yesItem = { title: "Yes", isCloseAffordance: false };
-      const alwaysItem = { title: "Always Save", isCloseAffordance: false };
-      const noItem = {
-        title: "No (run version on disk)",
-        isCloseAffordance: false,
-      };
-      const cancelItem = { title: "Cancel", isCloseAffordance: true };
-      const message = `Query file '${basename(
-        document.uri.fsPath,
-      )}' has unsaved changes. Save now?`;
-      const chosenItem = await window.showInformationMessage(
-        message,
-        { modal: true },
-        yesItem,
-        alwaysItem,
-        noItem,
-        cancelItem,
-      );
+export async function saveBeforeStart(): Promise<void> {
+  const mode: SaveBeforeStartMode =
+    (VSCODE_SAVE_BEFORE_START_SETTING.getValue<string>() as SaveBeforeStartMode) ??
+    "nonUntitledEditorsInActiveGroup";
 
-      if (chosenItem === alwaysItem) {
-        await AUTOSAVE_SETTING.updateValue(true, ConfigurationTarget.Workspace);
-        return true;
-      }
+  switch (mode) {
+    case "nonUntitledEditorsInActiveGroup":
+      await saveAllInGroup(false);
+      break;
 
-      if (chosenItem === yesItem) {
-        return true;
-      }
+    case "allEditorsInActiveGroup":
+      await saveAllInGroup(true);
+      break;
 
-      if (chosenItem === cancelItem) {
-        throw new UserCancellationException("Query run cancelled.", true);
+    case "none":
+      break;
+
+    default:
+      // Unexpected value. Fall back to the default behavior.
+      await saveAllInGroup(false);
+      break;
+  }
+}
+
+// Used in tests
+export async function saveAllInGroup(includeUntitled: boolean): Promise<void> {
+  // There's no good way to get from a `Tab` to a `TextDocument`, so we'll collect all of the dirty
+  // documents indexed by their URI, and then compare those URIs against the URIs of the tabs.
+  const dirtyDocumentUris = new Map<string, TextDocument>();
+  for (const openDocument of workspace.textDocuments) {
+    if (openDocument.isDirty) {
+      console.warn(`${openDocument.uri.toString()} is dirty.`);
+      if (!openDocument.isUntitled || includeUntitled) {
+        dirtyDocumentUris.set(openDocument.uri.toString(), openDocument);
+      }
+    }
+  }
+  if (dirtyDocumentUris.size > 0) {
+    console.warn(`${window.tabGroups.all.length} tab groups open`);
+    console.warn(`${workspace.textDocuments.length} documents open`);
+    const tabGroup = window.tabGroups.activeTabGroup;
+    console.warn(`${tabGroup.tabs.length} tabs open in active group`);
+    for (const tab of tabGroup.tabs) {
+      const input = tab.input;
+      // The `input` property can be of an arbitrary type, depending on the underlying tab type. For
+      // text editors (and potentially others), it's an object with a `uri` property. That's all we
+      // need to know to match it up with a dirty document.
+      if (typeof input === "object") {
+        const uri = (input as any).uri;
+        if (uri instanceof Uri) {
+          const document = dirtyDocumentUris.get(uri.toString());
+          if (document !== undefined) {
+            console.warn(`Saving ${uri.toString()}`);
+            await document.save();
+            // Remove the URI from the dirty list so we don't wind up saving the same file twice
+            // if it's open in multiple editors.
+            dirtyDocumentUris.delete(uri.toString());
+          } else {
+            console.warn(`Can't find ${uri.toString()}`);
+          }
+        }
       }
     }
   }
-  return false;
 }
 
 /**

extensions/ql-vscode/test/data/debugger/clean.ql

@@ -0,0 +1 @@
+select "clean"

extensions/ql-vscode/test/data/debugger/dirty.ql

@@ -0,0 +1 @@
+testtesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttestselect "dirty"

extensions/ql-vscode/test/data/debugger/other-clean.ql

@@ -0,0 +1 @@
+select "other-clean"

extensions/ql-vscode/test/data/debugger/other-dirty.ql

@@ -0,0 +1 @@
+testtesttesttesttesttestselect "other-dirty"