From 7ce3dc2c43deac21168e3eb2e6ff6b76a4b0e226 Mon Sep 17 00:00:00 2001 From: Andrew Eisenberg Date: Fri, 20 Mar 2020 15:35:39 -0700 Subject: [PATCH] feat: Display warning when codeql.cmd is used The old launcher has been deprecated and codeql.exe is recommended. Fixes #287. --- extensions/ql-vscode/CHANGELOG.md | 1 + extensions/ql-vscode/src/distribution.ts | 44 ++++++- .../no-workspace/distribution.test.ts | 115 +++++++++++++++++- .../ql-vscode/test/pure-tests/logging.test.ts | 2 +- 4 files changed, 153 insertions(+), 9 deletions(-) diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index 85dd2fd2933..cd5fb1c6cb6 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -5,6 +5,7 @@ - Add new command in query history view to view the log file of a query. - Request user acknowledgment before updating the CodeQL binaries. +- Warn when using the deprecated `codeql.cmd` launcher on windows. ## 1.1.0 - 17 March 2020 diff --git a/extensions/ql-vscode/src/distribution.ts b/extensions/ql-vscode/src/distribution.ts index 04b53ebe5bf..3d27af582bb 100644 --- a/extensions/ql-vscode/src/distribution.ts +++ b/extensions/ql-vscode/src/distribution.ts @@ -8,6 +8,7 @@ import { ExtensionContext, Event } from "vscode"; import { DistributionConfig } from "./config"; import { InvocationRateLimiter, InvocationRateLimiterResultKind, ProgressUpdate, showAndLogErrorMessage } from "./helpers"; import { logger } from "./logging"; +import * as helpers from "./helpers"; import { getCodeQlCliVersion, tryParseVersionString, Version } from "./cli-version"; /** @@ -111,6 +112,9 @@ export class DistributionManager implements DistributionProvider { "that a CodeQL executable exists at the specified path or remove the setting."); return undefined; } + if (deprecatedCodeQlLauncherName() && this._config.customCodeQlPath.endsWith(deprecatedCodeQlLauncherName()!)) { + warnDeprecatedLauncher(); + } return this._config.customCodeQlPath; } @@ -121,8 +125,8 @@ export class DistributionManager implements DistributionProvider { if (process.env.PATH) { for (const searchDirectory of process.env.PATH.split(path.delimiter)) { - const expectedLauncherPath = path.join(searchDirectory, codeQlLauncherName()); - if (await fs.pathExists(expectedLauncherPath)) { + const expectedLauncherPath = await getExecutableFromDirectory(searchDirectory); + if (expectedLauncherPath) { return expectedLauncherPath; } } @@ -186,12 +190,11 @@ class ExtensionSpecificDistributionManager { public async getCodeQlPathWithoutVersionCheck(): Promise { if (this.getInstalledRelease() !== undefined) { // An extension specific distribution has been installed. - const expectedLauncherPath = path.join(this.getDistributionRootPath(), codeQlLauncherName()); - if (await fs.pathExists(expectedLauncherPath)) { + const expectedLauncherPath = await getExecutableFromDirectory(this.getDistributionRootPath(), true); + if (expectedLauncherPath) { return expectedLauncherPath; } - logger.log(`WARNING: Expected to find a CodeQL CLI executable at ${expectedLauncherPath} but one was not found. ` + - "Will try PATH."); + try { await this.removeDistribution(); } catch (e) { @@ -514,6 +517,10 @@ function codeQlLauncherName(): string { return (os.platform() === "win32") ? "codeql.exe" : "codeql"; } +function deprecatedCodeQlLauncherName(): string | undefined { + return (os.platform() === "win32") ? "codeql.cmd" : undefined; +} + function isRedirectStatusCode(statusCode: number): boolean { return statusCode === 301 || statusCode === 302 || statusCode === 303 || statusCode === 307 || statusCode === 308; } @@ -614,6 +621,31 @@ function createUpdateAvailableResult(updatedRelease: Release): UpdateAvailableRe }; } +// Exported for testing +export async function getExecutableFromDirectory(directory: string, warnWhenNotFound = false): Promise { + const expectedLauncherPath = path.join(directory, codeQlLauncherName()); + const deprecatedLauncherName = deprecatedCodeQlLauncherName(); + const alternateExpectedLauncherPath = deprecatedLauncherName ? path.join(directory, deprecatedLauncherName) : undefined; + if (await fs.pathExists(expectedLauncherPath)) { + return expectedLauncherPath; + } else if (alternateExpectedLauncherPath && (await fs.pathExists(alternateExpectedLauncherPath))) { + warnDeprecatedLauncher(); + return alternateExpectedLauncherPath; + } + if (warnWhenNotFound) { + logger.log(`WARNING: Expected to find a CodeQL CLI executable at ${expectedLauncherPath} but one was not found. ` + + "Will try PATH."); + } + return undefined; +} + +function warnDeprecatedLauncher() { + helpers.showAndLogWarningMessage( + `The "${deprecatedCodeQlLauncherName()!}" launcher has been deprecated and will be removed in a future version. ` + + `Please use "${codeQlLauncherName()}" instead. It is recommended to update to the latest CodeQL binaries.` + ); +} + /** * A release on GitHub. */ diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/distribution.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/distribution.test.ts index 9fe0a10e2a1..64d425e9287 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/distribution.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/distribution.test.ts @@ -1,8 +1,18 @@ -import { expect } from "chai"; +import * as chai from "chai"; +import * as path from "path"; import * as fetch from "node-fetch"; +import 'chai/register-should'; +import * as sinonChai from 'sinon-chai'; +import * as sinon from 'sinon'; +import * as pq from "proxyquire"; import "mocha"; + import { Version } from "../../cli-version"; -import { GithubRelease, GithubReleaseAsset, ReleasesApiConsumer, versionCompare } from "../../distribution" +import { GithubRelease, GithubReleaseAsset, ReleasesApiConsumer, versionCompare } from "../../distribution"; + +const proxyquire = pq.noPreserveCache(); +chai.use(sinonChai); +const expect = chai.expect; describe("Releases API consumer", () => { const owner = "someowner"; @@ -176,3 +186,104 @@ describe("Release version ordering", () => { expect(versionCompare(createVersion(2, 1, 0, "alpha.1", "abcdef0"), createVersion(2, 1, 0, "alpha.1", "bcdef01"))).to.equal(0); }); }); + +describe('Launcher path', () => { + let sandbox: sinon.SinonSandbox; + let warnSpy: sinon.SinonSpy; + let logSpy: sinon.SinonSpy; + let fsSpy: sinon.SinonSpy; + let platformSpy: sinon.SinonSpy; + + let getExecutableFromDirectory: Function; + + let launcherThatExists = ''; + + beforeEach(() => { + getExecutableFromDirectory = createModule().getExecutableFromDirectory; + }); + + beforeEach(() => { + sandbox.restore(); + }); + + it('should not warn with proper launcher name', async () => { + launcherThatExists = 'codeql.exe'; + const result = await getExecutableFromDirectory('abc'); + expect(fsSpy).to.have.been.calledWith(`abc${path.sep}codeql.exe`); + + // correct launcher has been found, so alternate one not looked for + expect(fsSpy).not.to.have.been.calledWith(`abc${path.sep}codeql.cmd`); + + // no warning message + expect(warnSpy).not.to.have.been.calledWith(sinon.match.string); + // No log message + expect(logSpy).not.to.have.been.calledWith(sinon.match.string); + expect(result).to.equal(`abc${path.sep}codeql.exe`); + }); + + it('should warn when using a hard-coded deprecated launcher name', async () => { + launcherThatExists = 'codeql.cmd'; + path.sep; + const result = await getExecutableFromDirectory('abc'); + expect(fsSpy).to.have.been.calledWith(`abc${path.sep}codeql.exe`); + expect(fsSpy).to.have.been.calledWith(`abc${path.sep}codeql.cmd`); + + // Should have opened a warning message + expect(warnSpy).to.have.been.calledWith(sinon.match.string); + // No log message + expect(logSpy).not.to.have.been.calledWith(sinon.match.string); + expect(result).to.equal(`abc${path.sep}codeql.cmd`); + }); + + it('should avoid warn when no launcher is found', async () => { + launcherThatExists = 'xxx'; + const result = await getExecutableFromDirectory('abc', false); + expect(fsSpy).to.have.been.calledWith(`abc${path.sep}codeql.exe`); + expect(fsSpy).to.have.been.calledWith(`abc${path.sep}codeql.cmd`); + + // no warning message + expect(warnSpy).not.to.have.been.calledWith(sinon.match.string); + // log message sent out + expect(logSpy).not.to.have.been.calledWith(sinon.match.string); + expect(result).to.equal(undefined); + }); + + it('should warn when no launcher is found', async () => { + launcherThatExists = 'xxx'; + const result = await getExecutableFromDirectory('abc', true); + expect(fsSpy).to.have.been.calledWith(`abc${path.sep}codeql.exe`); + expect(fsSpy).to.have.been.calledWith(`abc${path.sep}codeql.cmd`); + + // no warning message + expect(warnSpy).not.to.have.been.calledWith(sinon.match.string); + // log message sent out + expect(logSpy).to.have.been.calledWith(sinon.match.string); + expect(result).to.equal(undefined); + }); + + function createModule() { + sandbox = sinon.createSandbox(); + warnSpy = sandbox.spy(); + logSpy = sandbox.spy(); + // pretend that only the .cmd file exists + fsSpy = sandbox.stub().callsFake(arg => arg.endsWith(launcherThatExists) ? true : false); + platformSpy = sandbox.stub().returns('win32'); + + return proxyquire('../../distribution', { + './helpers': { + showAndLogWarningMessage: warnSpy + }, + './logging': { + 'logger': { + log: logSpy + } + }, + 'fs-extra': { + pathExists: fsSpy + }, + os: { + platform: platformSpy + } + }); + } +}); diff --git a/extensions/ql-vscode/test/pure-tests/logging.test.ts b/extensions/ql-vscode/test/pure-tests/logging.test.ts index 5ea888458f5..e39f6392b44 100644 --- a/extensions/ql-vscode/test/pure-tests/logging.test.ts +++ b/extensions/ql-vscode/test/pure-tests/logging.test.ts @@ -1,3 +1,4 @@ +import 'chai/register-should'; import * as chai from 'chai'; import * as fs from 'fs-extra'; import * as path from 'path'; @@ -8,7 +9,6 @@ import * as sinon from 'sinon'; import * as pq from 'proxyquire'; const proxyquire = pq.noPreserveCache().noCallThru(); -chai.should(); chai.use(sinonChai); const expect = chai.expect;