mrva/vscode-codeql
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

_Correctly_ emulate VS Code's saveBeforeStart

Browse Source
This commit is contained in:
Dave Bartolomeo
2023-06-22 07:30:53 -04:00
parent b182d7afef
commit 0bd0bf1944
17 changed files with 106 additions and 202 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
extensions/ql-vscode/src/local-queries/local-queries.ts
View File

@@ -358,6 +358,8 @@ export class LocalQueries extends DisposableObject {
range?: Range,
templates?: Record<string, string>,
): Promise<CoreCompletedQuery> {
await saveBeforeStart();
let queryPath: string;
if (queryUri !== undefined) {
// The query URI is provided by the command, most likely because the command was run from an
@@ -388,8 +390,6 @@ export class LocalQueries extends DisposableObject {
const additionalPacks = getOnDiskWorkspaceFolders();
const extensionPacks = await this.getDefaultExtensionPacks(additionalPacks);
await saveBeforeStart();
const coreQueryRun = this.queryRunner.createQueryRun(
databaseItem.databaseUri.fsPath,
{

55
extensions/ql-vscode/src/run-queries-shared.ts
View File

@@ -2,14 +2,7 @@ import * as messages from "./pure/messages-shared";
import * as legacyMessages from "./pure/legacy-messages";
import { DatabaseInfo, QueryMetadata } from "./common/interface-types";
import { join, parse, dirname, basename } from "path";
import {
Range,
TextDocument,
TextEditor,
Uri,
window,
workspace,
} from "vscode";
import { Range, TextEditor, Uri, window, workspace } from "vscode";
import { isCanary, VSCODE_SAVE_BEFORE_START_SETTING } from "./config";
import {
pathExists,
@@ -503,13 +496,17 @@ export async function saveBeforeStart(): Promise<void> {
(VSCODE_SAVE_BEFORE_START_SETTING.getValue<string>() as SaveBeforeStartMode) ??
"nonUntitledEditorsInActiveGroup";
// Despite the names of the modes, the VS Code implementation doesn't restrict itself to the
// current tab group. It saves all dirty files in all groups. We'll do the same.
switch (mode) {
case "nonUntitledEditorsInActiveGroup":
await saveAllInGroup(false);
await workspace.saveAll(false);
break;
case "allEditorsInActiveGroup":
await saveAllInGroup(true);
// The VS Code implementation of this mode only saves an untitled file if it is the document
// in the active editor. That's too much work for us, so we'll just live with the inconsistency.
await workspace.saveAll(true);
break;
case "none":
@@ -517,47 +514,11 @@ export async function saveBeforeStart(): Promise<void> {
default:
// Unexpected value. Fall back to the default behavior.
await saveAllInGroup(false);
await workspace.saveAll(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) {
const tabGroup = window.tabGroups.activeTabGroup;
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) {
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());
}
}
}
}
}
}
/**
* @param filePath This needs to be equivalent to Java's `Path.toRealPath(NO_FOLLOW_LINKS)`
*/

0
extensions/ql-vscode/test/vscode-tests/cli-integration/data/QuickEvalLib.qll → extensions/ql-vscode/test/data/debugger/QuickEvalLib.qll
View File

0
extensions/ql-vscode/test/vscode-tests/cli-integration/data/QuickEvalQuery.ql → extensions/ql-vscode/test/data/debugger/QuickEvalQuery.ql
View File

1
extensions/ql-vscode/test/data/debugger/clean.ql
View File

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

20
extensions/ql-vscode/test/data/debugger/codeql-pack.lock.yml Normal file
View File

@@ -0,0 +1,20 @@
---
lockVersion: 1.0.0
dependencies:
codeql/javascript-all:
version: 0.6.3
codeql/javascript-queries:
version: 0.6.3
codeql/regex:
version: 0.0.14
codeql/suite-helpers:
version: 0.5.3
codeql/tutorial:
version: 0.0.11
codeql/typos:
version: 0.0.18
codeql/util:
version: 0.0.11
codeql/yaml:
version: 0.0.3
compiled: false

1
extensions/ql-vscode/test/data/debugger/dirty.ql
View File

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

1
extensions/ql-vscode/test/data/debugger/other-clean.ql
View File

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

1
extensions/ql-vscode/test/data/debugger/other-dirty.ql
View File

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

4
extensions/ql-vscode/test/data/debugger/qlpack.yml Normal file
View File

@@ -0,0 +1,4 @@
name: integration-test-debugger-javascript
version: 0.0.0
dependencies:
codeql/javascript-queries: "*"

0
extensions/ql-vscode/test/vscode-tests/cli-integration/data/simple-query.ql → extensions/ql-vscode/test/data/debugger/simple-query.ql
View File

3
extensions/ql-vscode/test/vscode-tests/cli-integration/codeql-cli/query-metadata.test.ts
View File

@@ -3,6 +3,7 @@ import { join, resolve } from "path";
import { CodeQLCliServer } from "../../../../src/codeql-cli/cli";
import { getActivatedExtension } from "../../global.helper";
import { tryGetQueryMetadata } from "../../../../src/codeql-cli/query-metadata";
import { getDataFolderFilePath } from "../utils";
describe("tryGetQueryMetadata", () => {
const baseDir = resolve(__dirname, "..");
@@ -30,7 +31,7 @@ describe("tryGetQueryMetadata", () => {
// Query with empty metadata
const noMetadata = await tryGetQueryMetadata(
cli,
join(baseDir, "data", "simple-query.ql"),
getDataFolderFilePath("debugger/simple-query.ql"),
);
expect(noMetadata).toEqual({});

54
extensions/ql-vscode/test/vscode-tests/cli-integration/debugger/debugger.test.ts
View File

@@ -1,5 +1,11 @@
import { Selection, Uri, window, workspace } from "vscode";
import { join } from "path";
import {
Position,
Selection,
TextEditor,
Uri,
window,
workspace,
} from "vscode";
import { DatabaseManager } from "../../../../src/databases/local-databases";
import {
@@ -13,6 +19,7 @@ import { CodeQLCliServer } from "../../../../src/codeql-cli/cli";
import { QueryOutputDir } from "../../../../src/run-queries-shared";
import { createVSCodeCommandManager } from "../../../../src/common/vscode/commands";
import { AllCommands } from "../../../../src/common/commands";
import { getDataFolderFilePath } from "../utils";
async function selectForQuickEval(
path: string,
@@ -20,10 +27,12 @@ async function selectForQuickEval(
column: number,
endLine: number,
endColumn: number,
): Promise<void> {
): Promise<TextEditor> {
const document = await workspace.openTextDocument(path);
const editor = await window.showTextDocument(document);
editor.selection = new Selection(line, column, endLine, endColumn);
return editor;
}
async function getResultCount(
@@ -42,9 +51,11 @@ describeWithCodeQL()("Debugger", () => {
let databaseManager: DatabaseManager;
let cli: CodeQLCliServer;
const appCommands = createVSCodeCommandManager<AllCommands>();
const simpleQueryPath = join(__dirname, "..", "data", "simple-query.ql");
const quickEvalQueryPath = join(__dirname, "..", "data", "QuickEvalQuery.ql");
const quickEvalLibPath = join(__dirname, "..", "data", "QuickEvalLib.qll");
const simpleQueryPath = getDataFolderFilePath("debugger/simple-query.ql");
const quickEvalQueryPath = getDataFolderFilePath(
"debugger/QuickEvalQuery.ql",
);
const quickEvalLibPath = getDataFolderFilePath("debugger/QuickEvalLib.qll");
beforeEach(async () => {
const extension = await getActivatedExtension();
@@ -138,4 +149,35 @@ describeWithCodeQL()("Debugger", () => {
await controller.expectStopped();
});
});
it("should save dirty documents before launching a debug session", async () => {
await withDebugController(appCommands, async (controller) => {
const editor = await selectForQuickEval(quickEvalLibPath, 4, 15, 4, 32);
await editor.edit((editBuilder) => {
editBuilder.insert(new Position(0, 0), "/* comment */");
});
expect(editor.document.isDirty).toBe(true);
await controller.startDebuggingSelection({
query: quickEvalQueryPath, // The query context. This query extends the abstract class.
});
await controller.expectLaunched();
// Should have saved the dirty document.
expect(editor.document.isDirty).toBe(false);
await controller.expectSucceeded();
await controller.expectStopped();
await editor.edit((editBuilder) => {
editBuilder.insert(new Position(0, 0), "/* another comment */");
});
expect(editor.document.isDirty).toBe(true);
await controller.continueDebuggingSelection();
await controller.expectSucceeded();
await controller.expectStopped();
expect(editor.document.isDirty).toBe(false);
});
});
});

9
extensions/ql-vscode/test/vscode-tests/cli-integration/jest-runner-vscode.config.js
View File

@@ -1,10 +1,17 @@
const path = require("path");
const fs = require("fs");
const {
config: baseConfig,
tmpDir,
rootDir,
} = require("../jest-runner-vscode.config.base");
const tmpDataDir = path.join(tmpDir.name, "data");
fs.cpSync(path.resolve(rootDir, "test/data"), tmpDataDir, {
recursive: true,
});
/** @type import("jest-runner-vscode").RunnerOptions */
const config = {
...baseConfig,
@@ -17,7 +24,7 @@ const config = {
"github.codespaces",
"--disable-extension",
"github.copilot",
path.resolve(rootDir, "test/data"),
tmpDataDir,
path.resolve(rootDir, "test/data-extensions"), // folder containing the extension packs and packs that are targeted by the extension pack
// CLI integration tests requires a multi-root workspace so that the data and the QL sources are accessible.
...(process.env.TEST_CODEQL_PATH ? [process.env.TEST_CODEQL_PATH] : []),

9
extensions/ql-vscode/test/vscode-tests/cli-integration/queries.test.ts
View File

@@ -38,6 +38,9 @@ import {
} from "../../../src/common/commands";
import { ProgressCallback } from "../../../src/common/vscode/progress";
import { withDebugController } from "./debugger/debug-controller";
import { getDataFolderFilePath } from "./utils";
const simpleQueryPath = getDataFolderFilePath("debugger/simple-query.ql");
type DebugMode = "localQueries" | "debug";
@@ -213,13 +216,12 @@ describeWithCodeQL()("Queries", () => {
describe.each(MODES)("running queries (%s)", (mode) => {
it("should run a query", async () => {
const queryPath = join(__dirname, "data", "simple-query.ql");
const result = await compileAndRunQuery(
mode,
appCommandManager,
localQueries,
QuickEvalType.None,
Uri.file(queryPath),
Uri.file(simpleQueryPath),
progress,
token,
dbItem,
@@ -233,13 +235,12 @@ describeWithCodeQL()("Queries", () => {
// Asserts a fix for bug https://github.com/github/vscode-codeql/issues/733
it("should restart the database and run a query", async () => {
await appCommandManager.execute("codeQL.restartQueryServer");
const queryPath = join(__dirname, "data", "simple-query.ql");
const result = await compileAndRunQuery(
mode,
appCommandManager,
localQueries,
QuickEvalType.None,
Uri.file(queryPath),
Uri.file(simpleQueryPath),
progress,
token,
dbItem,

9
extensions/ql-vscode/test/vscode-tests/cli-integration/utils.ts Normal file
View File

@@ -0,0 +1,9 @@
import { workspace } from "vscode";
import { join } from "path";
/**
* Get the absolute path to a file in the temporary copy of the `data` folder.
*/
export function getDataFolderFilePath(path: string): string {
return join(workspace.workspaceFolders![0].uri.fsPath, path);
}

137
extensions/ql-vscode/test/vscode-tests/minimal-workspace/debugger/save-before-start.test.ts
View File

@@ -1,137 +0,0 @@
import * as path from "path";
import {
Position,
TextDocument,
ViewColumn,
commands,
window,
workspace,
} from "vscode";
import { saveAllInGroup } from "../../../../src/run-queries-shared";
/**
* Returns a new `TextDocument` with the given file name.
*
* @param file Path to the file to open, or undefined to create an untitled document.
* @param isDirty True if the document should have an edit applied.
* @param viewColumn The `ViewColumn` in which to open the file's editor.
* @returns The new `TextDocument`
*/
async function mockDocument(
file: string | undefined,
isDirty: boolean,
viewColumn: ViewColumn,
): Promise<TextDocument> {
let doc: TextDocument;
if (file !== undefined) {
doc = await workspace.openTextDocument(
path.join(workspace.workspaceFolders![0].uri.fsPath, file),
);
const editor = await window.showTextDocument(doc, viewColumn, true);
if (isDirty) {
await editor.edit((edit) => {
edit.insert(new Position(0, 0), "test");
});
}
} else {
doc = await workspace.openTextDocument({
content: 'select "untitled"',
language: "ql",
});
await window.showTextDocument(doc, viewColumn, true);
}
return doc;
}
/**
* Returns a promise that resolves after the given number of milliseconds, and returns the string
* "timeout".
*/
function delay(ms: number): Promise<string> {
return new Promise((resolve) => setTimeout(() => resolve("timeout"), ms));
}
jest.setTimeout(10000); // A little extra time for the save dialog to appear.
const dirtyFile = "debugger/dirty.ql";
const cleanFile = "debugger/clean.ql";
const otherGroupDirtyFile = "debugger/other-dirty.ql";
const otherGroupCleanFile = "debugger/other-clean.ql";
describe("saveBeforeStart", () => {
// We can't easily mock `TextDocument` because its properties are read-only and/or not
// configurable, so we rely on the actual `save()` method and the actual `isDirty` property.
beforeEach(async () => {
await commands.executeCommand("workbench.action.closeAllEditors");
});
it("should not save untitled documents without `includeUntitled`", async () => {
const dirtyDoc = await mockDocument(dirtyFile, true, ViewColumn.One);
const cleanDoc = await mockDocument(cleanFile, false, ViewColumn.One);
await saveAllInGroup(false);
expect(dirtyDoc.isDirty).toBe(false);
expect(cleanDoc.isDirty).toBe(false);
});
it("should not save dirty documents in other tab groups", async () => {
const dirtyDoc = await mockDocument(dirtyFile, true, ViewColumn.One);
const cleanDoc = await mockDocument(cleanFile, false, ViewColumn.One);
const otherGroupDirtyDoc = await mockDocument(
otherGroupDirtyFile,
true,
ViewColumn.Two,
);
const otherGroupCleanDoc = await mockDocument(
otherGroupCleanFile,
false,
ViewColumn.Two,
);
await saveAllInGroup(false);
expect(dirtyDoc.isDirty).toBe(false);
expect(cleanDoc.isDirty).toBe(false);
expect(otherGroupDirtyDoc.isDirty).toBe(true);
expect(otherGroupCleanDoc.isDirty).toBe(false);
});
it("should save untitled documents with `includeUntitled`", async () => {
const dirtyDoc = await mockDocument(dirtyFile, true, ViewColumn.One);
const cleanDoc = await mockDocument(cleanFile, false, ViewColumn.One);
const untitledDoc = await mockDocument(undefined, true, ViewColumn.One);
const otherGroupDirtyDoc = await mockDocument(
otherGroupDirtyFile,
true,
ViewColumn.Two,
);
const otherGroupCleanDoc = await mockDocument(
otherGroupCleanFile,
false,
ViewColumn.Two,
);
// Calling `save()` on an untitled document will bring up the file save dialog, and there's no
// good way to spy on `save()` because it's defined as read-only. Instead, we'll do the save all
// operation _anyway_, and _expect_ it to time out after 4 seconds. If it doesn't time out, then
// we know that the save dialog never popped up.
// This is pretty horrible, but it's the best I can come up with. It does need to be the last
// test in the suite, because it leaves the save dialog open.
const saveAll = async () => {
await saveAllInGroup(true);
return "saved";
};
const result = await Promise.race([saveAll(), delay(4000)]);
expect(result).toBe("timeout");
expect(dirtyDoc.isDirty).toBe(false);
expect(cleanDoc.isDirty).toBe(false);
expect(untitledDoc.isDirty).toBe(true);
expect(otherGroupDirtyDoc.isDirty).toBe(true);
expect(otherGroupCleanDoc.isDirty).toBe(false);
});
});