From 28952e87a1ff2289acd4e462e6d3bca03c4aee13 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Mon, 10 Jul 2017 11:44:52 +0900 Subject: [PATCH 1/3] Update: add question to confirm downgrade (fixes #8870) --- lib/config/config-initializer.js | 137 ++++++++++++++++++++++--- lib/util/npm-util.js | 2 +- package.json | 2 +- tests/lib/config/config-initializer.js | 70 ++++++++++++- 4 files changed, 193 insertions(+), 18 deletions(-) diff --git a/lib/config/config-initializer.js b/lib/config/config-initializer.js index e87cb79b6db..ef6cbfdee56 100644 --- a/lib/config/config-initializer.js +++ b/lib/config/config-initializer.js @@ -12,10 +12,12 @@ const util = require("util"), inquirer = require("inquirer"), ProgressBar = require("progress"), + semver = require("semver"), autoconfig = require("./autoconfig.js"), ConfigFile = require("./config-file"), ConfigOps = require("./config-ops"), getSourceCodeOfFiles = require("../util/source-code-util").getSourceCodeOfFiles, + ModuleResolver = require("../util/module-resolver"), npmUtil = require("../util/npm-util"), recConfig = require("../../conf/eslint-recommended"), log = require("../logging"); @@ -56,12 +58,35 @@ function writeFile(config, format) { } } +/** + * Get the peer dependencies of the given module. + * This adds the gotten value to cache at the first time, then reuses it. + * In a process, this function is called twice, but `npmUtil.fetchPeerDependencies` needs to access network which is relatively slow. + * @param {string} moduleName The module name to get. + * @returns {Object} The peer dependencies of the given module. + * This object is the object of `peerDependencies` field of `package.json`. + */ +function getPeerDependencies(moduleName) { + let result = getPeerDependencies.cache.get(moduleName); + + if (!result) { + log.info(`Checking peerDependencies of ${moduleName}`); + + result = npmUtil.fetchPeerDependencies(moduleName); + getPeerDependencies.cache.set(moduleName, result); + } + + return result; +} +getPeerDependencies.cache = new Map(); + /** * Synchronously install necessary plugins, configs, parsers, etc. based on the config * @param {Object} config config object + * @param {boolean} [installESLint=true] If `false` is given, it does not install eslint. * @returns {void} */ -function installModules(config) { +function installModules(config, installESLint) { const modules = {}; // Create a list of modules which should be installed based on config @@ -73,11 +98,10 @@ function installModules(config) { if (config.extends && config.extends.indexOf("eslint:") === -1) { const moduleName = `eslint-config-${config.extends}`; - log.info(`Checking peerDependencies of ${moduleName}`); modules[moduleName] = "latest"; Object.assign( modules, - npmUtil.fetchPeerDependencies(`${moduleName}@latest`) + getPeerDependencies(`${moduleName}@latest`) ); } @@ -86,15 +110,17 @@ function installModules(config) { return; } - // Add eslint to list in case user does not have it installed locally - modules.eslint = modules.eslint || "latest"; - - // Mark to show messages if it's new installation of eslint. - const installStatus = npmUtil.checkDevDeps(["eslint"]); + if (installESLint === false) { + delete modules.eslint; + } else { + const installStatus = npmUtil.checkDevDeps(["eslint"]); - if (installStatus.eslint === false) { - log.info("Local ESLint installation not found."); - config.installedESLint = true; + // Mark to show messages if it's new installation of eslint. + if (installStatus.eslint === false) { + log.info("Local ESLint installation not found."); + modules.eslint = modules.eslint || "latest"; + config.installedESLint = true; + } } // Install packages @@ -265,9 +291,10 @@ function processAnswers(answers) { /** * process user's style guide of choice and return an appropriate config object. * @param {string} guide name of the chosen style guide + * @param {boolean} [installESLint=true] If `false` is given, it does not install eslint. * @returns {Object} config object */ -function getConfigForStyleGuide(guide) { +function getConfigForStyleGuide(guide, installESLint) { const guides = { google: { extends: "google" }, airbnb: { extends: "airbnb" }, @@ -279,11 +306,74 @@ function getConfigForStyleGuide(guide) { throw new Error("You referenced an unsupported guide."); } - installModules(guides[guide]); + installModules(guides[guide], installESLint); return guides[guide]; } +/** + * Get the version of the local ESLint. + * @returns {string|null} The version. If the local ESLint was not found, returns null. + */ +function getLocalESLintVersion() { + try { + const resolver = new ModuleResolver(); + const eslintPath = resolver.resolve("eslint", process.cwd()); + const eslint = require(eslintPath); + + return eslint.linter.version || null; + } catch (_err) { + return null; + } +} + +/** + * Get the shareable config name of the chosen style guide. + * @param {Object} answers The answers object. + * @returns {string} The shareable config name. + */ +function getStyleGuideName(answers) { + if (answers.styleguide === "airbnb" && !answers.airbnbReact) { + return "airbnb-base"; + } + return answers.styleguide; +} + +/** + * Check the local ESLint version satisfies the required version of the chosen shareable config. + * @param {Object} answers The answers object. + * @returns {boolean} `true` if the local ESLint is not found or it satisfies the required version of the chosen shareable config. + */ +function checkLocalESLintVersion(answers) { + + // Get the local ESLint version. + const localESLintVersion = getLocalESLintVersion(); + + if (!localESLintVersion) { + return true; + } + + // Get the required range of ESLint version. + const configName = getStyleGuideName(answers); + const moduleName = `eslint-config-${configName}@latest`; + const requiredESLintVersionRange = getPeerDependencies(moduleName).eslint; + + if (!requiredESLintVersionRange) { + return true; + } + + answers.localESLintVersion = localESLintVersion; + answers.requiredESLintVersionRange = requiredESLintVersionRange; + + // Check the version. + if (semver.satisfies(localESLintVersion, requiredESLintVersionRange)) { + answers.installESLint = false; + return true; + } + + return false; +} + /* istanbul ignore next: no need to test inquirer*/ /** * Ask use a few questions on command prompt @@ -346,6 +436,21 @@ function promptUser() { when(answers) { return ((answers.source === "guide" && answers.packageJsonExists) || answers.source === "auto"); } + }, + { + type: "confirm", + name: "installESLint", + message(answers) { + const verb = semver.ltr(answers.localESLintVersion, answers.requiredESLintVersionRange) + ? "upgrade" + : "downgrade"; + + return `The style guide "${answers.styleguide}" requires eslint@${answers.requiredESLintVersionRange}. You are currently using eslint@${answers.localESLintVersion}.\n Do you want to ${verb}?`; + }, + default: true, + when(answers) { + return answers.source === "guide" && answers.packageJsonExists && !checkLocalESLintVersion(answers); + } } ]).then(earlyAnswers => { @@ -355,11 +460,14 @@ function promptUser() { log.info("A package.json is necessary to install plugins such as style guides. Run `npm init` to create a package.json file and try again."); return void 0; } + if (earlyAnswers.installESLint === false && !semver.satisfies(earlyAnswers.localESLintVersion, earlyAnswers.requiredESLintVersionRange)) { + log.info(`Note: it might not work since ESLint's version is mismatched with the ${earlyAnswers.styleguide} config.`); + } if (earlyAnswers.styleguide === "airbnb" && !earlyAnswers.airbnbReact) { earlyAnswers.styleguide = "airbnb-base"; } - config = getConfigForStyleGuide(earlyAnswers.styleguide); + config = getConfigForStyleGuide(earlyAnswers.styleguide, earlyAnswers.installESLint); writeFile(config, earlyAnswers.format); return void 0; @@ -479,6 +587,7 @@ function promptUser() { const init = { getConfigForStyleGuide, + checkLocalESLintVersion, processAnswers, /* istanbul ignore next */initializeConfig() { return promptUser(); diff --git a/lib/util/npm-util.js b/lib/util/npm-util.js index 057dbfea4d0..23334b35b24 100644 --- a/lib/util/npm-util.js +++ b/lib/util/npm-util.js @@ -59,7 +59,7 @@ function installSyncSaveDev(packages) { /** * Fetch `peerDependencies` of the given package by `npm show` command. * @param {string} packageName The package name to fetch peerDependencies. - * @returns {string[]} Gotten peerDependencies. + * @returns {Object} Gotten peerDependencies. */ function fetchPeerDependencies(packageName) { const fetchedText = childProcess.execSync( diff --git a/package.json b/package.json index fdd950684fa..1be6f7db9fe 100644 --- a/package.json +++ b/package.json @@ -65,6 +65,7 @@ "pluralize": "^4.0.0", "progress": "^2.0.0", "require-uncached": "^1.0.3", + "semver": "^5.3.0", "strip-json-comments": "~2.0.1", "table": "^4.0.1", "text-table": "~0.2.0" @@ -102,7 +103,6 @@ "npm-license": "^0.3.3", "phantomjs-prebuilt": "^2.1.14", "proxyquire": "^1.8.0", - "semver": "^5.3.0", "shelljs": "^0.7.7", "shelljs-nodecli": "~0.1.1", "sinon": "^2.3.2", diff --git a/tests/lib/config/config-initializer.js b/tests/lib/config/config-initializer.js index 1a7d0a90617..0754b9ed2ca 100644 --- a/tests/lib/config/config-initializer.js +++ b/tests/lib/config/config-initializer.js @@ -33,14 +33,30 @@ describe("configInitializer", () => { npmCheckStub, npmInstallStub, npmFetchPeerDependenciesStub, - init; + init, + localESLintVersion = null; const log = { info: sinon.spy(), error: sinon.spy() }; const requireStubs = { - "../logging": log + "../logging": log, + "../util/module-resolver": class ModuleResolver { + + /** + * @returns {string} The path to local eslint to test. + */ + resolve() { // eslint-disable-line class-methods-use-this + if (localESLintVersion) { + return `local-eslint-${localESLintVersion}`; + } + throw new Error("Cannot find module"); + } + }, + "local-eslint-3.18.0": { linter: { version: "3.18.0" }, "@noCallThru": true }, + "local-eslint-3.19.0": { linter: { version: "3.19.0" }, "@noCallThru": true }, + "local-eslint-4.0.0": { linter: { version: "4.0.0" }, "@noCallThru": true } }; /** @@ -245,6 +261,56 @@ describe("configInitializer", () => { ] ); }); + + describe("checkLocalESLintVersion (Note: peerDependencies always `eslint: \"^3.19.0\"` by stubs)", () => { + describe("if local ESLint is not found,", () => { + before(() => { + localESLintVersion = null; + }); + + it("should returns true.", () => { + const result = init.checkLocalESLintVersion({ styleguide: "airbnb" }); + + assert.equal(result, true); + }); + }); + + describe("if local ESLint is 3.19.0,", () => { + before(() => { + localESLintVersion = "3.19.0"; + }); + + it("should returns true.", () => { + const result = init.checkLocalESLintVersion({ styleguide: "airbnb" }); + + assert.equal(result, true); + }); + }); + + describe("if local ESLint is 4.0.0,", () => { + before(() => { + localESLintVersion = "4.0.0"; + }); + + it("should returns false.", () => { + const result = init.checkLocalESLintVersion({ styleguide: "airbnb" }); + + assert.equal(result, false); + }); + }); + + describe("if local ESLint is 3.18.0,", () => { + before(() => { + localESLintVersion = "3.18.0"; + }); + + it("should returns false.", () => { + const result = init.checkLocalESLintVersion({ styleguide: "airbnb" }); + + assert.equal(result, false); + }); + }); + }); }); describe("auto", () => { From abfe86d05e67f1977fa8e01feab6d7edf6de18c0 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 13 Jul 2017 15:42:54 +0900 Subject: [PATCH 2/3] =?UTF-8?q?fix:=20checkLocalESLintVersion=20=E2=86=92?= =?UTF-8?q?=20hasESLintVersionConflict?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/config/config-initializer.js | 18 +++++++++--------- tests/lib/config/config-initializer.js | 26 +++++++++++++------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/lib/config/config-initializer.js b/lib/config/config-initializer.js index ef6cbfdee56..d344fa0ac77 100644 --- a/lib/config/config-initializer.js +++ b/lib/config/config-initializer.js @@ -340,17 +340,17 @@ function getStyleGuideName(answers) { } /** - * Check the local ESLint version satisfies the required version of the chosen shareable config. + * Check whether the local ESLint version conflicts with the required version of the chosen shareable config. * @param {Object} answers The answers object. - * @returns {boolean} `true` if the local ESLint is not found or it satisfies the required version of the chosen shareable config. + * @returns {boolean} `true` if the local ESLint is found then it conflicts with the required version of the chosen shareable config. */ -function checkLocalESLintVersion(answers) { +function hasESLintVersionConflict(answers) { // Get the local ESLint version. const localESLintVersion = getLocalESLintVersion(); if (!localESLintVersion) { - return true; + return false; } // Get the required range of ESLint version. @@ -359,7 +359,7 @@ function checkLocalESLintVersion(answers) { const requiredESLintVersionRange = getPeerDependencies(moduleName).eslint; if (!requiredESLintVersionRange) { - return true; + return false; } answers.localESLintVersion = localESLintVersion; @@ -368,10 +368,10 @@ function checkLocalESLintVersion(answers) { // Check the version. if (semver.satisfies(localESLintVersion, requiredESLintVersionRange)) { answers.installESLint = false; - return true; + return false; } - return false; + return true; } /* istanbul ignore next: no need to test inquirer*/ @@ -449,7 +449,7 @@ function promptUser() { }, default: true, when(answers) { - return answers.source === "guide" && answers.packageJsonExists && !checkLocalESLintVersion(answers); + return answers.source === "guide" && answers.packageJsonExists && hasESLintVersionConflict(answers); } } ]).then(earlyAnswers => { @@ -587,7 +587,7 @@ function promptUser() { const init = { getConfigForStyleGuide, - checkLocalESLintVersion, + hasESLintVersionConflict, processAnswers, /* istanbul ignore next */initializeConfig() { return promptUser(); diff --git a/tests/lib/config/config-initializer.js b/tests/lib/config/config-initializer.js index 0754b9ed2ca..9a337b16fb0 100644 --- a/tests/lib/config/config-initializer.js +++ b/tests/lib/config/config-initializer.js @@ -262,16 +262,16 @@ describe("configInitializer", () => { ); }); - describe("checkLocalESLintVersion (Note: peerDependencies always `eslint: \"^3.19.0\"` by stubs)", () => { + describe("hasESLintVersionConflict (Note: peerDependencies always `eslint: \"^3.19.0\"` by stubs)", () => { describe("if local ESLint is not found,", () => { before(() => { localESLintVersion = null; }); - it("should returns true.", () => { - const result = init.checkLocalESLintVersion({ styleguide: "airbnb" }); + it("should returns false.", () => { + const result = init.hasESLintVersionConflict({ styleguide: "airbnb" }); - assert.equal(result, true); + assert.equal(result, false); }); }); @@ -280,10 +280,10 @@ describe("configInitializer", () => { localESLintVersion = "3.19.0"; }); - it("should returns true.", () => { - const result = init.checkLocalESLintVersion({ styleguide: "airbnb" }); + it("should returns false.", () => { + const result = init.hasESLintVersionConflict({ styleguide: "airbnb" }); - assert.equal(result, true); + assert.equal(result, false); }); }); @@ -292,10 +292,10 @@ describe("configInitializer", () => { localESLintVersion = "4.0.0"; }); - it("should returns false.", () => { - const result = init.checkLocalESLintVersion({ styleguide: "airbnb" }); + it("should returns true.", () => { + const result = init.hasESLintVersionConflict({ styleguide: "airbnb" }); - assert.equal(result, false); + assert.equal(result, true); }); }); @@ -304,10 +304,10 @@ describe("configInitializer", () => { localESLintVersion = "3.18.0"; }); - it("should returns false.", () => { - const result = init.checkLocalESLintVersion({ styleguide: "airbnb" }); + it("should returns true.", () => { + const result = init.hasESLintVersionConflict({ styleguide: "airbnb" }); - assert.equal(result, false); + assert.equal(result, true); }); }); }); From cfca6837c6f065c7d0a0a730bdb6bcecd4d2cef4 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 13 Jul 2017 15:43:17 +0900 Subject: [PATCH 3/3] fix typo --- tests/lib/config/config-initializer.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/lib/config/config-initializer.js b/tests/lib/config/config-initializer.js index 9a337b16fb0..9cb44ae1e28 100644 --- a/tests/lib/config/config-initializer.js +++ b/tests/lib/config/config-initializer.js @@ -268,7 +268,7 @@ describe("configInitializer", () => { localESLintVersion = null; }); - it("should returns false.", () => { + it("should return false.", () => { const result = init.hasESLintVersionConflict({ styleguide: "airbnb" }); assert.equal(result, false); @@ -280,7 +280,7 @@ describe("configInitializer", () => { localESLintVersion = "3.19.0"; }); - it("should returns false.", () => { + it("should return false.", () => { const result = init.hasESLintVersionConflict({ styleguide: "airbnb" }); assert.equal(result, false); @@ -292,7 +292,7 @@ describe("configInitializer", () => { localESLintVersion = "4.0.0"; }); - it("should returns true.", () => { + it("should return true.", () => { const result = init.hasESLintVersionConflict({ styleguide: "airbnb" }); assert.equal(result, true); @@ -304,7 +304,7 @@ describe("configInitializer", () => { localESLintVersion = "3.18.0"; }); - it("should returns true.", () => { + it("should return true.", () => { const result = init.hasESLintVersionConflict({ styleguide: "airbnb" }); assert.equal(result, true);