Merge pull request #303 from aeisenberg/aeisenberg/deprecate

feat: Display warning when codeql.cmd is used

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
 

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<string | undefined> {
     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<string | undefined> {
+  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.
  */

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
+      }
+    });
+  }
+});

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;