diff --git a/extensions/ql-vscode/media/dark/lgtm-plus.svg b/extensions/ql-vscode/media/dark/lgtm-plus.svg deleted file mode 100644 index cfa1bb7a9..000000000 --- a/extensions/ql-vscode/media/dark/lgtm-plus.svg +++ /dev/null @@ -1,5 +0,0 @@ - - - - - diff --git a/extensions/ql-vscode/media/light/lgtm-plus.svg b/extensions/ql-vscode/media/light/lgtm-plus.svg deleted file mode 100644 index 4ca2d548f..000000000 --- a/extensions/ql-vscode/media/light/lgtm-plus.svg +++ /dev/null @@ -1,5 +0,0 @@ - - - - - diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index bd178ab52..5cd9b16a6 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -48,7 +48,6 @@ "onCommand:codeQLDatabases.chooseDatabaseArchive", "onCommand:codeQLDatabases.chooseDatabaseInternet", "onCommand:codeQLDatabases.chooseDatabaseGithub", - "onCommand:codeQLDatabases.chooseDatabaseLgtm", "onCommand:codeQL.setCurrentDatabase", "onCommand:codeQL.viewAst", "onCommand:codeQL.viewCfg", @@ -58,7 +57,6 @@ "onCommand:codeQL.chooseDatabaseArchive", "onCommand:codeQL.chooseDatabaseInternet", "onCommand:codeQL.chooseDatabaseGithub", - "onCommand:codeQL.chooseDatabaseLgtm", "onCommand:codeQLDatabases.chooseDatabase", "onCommand:codeQLDatabases.setCurrentDatabase", "onCommand:codeQLDatabasesExperimental.openConfigFile", @@ -410,14 +408,6 @@ "dark": "media/dark/github.svg" } }, - { - "command": "codeQLDatabases.chooseDatabaseLgtm", - "title": "Download from LGTM", - "icon": { - "light": "media/light/lgtm-plus.svg", - "dark": "media/dark/lgtm-plus.svg" - } - }, { "command": "codeQL.setCurrentDatabase", "title": "CodeQL: Set Current Database" @@ -486,10 +476,6 @@ "command": "codeQL.chooseDatabaseGithub", "title": "CodeQL: Download Database from GitHub" }, - { - "command": "codeQL.chooseDatabaseLgtm", - "title": "CodeQL: Download Database from LGTM" - }, { "command": "codeQLDatabases.sortByName", "title": "Sort by Name", @@ -728,11 +714,6 @@ "when": "view == codeQLDatabases", "group": "navigation" }, - { - "command": "codeQLDatabases.chooseDatabaseLgtm", - "when": "config.codeQL.canary && view == codeQLDatabases", - "group": "navigation" - }, { "command": "codeQLQueryHistory.openQuery", "when": "view == codeQLQueryHistory", @@ -780,7 +761,7 @@ }, { "command": "codeQLDatabasesExperimental.addNewList", - "when": "view == codeQLDatabasesExperimental", + "when": "view == codeQLDatabasesExperimental && codeQLDatabasesExperimental.configError == false", "group": "navigation" } ], @@ -997,10 +978,6 @@ "command": "codeQL.viewCfg", "when": "resourceScheme == codeql-zip-archive && config.codeQL.canary" }, - { - "command": "codeQL.chooseDatabaseLgtm", - "when": "config.codeQL.canary" - }, { "command": "codeQLDatabasesExperimental.openConfigFile", "when": "false" @@ -1061,10 +1038,6 @@ "command": "codeQLDatabases.chooseDatabaseGithub", "when": "false" }, - { - "command": "codeQLDatabases.chooseDatabaseLgtm", - "when": "false" - }, { "command": "codeQLDatabases.upgradeDatabase", "when": "false" diff --git a/extensions/ql-vscode/src/common/app.ts b/extensions/ql-vscode/src/common/app.ts index 64b441665..f9d35f315 100644 --- a/extensions/ql-vscode/src/common/app.ts +++ b/extensions/ql-vscode/src/common/app.ts @@ -1,10 +1,12 @@ import { Disposable } from "../pure/disposable-object"; import { AppEventEmitter } from "./events"; +import { Logger } from "./logging"; export interface App { createEventEmitter(): AppEventEmitter; executeCommand(command: string, ...args: any): Thenable; mode: AppMode; + logger: Logger; subscriptions: Disposable[]; extensionPath: string; globalStoragePath: string; diff --git a/extensions/ql-vscode/src/common/vscode/vscode-app.ts b/extensions/ql-vscode/src/common/vscode/vscode-app.ts index fcbf5d66a..69f262fc9 100644 --- a/extensions/ql-vscode/src/common/vscode/vscode-app.ts +++ b/extensions/ql-vscode/src/common/vscode/vscode-app.ts @@ -2,6 +2,7 @@ import * as vscode from "vscode"; import { Disposable } from "../../pure/disposable-object"; import { App, AppMode } from "../app"; import { AppEventEmitter } from "../events"; +import { extLogger, Logger } from "../logging"; import { VSCodeAppEventEmitter } from "./events"; export class ExtensionApp implements App { @@ -36,6 +37,10 @@ export class ExtensionApp implements App { } } + public get logger(): Logger { + return extLogger; + } + public createEventEmitter(): AppEventEmitter { return new VSCodeAppEventEmitter(); } diff --git a/extensions/ql-vscode/src/databaseFetcher.ts b/extensions/ql-vscode/src/databaseFetcher.ts index 8c5b55425..f3d46e370 100644 --- a/extensions/ql-vscode/src/databaseFetcher.ts +++ b/extensions/ql-vscode/src/databaseFetcher.ts @@ -153,74 +153,6 @@ export async function promptImportGithubDatabase( return; } -/** - * Prompts a user to fetch a database from lgtm. - * User enters a project url and then the user is asked which language - * to download (if there is more than one) - * - * @param databaseManager the DatabaseManager - * @param storagePath where to store the unzipped database. - */ -export async function promptImportLgtmDatabase( - databaseManager: DatabaseManager, - storagePath: string, - progress: ProgressCallback, - token: CancellationToken, - cli?: CodeQLCliServer, -): Promise { - progress({ - message: "Choose project", - step: 1, - maxStep: 2, - }); - const lgtmUrl = await window.showInputBox({ - prompt: - "Enter the project slug or URL on LGTM (e.g., g/github/codeql or https://lgtm.com/projects/g/github/codeql)", - }); - if (!lgtmUrl) { - return; - } - - if (looksLikeLgtmUrl(lgtmUrl)) { - const databaseUrl = await convertLgtmUrlToDatabaseUrl(lgtmUrl, progress); - if (databaseUrl) { - const item = await databaseArchiveFetcher( - databaseUrl, - {}, - databaseManager, - storagePath, - undefined, - progress, - token, - cli, - ); - if (item) { - await commands.executeCommand("codeQLDatabases.focus"); - void showAndLogInformationMessage( - "Database downloaded and imported successfully.", - ); - } - return item; - } - } else { - throw new Error(`Invalid LGTM URL: ${lgtmUrl}`); - } - return; -} - -export async function retrieveCanonicalRepoName(lgtmUrl: string) { - const givenRepoName = extractProjectSlug(lgtmUrl); - const response = await checkForFailingResponse( - await fetch(`https://api.github.com/repos/${givenRepoName}`), - "Failed to locate the repository on github", - ); - const repo = await response.json(); - if (!repo || !repo.full_name) { - return; - } - return repo.full_name; -} - /** * Imports a database from a local archive. * @@ -552,127 +484,6 @@ export async function convertGithubNwoToDatabaseUrl( } } -/** - * The URL pattern is https://lgtm.com/projects/{provider}/{org}/{name}/{irrelevant-subpages}. - * There are several possibilities for the provider: in addition to GitHub.com (g), - * LGTM currently hosts projects from Bitbucket (b), GitLab (gl) and plain git (git). - * - * This function accepts any url that matches the pattern above. It also accepts the - * raw project slug, e.g., `g/myorg/myproject` - * - * After the `{provider}/{org}/{name}` path components, there may be the components - * related to sub pages. - * - * @param lgtmUrl The URL to the lgtm project - * - * @return true if this looks like an LGTM project url - */ -// exported for testing -export function looksLikeLgtmUrl( - lgtmUrl: string | undefined, -): lgtmUrl is string { - if (!lgtmUrl) { - return false; - } - - if (convertRawLgtmSlug(lgtmUrl)) { - return true; - } - - try { - const uri = Uri.parse(lgtmUrl, true); - if (uri.scheme !== "https") { - return false; - } - - if (uri.authority !== "lgtm.com" && uri.authority !== "www.lgtm.com") { - return false; - } - - const paths = uri.path.split("/").filter((segment: string) => segment); - return paths.length >= 4 && paths[0] === "projects"; - } catch (e) { - return false; - } -} - -function convertRawLgtmSlug(maybeSlug: string): string | undefined { - if (!maybeSlug) { - return; - } - const segments = maybeSlug.split("/"); - const providers = ["g", "gl", "b", "git"]; - if (segments.length === 3 && providers.includes(segments[0])) { - return `https://lgtm.com/projects/${maybeSlug}`; - } - return; -} - -function extractProjectSlug(lgtmUrl: string): string | undefined { - // Only matches the '/g/' provider (github) - const re = new RegExp("https://lgtm.com/projects/g/(.*[^/])"); - const match = lgtmUrl.match(re); - if (!match) { - return; - } - return match[1]; -} - -// exported for testing -export async function convertLgtmUrlToDatabaseUrl( - lgtmUrl: string, - progress: ProgressCallback, -) { - try { - lgtmUrl = convertRawLgtmSlug(lgtmUrl) || lgtmUrl; - let projectJson = await downloadLgtmProjectMetadata(lgtmUrl); - - if (projectJson.code === 404) { - // fallback check for github repositories with same name but different case - // will fail for other providers - let canonicalName = await retrieveCanonicalRepoName(lgtmUrl); - if (!canonicalName) { - throw new Error(`Project was not found at ${lgtmUrl}.`); - } - canonicalName = convertRawLgtmSlug(`g/${canonicalName}`); - projectJson = await downloadLgtmProjectMetadata(canonicalName); - if (projectJson.code === 404) { - throw new Error("Failed to download project from LGTM."); - } - } - - const languages = - projectJson?.languages?.map( - (lang: { language: string }) => lang.language, - ) || []; - - const language = await promptForLanguage(languages, progress); - if (!language) { - return; - } - return `https://lgtm.com/${[ - "api", - "v1.0", - "snapshots", - projectJson.id, - language, - ].join("/")}`; - } catch (e) { - void extLogger.log(`Error: ${getErrorMessage(e)}`); - throw new Error(`Invalid LGTM URL: ${lgtmUrl}`); - } -} - -async function downloadLgtmProjectMetadata(lgtmUrl: string): Promise { - const uri = Uri.parse(lgtmUrl, true); - const paths = ["api", "v1.0"] - .concat(uri.path.split("/").filter((segment: string) => segment)) - .slice(0, 6); - const projectUrl = `https://lgtm.com/${paths.join("/")}`; - const projectResponse = await fetch(projectUrl); - return projectResponse.json(); -} - async function promptForLanguage( languages: string[], progress: ProgressCallback, diff --git a/extensions/ql-vscode/src/databases-ui.ts b/extensions/ql-vscode/src/databases-ui.ts index 0f13731ab..80f7ac2de 100644 --- a/extensions/ql-vscode/src/databases-ui.ts +++ b/extensions/ql-vscode/src/databases-ui.ts @@ -33,7 +33,6 @@ import { importArchiveDatabase, promptImportGithubDatabase, promptImportInternetDatabase, - promptImportLgtmDatabase, } from "./databaseFetcher"; import { asyncFilter, getErrorMessage } from "./pure/helpers-pure"; import { Credentials } from "./authentication"; @@ -308,15 +307,6 @@ export class DatabaseUI extends DisposableObject { }, ), ); - this.push( - commandRunnerWithProgress( - "codeQLDatabases.chooseDatabaseLgtm", - this.handleChooseDatabaseLgtm, - { - title: "Adding database from LGTM", - }, - ), - ); this.push( commandRunner( "codeQLDatabases.setCurrentDatabase", @@ -491,19 +481,6 @@ export class DatabaseUI extends DisposableObject { ); }; - handleChooseDatabaseLgtm = async ( - progress: ProgressCallback, - token: CancellationToken, - ): Promise => { - return await promptImportLgtmDatabase( - this.databaseManager, - this.storagePath, - progress, - token, - this.queryServer?.cliServer, - ); - }; - async tryUpgradeCurrentDatabase( progress: ProgressCallback, token: CancellationToken, diff --git a/extensions/ql-vscode/src/databases/config/db-config-store.ts b/extensions/ql-vscode/src/databases/config/db-config-store.ts index fe71a0b4f..fc5536816 100644 --- a/extensions/ql-vscode/src/databases/config/db-config-store.ts +++ b/extensions/ql-vscode/src/databases/config/db-config-store.ts @@ -1,4 +1,4 @@ -import { pathExists, writeJSON, readJSON, readJSONSync } from "fs-extra"; +import { pathExists, outputJSON, readJSON, readJSONSync } from "fs-extra"; import { join } from "path"; import { cloneDbConfig, @@ -9,9 +9,13 @@ import { import * as chokidar from "chokidar"; import { DisposableObject, DisposeHandler } from "../../pure/disposable-object"; import { DbConfigValidator } from "./db-config-validator"; -import { ValueResult } from "../../common/value-result"; import { App } from "../../common/app"; import { AppEvent, AppEventEmitter } from "../../common/events"; +import { + DbConfigValidationError, + DbConfigValidationErrorKind, +} from "../db-validation-errors"; +import { ValueResult } from "../../common/value-result"; export class DbConfigStore extends DisposableObject { public readonly onDidChangeConfig: AppEvent; @@ -21,10 +25,10 @@ export class DbConfigStore extends DisposableObject { private readonly configValidator: DbConfigValidator; private config: DbConfig | undefined; - private configErrors: string[]; + private configErrors: DbConfigValidationError[]; private configWatcher: chokidar.FSWatcher | undefined; - public constructor(app: App) { + public constructor(private readonly app: App) { super(); const storagePath = app.workspaceStoragePath || app.globalStoragePath; @@ -48,7 +52,7 @@ export class DbConfigStore extends DisposableObject { this.configWatcher?.unwatch(this.configPath); } - public getConfig(): ValueResult { + public getConfig(): ValueResult { if (this.config) { // Clone the config so that it's not modified outside of this class. return ValueResult.ok(cloneDbConfig(this.config)); @@ -95,28 +99,45 @@ export class DbConfigStore extends DisposableObject { throw Error("Cannot add remote list if config is not loaded"); } + if (this.doesRemoteListExist(listName)) { + throw Error(`A remote list with the name '${listName}' already exists`); + } + const config: DbConfig = cloneDbConfig(this.config); config.databases.remote.repositoryLists.push({ name: listName, repositories: [], }); - // TODO: validate that the name doesn't already exist await this.writeConfig(config); } + public doesRemoteListExist(listName: string): boolean { + if (!this.config) { + throw Error("Cannot check remote list existence if config is not loaded"); + } + + return this.config.databases.remote.repositoryLists.some( + (l) => l.name === listName, + ); + } + private async writeConfig(config: DbConfig): Promise { - await writeJSON(this.configPath, config, { + await outputJSON(this.configPath, config, { spaces: 2, }); } private async loadConfig(): Promise { if (!(await pathExists(this.configPath))) { + void this.app.logger.log( + `Creating new database config file at ${this.configPath}`, + ); await this.writeConfig(this.createEmptyConfig()); } await this.readConfig(); + void this.app.logger.log(`Database config loaded from ${this.configPath}`); } private async readConfig(): Promise { @@ -124,14 +145,33 @@ export class DbConfigStore extends DisposableObject { try { newConfig = await readJSON(this.configPath); } catch (e) { - this.configErrors = [`Failed to read config file: ${this.configPath}`]; + this.configErrors = [ + { + kind: DbConfigValidationErrorKind.InvalidJson, + message: `Failed to read config file: ${this.configPath}`, + }, + ]; } if (newConfig) { this.configErrors = this.configValidator.validate(newConfig); } - this.config = this.configErrors.length === 0 ? newConfig : undefined; + if (this.configErrors.length === 0) { + this.config = newConfig; + await this.app.executeCommand( + "setContext", + "codeQLDatabasesExperimental.configError", + false, + ); + } else { + this.config = undefined; + await this.app.executeCommand( + "setContext", + "codeQLDatabasesExperimental.configError", + true, + ); + } } private readConfigSync(): void { @@ -139,22 +179,51 @@ export class DbConfigStore extends DisposableObject { try { newConfig = readJSONSync(this.configPath); } catch (e) { - this.configErrors = [`Failed to read config file: ${this.configPath}`]; + this.configErrors = [ + { + kind: DbConfigValidationErrorKind.InvalidJson, + message: `Failed to read config file: ${this.configPath}`, + }, + ]; } if (newConfig) { this.configErrors = this.configValidator.validate(newConfig); } - this.config = this.configErrors.length === 0 ? newConfig : undefined; - + if (this.configErrors.length === 0) { + this.config = newConfig; + void this.app.executeCommand( + "setContext", + "codeQLDatabasesExperimental.configError", + false, + ); + } else { + this.config = undefined; + void this.app.executeCommand( + "setContext", + "codeQLDatabasesExperimental.configError", + true, + ); + } this.onDidChangeConfigEventEmitter.fire(); } private watchConfig(): void { - this.configWatcher = chokidar.watch(this.configPath).on("change", () => { - this.readConfigSync(); - }); + this.configWatcher = chokidar + .watch(this.configPath, { + // In some cases, change events are emitted while the file is still + // being written. The awaitWriteFinish option tells the watcher to + // poll the file size, holding its add and change events until the size + // does not change for a configurable amount of time. We set that time + // to 1 second, but it may need to be adjusted if there are issues. + awaitWriteFinish: { + stabilityThreshold: 1000, + }, + }) + .on("change", () => { + this.readConfigSync(); + }); } private createEmptyConfig(): DbConfig { diff --git a/extensions/ql-vscode/src/databases/config/db-config-validator.ts b/extensions/ql-vscode/src/databases/config/db-config-validator.ts index 211a815f5..ee820181b 100644 --- a/extensions/ql-vscode/src/databases/config/db-config-validator.ts +++ b/extensions/ql-vscode/src/databases/config/db-config-validator.ts @@ -2,6 +2,11 @@ import { readJsonSync } from "fs-extra"; import { resolve } from "path"; import Ajv from "ajv"; import { DbConfig } from "./db-config"; +import { findDuplicateStrings } from "../../text-utils"; +import { + DbConfigValidationError, + DbConfigValidationErrorKind, +} from "../db-validation-errors"; export class DbConfigValidator { private readonly schema: any; @@ -14,16 +19,118 @@ export class DbConfigValidator { this.schema = readJsonSync(schemaPath); } - public validate(dbConfig: DbConfig): string[] { + public validate(dbConfig: DbConfig): DbConfigValidationError[] { const ajv = new Ajv({ allErrors: true }); ajv.validate(this.schema, dbConfig); if (ajv.errors) { - return ajv.errors.map( - (error) => `${error.instancePath} ${error.message}`, - ); + return ajv.errors.map((error) => ({ + kind: DbConfigValidationErrorKind.InvalidConfig, + message: `${error.instancePath} ${error.message}`, + })); } - return []; + return [ + ...this.validateDbListNames(dbConfig), + ...this.validateDbNames(dbConfig), + ...this.validateDbNamesInLists(dbConfig), + ...this.validateOwners(dbConfig), + ]; + } + + private validateDbListNames(dbConfig: DbConfig): DbConfigValidationError[] { + const errors: DbConfigValidationError[] = []; + + const buildError = (dups: string[]) => ({ + kind: DbConfigValidationErrorKind.DuplicateNames, + message: `There are database lists with the same name: ${dups.join( + ", ", + )}`, + }); + + const duplicateLocalDbLists = findDuplicateStrings( + dbConfig.databases.local.lists.map((n) => n.name), + ); + + if (duplicateLocalDbLists.length > 0) { + errors.push(buildError(duplicateLocalDbLists)); + } + + const duplicateRemoteDbLists = findDuplicateStrings( + dbConfig.databases.remote.repositoryLists.map((n) => n.name), + ); + if (duplicateRemoteDbLists.length > 0) { + errors.push(buildError(duplicateRemoteDbLists)); + } + + return errors; + } + + private validateDbNames(dbConfig: DbConfig): DbConfigValidationError[] { + const errors: DbConfigValidationError[] = []; + + const buildError = (dups: string[]) => ({ + kind: DbConfigValidationErrorKind.DuplicateNames, + message: `There are databases with the same name: ${dups.join(", ")}`, + }); + + const duplicateLocalDbs = findDuplicateStrings( + dbConfig.databases.local.databases.map((d) => d.name), + ); + + if (duplicateLocalDbs.length > 0) { + errors.push(buildError(duplicateLocalDbs)); + } + + const duplicateRemoteDbs = findDuplicateStrings( + dbConfig.databases.remote.repositories, + ); + if (duplicateRemoteDbs.length > 0) { + errors.push(buildError(duplicateRemoteDbs)); + } + + return errors; + } + + private validateDbNamesInLists( + dbConfig: DbConfig, + ): DbConfigValidationError[] { + const errors: DbConfigValidationError[] = []; + + const buildError = (listName: string, dups: string[]) => ({ + kind: DbConfigValidationErrorKind.DuplicateNames, + message: `There are databases with the same name in the ${listName} list: ${dups.join( + ", ", + )}`, + }); + + for (const list of dbConfig.databases.local.lists) { + const dups = findDuplicateStrings(list.databases.map((d) => d.name)); + if (dups.length > 0) { + errors.push(buildError(list.name, dups)); + } + } + + for (const list of dbConfig.databases.remote.repositoryLists) { + const dups = findDuplicateStrings(list.repositories); + if (dups.length > 0) { + errors.push(buildError(list.name, dups)); + } + } + + return errors; + } + + private validateOwners(dbConfig: DbConfig): DbConfigValidationError[] { + const errors: DbConfigValidationError[] = []; + + const dups = findDuplicateStrings(dbConfig.databases.remote.owners); + if (dups.length > 0) { + errors.push({ + kind: DbConfigValidationErrorKind.DuplicateNames, + message: `There are owners with the same name: ${dups.join(", ")}`, + }); + } + return errors; } } diff --git a/extensions/ql-vscode/src/databases/db-manager.ts b/extensions/ql-vscode/src/databases/db-manager.ts index 3e9d0a0db..a07a63177 100644 --- a/extensions/ql-vscode/src/databases/db-manager.ts +++ b/extensions/ql-vscode/src/databases/db-manager.ts @@ -9,6 +9,7 @@ import { mapDbItemToSelectedDbItem, } from "./db-item-selection"; import { createLocalTree, createRemoteTree } from "./db-tree-creator"; +import { DbConfigValidationError } from "./db-validation-errors"; export class DbManager { public readonly onDbItemsChanged: AppEvent; @@ -24,16 +25,16 @@ export class DbManager { } public getSelectedDbItem(): DbItem | undefined { - const dbItems = this.getDbItems(); + const dbItemsResult = this.getDbItems(); - if (dbItems.isFailure) { + if (dbItemsResult.errors.length > 0) { return undefined; } - return getSelectedDbItem(dbItems.value); + return getSelectedDbItem(dbItemsResult.value); } - public getDbItems(): ValueResult { + public getDbItems(): ValueResult { const configResult = this.dbConfigStore.getConfig(); if (configResult.isFailure) { return ValueResult.fail(configResult.errors); @@ -75,6 +76,14 @@ export class DbManager { } public async addNewRemoteList(listName: string): Promise { + if (this.dbConfigStore.doesRemoteListExist(listName)) { + throw Error(`A list with the name '${listName}' already exists`); + } + await this.dbConfigStore.addRemoteList(listName); } + + public doesRemoteListExist(listName: string): boolean { + return this.dbConfigStore.doesRemoteListExist(listName); + } } diff --git a/extensions/ql-vscode/src/databases/db-module.ts b/extensions/ql-vscode/src/databases/db-module.ts index cabdef7ed..588cbd7ff 100644 --- a/extensions/ql-vscode/src/databases/db-module.ts +++ b/extensions/ql-vscode/src/databases/db-module.ts @@ -20,22 +20,25 @@ export class DbModule extends DisposableObject { } public static async initialize(app: App): Promise { - if ( - isCanary() && - isNewQueryRunExperienceEnabled() && - app.mode === AppMode.Development - ) { + if (DbModule.shouldEnableModule(app.mode)) { const dbModule = new DbModule(app); app.subscriptions.push(dbModule); await dbModule.initialize(); - return dbModule; } return undefined; } + private static shouldEnableModule(app: AppMode): boolean { + if (app === AppMode.Development || app === AppMode.Test) { + return true; + } + + return isCanary() && isNewQueryRunExperienceEnabled(); + } + private async initialize(): Promise { void extLogger.log("Initializing database module"); diff --git a/extensions/ql-vscode/src/databases/db-validation-errors.ts b/extensions/ql-vscode/src/databases/db-validation-errors.ts new file mode 100644 index 000000000..cc614694c --- /dev/null +++ b/extensions/ql-vscode/src/databases/db-validation-errors.ts @@ -0,0 +1,10 @@ +export enum DbConfigValidationErrorKind { + InvalidJson = "InvalidJson", + InvalidConfig = "InvalidConfig", + DuplicateNames = "DuplicateNames", +} + +export interface DbConfigValidationError { + kind: DbConfigValidationErrorKind; + message: string; +} diff --git a/extensions/ql-vscode/src/databases/ui/db-panel.ts b/extensions/ql-vscode/src/databases/ui/db-panel.ts index 77d6bdc59..6b4f89050 100644 --- a/extensions/ql-vscode/src/databases/ui/db-panel.ts +++ b/extensions/ql-vscode/src/databases/ui/db-panel.ts @@ -1,5 +1,6 @@ import { TreeViewExpansionEvent, window, workspace } from "vscode"; import { commandRunner } from "../../commandRunner"; +import { showAndLogErrorMessage } from "../../helpers"; import { DisposableObject } from "../../pure/disposable-object"; import { DbManager } from "../db-manager"; import { DbTreeDataProvider } from "./db-tree-data-provider"; @@ -58,7 +59,6 @@ export class DbPanel extends DisposableObject { } private async addNewRemoteList(): Promise { - // TODO: check that config exists *before* showing the input box const listName = await window.showInputBox({ prompt: "Enter a name for the new list", placeHolder: "example-list", @@ -66,7 +66,14 @@ export class DbPanel extends DisposableObject { if (listName === undefined) { return; } - await this.dbManager.addNewRemoteList(listName); + + if (this.dbManager.doesRemoteListExist(listName)) { + void showAndLogErrorMessage( + `A list with the name '${listName}' already exists`, + ); + } else { + await this.dbManager.addNewRemoteList(listName); + } } private async setSelectedItem(treeViewItem: DbTreeViewItem): Promise { diff --git a/extensions/ql-vscode/src/databases/ui/db-tree-data-provider.ts b/extensions/ql-vscode/src/databases/ui/db-tree-data-provider.ts index 1040b8f60..ee569eb73 100644 --- a/extensions/ql-vscode/src/databases/ui/db-tree-data-provider.ts +++ b/extensions/ql-vscode/src/databases/ui/db-tree-data-provider.ts @@ -9,6 +9,10 @@ import { createDbTreeViewItemError, DbTreeViewItem } from "./db-tree-view-item"; import { DbManager } from "../db-manager"; import { mapDbItemToTreeViewItem } from "./db-item-mapper"; import { DisposableObject } from "../../pure/disposable-object"; +import { + DbConfigValidationError, + DbConfigValidationErrorKind, +} from "../db-validation-errors"; export class DbTreeDataProvider extends DisposableObject @@ -61,14 +65,34 @@ export class DbTreeDataProvider const dbItemsResult = this.dbManager.getDbItems(); if (dbItemsResult.isFailure) { + return this.createErrorItems(dbItemsResult.errors); + } + + return dbItemsResult.value.map(mapDbItemToTreeViewItem); + } + + private createErrorItems( + errors: DbConfigValidationError[], + ): DbTreeViewItem[] { + if ( + errors.some( + (e) => + e.kind === DbConfigValidationErrorKind.InvalidJson || + e.kind === DbConfigValidationErrorKind.InvalidConfig, + ) + ) { const errorTreeViewItem = createDbTreeViewItemError( "Error when reading databases config", "Please open your databases config and address errors", ); return [errorTreeViewItem]; + } else { + return errors + .filter((e) => e.kind === DbConfigValidationErrorKind.DuplicateNames) + .map((e) => + createDbTreeViewItemError(e.message, "Please remove duplicates"), + ); } - - return dbItemsResult.value.map(mapDbItemToTreeViewItem); } } diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index 66331dd16..abf49939b 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -1369,16 +1369,6 @@ async function activateWithInstalledDistribution( }, ), ); - ctx.subscriptions.push( - commandRunnerWithProgress( - "codeQL.chooseDatabaseLgtm", - (progress: ProgressCallback, token: CancellationToken) => - databaseUI.handleChooseDatabaseLgtm(progress, token), - { - title: "Adding database from LGTM", - }, - ), - ); ctx.subscriptions.push( commandRunnerWithProgress( "codeQL.chooseDatabaseInternet", diff --git a/extensions/ql-vscode/src/stories/variant-analysis/VariantAnalysisAnalyzedRepos.stories.tsx b/extensions/ql-vscode/src/stories/variant-analysis/VariantAnalysisAnalyzedRepos.stories.tsx index 1b32570a4..9b7c5f52f 100644 --- a/extensions/ql-vscode/src/stories/variant-analysis/VariantAnalysisAnalyzedRepos.stories.tsx +++ b/extensions/ql-vscode/src/stories/variant-analysis/VariantAnalysisAnalyzedRepos.stories.tsx @@ -8,6 +8,7 @@ import { VariantAnalysisContainer } from "../../view/variant-analysis/VariantAna import { VariantAnalysisAnalyzedRepos } from "../../view/variant-analysis/VariantAnalysisAnalyzedRepos"; import { VariantAnalysisRepoStatus, + VariantAnalysisScannedRepositoryDownloadStatus, VariantAnalysisStatus, } from "../../remote-queries/shared/variant-analysis"; import { AnalysisAlert } from "../../remote-queries/shared/analysis-result"; @@ -148,8 +149,8 @@ const manyScannedRepos = Array.from({ length: 1000 }, (_, i) => { }; }); -export const PerformanceExample = Template.bind({}); -PerformanceExample.args = { +export const ManyRepositoriesPerformanceExample = Template.bind({}); +ManyRepositoriesPerformanceExample.args = { variantAnalysis: { ...createMockVariantAnalysis({ status: VariantAnalysisStatus.Succeeded, @@ -163,3 +164,39 @@ PerformanceExample.args = { interpretedResults: interpretedResultsForRepo("facebook/create-react-app"), })), }; + +const mockAnalysisAlert = interpretedResultsForRepo( + "facebook/create-react-app", +)![0]; + +const performanceNumbers = [10, 50, 100, 500, 1000, 2000, 5000, 10_000]; + +export const ManyResultsPerformanceExample = Template.bind({}); +ManyResultsPerformanceExample.args = { + variantAnalysis: { + ...createMockVariantAnalysis({ + status: VariantAnalysisStatus.Succeeded, + scannedRepos: performanceNumbers.map((resultCount, i) => ({ + repository: { + ...createMockRepositoryWithMetadata(), + id: resultCount, + fullName: `octodemo/${i}-${resultCount}-results`, + }, + analysisStatus: VariantAnalysisRepoStatus.Succeeded, + resultCount, + })), + }), + id: 1, + }, + repositoryStates: performanceNumbers.map((resultCount) => ({ + repositoryId: resultCount, + downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Succeeded, + })), + repositoryResults: performanceNumbers.map((resultCount) => ({ + variantAnalysisId: 1, + repositoryId: resultCount, + interpretedResults: Array.from({ length: resultCount }, (_, i) => ({ + ...mockAnalysisAlert, + })), + })), +}; diff --git a/extensions/ql-vscode/src/text-utils.ts b/extensions/ql-vscode/src/text-utils.ts index 7a0d1560a..5e5e2b083 100644 --- a/extensions/ql-vscode/src/text-utils.ts +++ b/extensions/ql-vscode/src/text-utils.ts @@ -31,3 +31,11 @@ export function convertNonPrintableChars(label: string | undefined) { return convertedLabelArray.join(""); } } + +export function findDuplicateStrings(strings: string[]): string[] { + const dups = strings.filter( + (string, index, strings) => strings.indexOf(string) !== index, + ); + + return [...new Set(dups)]; +} diff --git a/extensions/ql-vscode/src/vscode-tests/cli-integration/databases.test.ts b/extensions/ql-vscode/src/vscode-tests/cli-integration/databases.test.ts index 2ecbb9a8c..15fd96e01 100644 --- a/extensions/ql-vscode/src/vscode-tests/cli-integration/databases.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/cli-integration/databases.test.ts @@ -5,7 +5,6 @@ import { CodeQLExtensionInterface } from "../../extension"; import { CodeQLCliServer } from "../../cli"; import { DatabaseManager } from "../../databases"; import { - promptImportLgtmDatabase, importArchiveDatabase, promptImportInternetDatabase, } from "../../databaseFetcher"; @@ -17,9 +16,6 @@ jest.setTimeout(60_000); * Run various integration tests for databases */ describe("Databases", () => { - const LGTM_URL = - "https://lgtm.com/projects/g/aeisenberg/angular-bind-notifier/"; - let databaseManager: DatabaseManager; let inputBoxStub: jest.SpiedFunction; let cli: CodeQLCliServer; @@ -71,27 +67,6 @@ describe("Databases", () => { expect(dbItem.databaseUri.fsPath).toBe(join(storagePath, "db", "db")); }); - it("should add a database from lgtm with only one language", async () => { - inputBoxStub.mockResolvedValue(LGTM_URL); - let dbItem = await promptImportLgtmDatabase( - databaseManager, - storagePath, - progressCallback, - {} as CancellationToken, - cli, - ); - expect(dbItem).toBeDefined(); - dbItem = dbItem!; - expect(dbItem.name).toBe("aeisenberg_angular-bind-notifier_106179a"); - expect(dbItem.databaseUri.fsPath).toBe( - join( - storagePath, - "javascript", - "aeisenberg_angular-bind-notifier_106179a", - ), - ); - }); - it("should add a database from a url", async () => { inputBoxStub.mockResolvedValue(DB_URL); diff --git a/extensions/ql-vscode/src/vscode-tests/cli-integration/databases/db-panel.test.ts b/extensions/ql-vscode/src/vscode-tests/cli-integration/databases/db-panel.test.ts new file mode 100644 index 000000000..3eb014b51 --- /dev/null +++ b/extensions/ql-vscode/src/vscode-tests/cli-integration/databases/db-panel.test.ts @@ -0,0 +1,36 @@ +import { commands, extensions, window } from "vscode"; + +import { CodeQLExtensionInterface } from "../../../extension"; +import { readJson } from "fs-extra"; +import * as path from "path"; +import { DbConfig } from "../../../databases/config/db-config"; + +jest.setTimeout(60_000); + +describe("Db panel UI commands", () => { + let extension: CodeQLExtensionInterface | Record; + let storagePath: string; + + beforeEach(async () => { + extension = await extensions + .getExtension>( + "GitHub.vscode-codeql", + )! + .activate(); + + storagePath = + extension.ctx.storageUri?.fsPath || extension.ctx.globalStorageUri.fsPath; + }); + + it("should add new remote db list", async () => { + // Add db list + jest.spyOn(window, "showInputBox").mockResolvedValue("my-list-1"); + await commands.executeCommand("codeQLDatabasesExperimental.addNewList"); + + // Check db config + const dbConfigFilePath = path.join(storagePath, "workspace-databases.json"); + const dbConfig: DbConfig = await readJson(dbConfigFilePath); + expect(dbConfig.databases.remote.repositoryLists).toHaveLength(1); + expect(dbConfig.databases.remote.repositoryLists[0].name).toBe("my-list-1"); + }); +}); diff --git a/extensions/ql-vscode/test/factories/db-config-factories.ts b/extensions/ql-vscode/src/vscode-tests/factories/db-config-factories.ts similarity index 60% rename from extensions/ql-vscode/test/factories/db-config-factories.ts rename to extensions/ql-vscode/src/vscode-tests/factories/db-config-factories.ts index cf171e020..48f523d66 100644 --- a/extensions/ql-vscode/test/factories/db-config-factories.ts +++ b/extensions/ql-vscode/src/vscode-tests/factories/db-config-factories.ts @@ -1,3 +1,4 @@ +import { faker } from "@faker-js/faker"; import { DbConfig, ExpandedDbItem, @@ -5,7 +6,7 @@ import { LocalList, RemoteRepositoryList, SelectedDbItem, -} from "../../src/databases/config/db-config"; +} from "../../databases/config/db-config"; export function createDbConfig({ remoteLists = [], @@ -40,3 +41,22 @@ export function createDbConfig({ selected, }; } + +export function createLocalDbConfigItem({ + name = `database${faker.datatype.number()}`, + dateAdded = faker.date.past().getTime(), + language = `language${faker.datatype.number()}`, + storagePath = `storagePath${faker.datatype.number()}`, +}: { + name?: string; + dateAdded?: number; + language?: string; + storagePath?: string; +} = {}): LocalDatabase { + return { + name, + dateAdded, + language, + storagePath, + }; +} diff --git a/extensions/ql-vscode/src/vscode-tests/minimal-workspace/databases/db-panel.test.ts b/extensions/ql-vscode/src/vscode-tests/minimal-workspace/databases/db-panel.test.ts index 22390bc18..291326a84 100644 --- a/extensions/ql-vscode/src/vscode-tests/minimal-workspace/databases/db-panel.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/minimal-workspace/databases/db-panel.test.ts @@ -1,4 +1,4 @@ -import { TreeItemCollapsibleState, ThemeIcon } from "vscode"; +import { TreeItemCollapsibleState, ThemeIcon, ThemeColor } from "vscode"; import { join } from "path"; import { ensureDir, readJSON, remove, writeJson } from "fs-extra"; import { @@ -12,6 +12,7 @@ import { DbItemKind, LocalDatabaseDbItem } from "../../../databases/db-item"; import { DbTreeViewItem } from "../../../databases/ui/db-tree-view-item"; import { ExtensionApp } from "../../../common/vscode/vscode-app"; import { createMockExtensionContext } from "../../factories/extension-context"; +import { createDbConfig } from "../../factories/db-config-factories"; describe("db panel", () => { const workspaceStoragePath = join(__dirname, "test-workspace-storage"); @@ -48,20 +49,7 @@ describe("db panel", () => { }); it("should render default local and remote nodes when the config is empty", async () => { - const dbConfig: DbConfig = { - databases: { - remote: { - repositoryLists: [], - owners: [], - repositories: [], - }, - local: { - lists: [], - databases: [], - }, - }, - expanded: [], - }; + const dbConfig: DbConfig = createDbConfig(); await saveDbConfig(dbConfig); @@ -103,29 +91,18 @@ describe("db panel", () => { }); it("should render remote repository list nodes", async () => { - const dbConfig: DbConfig = { - databases: { - remote: { - repositoryLists: [ - { - name: "my-list-1", - repositories: ["owner1/repo1", "owner1/repo2"], - }, - { - name: "my-list-2", - repositories: ["owner1/repo1", "owner2/repo1", "owner2/repo2"], - }, - ], - owners: [], - repositories: [], + const dbConfig: DbConfig = createDbConfig({ + remoteLists: [ + { + name: "my-list-1", + repositories: ["owner1/repo1", "owner1/repo2"], }, - local: { - lists: [], - databases: [], + { + name: "my-list-2", + repositories: ["owner1/repo1", "owner2/repo1", "owner2/repo2"], }, - }, - expanded: [], - }; + ], + }); await saveDbConfig(dbConfig); @@ -164,20 +141,9 @@ describe("db panel", () => { }); it("should render owner list nodes", async () => { - const dbConfig: DbConfig = { - databases: { - remote: { - repositoryLists: [], - owners: ["owner1", "owner2"], - repositories: [], - }, - local: { - lists: [], - databases: [], - }, - }, - expanded: [], - }; + const dbConfig: DbConfig = createDbConfig({ + remoteOwners: ["owner1", "owner2"], + }); await saveDbConfig(dbConfig); @@ -204,20 +170,9 @@ describe("db panel", () => { }); it("should render repository nodes", async () => { - const dbConfig: DbConfig = { - databases: { - remote: { - repositoryLists: [], - owners: [], - repositories: ["owner1/repo1", "owner1/repo2"], - }, - local: { - lists: [], - databases: [], - }, - }, - expanded: [], - }; + const dbConfig: DbConfig = createDbConfig({ + remoteRepos: ["owner1/repo1", "owner1/repo2"], + }); await saveDbConfig(dbConfig); @@ -244,49 +199,38 @@ describe("db panel", () => { }); it("should render local list nodes", async () => { - const dbConfig: DbConfig = { - databases: { - remote: { - repositoryLists: [], - owners: [], - repositories: [], - }, - local: { - lists: [ + const dbConfig: DbConfig = createDbConfig({ + localLists: [ + { + name: "my-list-1", + databases: [ { - name: "my-list-1", - databases: [ - { - name: "db1", - dateAdded: 1668428293677, - language: "cpp", - storagePath: "/path/to/db1/", - }, - { - name: "db2", - dateAdded: 1668428472731, - language: "cpp", - storagePath: "/path/to/db2/", - }, - ], + name: "db1", + dateAdded: 1668428293677, + language: "cpp", + storagePath: "/path/to/db1/", }, { - name: "my-list-2", - databases: [ - { - name: "db3", - dateAdded: 1668428472731, - language: "ruby", - storagePath: "/path/to/db3/", - }, - ], + name: "db2", + dateAdded: 1668428472731, + language: "cpp", + storagePath: "/path/to/db2/", }, ], - databases: [], }, - }, - expanded: [], - }; + { + name: "my-list-2", + databases: [ + { + name: "db3", + dateAdded: 1668428472731, + language: "ruby", + storagePath: "/path/to/db3/", + }, + ], + }, + ], + }); await saveDbConfig(dbConfig); @@ -339,33 +283,22 @@ describe("db panel", () => { }); it("should render local database nodes", async () => { - const dbConfig: DbConfig = { - databases: { - remote: { - repositoryLists: [], - owners: [], - repositories: [], + const dbConfig: DbConfig = createDbConfig({ + localDbs: [ + { + name: "db1", + dateAdded: 1668428293677, + language: "csharp", + storagePath: "/path/to/db1/", }, - local: { - lists: [], - databases: [ - { - name: "db1", - dateAdded: 1668428293677, - language: "csharp", - storagePath: "/path/to/db1/", - }, - { - name: "db2", - dateAdded: 1668428472731, - language: "go", - storagePath: "/path/to/db2/", - }, - ], + { + name: "db2", + dateAdded: 1668428472731, + language: "go", + storagePath: "/path/to/db2/", }, - }, - expanded: [], - }; + ], + }); await saveDbConfig(dbConfig); @@ -406,33 +339,22 @@ describe("db panel", () => { }); it("should mark selected remote db list as selected", async () => { - const dbConfig: DbConfig = { - databases: { - remote: { - repositoryLists: [ - { - name: "my-list-1", - repositories: ["owner1/repo1", "owner1/repo2"], - }, - { - name: "my-list-2", - repositories: ["owner2/repo1", "owner2/repo2"], - }, - ], - owners: [], - repositories: [], + const dbConfig: DbConfig = createDbConfig({ + remoteLists: [ + { + name: "my-list-1", + repositories: ["owner1/repo1", "owner1/repo2"], }, - local: { - lists: [], - databases: [], + { + name: "my-list-2", + repositories: ["owner2/repo1", "owner2/repo2"], }, - }, - expanded: [], + ], selected: { kind: SelectedDbItemKind.RemoteUserDefinedList, listName: "my-list-2", }, - }; + }); await saveDbConfig(dbConfig); @@ -463,34 +385,24 @@ describe("db panel", () => { }); it("should mark selected remote db inside list as selected", async () => { - const dbConfig: DbConfig = { - databases: { - remote: { - repositoryLists: [ - { - name: "my-list-1", - repositories: ["owner1/repo1", "owner1/repo2"], - }, - { - name: "my-list-2", - repositories: ["owner1/repo1", "owner2/repo2"], - }, - ], - owners: [], - repositories: ["owner1/repo1"], + const dbConfig: DbConfig = createDbConfig({ + remoteLists: [ + { + name: "my-list-1", + repositories: ["owner1/repo1", "owner1/repo2"], }, - local: { - lists: [], - databases: [], + { + name: "my-list-2", + repositories: ["owner1/repo1", "owner2/repo2"], }, - }, - expanded: [], + ], + remoteRepos: ["owner1/repo1"], selected: { kind: SelectedDbItemKind.RemoteRepository, repositoryName: "owner1/repo1", listName: "my-list-2", }, - }; + }); await saveDbConfig(dbConfig); @@ -532,29 +444,18 @@ describe("db panel", () => { }); it("should add a new list to the remote db list", async () => { - const dbConfig: DbConfig = { - databases: { - remote: { - repositoryLists: [ - { - name: "my-list-1", - repositories: ["owner1/repo1", "owner1/repo2"], - }, - ], - owners: [], - repositories: [], + const dbConfig: DbConfig = createDbConfig({ + remoteLists: [ + { + name: "my-list-1", + repositories: ["owner1/repo1", "owner1/repo2"], }, - local: { - lists: [], - databases: [], - }, - }, - expanded: [], + ], selected: { kind: SelectedDbItemKind.RemoteUserDefinedList, listName: "my-list-1", }, - }; + }); await saveDbConfig(dbConfig); @@ -591,6 +492,63 @@ describe("db panel", () => { }); }); + it("should show error for invalid config", async () => { + // We're intentionally bypassing the type check because we'd + // like to make sure validation errors are highlighted. + const dbConfig = { + databases: {}, + } as any as DbConfig; + + await saveDbConfig(dbConfig); + + const dbTreeItems = await dbTreeDataProvider.getChildren(); + + expect(dbTreeItems).toBeTruthy(); + const items = dbTreeItems!; + expect(items.length).toBe(1); + + checkErrorItem( + items[0], + "Error when reading databases config", + "Please open your databases config and address errors", + ); + }); + + it("should show errors for duplicate names", async () => { + const dbConfig: DbConfig = createDbConfig({ + remoteLists: [ + { + name: "my-list-1", + repositories: ["owner1/repo1", "owner1/repo2"], + }, + { + name: "my-list-1", + repositories: ["owner1/repo1", "owner2/repo2"], + }, + ], + remoteRepos: ["owner1/repo1", "owner1/repo1"], + }); + + await saveDbConfig(dbConfig); + + const dbTreeItems = await dbTreeDataProvider.getChildren(); + + expect(dbTreeItems).toBeTruthy(); + const items = dbTreeItems!; + expect(items.length).toBe(2); + + checkErrorItem( + items[0], + "There are database lists with the same name: my-list-1", + "Please remove duplicates", + ); + checkErrorItem( + items[1], + "There are databases with the same name: owner1/repo1", + "Please remove duplicates", + ); + }); + async function saveDbConfig(dbConfig: DbConfig): Promise { await writeJson(dbConfigFilePath, dbConfig); @@ -672,6 +630,21 @@ describe("db panel", () => { expect(item.collapsibleState).toBe(TreeItemCollapsibleState.None); } + function checkErrorItem( + item: DbTreeViewItem, + label: string, + tooltip: string, + ): void { + expect(item.dbItem).toBe(undefined); + expect(item.iconPath).toEqual( + new ThemeIcon("error", new ThemeColor("problemsErrorIcon.foreground")), + ); + expect(item.label).toBe(label); + expect(item.tooltip).toBe(tooltip); + expect(item.collapsibleState).toBe(TreeItemCollapsibleState.None); + expect(item.children.length).toBe(0); + } + function isTreeViewItemSelectable(treeViewItem: DbTreeViewItem) { return ( treeViewItem.resourceUri === undefined && diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts index 32c6455b1..15d7e4018 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts @@ -5,8 +5,6 @@ import { QuickPickItem, window } from "vscode"; import { convertGithubNwoToDatabaseUrl, - convertLgtmUrlToDatabaseUrl, - looksLikeLgtmUrl, findDirWithFile, } from "../../databaseFetcher"; import * as Octokit from "@octokit/rest"; @@ -131,64 +129,6 @@ describe("databaseFetcher", () => { }); }); - describe("convertLgtmUrlToDatabaseUrl", () => { - let quickPickSpy: jest.SpiedFunction; - const progressSpy = jest.fn(); - - beforeEach(() => { - quickPickSpy = jest - .spyOn(window, "showQuickPick") - .mockResolvedValue(undefined); - }); - - it("should convert a project url to a database url", async () => { - quickPickSpy.mockResolvedValue("javascript" as unknown as QuickPickItem); - const lgtmUrl = "https://lgtm.com/projects/g/github/codeql"; - const dbUrl = await convertLgtmUrlToDatabaseUrl(lgtmUrl, progressSpy); - - expect(dbUrl).toBe( - "https://lgtm.com/api/v1.0/snapshots/1506465042581/javascript", - ); - expect(quickPickSpy).toHaveBeenNthCalledWith( - 1, - expect.arrayContaining(["javascript", "python"]), - expect.anything(), - ); - }); - - it("should convert a project url to a database url with extra path segments", async () => { - quickPickSpy.mockResolvedValue("python" as unknown as QuickPickItem); - const lgtmUrl = - "https://lgtm.com/projects/g/github/codeql/subpage/subpage2?query=xxx"; - const dbUrl = await convertLgtmUrlToDatabaseUrl(lgtmUrl, progressSpy); - - expect(dbUrl).toBe( - "https://lgtm.com/api/v1.0/snapshots/1506465042581/python", - ); - expect(progressSpy).toBeCalledTimes(1); - }); - - it("should convert a raw slug to a database url with extra path segments", async () => { - quickPickSpy.mockResolvedValue("python" as unknown as QuickPickItem); - const lgtmUrl = "g/github/codeql"; - const dbUrl = await convertLgtmUrlToDatabaseUrl(lgtmUrl, progressSpy); - - expect(dbUrl).toBe( - "https://lgtm.com/api/v1.0/snapshots/1506465042581/python", - ); - expect(progressSpy).toBeCalledTimes(1); - }); - - it("should fail on a nonexistent project", async () => { - quickPickSpy.mockResolvedValue("javascript" as unknown as QuickPickItem); - const lgtmUrl = "https://lgtm.com/projects/g/github/hucairz"; - await expect( - convertLgtmUrlToDatabaseUrl(lgtmUrl, progressSpy), - ).rejects.toThrow(/Invalid LGTM URL/); - expect(progressSpy).toBeCalledTimes(0); - }); - }); - describe("looksLikeGithubRepo", () => { it("should handle invalid urls", () => { expect(looksLikeGithubRepo("")).toBe(false); @@ -208,42 +148,6 @@ describe("databaseFetcher", () => { }); }); - describe("looksLikeLgtmUrl", () => { - it("should handle invalid urls", () => { - expect(looksLikeLgtmUrl("")).toBe(false); - expect(looksLikeLgtmUrl("http://lgtm.com/projects/g/github/codeql")).toBe( - false, - ); - expect( - looksLikeLgtmUrl("https://ww.lgtm.com/projects/g/github/codeql"), - ).toBe(false); - expect(looksLikeLgtmUrl("https://ww.lgtm.com/projects/g/github")).toBe( - false, - ); - expect(looksLikeLgtmUrl("g/github")).toBe(false); - expect(looksLikeLgtmUrl("ggg/github/myproj")).toBe(false); - }); - - it("should handle valid urls", () => { - expect( - looksLikeLgtmUrl("https://lgtm.com/projects/g/github/codeql"), - ).toBe(true); - expect( - looksLikeLgtmUrl("https://www.lgtm.com/projects/g/github/codeql"), - ).toBe(true); - expect( - looksLikeLgtmUrl("https://lgtm.com/projects/g/github/codeql/sub/pages"), - ).toBe(true); - expect( - looksLikeLgtmUrl( - "https://lgtm.com/projects/g/github/codeql/sub/pages?query=string", - ), - ).toBe(true); - expect(looksLikeLgtmUrl("g/github/myproj")).toBe(true); - expect(looksLikeLgtmUrl("git/github/myproj")).toBe(true); - }); - }); - describe("findDirWithFile", () => { let dir: tmp.DirResult; beforeEach(() => { diff --git a/extensions/ql-vscode/test/__mocks__/appMock.ts b/extensions/ql-vscode/test/__mocks__/appMock.ts index 4c75a9d4c..01f54ae85 100644 --- a/extensions/ql-vscode/test/__mocks__/appMock.ts +++ b/extensions/ql-vscode/test/__mocks__/appMock.ts @@ -1,6 +1,7 @@ import { App, AppMode } from "../../src/common/app"; import { AppEvent, AppEventEmitter } from "../../src/common/events"; import { Disposable } from "../../src/pure/disposable-object"; +import { createMockLogger } from "./loggerMock"; export function createMockApp({ extensionPath = "/mock/extension/path", @@ -17,6 +18,7 @@ export function createMockApp({ }): App { return { mode: AppMode.Test, + logger: createMockLogger(), subscriptions: [], extensionPath, workspaceStoragePath, diff --git a/extensions/ql-vscode/test/__mocks__/loggerMock.ts b/extensions/ql-vscode/test/__mocks__/loggerMock.ts new file mode 100644 index 000000000..665720357 --- /dev/null +++ b/extensions/ql-vscode/test/__mocks__/loggerMock.ts @@ -0,0 +1,9 @@ +import { Logger } from "../../src/common"; + +export function createMockLogger(): Logger { + return { + log: jest.fn(() => Promise.resolve()), + show: jest.fn(), + removeAdditionalLogLocation: jest.fn(), + }; +} diff --git a/extensions/ql-vscode/test/pure-tests/databases/config/data/invalid/workspace-databases.json b/extensions/ql-vscode/test/pure-tests/databases/config/data/invalid/workspace-databases.json new file mode 100644 index 000000000..0967ef424 --- /dev/null +++ b/extensions/ql-vscode/test/pure-tests/databases/config/data/invalid/workspace-databases.json @@ -0,0 +1 @@ +{} diff --git a/extensions/ql-vscode/test/pure-tests/databases/config/db-config-store.test.ts b/extensions/ql-vscode/test/pure-tests/databases/config/db-config-store.test.ts index 8f3c3c8b5..1083d6e7e 100644 --- a/extensions/ql-vscode/test/pure-tests/databases/config/db-config-store.test.ts +++ b/extensions/ql-vscode/test/pure-tests/databases/config/db-config-store.test.ts @@ -128,4 +128,39 @@ describe("db config store", () => { configStore.dispose(); }); + + it("should set codeQLDatabasesExperimental.configError to true when config has error", async () => { + const testDataStoragePathInvalid = join(__dirname, "data", "invalid"); + + const app = createMockApp({ + extensionPath, + workspaceStoragePath: testDataStoragePathInvalid, + }); + const configStore = new DbConfigStore(app); + await configStore.initialize(); + + expect(app.executeCommand).toBeCalledWith( + "setContext", + "codeQLDatabasesExperimental.configError", + true, + ); + configStore.dispose(); + }); + + it("should set codeQLDatabasesExperimental.configError to false when config is valid", async () => { + const app = createMockApp({ + extensionPath, + workspaceStoragePath: testDataStoragePath, + }); + const configStore = new DbConfigStore(app); + await configStore.initialize(); + + expect(app.executeCommand).toBeCalledWith( + "setContext", + "codeQLDatabasesExperimental.configError", + false, + ); + + configStore.dispose(); + }); }); diff --git a/extensions/ql-vscode/test/pure-tests/databases/config/db-config-validator.test.ts b/extensions/ql-vscode/test/pure-tests/databases/config/db-config-validator.test.ts index 565d76735..6425ac068 100644 --- a/extensions/ql-vscode/test/pure-tests/databases/config/db-config-validator.test.ts +++ b/extensions/ql-vscode/test/pure-tests/databases/config/db-config-validator.test.ts @@ -1,6 +1,11 @@ import { join } from "path"; import { DbConfig } from "../../../../src/databases/config/db-config"; import { DbConfigValidator } from "../../../../src/databases/config/db-config-validator"; +import { DbConfigValidationErrorKind } from "../../../../src/databases/db-validation-errors"; +import { + createDbConfig, + createLocalDbConfigItem, +} from "../../../../src/vscode-tests/factories/db-config-factories"; describe("db config validation", () => { const extensionPath = join(__dirname, "../../../.."); @@ -29,14 +34,139 @@ describe("db config validation", () => { expect(validationOutput).toHaveLength(3); - expect(validationOutput[0]).toEqual( - "/databases must have required property 'local'", - ); - expect(validationOutput[1]).toEqual( - "/databases/remote must have required property 'owners'", - ); - expect(validationOutput[2]).toEqual( - "/databases/remote must NOT have additional properties", - ); + expect(validationOutput[0]).toEqual({ + kind: DbConfigValidationErrorKind.InvalidConfig, + message: "/databases must have required property 'local'", + }); + expect(validationOutput[1]).toEqual({ + kind: DbConfigValidationErrorKind.InvalidConfig, + message: "/databases/remote must have required property 'owners'", + }); + expect(validationOutput[2]).toEqual({ + kind: DbConfigValidationErrorKind.InvalidConfig, + message: "/databases/remote must NOT have additional properties", + }); + }); + + it("should return error when there are multiple remote db lists with the same name", async () => { + const dbConfig = createDbConfig({ + remoteLists: [ + { + name: "repoList1", + repositories: ["owner1/repo1", "owner1/repo2"], + }, + { + name: "repoList1", + repositories: ["owner2/repo1", "owner2/repo2"], + }, + ], + }); + + const validationOutput = configValidator.validate(dbConfig); + + expect(validationOutput).toHaveLength(1); + expect(validationOutput[0]).toEqual({ + kind: DbConfigValidationErrorKind.DuplicateNames, + message: "There are database lists with the same name: repoList1", + }); + }); + + it("should return error when there are multiple remote dbs with the same name", async () => { + const dbConfig = createDbConfig({ + remoteRepos: ["owner1/repo1", "owner1/repo2", "owner1/repo2"], + }); + + const validationOutput = configValidator.validate(dbConfig); + + expect(validationOutput).toHaveLength(1); + expect(validationOutput[0]).toEqual({ + kind: DbConfigValidationErrorKind.DuplicateNames, + message: "There are databases with the same name: owner1/repo2", + }); + }); + + it("should return error when there are multiple remote dbs with the same name in the same list", async () => { + const dbConfig = createDbConfig({ + remoteLists: [ + { + name: "repoList1", + repositories: ["owner1/repo1", "owner1/repo2", "owner1/repo2"], + }, + ], + }); + + const validationOutput = configValidator.validate(dbConfig); + + expect(validationOutput).toHaveLength(1); + expect(validationOutput[0]).toEqual({ + kind: DbConfigValidationErrorKind.DuplicateNames, + message: + "There are databases with the same name in the repoList1 list: owner1/repo2", + }); + }); + + it("should return error when there are multiple local db lists with the same name", async () => { + const dbConfig = createDbConfig({ + localLists: [ + { + name: "dbList1", + databases: [createLocalDbConfigItem()], + }, + { + name: "dbList1", + databases: [createLocalDbConfigItem()], + }, + ], + }); + + const validationOutput = configValidator.validate(dbConfig); + + expect(validationOutput).toHaveLength(1); + expect(validationOutput[0]).toEqual({ + kind: DbConfigValidationErrorKind.DuplicateNames, + message: "There are database lists with the same name: dbList1", + }); + }); + + it("should return error when there are multiple local dbs with the same name", async () => { + const dbConfig = createDbConfig({ + localDbs: [ + createLocalDbConfigItem({ name: "db1" }), + createLocalDbConfigItem({ name: "db2" }), + createLocalDbConfigItem({ name: "db1" }), + ], + }); + + const validationOutput = configValidator.validate(dbConfig); + + expect(validationOutput).toHaveLength(1); + expect(validationOutput[0]).toEqual({ + kind: DbConfigValidationErrorKind.DuplicateNames, + message: "There are databases with the same name: db1", + }); + }); + + it("should return error when there are multiple local dbs with the same name in the same list", async () => { + const dbConfig = createDbConfig({ + localLists: [ + { + name: "dbList1", + databases: [ + createLocalDbConfigItem({ name: "db1" }), + createLocalDbConfigItem({ name: "db2" }), + createLocalDbConfigItem({ name: "db1" }), + ], + }, + ], + }); + + const validationOutput = configValidator.validate(dbConfig); + + expect(validationOutput).toHaveLength(1); + expect(validationOutput[0]).toEqual({ + kind: DbConfigValidationErrorKind.DuplicateNames, + message: + "There are databases with the same name in the dbList1 list: db1", + }); }); }); diff --git a/extensions/ql-vscode/test/pure-tests/databases/db-tree-creator.test.ts b/extensions/ql-vscode/test/pure-tests/databases/db-tree-creator.test.ts index 99ce1e224..dc79ed245 100644 --- a/extensions/ql-vscode/test/pure-tests/databases/db-tree-creator.test.ts +++ b/extensions/ql-vscode/test/pure-tests/databases/db-tree-creator.test.ts @@ -13,7 +13,7 @@ import { createLocalTree, createRemoteTree, } from "../../../src/databases/db-tree-creator"; -import { createDbConfig } from "../../factories/db-config-factories"; +import { createDbConfig } from "../../../src/vscode-tests/factories/db-config-factories"; describe("db tree creator", () => { describe("createRemoteTree", () => { @@ -103,20 +103,9 @@ describe("db tree creator", () => { }); it("should create remote owner nodes", () => { - const dbConfig: DbConfig = { - databases: { - remote: { - repositoryLists: [], - owners: ["owner1", "owner2"], - repositories: [], - }, - local: { - lists: [], - databases: [], - }, - }, - expanded: [], - }; + const dbConfig: DbConfig = createDbConfig({ + remoteOwners: ["owner1", "owner2"], + }); const dbTreeRoot = createRemoteTree(dbConfig); diff --git a/extensions/ql-vscode/test/pure-tests/text-utils.test.ts b/extensions/ql-vscode/test/pure-tests/text-utils.test.ts new file mode 100644 index 000000000..bc2499d42 --- /dev/null +++ b/extensions/ql-vscode/test/pure-tests/text-utils.test.ts @@ -0,0 +1,15 @@ +import { findDuplicateStrings } from "../../src/text-utils"; + +describe("findDuplicateStrings", () => { + it("should find duplicates strings in an array of strings", () => { + const strings = ["a", "b", "c", "a", "aa", "bb"]; + const duplicates = findDuplicateStrings(strings); + expect(duplicates).toEqual(["a"]); + }); + + it("should not find duplicates strings if there aren't any", () => { + const strings = ["a", "b", "c", "aa", "bb"]; + const duplicates = findDuplicateStrings(strings); + expect(duplicates).toEqual([]); + }); +}); diff --git a/extensions/ql-vscode/test/tsconfig.json b/extensions/ql-vscode/test/tsconfig.json index 0e09e2c63..ead73b331 100644 --- a/extensions/ql-vscode/test/tsconfig.json +++ b/extensions/ql-vscode/test/tsconfig.json @@ -1,6 +1,9 @@ { "extends": "../tsconfig.json", - "include": ["**/*.ts"], + "include": [ + "**/*.ts", + "../src/vscode-tests/factories/db-config-factories.ts" + ], "exclude": [], "compilerOptions": { "noEmit": true,