Merge remote-tracking branch 'origin/main' into koesie10/split-helpers-3

This commit is contained in:
Koen Vlaswinkel
2023-06-14 09:30:41 +02:00
17 changed files with 119 additions and 253 deletions

View File

@@ -4,11 +4,6 @@ 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>;
@@ -19,8 +14,6 @@ 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;
readonly environment: EnvironmentContext;

View File

@@ -40,14 +40,6 @@ 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

@@ -14,6 +14,7 @@ import { getQlPackPath, QLPACK_FILENAMES } from "../pure/ql";
import { getErrorMessage } from "../pure/helpers-pure";
import { ExtensionPack, ExtensionPackModelFile } from "./shared/extension-pack";
import { showAndLogErrorMessage } from "../common/vscode/log";
import { containsPath } from "../pure/files";
const maxStep = 3;
@@ -347,11 +348,7 @@ async function pickNewModelFile(
return "File already exists";
}
const notInExtensionPack = relative(
extensionPack.path,
path,
).startsWith("..");
if (notInExtensionPack) {
if (!containsPath(extensionPack.path, path)) {
return "File must be in the extension pack";
}

View File

@@ -147,14 +147,10 @@ export class DbConfigStore extends DisposableObject {
await this.writeConfig(config);
}
/**
* Adds a list of remote repositories to an existing repository list and removes duplicates.
* @returns a list of repositories that were not added because the list reached 1000 entries.
*/
public async addRemoteReposToList(
repoNwoList: string[],
parentList: string,
): Promise<string[]> {
): Promise<void> {
if (!this.config) {
throw Error("Cannot add variant analysis repos if config is not loaded");
}
@@ -172,21 +168,15 @@ export class DbConfigStore extends DisposableObject {
...new Set([...parent.repositories, ...repoNwoList]),
];
parent.repositories = newRepositoriesList.slice(0, 1000);
const truncatedRepositories = newRepositoriesList.slice(1000);
parent.repositories = newRepositoriesList;
await this.writeConfig(config);
return truncatedRepositories;
}
/**
* Adds one remote repository
* @returns either nothing, or, if a parentList is given AND the number of repos on that list reaches 1000 returns the repo that was not added.
*/
public async addRemoteRepo(
repoNwo: string,
parentList?: string,
): Promise<string[]> {
): Promise<void> {
if (!this.config) {
throw Error("Cannot add variant analysis repo if config is not loaded");
}
@@ -201,7 +191,6 @@ export class DbConfigStore extends DisposableObject {
);
}
const truncatedRepositories = [];
const config = cloneDbConfig(this.config);
if (parentList) {
const parent = config.databases.variantAnalysis.repositoryLists.find(
@@ -210,15 +199,12 @@ export class DbConfigStore extends DisposableObject {
if (!parent) {
throw Error(`Cannot find parent list '${parentList}'`);
} else {
const newRepositories = [...parent.repositories, repoNwo];
parent.repositories = newRepositories.slice(0, 1000);
truncatedRepositories.push(...newRepositories.slice(1000));
parent.repositories = [...parent.repositories, repoNwo];
}
} else {
config.databases.variantAnalysis.repositories.push(repoNwo);
}
await this.writeConfig(config);
return truncatedRepositories;
}
public async addRemoteOwner(owner: string): Promise<void> {

View File

@@ -101,15 +101,15 @@ export class DbManager extends DisposableObject {
public async addNewRemoteRepo(
nwo: string,
parentList?: string,
): Promise<string[]> {
return await this.dbConfigStore.addRemoteRepo(nwo, parentList);
): Promise<void> {
await this.dbConfigStore.addRemoteRepo(nwo, parentList);
}
public async addNewRemoteReposToList(
nwoList: string[],
parentList: string,
): Promise<string[]> {
return await this.dbConfigStore.addRemoteReposToList(nwoList, parentList);
): Promise<void> {
await this.dbConfigStore.addRemoteReposToList(nwoList, parentList);
}
public async addNewRemoteOwner(owner: string): Promise<void> {

View File

@@ -2,7 +2,7 @@
import * as cli from "../../codeql-cli/cli";
import vscode from "vscode";
import { FullDatabaseOptions } from "./database-options";
import { basename, dirname, extname, join, relative } from "path";
import { basename, dirname, extname, join } from "path";
import {
decodeSourceArchiveUri,
encodeArchiveBasePath,
@@ -12,7 +12,7 @@ import {
import { DatabaseItem, PersistedDatabaseItem } from "./database-item";
import { isLikelyDatabaseRoot } from "./db-contents-heuristics";
import { stat } from "fs-extra";
import { pathsEqual } from "../../pure/files";
import { containsPath, pathsEqual } from "../../pure/files";
import { DatabaseContents } from "./database-contents";
export class DatabaseItemImpl implements DatabaseItem {
@@ -199,7 +199,7 @@ export class DatabaseItemImpl implements DatabaseItem {
try {
const stats = await stat(testPath);
if (stats.isDirectory()) {
return !relative(testPath, databasePath).startsWith("..");
return containsPath(testPath, databasePath);
} else {
// database for /one/two/three/test.ql is at /one/two/three/three.testproj
const testdir = dirname(testPath);
@@ -207,7 +207,6 @@ export class DatabaseItemImpl implements DatabaseItem {
return pathsEqual(
databasePath,
join(testdir, `${testdirbase}.testproj`),
process.platform,
);
}
} catch {

View File

@@ -652,7 +652,7 @@ export class DatabaseManager extends DisposableObject {
private isExtensionControlledLocation(uri: vscode.Uri) {
const storageUri = this.ctx.storageUri || this.ctx.globalStorageUri;
if (storageUri) {
return containsPath(storageUri.fsPath, uri.fsPath, process.platform);
return containsPath(storageUri.fsPath, uri.fsPath);
}
return false;
}

View File

@@ -183,14 +183,7 @@ export class DbPanel extends DisposableObject {
return;
}
const truncatedRepositories = await this.dbManager.addNewRemoteRepo(
nwo,
parentList,
);
if (parentList) {
this.reportAnyTruncatedRepos(truncatedRepositories, parentList);
}
await this.dbManager.addNewRemoteRepo(nwo, parentList);
}
private async addNewRemoteOwner(): Promise<void> {
@@ -415,26 +408,11 @@ export class DbPanel extends DisposableObject {
progress.report({ increment: 10, message: "Processing results..." });
const truncatedRepositories =
await this.dbManager.addNewRemoteReposToList(repositories, listName);
this.reportAnyTruncatedRepos(truncatedRepositories, listName);
await this.dbManager.addNewRemoteReposToList(repositories, listName);
},
);
}
private reportAnyTruncatedRepos(
truncatedRepositories: string[],
listName: string,
) {
if (truncatedRepositories.length > 0) {
void showAndLogErrorMessage(
`Some repositories were not added to '${listName}' because a list can only have 1000 entries. Excluded repositories: ${truncatedRepositories.join(
", ",
)}`,
);
}
}
private async onDidCollapseElement(
event: TreeViewExpansionEvent<DbTreeViewItem>,
): Promise<void> {

View File

@@ -1,5 +1,5 @@
import { pathExists, stat, readdir, opendir } from "fs-extra";
import { join, resolve } from "path";
import { isAbsolute, join, relative, resolve } from "path";
/**
* Recursively finds all .ql files in this set of Uris.
@@ -51,36 +51,32 @@ export async function getDirectoryNamesInsidePath(
return dirNames;
}
function normalizePath(path: string, platform: NodeJS.Platform): string {
function normalizePath(path: string): string {
// On Windows, "C:/", "C:\", and "c:/" are all equivalent. We need
// to normalize the paths to ensure they all get resolved to the
// same format. On Windows, we also need to do the comparison
// case-insensitively.
path = resolve(path);
if (platform === "win32") {
if (process.platform === "win32") {
path = path.toLowerCase();
}
return path;
}
export function pathsEqual(
path1: string,
path2: string,
platform: NodeJS.Platform,
): boolean {
return normalizePath(path1, platform) === normalizePath(path2, platform);
export function pathsEqual(path1: string, path2: string): boolean {
return normalizePath(path1) === normalizePath(path2);
}
/**
* Returns true if path1 contains path2.
* Returns true if `parent` contains `child`, or if they are equal.
*/
export function containsPath(
path1: string,
path2: string,
platform: NodeJS.Platform,
): boolean {
return normalizePath(path2, platform).startsWith(
normalizePath(path1, platform),
export function containsPath(parent: string, child: string): boolean {
const relativePath = relative(parent, child);
return (
!relativePath.startsWith("..") &&
// On windows, if the two paths are in different drives, then the
// relative path will be an absolute path to the other drive.
!isAbsolute(relativePath)
);
}

View File

@@ -19,7 +19,7 @@ export class QueriesModule extends DisposableObject {
}
void extLogger.log("Initializing queries panel.");
const queryDiscovery = new QueryDiscovery(app, cliServer);
const queryDiscovery = new QueryDiscovery(app.environment, cliServer);
this.push(queryDiscovery);
void queryDiscovery.refresh();

View File

@@ -1,9 +1,16 @@
import { dirname, basename, normalize, relative } from "path";
import { Discovery } from "../common/discovery";
import { CodeQLCliServer } from "../codeql-cli/cli";
import { Event, RelativePattern, Uri, WorkspaceFolder } from "vscode";
import {
Event,
EventEmitter,
RelativePattern,
Uri,
WorkspaceFolder,
workspace,
} from "vscode";
import { MultiFileSystemWatcher } from "../common/vscode/multi-file-system-watcher";
import { App } from "../common/app";
import { EnvironmentContext } from "../common/app";
import { FileTreeDirectory, FileTreeLeaf } from "../common/file-tree-nodes";
import { getOnDiskWorkspaceFoldersObjects } from "../common/vscode/workspace-folders";
import { AppEventEmitter } from "../common/events";
@@ -42,13 +49,13 @@ export class QueryDiscovery
);
constructor(
private readonly app: App,
private readonly env: EnvironmentContext,
private readonly cliServer: CodeQLCliServer,
) {
super("Query Discovery", extLogger);
this.onDidChangeQueriesEmitter = this.push(app.createEventEmitter<void>());
this.push(app.onDidChangeWorkspaceFolders(this.refresh.bind(this)));
this.onDidChangeQueriesEmitter = this.push(new EventEmitter<void>());
this.push(workspace.onDidChangeWorkspaceFolders(this.refresh.bind(this)));
this.push(this.watcher.onDidChange(this.refresh.bind(this)));
}
@@ -130,7 +137,7 @@ export class QueryDiscovery
const rootDirectory = new FileTreeDirectory<string>(
fullPath,
name,
this.app.environment,
this.env,
);
for (const queryPath of resolvedQueries) {
const relativePath = normalize(relative(fullPath, queryPath));

View File

@@ -8,11 +8,6 @@ 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",
@@ -20,8 +15,6 @@ export function createMockApp({
globalStoragePath = "/mock/global/storage/path",
createEventEmitter = <T>() => new MockAppEventEmitter<T>(),
workspaceState = createMockMemento(),
workspaceFolders = [],
onDidChangeWorkspaceFolders = jest.fn(),
credentials = testCredentialsWithStub(),
commands = createMockCommandManager(),
environment = createMockEnvironmentContext(),
@@ -31,8 +24,6 @@ export function createMockApp({
globalStoragePath?: string;
createEventEmitter?: <T>() => AppEventEmitter<T>;
workspaceState?: Memento;
workspaceFolders?: readonly WorkspaceFolder[] | undefined;
onDidChangeWorkspaceFolders?: Event<WorkspaceFoldersChangeEvent>;
credentials?: Credentials;
commands?: AppCommandManager;
environment?: EnvironmentContext;
@@ -45,8 +36,6 @@ export function createMockApp({
workspaceStoragePath,
globalStoragePath,
workspaceState,
workspaceFolders,
onDidChangeWorkspaceFolders,
createEventEmitter,
credentials,
commands,

View File

@@ -10,7 +10,7 @@ const toEqualPath: MatcherFunction<[expectedPath: unknown]> = function (
throw new Error("These must be of type string!");
}
const pass = pathsEqual(actual, expectedPath, process.platform);
const pass = pathsEqual(actual, expectedPath);
if (pass) {
return {
message: () =>

View File

@@ -262,7 +262,7 @@ describe("db config store", () => {
});
// Add
const response = await configStore.addRemoteReposToList(
await configStore.addRemoteReposToList(
["owner/repo1", "owner/repo2"],
"list1",
);
@@ -278,72 +278,6 @@ describe("db config store", () => {
name: "list1",
repositories: ["owner/repo1", "owner/repo2"],
});
expect(response).toEqual([]);
configStore.dispose();
});
it("should add no more than 1000 repositories to a remote list when adding multiple repos", async () => {
// Initial set up
const dbConfig = createDbConfig({
remoteLists: [
{
name: "list1",
repositories: [],
},
],
});
const configStore = await initializeConfig(dbConfig, configPath, app);
// Add
const response = await configStore.addRemoteReposToList(
[...Array(1001).keys()].map((i) => `owner/db${i}`),
"list1",
);
// Read the config file
const updatedDbConfig = (await readJSON(configPath)) as DbConfig;
// Check that the config file has been updated
const updatedRemoteDbs = updatedDbConfig.databases.variantAnalysis;
expect(updatedRemoteDbs.repositories).toHaveLength(0);
expect(updatedRemoteDbs.repositoryLists).toHaveLength(1);
expect(updatedRemoteDbs.repositoryLists[0].repositories).toHaveLength(
1000,
);
expect(response).toEqual(["owner/db1000"]);
configStore.dispose();
});
it("should add no more than 1000 repositories to a remote list when adding one repo", async () => {
// Initial set up
const dbConfig = createDbConfig({
remoteLists: [
{
name: "list1",
repositories: [...Array(1000).keys()].map((i) => `owner/db${i}`),
},
],
});
const configStore = await initializeConfig(dbConfig, configPath, app);
// Add
const reponse = await configStore.addRemoteRepo("owner/db1000", "list1");
// Read the config file
const updatedDbConfig = (await readJSON(configPath)) as DbConfig;
// Check that the config file has been updated
const updatedRemoteDbs = updatedDbConfig.databases.variantAnalysis;
expect(updatedRemoteDbs.repositories).toHaveLength(0);
expect(updatedRemoteDbs.repositoryLists).toHaveLength(1);
expect(updatedRemoteDbs.repositoryLists[0].repositories).toHaveLength(
1000,
);
expect(reponse).toEqual(["owner/db1000"]);
configStore.dispose();
});

View File

@@ -115,46 +115,6 @@ describe("db manager", () => {
});
});
it("should return truncated repos when adding multiple repos to a user defined list", async () => {
const dbConfig: DbConfig = createDbConfig({
remoteLists: [
{
name: "my-list-1",
repositories: [...Array(1000).keys()].map((i) => `owner/db${i}`),
},
],
});
await saveDbConfig(dbConfig);
const response = await dbManager.addNewRemoteReposToList(
["owner2/repo2"],
"my-list-1",
);
expect(response).toEqual(["owner2/repo2"]);
});
it("should return truncated repos when adding one repo to a user defined list", async () => {
const dbConfig: DbConfig = createDbConfig({
remoteLists: [
{
name: "my-list-1",
repositories: [...Array(1000).keys()].map((i) => `owner/db${i}`),
},
],
});
await saveDbConfig(dbConfig);
const response = await dbManager.addNewRemoteRepo(
"owner2/repo2",
"my-list-1",
);
expect(response).toEqual(["owner2/repo2"]);
});
it("should add a new remote repo to a user defined list", async () => {
const dbConfig: DbConfig = createDbConfig({
remoteLists: [

View File

@@ -204,102 +204,142 @@ describe("pathsEqual", () => {
return;
}
expect(pathsEqual(path1, path2, platform)).toEqual(expected);
expect(pathsEqual(path1, path2)).toEqual(expected);
},
);
});
describe("containsPath", () => {
const testCases: Array<{
path1: string;
path2: string;
parent: string;
child: string;
platform: NodeJS.Platform;
expected: boolean;
}> = [
{
path1:
parent:
"/home/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript",
path2:
child:
"/home/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql",
platform: "linux",
expected: true,
},
{
path1:
parent:
"/HOME/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript",
path2:
child:
"/home/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql",
platform: "linux",
expected: false,
},
{
path1:
parent:
"/home/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql",
path2:
child:
"/home/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript",
platform: "linux",
expected: false,
},
{
path1:
parent:
"/home/github/projects/vscode-codeql-starter/codeql-custom-queries-java",
child:
"/home/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript",
platform: "linux",
expected: false,
},
{
parent:
"/home/github/projects/vscode-codeql-starter/codeql-custom-queries-java",
child:
"/home/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql",
platform: "linux",
expected: false,
},
{
parent:
"C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript",
path2:
child:
"C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql",
platform: "win32",
expected: true,
},
{
path1:
parent:
"C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript",
path2:
child:
"c:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql",
platform: "win32",
expected: true,
},
{
path1:
parent:
"C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript",
path2:
child:
"C:/USERS/GITHUB/PROJECTS/VSCODE-CODEQL-STARTER/CODEQL-CUSTOM-QUERIES-JAVASCRIPT/EXAMPLE.QL",
platform: "win32",
expected: true,
},
{
parent:
"C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript",
child:
"D:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql",
platform: "win32",
expected: false,
},
{
path1:
parent:
"C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql",
path2:
child:
"C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript",
platform: "win32",
expected: false,
},
{
path1:
parent:
"C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript",
path2:
child:
"C:\\Users\\github\\projects\\vscode-codeql-starter\\codeql-custom-queries-javascript\\example.ql",
platform: "win32",
expected: true,
},
{
path1:
parent:
"C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript",
path2:
child:
"D:\\Users\\github\\projects\\vscode-codeql-starter\\codeql-custom-queries-javascript\\example.ql",
platform: "win32",
expected: false,
},
{
parent:
"C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-java",
child:
"C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript",
platform: "win32",
expected: false,
},
{
parent:
"C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-java",
child:
"C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql",
platform: "win32",
expected: false,
},
];
test.each(testCases)(
"$path1 contains $path2 on $platform = $expected",
({ path1, path2, platform, expected }) => {
"$parent contains $child on $platform = $expected",
({ parent, child, platform, expected }) => {
if (platform !== process.platform) {
// We're using the platform-specific path.resolve, so we can't really run
// these tests on all platforms.
return;
}
expect(containsPath(path1, path2, platform)).toEqual(expected);
expect(containsPath(parent, child)).toEqual(expected);
},
);
});

View File

@@ -7,7 +7,7 @@ import {
} from "vscode";
import { CodeQLCliServer } from "../../../../src/codeql-cli/cli";
import { QueryDiscovery } from "../../../../src/queries-panel/query-discovery";
import { createMockApp } from "../../../__mocks__/appMock";
import { createMockEnvironmentContext } from "../../../__mocks__/appMock";
import { mockedObject } from "../../utils/mocking.helpers";
import { basename, join, sep } from "path";
@@ -23,7 +23,7 @@ describe("QueryDiscovery", () => {
resolveQueries,
});
const discovery = new QueryDiscovery(createMockApp({}), cli);
const discovery = new QueryDiscovery(createMockEnvironmentContext(), cli);
await discovery.refresh();
const queries = discovery.queries;
@@ -43,7 +43,7 @@ describe("QueryDiscovery", () => {
]),
});
const discovery = new QueryDiscovery(createMockApp({}), cli);
const discovery = new QueryDiscovery(createMockEnvironmentContext(), cli);
await discovery.refresh();
const queries = discovery.queries;
expect(queries).toBeDefined();
@@ -69,7 +69,7 @@ describe("QueryDiscovery", () => {
]),
});
const discovery = new QueryDiscovery(createMockApp({}), cli);
const discovery = new QueryDiscovery(createMockEnvironmentContext(), cli);
await discovery.refresh();
const queries = discovery.queries;
expect(queries).toBeDefined();
@@ -114,7 +114,7 @@ describe("QueryDiscovery", () => {
resolveQueries,
});
const discovery = new QueryDiscovery(createMockApp({}), cli);
const discovery = new QueryDiscovery(createMockEnvironmentContext(), cli);
await discovery.refresh();
const queries = discovery.queries;
expect(queries).toBeDefined();
@@ -153,12 +153,7 @@ describe("QueryDiscovery", () => {
.mockResolvedValue([join(workspaceRoot, "query1.ql")]),
});
const discovery = new QueryDiscovery(
createMockApp({
createEventEmitter: () => new EventEmitter(),
}),
cli,
);
const discovery = new QueryDiscovery(createMockEnvironmentContext(), cli);
const onDidChangeQueriesSpy = jest.fn();
discovery.onDidChangeQueries(onDidChangeQueriesSpy);
@@ -180,12 +175,12 @@ describe("QueryDiscovery", () => {
it("should refresh when workspace folders change", async () => {
const onDidChangeWorkspaceFoldersEvent =
new EventEmitter<WorkspaceFoldersChangeEvent>();
jest
.spyOn(workspace, "onDidChangeWorkspaceFolders")
.mockImplementation(onDidChangeWorkspaceFoldersEvent.event);
const discovery = new QueryDiscovery(
createMockApp({
createEventEmitter: () => new EventEmitter(),
onDidChangeWorkspaceFolders: onDidChangeWorkspaceFoldersEvent.event,
}),
createMockEnvironmentContext(),
mockedObject<CodeQLCliServer>({
resolveQueries: jest.fn().mockResolvedValue([]),
}),