From 1118fceb49af3436b8dcd0c6089f913cedf9a329 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 7 Jan 2020 20:11:26 +0900 Subject: [PATCH] Breaking: runtime-deprecation on '~/.eslintrc' (refs eslint/rfcs#32) (#12678) Co-Authored-By: Kai Cataldo --- docs/user-guide/configuring.md | 2 +- .../cascading-config-array-factory.js | 44 ++- lib/cli-engine/config-array-factory.js | 23 ++ lib/shared/config-validator.js | 29 +- lib/shared/deprecation-warnings.js | 56 +++ tests/bin/eslint.js | 2 +- .../cascading-config-array-factory.js | 363 +++++++++++++++++- 7 files changed, 483 insertions(+), 36 deletions(-) create mode 100644 lib/shared/deprecation-warnings.js diff --git a/docs/user-guide/configuring.md b/docs/user-guide/configuring.md index 2a47a3e9e3d..6771287d363 100644 --- a/docs/user-guide/configuring.md +++ b/docs/user-guide/configuring.md @@ -1100,7 +1100,7 @@ This message occurs because ESLint is unsure if you wanted to actually lint the ## Personal Configuration File (deprecated) -⚠️ **This feature has been deprecated**. ESLint will print deprecation warnings beginning with the 7.0.0 release. This feature will then be removed in the 8.0.0 release. If you want to continue to use personal configuration files, please use the [`--config` CLI option](https://eslint.org/docs/user-guide/command-line-interface#-c---config). For more information regarding this decision, please see [RFC 28](https://github.com/eslint/rfcs/pull/28) and [RFC 32](https://github.com/eslint/rfcs/pull/32). +⚠️ **This feature has been deprecated**. This feature will be removed in the 8.0.0 release. If you want to continue to use personal configuration files, please use the [`--config` CLI option](https://eslint.org/docs/user-guide/command-line-interface#-c---config). For more information regarding this decision, please see [RFC 28](https://github.com/eslint/rfcs/pull/28) and [RFC 32](https://github.com/eslint/rfcs/pull/32). `~/` refers to [the home directory of the current user on your preferred operating system](https://nodejs.org/api/os.html#os_os_homedir). The personal configuration file being referred to here is `~/.eslintrc.*` file, which is currently handled differently than other configuration files. diff --git a/lib/cli-engine/cascading-config-array-factory.js b/lib/cli-engine/cascading-config-array-factory.js index 52703a873bb..ca03bec0638 100644 --- a/lib/cli-engine/cascading-config-array-factory.js +++ b/lib/cli-engine/cascading-config-array-factory.js @@ -26,6 +26,7 @@ const os = require("os"); const path = require("path"); const { validateConfigArray } = require("../shared/config-validator"); +const { emitDeprecationWarning } = require("../shared/deprecation-warnings"); const { ConfigArrayFactory } = require("./config-array-factory"); const { ConfigArray, ConfigDependency, IgnorePattern } = require("./config-array"); const loadRules = require("./load-rules"); @@ -290,10 +291,11 @@ class CascadingConfigArrayFactory { /** * Load and normalize config files from the ancestor directories. * @param {string} directoryPath The path to a leaf directory. + * @param {boolean} configsExistInSubdirs `true` if configurations exist in subdirectories. * @returns {ConfigArray} The loaded config. * @private */ - _loadConfigInAncestors(directoryPath) { + _loadConfigInAncestors(directoryPath, configsExistInSubdirs = false) { const { baseConfigArray, configArrayFactory, @@ -320,6 +322,16 @@ class CascadingConfigArrayFactory { // Consider this is root. if (directoryPath === homePath && cwd !== homePath) { debug("Stop traversing because of considered root."); + if (configsExistInSubdirs) { + const filePath = ConfigArrayFactory.getPathToConfigFileInDirectory(directoryPath); + + if (filePath) { + emitDeprecationWarning( + filePath, + "ESLINT_PERSONAL_CONFIG_SUPPRESS" + ); + } + } return this._cacheConfig(directoryPath, baseConfigArray); } @@ -344,7 +356,10 @@ class CascadingConfigArrayFactory { // Load from the ancestors and merge it. const parentPath = path.dirname(directoryPath); const parentConfigArray = parentPath && parentPath !== directoryPath - ? this._loadConfigInAncestors(parentPath) + ? this._loadConfigInAncestors( + parentPath, + configsExistInSubdirs || configArray.length > 0 + ) : baseConfigArray; if (configArray.length > 0) { @@ -400,12 +415,29 @@ class CascadingConfigArrayFactory { configArray.every(c => !c.filePath) && cliConfigArray.every(c => !c.filePath) // `--config` option can be a file. ) { - debug("Loading the config file of the home directory."); + const homePath = os.homedir(); + + debug("Loading the config file of the home directory:", homePath); - finalConfigArray = configArrayFactory.loadInDirectory( - os.homedir(), - { name: "PersonalConfig", parent: finalConfigArray } + const personalConfigArray = configArrayFactory.loadInDirectory( + homePath, + { name: "PersonalConfig" } ); + + if ( + personalConfigArray.length > 0 && + !directoryPath.startsWith(homePath) + ) { + const lastElement = + personalConfigArray[personalConfigArray.length - 1]; + + emitDeprecationWarning( + lastElement.filePath, + "ESLINT_PERSONAL_CONFIG_LOAD" + ); + } + + finalConfigArray = finalConfigArray.concat(personalConfigArray); } // Apply CLI options. diff --git a/lib/cli-engine/config-array-factory.js b/lib/cli-engine/config-array-factory.js index 76c4ccd7021..2f6595355eb 100644 --- a/lib/cli-engine/config-array-factory.js +++ b/lib/cli-engine/config-array-factory.js @@ -436,6 +436,29 @@ class ConfigArrayFactory { ); } + /** + * Check if a config file on a given directory exists or not. + * @param {string} directoryPath The path to a directory. + * @returns {string | null} The path to the found config file. If not found then null. + */ + static getPathToConfigFileInDirectory(directoryPath) { + for (const filename of configFilenames) { + const filePath = path.join(directoryPath, filename); + + if (fs.existsSync(filePath)) { + if (filename === "package.json") { + try { + loadPackageJSONConfigFile(filePath); + return filePath; + } catch (error) { /* ignore */ } + } else { + return filePath; + } + } + } + return null; + } + /** * Load `.eslintignore` file. * @param {string} filePath The path to a `.eslintignore` file to load. diff --git a/lib/shared/config-validator.js b/lib/shared/config-validator.js index aca6e1fb274..70eaf0a9670 100644 --- a/lib/shared/config-validator.js +++ b/lib/shared/config-validator.js @@ -10,13 +10,12 @@ //------------------------------------------------------------------------------ const - path = require("path"), util = require("util"), - lodash = require("lodash"), configSchema = require("../../conf/config-schema"), BuiltInEnvironments = require("../../conf/environments"), BuiltInRules = require("../rules"), - ConfigOps = require("./config-ops"); + ConfigOps = require("./config-ops"), + { emitDeprecationWarning } = require("./deprecation-warnings"); const ajv = require("./ajv")(); const ruleValidators = new WeakMap(); @@ -26,11 +25,6 @@ const noop = Function.prototype; // Private //------------------------------------------------------------------------------ let validateSchema; - -// Defitions for deprecation warnings. -const deprecationWarningMessages = { - ESLINT_LEGACY_ECMAFEATURES: "The 'ecmaFeatures' config file property is deprecated, and has no effect." -}; const severityMap = { error: 2, warn: 1, @@ -254,25 +248,6 @@ function formatErrors(errors) { }).map(message => `\t- ${message}.\n`).join(""); } -/** - * Emits a deprecation warning containing a given filepath. A new deprecation warning is emitted - * for each unique file path, but repeated invocations with the same file path have no effect. - * No warnings are emitted if the `--no-deprecation` or `--no-warnings` Node runtime flags are active. - * @param {string} source The name of the configuration source to report the warning for. - * @param {string} errorCode The warning message to show. - * @returns {void} - */ -const emitDeprecationWarning = lodash.memoize((source, errorCode) => { - const rel = path.relative(process.cwd(), source); - const message = deprecationWarningMessages[errorCode]; - - process.emitWarning( - `${message} (found in "${rel}")`, - "DeprecationWarning", - errorCode - ); -}); - /** * Validates the top level properties of the config object. * @param {Object} config The config object to validate. diff --git a/lib/shared/deprecation-warnings.js b/lib/shared/deprecation-warnings.js new file mode 100644 index 00000000000..e1481a2e9aa --- /dev/null +++ b/lib/shared/deprecation-warnings.js @@ -0,0 +1,56 @@ +/** + * @fileoverview Provide the function that emits deprecation warnings. + * @author Toru Nagashima + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const path = require("path"); +const lodash = require("lodash"); + +//------------------------------------------------------------------------------ +// Private +//------------------------------------------------------------------------------ + +// Defitions for deprecation warnings. +const deprecationWarningMessages = { + ESLINT_LEGACY_ECMAFEATURES: + "The 'ecmaFeatures' config file property is deprecated and has no effect.", + ESLINT_PERSONAL_CONFIG_LOAD: + "'~/.eslintrc.*' config files have been deprecated. " + + "Please use a config file per project or the '--config' option.", + ESLINT_PERSONAL_CONFIG_SUPPRESS: + "'~/.eslintrc.*' config files have been deprecated. " + + "Please remove it or add 'root:true' to the config files in your " + + "projects in order to avoid loading '~/.eslintrc.*' accidentally." +}; + +/** + * Emits a deprecation warning containing a given filepath. A new deprecation warning is emitted + * for each unique file path, but repeated invocations with the same file path have no effect. + * No warnings are emitted if the `--no-deprecation` or `--no-warnings` Node runtime flags are active. + * @param {string} source The name of the configuration source to report the warning for. + * @param {string} errorCode The warning message to show. + * @returns {void} + */ +const emitDeprecationWarning = lodash.memoize((source, errorCode) => { + const rel = path.relative(process.cwd(), source); + const message = deprecationWarningMessages[errorCode]; + + process.emitWarning( + `${message} (found in "${rel}")`, + "DeprecationWarning", + errorCode + ); +}, (...args) => JSON.stringify(args)); + +//------------------------------------------------------------------------------ +// Public Interface +//------------------------------------------------------------------------------ + +module.exports = { + emitDeprecationWarning +}; diff --git a/tests/bin/eslint.js b/tests/bin/eslint.js index 932dd9ec451..774a1ace5c3 100644 --- a/tests/bin/eslint.js +++ b/tests/bin/eslint.js @@ -388,7 +388,7 @@ describe("bin/eslint.js", () => { const exitCodePromise = assertExitCode(child, 0); const outputPromise = getOutput(child).then(output => { - assert.include(output.stderr, "The 'ecmaFeatures' config file property is deprecated, and has no effect."); + assert.include(output.stderr, "The 'ecmaFeatures' config file property is deprecated and has no effect."); }); return Promise.all([exitCodePromise, outputPromise]); diff --git a/tests/lib/cli-engine/cascading-config-array-factory.js b/tests/lib/cli-engine/cascading-config-array-factory.js index 2748aa608a4..32d094aba81 100644 --- a/tests/lib/cli-engine/cascading-config-array-factory.js +++ b/tests/lib/cli-engine/cascading-config-array-factory.js @@ -14,6 +14,9 @@ const { ConfigArrayFactory } = require("../../../lib/cli-engine/config-array-fac const { ExtractedConfig } = require("../../../lib/cli-engine/config-array/extracted-config"); const { defineCascadingConfigArrayFactoryWithInMemoryFileSystem } = require("./_utils"); +/** @typedef {InstanceType["CascadingConfigArrayFactory"]>} CascadingConfigArrayFactory */ +/** @typedef {ReturnType} ConfigArray */ + const cwdIgnorePatterns = new ConfigArrayFactory() .loadDefaultESLintIgnore()[0] .ignorePattern @@ -87,6 +90,364 @@ describe("CascadingConfigArrayFactory", () => { }); }); + describe("deprecation warnings", () => { + let uid = 0; + let uniqueHomeDirName = ""; + let homeDir = ""; + let cwd = ""; + + /** @type {{code:string, message:string}[]} */ + let warnings = []; + + /** @type {CascadingConfigArrayFactory} */ + let factory = null; + + /** @type {ConfigArray} */ + let config = null; + + /** + * Store a reported warning object if that code starts with `ESLINT_`. + * @param {{code:string, message:string}} w The warning object to store. + * @returns {void} + */ + function onWarning(w) { + if (w.code.startsWith("ESLINT_")) { + warnings.push({ code: w.code, message: w.message }); + } + } + + /** + * Delay to wait for 'warning' events. + * @returns {Promise} The promise that will be fulfilled after wait a timer. + */ + function delay() { + return new Promise(resolve => setTimeout(resolve, 0)); + } + + beforeEach(() => { + uniqueHomeDirName = `home_${++uid}`; + homeDir = path.join(__dirname, `../../../${uniqueHomeDirName}`); + warnings = []; + sinon.stub(os, "homedir").returns(homeDir); + process.on("warning", onWarning); + }); + afterEach(() => { + os.homedir.restore(); + process.removeListener("warning", onWarning); + }); + + describe("when '~/.eslintrc.json' exists and CWD is `~/`", () => { + beforeEach(() => { + cwd = homeDir; + const { CascadingConfigArrayFactory } = defineCascadingConfigArrayFactoryWithInMemoryFileSystem({ + cwd: () => cwd, + files: { + + // ~/.eslintrc.json + ".eslintrc.json": JSON.stringify({ rules: { eqeqeq: "error" } }), + + // other files + "exist-with-root/test.js": "", + "exist-with-root/.eslintrc.json": JSON.stringify({ root: true, rules: { yoda: "error" } }), + "exist/test.js": "", + "exist/.eslintrc.json": JSON.stringify({ rules: { yoda: "error" } }), + "not-exist/test.js": "" + } + }); + + factory = new CascadingConfigArrayFactory({ cwd }); + }); + + // no warning. + describe("when it lints 'subdir/exist-with-root/test.js'", () => { + beforeEach(async() => { + config = factory.getConfigArrayForFile("exist-with-root/test.js"); + await delay(); + }); + + it("should not raise any warnings.", () => { + assert.deepStrictEqual(warnings, []); + }); + + it("should not load '~/.eslintrc.json'.", () => { + assert.deepStrictEqual( + config.extractConfig("a.js").rules, + { yoda: ["error"] } + ); + }); + }); + + // no warning. + describe("when it lints 'subdir/exist/test.js'", () => { + beforeEach(async() => { + config = factory.getConfigArrayForFile("exist/test.js"); + await delay(); + }); + + it("should not raise any warnings.", () => { + assert.deepStrictEqual(warnings, []); + }); + + it("should load '~/.eslintrc.json'.", () => { + assert.deepStrictEqual( + config.extractConfig("a.js").rules, + { eqeqeq: ["error"], yoda: ["error"] } + ); + }); + }); + + // no warning + describe("when it lints 'subdir/not-exist/test.js'", () => { + beforeEach(async() => { + config = factory.getConfigArrayForFile("not-exist/test.js"); + await delay(); + }); + + it("should not raise any warnings.", () => { + assert.deepStrictEqual(warnings, []); + }); + + it("should load '~/.eslintrc.json'.", () => { + assert.deepStrictEqual( + config.extractConfig("a.js").rules, + { eqeqeq: ["error"] } + ); + }); + }); + }); + + describe("when '~/.eslintrc.json' exists and CWD is `~/subdir`", () => { + beforeEach(() => { + cwd = path.join(homeDir, "subdir"); + const { CascadingConfigArrayFactory } = defineCascadingConfigArrayFactoryWithInMemoryFileSystem({ + cwd: () => cwd, + files: { + + // ~/.eslintrc.json + "../.eslintrc.json": JSON.stringify({ rules: { eqeqeq: "error" } }), + + // other files + "exist-with-root/test.js": "", + "exist-with-root/.eslintrc.json": JSON.stringify({ root: true, rules: { yoda: "error" } }), + "exist/test.js": "", + "exist/.eslintrc.json": JSON.stringify({ rules: { yoda: "error" } }), + "not-exist/test.js": "" + } + }); + + factory = new CascadingConfigArrayFactory({ cwd }); + }); + + // Project's config file has `root:true`, then no warning. + describe("when it lints 'subdir/exist-with-root/test.js'", () => { + beforeEach(async() => { + config = factory.getConfigArrayForFile("exist-with-root/test.js"); + await delay(); + }); + + it("should not raise any warnings.", () => { + assert.deepStrictEqual(warnings, []); + }); + + it("should not load '~/.eslintrc.json'.", () => { + assert.deepStrictEqual( + config.extractConfig("a.js").rules, + { yoda: ["error"] } + ); + }); + }); + + // Project's config file doesn't have `root:true` and home is ancestor, then ESLINT_PERSONAL_CONFIG_SUPPRESS. + describe("when it lints 'subdir/exist/test.js'", () => { + beforeEach(async() => { + config = factory.getConfigArrayForFile("exist/test.js"); + await delay(); + }); + + it("should raise an ESLINT_PERSONAL_CONFIG_SUPPRESS warning.", () => { + assert.deepStrictEqual(warnings, [ + { + code: "ESLINT_PERSONAL_CONFIG_SUPPRESS", + message: `'~/.eslintrc.*' config files have been deprecated. Please remove it or add 'root:true' to the config files in your projects in order to avoid loading '~/.eslintrc.*' accidentally. (found in "${uniqueHomeDirName}${path.sep}.eslintrc.json")` + } + ]); + }); + + it("should not load '~/.eslintrc.json'.", () => { + assert.deepStrictEqual( + config.extractConfig("a.js").rules, + { yoda: ["error"] } + ); + }); + }); + + /* + * Project's config file doesn't exist and home is ancestor, then no warning. + * In this case, ESLint will continue to use `~/.eslintrc.json` even if personal config file feature is removed. + */ + describe("when it lints 'subdir/not-exist/test.js'", () => { + beforeEach(async() => { + config = factory.getConfigArrayForFile("not-exist/test.js"); + await delay(); + }); + + it("should not raise any warnings.", () => { + assert.deepStrictEqual(warnings, []); + }); + + it("should load '~/.eslintrc.json'.", () => { + assert.deepStrictEqual( + config.extractConfig("a.js").rules, + { eqeqeq: ["error"] } + ); + }); + }); + }); + + describe("when '~/.eslintrc.json' exists and CWD is `~/../another`", () => { + beforeEach(() => { + cwd = path.join(homeDir, "../another"); + const { CascadingConfigArrayFactory } = defineCascadingConfigArrayFactoryWithInMemoryFileSystem({ + cwd: () => cwd, + files: { + + // ~/.eslintrc.json + [`../${uniqueHomeDirName}/.eslintrc.json`]: JSON.stringify({ rules: { eqeqeq: "error" } }), + + // other files + "exist-with-root/test.js": "", + "exist-with-root/.eslintrc.json": JSON.stringify({ root: true, rules: { yoda: "error" } }), + "exist/test.js": "", + "exist/.eslintrc.json": JSON.stringify({ rules: { yoda: "error" } }), + "not-exist/test.js": "" + } + }); + + factory = new CascadingConfigArrayFactory({ cwd }); + }); + + // Project's config file has `root:true`, then no warning. + describe("when it lints 'exist-with-root/test.js'", () => { + beforeEach(async() => { + config = factory.getConfigArrayForFile("exist-with-root/test.js"); + await delay(); + }); + + it("should not raise any warnings.", () => { + assert.deepStrictEqual(warnings, []); + }); + + it("should not load '~/.eslintrc.json'.", () => { + assert.deepStrictEqual( + config.extractConfig("a.js").rules, + { yoda: ["error"] } + ); + }); + }); + + // Project's config file doesn't have `root:true` but home is not ancestor, then no warning. + describe("when it lints 'exist/test.js'", () => { + beforeEach(async() => { + config = factory.getConfigArrayForFile("exist/test.js"); + await delay(); + }); + + it("should not raise any warnings.", () => { + assert.deepStrictEqual(warnings, []); + }); + + it("should not load '~/.eslintrc.json'.", () => { + assert.deepStrictEqual( + config.extractConfig("a.js").rules, + { yoda: ["error"] } + ); + }); + }); + + // Project's config file doesn't exist and home is not ancestor, then ESLINT_PERSONAL_CONFIG_LOAD. + describe("when it lints 'not-exist/test.js'", () => { + beforeEach(async() => { + config = factory.getConfigArrayForFile("not-exist/test.js"); + await delay(); + }); + + it("should raise an ESLINT_PERSONAL_CONFIG_LOAD warning.", () => { + assert.deepStrictEqual(warnings, [ + { + code: "ESLINT_PERSONAL_CONFIG_LOAD", + message: `'~/.eslintrc.*' config files have been deprecated. Please use a config file per project or the '--config' option. (found in "${uniqueHomeDirName}${path.sep}.eslintrc.json")` + } + ]); + }); + + it("should load '~/.eslintrc.json'.", () => { + assert.deepStrictEqual( + config.extractConfig("a.js").rules, + { eqeqeq: ["error"] } + ); + }); + }); + }); + + describe("when '~/.eslintrc.json' doesn't exist and CWD is `~/subdir`", () => { + beforeEach(() => { + cwd = path.join(homeDir, "subdir"); + const { CascadingConfigArrayFactory } = defineCascadingConfigArrayFactoryWithInMemoryFileSystem({ + cwd: () => cwd, + files: { + "exist-with-root/test.js": "", + "exist-with-root/.eslintrc.json": JSON.stringify({ root: true, rules: { yoda: "error" } }), + "exist/test.js": "", + "exist/.eslintrc.json": JSON.stringify({ rules: { yoda: "error" } }), + "not-exist/test.js": "" + } + }); + + factory = new CascadingConfigArrayFactory({ cwd }); + }); + + describe("when it lints 'subdir/exist/test.js'", () => { + beforeEach(async() => { + config = factory.getConfigArrayForFile("exist/test.js"); + await delay(); + }); + + it("should not raise any warnings.", () => { + assert.deepStrictEqual(warnings, []); + }); + }); + }); + + describe("when '~/.eslintrc.json' doesn't exist and CWD is `~/../another`", () => { + beforeEach(() => { + cwd = path.join(homeDir, "../another"); + const { CascadingConfigArrayFactory } = defineCascadingConfigArrayFactoryWithInMemoryFileSystem({ + cwd: () => cwd, + files: { + "exist-with-root/test.js": "", + "exist-with-root/.eslintrc.json": JSON.stringify({ root: true, rules: { yoda: "error" } }), + "exist/test.js": "", + "exist/.eslintrc.json": JSON.stringify({ rules: { yoda: "error" } }), + "not-exist/test.js": "" + } + }); + + factory = new CascadingConfigArrayFactory({ cwd }); + }); + + describe("when it lints 'not-exist/test.js'", () => { + beforeEach(async() => { + config = factory.getConfigArrayForFile("not-exist/test.js", { ignoreNotFoundError: true }); + await delay(); + }); + + it("should not raise any warnings.", () => { + assert.deepStrictEqual(warnings, []); + }); + }); + }); + }); + // This group moved from 'tests/lib/config.js' when refactoring to keep the cumulated test cases. describe("with 'tests/fixtures/config-hierarchy' files", () => { const { CascadingConfigArrayFactory } = require("../../../lib/cli-engine/cascading-config-array-factory"); @@ -1162,7 +1523,7 @@ describe("CascadingConfigArrayFactory", () => { assert.notStrictEqual(warning, null); assert.strictEqual( warning.message, - `The 'ecmaFeatures' config file property is deprecated, and has no effect. (found in "ecma-features${path.sep}.eslintrc.yml")` + `The 'ecmaFeatures' config file property is deprecated and has no effect. (found in "ecma-features${path.sep}.eslintrc.yml")` ); }); });