Merge branch 'main' into koesie10/unsupported-cli-version-check

This commit is contained in:
Koen Vlaswinkel
2023-05-22 11:37:58 +02:00
committed by GitHub
13 changed files with 241 additions and 135 deletions

View File

@@ -3,7 +3,9 @@
## [UNRELEASED]
- Add settings `codeQL.variantAnalysis.defaultResultsFilter` and `codeQL.variantAnalysis.defaultResultsSort` for configuring how variant analysis results are filtered and sorted in the results view. The default is to show all repositories, and to sort by the number of results. [#2392](https://github.com/github/vscode-codeql/pull/2392)
- Fix bug to ensure error messages have complete stack trace in message logs. [#2425](https://github.com/github/vscode-codeql/pull/2425)
- Fix bug where the `CodeQL: Compare Query` command did not work for comparing quick-eval queries. [#2422](https://github.com/github/vscode-codeql/pull/2422)
- Update text of copy and export buttons in variant analysis results view to clarify that they only copy/export the selected/filtered results. [#2427](https://github.com/github/vscode-codeql/pull/2427)
- Add warning when using unsupported CodeQL CLI version. [#2428](https://github.com/github/vscode-codeql/pull/2428)
## 1.8.4 - 3 May 2023

View File

@@ -4,6 +4,11 @@ import { AppEventEmitter } from "./events";
import { Logger } from "./logging";
import { Memento } from "./memento";
import { AppCommandManager } from "./commands";
import type {
WorkspaceFolder,
Event,
WorkspaceFoldersChangeEvent,
} from "vscode";
export interface App {
createEventEmitter<T>(): AppEventEmitter<T>;
@@ -14,6 +19,8 @@ export interface App {
readonly globalStoragePath: string;
readonly workspaceStoragePath?: string;
readonly workspaceState: Memento;
readonly workspaceFolders: readonly WorkspaceFolder[] | undefined;
readonly onDidChangeWorkspaceFolders: Event<WorkspaceFoldersChangeEvent>;
readonly credentials: Credentials;
readonly commands: AppCommandManager;
}

View File

@@ -0,0 +1,103 @@
import { basename, dirname, join } from "path";
import { env } from "vscode";
/**
* A node in the tree of files. This will be either a `FileTreeDirectory` or a `FileTreeLeaf`.
*/
export abstract class FileTreeNode {
constructor(private _path: string, private _name: string) {}
public get path(): string {
return this._path;
}
public get name(): string {
return this._name;
}
public abstract get children(): readonly FileTreeNode[];
public abstract finish(): void;
}
/**
* A directory containing one or more files or other directories.
*/
export class FileTreeDirectory extends FileTreeNode {
constructor(
_path: string,
_name: string,
private _children: FileTreeNode[] = [],
) {
super(_path, _name);
}
public get children(): readonly FileTreeNode[] {
return this._children;
}
public addChild(child: FileTreeNode): void {
this._children.push(child);
}
public createDirectory(relativePath: string): FileTreeDirectory {
const dirName = dirname(relativePath);
if (dirName === ".") {
return this.createChildDirectory(relativePath);
} else {
const parent = this.createDirectory(dirName);
return parent.createDirectory(basename(relativePath));
}
}
public finish(): void {
// remove empty directories
this._children.filter(
(child) => child instanceof FileTreeLeaf || child.children.length > 0,
);
this._children.sort((a, b) => a.name.localeCompare(b.name, env.language));
this._children.forEach((child, i) => {
child.finish();
if (
child.children?.length === 1 &&
child.children[0] instanceof FileTreeDirectory
) {
// collapse children
const replacement = new FileTreeDirectory(
child.children[0].path,
`${child.name} / ${child.children[0].name}`,
Array.from(child.children[0].children),
);
this._children[i] = replacement;
}
});
}
private createChildDirectory(name: string): FileTreeDirectory {
const existingChild = this._children.find((child) => child.name === name);
if (existingChild !== undefined) {
return existingChild as FileTreeDirectory;
} else {
const newChild = new FileTreeDirectory(join(this.path, name), name);
this.addChild(newChild);
return newChild;
}
}
}
/**
* A single file.
*/
export class FileTreeLeaf extends FileTreeNode {
constructor(_path: string, _name: string) {
super(_path, _name);
}
public get children(): readonly FileTreeNode[] {
return [];
}
public finish(): void {
/**/
}
}

View File

@@ -49,7 +49,6 @@ export function registerCommandWithErrorHandling(
const errorMessage = redactableError(error)`${
getErrorMessage(e) || e
} (${commandId})`;
const errorStack = getErrorStack(e);
if (e instanceof UserCancellationException) {
// User has cancelled this action manually
if (e.silent) {
@@ -61,6 +60,7 @@ export function registerCommandWithErrorHandling(
}
} else {
// Include the full stack in the error log only.
const errorStack = getErrorStack(e);
const fullMessage = errorStack
? `${errorMessage.fullMessage}\n${errorStack}`
: errorMessage.fullMessage;

View File

@@ -39,6 +39,14 @@ export class ExtensionApp implements App {
return this.extensionContext.workspaceState;
}
public get workspaceFolders(): readonly vscode.WorkspaceFolder[] | undefined {
return vscode.workspace.workspaceFolders;
}
public get onDidChangeWorkspaceFolders(): vscode.Event<vscode.WorkspaceFoldersChangeEvent> {
return vscode.workspace.onDidChangeWorkspaceFolders;
}
public get subscriptions(): Disposable[] {
return this.extensionContext.subscriptions;
}

View File

@@ -98,7 +98,7 @@ export async function showAndLogErrorMessage(
return internalShowAndLog(
dropLinesExceptInitial(message),
Window.showErrorMessage,
options,
{ fullMessage: message, ...options },
);
}

View File

@@ -1,4 +1,4 @@
import { dirname, basename, join, normalize, relative, extname } from "path";
import { dirname, basename, normalize, relative, extname } from "path";
import { Discovery } from "../common/discovery";
import {
EventEmitter,
@@ -6,112 +6,11 @@ import {
Uri,
RelativePattern,
WorkspaceFolder,
env,
} from "vscode";
import { MultiFileSystemWatcher } from "../common/vscode/multi-file-system-watcher";
import { CodeQLCliServer } from "../codeql-cli/cli";
import { pathExists } from "fs-extra";
/**
* A node in the tree of tests. This will be either a `QLTestDirectory` or a `QLTestFile`.
*/
export abstract class QLTestNode {
constructor(private _path: string, private _name: string) {}
public get path(): string {
return this._path;
}
public get name(): string {
return this._name;
}
public abstract get children(): readonly QLTestNode[];
public abstract finish(): void;
}
/**
* A directory containing one or more QL tests or other test directories.
*/
export class QLTestDirectory extends QLTestNode {
constructor(
_path: string,
_name: string,
private _children: QLTestNode[] = [],
) {
super(_path, _name);
}
public get children(): readonly QLTestNode[] {
return this._children;
}
public addChild(child: QLTestNode): void {
this._children.push(child);
}
public createDirectory(relativePath: string): QLTestDirectory {
const dirName = dirname(relativePath);
if (dirName === ".") {
return this.createChildDirectory(relativePath);
} else {
const parent = this.createDirectory(dirName);
return parent.createDirectory(basename(relativePath));
}
}
public finish(): void {
// remove empty directories
this._children.filter(
(child) => child instanceof QLTestFile || child.children.length > 0,
);
this._children.sort((a, b) => a.name.localeCompare(b.name, env.language));
this._children.forEach((child, i) => {
child.finish();
if (
child.children?.length === 1 &&
child.children[0] instanceof QLTestDirectory
) {
// collapse children
const replacement = new QLTestDirectory(
child.children[0].path,
`${child.name} / ${child.children[0].name}`,
Array.from(child.children[0].children),
);
this._children[i] = replacement;
}
});
}
private createChildDirectory(name: string): QLTestDirectory {
const existingChild = this._children.find((child) => child.name === name);
if (existingChild !== undefined) {
return existingChild as QLTestDirectory;
} else {
const newChild = new QLTestDirectory(join(this.path, name), name);
this.addChild(newChild);
return newChild;
}
}
}
/**
* A single QL test. This will be either a `.ql` file or a `.qlref` file.
*/
export class QLTestFile extends QLTestNode {
constructor(_path: string, _name: string) {
super(_path, _name);
}
public get children(): readonly QLTestNode[] {
return [];
}
public finish(): void {
/**/
}
}
import { FileTreeDirectory, FileTreeLeaf } from "../common/file-tree-nodes";
/**
* The results of discovering QL tests.
@@ -120,7 +19,7 @@ interface QLTestDiscoveryResults {
/**
* A directory that contains one or more QL Tests, or other QLTestDirectories.
*/
testDirectory: QLTestDirectory | undefined;
testDirectory: FileTreeDirectory | undefined;
/**
* The file system path to a directory to watch. If any ql or qlref file changes in
@@ -137,7 +36,7 @@ export class QLTestDiscovery extends Discovery<QLTestDiscoveryResults> {
private readonly watcher: MultiFileSystemWatcher = this.push(
new MultiFileSystemWatcher(),
);
private _testDirectory: QLTestDirectory | undefined;
private _testDirectory: FileTreeDirectory | undefined;
constructor(
private readonly workspaceFolder: WorkspaceFolder,
@@ -159,7 +58,7 @@ export class QLTestDiscovery extends Discovery<QLTestDiscoveryResults> {
* The root directory. There is at least one test in this directory, or
* in a subdirectory of this.
*/
public get testDirectory(): QLTestDirectory | undefined {
public get testDirectory(): FileTreeDirectory | undefined {
return this._testDirectory;
}
@@ -194,10 +93,10 @@ export class QLTestDiscovery extends Discovery<QLTestDiscoveryResults> {
* @returns A `QLTestDirectory` object describing the contents of the directory, or `undefined` if
* no tests were found.
*/
private async discoverTests(): Promise<QLTestDirectory> {
private async discoverTests(): Promise<FileTreeDirectory> {
const fullPath = this.workspaceFolder.uri.fsPath;
const name = this.workspaceFolder.name;
const rootDirectory = new QLTestDirectory(fullPath, name);
const rootDirectory = new FileTreeDirectory(fullPath, name);
// Don't try discovery on workspace folders that don't exist on the filesystem
if (await pathExists(fullPath)) {
@@ -208,7 +107,9 @@ export class QLTestDiscovery extends Discovery<QLTestDiscoveryResults> {
const relativePath = normalize(relative(fullPath, testPath));
const dirName = dirname(relativePath);
const parentDirectory = rootDirectory.createDirectory(dirName);
parentDirectory.addChild(new QLTestFile(testPath, basename(testPath)));
parentDirectory.addChild(
new FileTreeLeaf(testPath, basename(testPath)),
);
}
rootDirectory.finish();

View File

@@ -13,17 +13,17 @@ import {
TestHub,
} from "vscode-test-adapter-api";
import { TestAdapterRegistrar } from "vscode-test-adapter-util";
import {
QLTestFile,
QLTestNode,
QLTestDirectory,
QLTestDiscovery,
} from "./qltest-discovery";
import { QLTestDiscovery } from "./qltest-discovery";
import { Event, EventEmitter, CancellationTokenSource } from "vscode";
import { DisposableObject } from "../pure/disposable-object";
import { CodeQLCliServer, TestCompleted } from "../codeql-cli/cli";
import { testLogger } from "../common";
import { TestRunner } from "./test-runner";
import {
FileTreeDirectory,
FileTreeLeaf,
FileTreeNode,
} from "../common/file-tree-nodes";
/**
* Get the full path of the `.expected` file for the specified QL test.
@@ -135,7 +135,7 @@ export class QLTestAdapter extends DisposableObject implements TestAdapter {
}
private static createTestOrSuiteInfos(
testNodes: readonly QLTestNode[],
testNodes: readonly FileTreeNode[],
): Array<TestSuiteInfo | TestInfo> {
return testNodes.map((childNode) => {
return QLTestAdapter.createTestOrSuiteInfo(childNode);
@@ -143,18 +143,18 @@ export class QLTestAdapter extends DisposableObject implements TestAdapter {
}
private static createTestOrSuiteInfo(
testNode: QLTestNode,
testNode: FileTreeNode,
): TestSuiteInfo | TestInfo {
if (testNode instanceof QLTestFile) {
if (testNode instanceof FileTreeLeaf) {
return QLTestAdapter.createTestInfo(testNode);
} else if (testNode instanceof QLTestDirectory) {
} else if (testNode instanceof FileTreeDirectory) {
return QLTestAdapter.createTestSuiteInfo(testNode, testNode.name);
} else {
throw new Error("Unexpected test type.");
}
}
private static createTestInfo(testFile: QLTestFile): TestInfo {
private static createTestInfo(testFile: FileTreeLeaf): TestInfo {
return {
type: "test",
id: testFile.path,
@@ -165,7 +165,7 @@ export class QLTestAdapter extends DisposableObject implements TestAdapter {
}
private static createTestSuiteInfo(
testDirectory: QLTestDirectory,
testDirectory: FileTreeDirectory,
label: string,
): TestSuiteInfo {
return {

View File

@@ -16,12 +16,7 @@ import {
workspace,
} from "vscode";
import { DisposableObject } from "../pure/disposable-object";
import {
QLTestDirectory,
QLTestDiscovery,
QLTestFile,
QLTestNode,
} from "./qltest-discovery";
import { QLTestDiscovery } from "./qltest-discovery";
import { CodeQLCliServer } from "../codeql-cli/cli";
import { getErrorMessage } from "../pure/helpers-pure";
import { BaseLogger, LogOptions } from "../common";
@@ -29,6 +24,11 @@ import { TestRunner } from "./test-runner";
import { TestManagerBase } from "./test-manager-base";
import { App } from "../common/app";
import { isWorkspaceFolderOnDisk } from "../helpers";
import {
FileTreeDirectory,
FileTreeLeaf,
FileTreeNode,
} from "../common/file-tree-nodes";
/**
* Returns the complete text content of the specified file. If there is an error reading the file,
@@ -209,7 +209,7 @@ export class TestManager extends TestManagerBase {
*/
public updateTestsForWorkspaceFolder(
workspaceFolder: WorkspaceFolder,
testDirectory: QLTestDirectory | undefined,
testDirectory: FileTreeDirectory | undefined,
): void {
if (testDirectory !== undefined) {
// Adding an item with the same ID as an existing item will replace it, which is exactly what
@@ -229,9 +229,9 @@ export class TestManager extends TestManagerBase {
/**
* Creates a tree of `TestItem`s from the root `QlTestNode` provided by test discovery.
*/
private createTestItemTree(node: QLTestNode, isRoot: boolean): TestItem {
private createTestItemTree(node: FileTreeNode, isRoot: boolean): TestItem {
// Prefix the ID to identify it as a directory or a test
const itemType = node instanceof QLTestDirectory ? "dir" : "test";
const itemType = node instanceof FileTreeDirectory ? "dir" : "test";
const testItem = this.testController.createTestItem(
// For the root of a workspace folder, use the full path as the ID. Otherwise, use the node's
// name as the ID, since it's shorter but still unique.
@@ -242,7 +242,7 @@ export class TestManager extends TestManagerBase {
for (const childNode of node.children) {
const childItem = this.createTestItemTree(childNode, false);
if (childNode instanceof QLTestFile) {
if (childNode instanceof FileTreeLeaf) {
childItem.range = new Range(0, 0, 0, 0);
}
testItem.children.add(childItem);

View File

@@ -14,6 +14,9 @@ export type VariantAnalysisActionsProps = {
onExportResultsClick: () => void;
copyRepositoryListDisabled?: boolean;
exportResultsDisabled?: boolean;
hasSelectedRepositories?: boolean;
hasFilteredRepositories?: boolean;
};
const Container = styled.div`
@@ -26,6 +29,28 @@ const Button = styled(VSCodeButton)`
white-space: nowrap;
`;
const chooseText = ({
hasSelectedRepositories,
hasFilteredRepositories,
normalText,
selectedText,
filteredText,
}: {
hasSelectedRepositories?: boolean;
hasFilteredRepositories?: boolean;
normalText: string;
selectedText: string;
filteredText: string;
}) => {
if (hasSelectedRepositories) {
return selectedText;
}
if (hasFilteredRepositories) {
return filteredText;
}
return normalText;
};
export const VariantAnalysisActions = ({
variantAnalysisStatus,
onStopQueryClick,
@@ -35,6 +60,8 @@ export const VariantAnalysisActions = ({
onExportResultsClick,
copyRepositoryListDisabled,
exportResultsDisabled,
hasSelectedRepositories,
hasFilteredRepositories,
}: VariantAnalysisActionsProps) => {
return (
<Container>
@@ -45,14 +72,26 @@ export const VariantAnalysisActions = ({
onClick={onCopyRepositoryListClick}
disabled={copyRepositoryListDisabled}
>
Copy repository list
{chooseText({
hasSelectedRepositories,
hasFilteredRepositories,
normalText: "Copy repository list",
selectedText: "Copy selected repositories as a list",
filteredText: "Copy filtered repositories as a list",
})}
</Button>
<Button
appearance="primary"
onClick={onExportResultsClick}
disabled={exportResultsDisabled}
>
Export results
{chooseText({
hasSelectedRepositories,
hasFilteredRepositories,
normalText: "Export results",
selectedText: "Export selected results",
filteredText: "Export filtered results",
})}
</Button>
</>
)}

View File

@@ -131,6 +131,13 @@ export const VariantAnalysisHeader = ({
stopQueryDisabled={!variantAnalysis.actionsWorkflowRunId}
exportResultsDisabled={!hasDownloadedRepos}
copyRepositoryListDisabled={!hasReposWithResults}
hasFilteredRepositories={
variantAnalysis.scannedRepos?.length !==
filteredRepositories?.length
}
hasSelectedRepositories={
selectedRepositoryIds && selectedRepositoryIds.length > 0
}
/>
</Row>
<VariantAnalysisStats

View File

@@ -93,4 +93,32 @@ describe(VariantAnalysisActions.name, () => {
expect(container.querySelectorAll("vscode-button").length).toEqual(0);
});
it("changes the text on the buttons when repositories are selected", async () => {
render({
variantAnalysisStatus: VariantAnalysisStatus.Succeeded,
showResultActions: true,
hasSelectedRepositories: true,
hasFilteredRepositories: true,
});
expect(screen.getByText("Export selected results")).toBeInTheDocument();
expect(
screen.getByText("Copy selected repositories as a list"),
).toBeInTheDocument();
});
it("changes the text on the buttons when repositories are filtered", async () => {
render({
variantAnalysisStatus: VariantAnalysisStatus.Succeeded,
showResultActions: true,
hasSelectedRepositories: false,
hasFilteredRepositories: true,
});
expect(screen.getByText("Export filtered results")).toBeInTheDocument();
expect(
screen.getByText("Copy filtered repositories as a list"),
).toBeInTheDocument();
});
});

View File

@@ -8,6 +8,11 @@ import { testCredentialsWithStub } from "../factories/authentication";
import { Credentials } from "../../src/common/authentication";
import { AppCommandManager } from "../../src/common/commands";
import { createMockCommandManager } from "./commandsMock";
import type {
Event,
WorkspaceFolder,
WorkspaceFoldersChangeEvent,
} from "vscode";
export function createMockApp({
extensionPath = "/mock/extension/path",
@@ -15,6 +20,8 @@ export function createMockApp({
globalStoragePath = "/mock/global/storage/path",
createEventEmitter = <T>() => new MockAppEventEmitter<T>(),
workspaceState = createMockMemento(),
workspaceFolders = [],
onDidChangeWorkspaceFolders = jest.fn(),
credentials = testCredentialsWithStub(),
commands = createMockCommandManager(),
}: {
@@ -23,6 +30,8 @@ export function createMockApp({
globalStoragePath?: string;
createEventEmitter?: <T>() => AppEventEmitter<T>;
workspaceState?: Memento;
workspaceFolders?: readonly WorkspaceFolder[] | undefined;
onDidChangeWorkspaceFolders?: Event<WorkspaceFoldersChangeEvent>;
credentials?: Credentials;
commands?: AppCommandManager;
}): App {
@@ -34,6 +43,8 @@ export function createMockApp({
workspaceStoragePath,
globalStoragePath,
workspaceState,
workspaceFolders,
onDidChangeWorkspaceFolders,
createEventEmitter,
credentials,
commands,