From 21dd2116c70b93aa8dd50d2b15e202724b11486a Mon Sep 17 00:00:00 2001 From: Teddy Katz Date: Fri, 10 May 2019 20:43:30 -0400 Subject: [PATCH] New: add --resolve-plugins-relative-to flag (#11696) With the [2018-simplified-package-loading RFC](https://github.com/eslint/rfcs/blob/8bc0b80e0b3e54d10991a4774c41f7375dfcbbfe/designs/2018-simplified-package-loading/README.md) implemented, ESLint always resolves plugins relative to the current working directory. The CWD works well for the most common use case, but is inconvenient for certain integrations. This commit proposes adding a CLI flag to specify an alternative place where plugins should be resolved from. (Implements https://github.com/eslint/rfcs/pull/18) --- docs/developer-guide/nodejs-api.md | 1 + docs/user-guide/command-line-interface.md | 8 ++++++ docs/user-guide/migrating-to-6.0.0.md | 6 ++--- lib/cli-engine.js | 2 ++ .../cascading-config-array-factory.js | 4 ++- lib/cli-engine/config-array-factory.js | 15 ++++++----- lib/cli.js | 3 ++- lib/options.js | 5 ++++ messages/plugin-missing.txt | 2 +- .../node_modules/eslint-plugin-with-rules.js | 14 ++++++++++ tests/lib/cli-engine.js | 18 +++++++++++++ tests/lib/cli-engine/config-array-factory.js | 27 ++++++++++++++++--- tests/lib/cli.js | 10 +++++++ 13 files changed, 99 insertions(+), 16 deletions(-) create mode 100644 tests/fixtures/plugins/node_modules/eslint-plugin-with-rules.js diff --git a/docs/developer-guide/nodejs-api.md b/docs/developer-guide/nodejs-api.md index 74b4796e9df..9e826c6371d 100644 --- a/docs/developer-guide/nodejs-api.md +++ b/docs/developer-guide/nodejs-api.md @@ -355,6 +355,7 @@ The `CLIEngine` is a constructor, and you can create a new instance by passing i * `parserOptions` - An object containing parser options (default: empty object). Corresponds to `--parser-options`. * `plugins` - An array of plugins to load (default: empty array). Corresponds to `--plugin`. * `reportUnusedDisableDirectives` - When set to `true`, adds reported errors for unused `eslint-disable` directives when no problems would be reported in the disabled area anyway (default: false). Corresponds to `--report-unused-disable-directives`. +* `resolvePluginsRelativeTo` - Determines the folder where plugins should be resolved from. Should be used when an integration installs plugins and uses those plugins to lint code on behalf of the end user. Corresponds to `--resolve-plugins-relative-to`. * `rulePaths` - An array of directories to load custom rules from (default: empty array). Corresponds to `--rulesdir`. * `rules` - An object of rules to use (default: null). Corresponds to `--rule`. * `useEslintrc` - Set to false to disable use of `.eslintrc` files (default: true). Corresponds to `--no-eslintrc`. diff --git a/docs/user-guide/command-line-interface.md b/docs/user-guide/command-line-interface.md index a71684f707b..532d8f94735 100644 --- a/docs/user-guide/command-line-interface.md +++ b/docs/user-guide/command-line-interface.md @@ -37,6 +37,7 @@ Basic configuration: --global [String] Define global variables --parser String Specify the parser to be used --parser-options Object Specify parser options + --resolve-plugins-relative-to path::String A folder where plugins should be resolved from, CWD by default Specifying rules and plugins: --rulesdir [path::String] Use additional rules from this directory @@ -164,6 +165,13 @@ Examples: echo '3 ** 4' | eslint --stdin --parser-options=ecmaVersion:6 # will fail with a parsing error echo '3 ** 4' | eslint --stdin --parser-options=ecmaVersion:7 # succeeds, yay! +#### `--resolve-plugins-relative-to` + +Changes the folder where plugins are resolved from. By default, plugins are resolved from the current working directory. This option should be used when plugins were installed by someone other than the end user. It should be set to the project directory of the project that has a dependency on the necessary plugins. For example: + +* When using a config file that is located outside of the current project (with the `--config` flag), if the config uses plugins which are installed locally to itself, `--resolve-plugins-relative-to` should be set to the directory containing the config file. +* If an integration has dependencies on ESLint and a set of plugins, and the tool invokes ESLint on behalf of the user with a preset configuration, the tool should set `--resolve-plugins-relative-to` to the top-level directory of the tool. + ### Specifying rules and plugins #### `--rulesdir` diff --git a/docs/user-guide/migrating-to-6.0.0.md b/docs/user-guide/migrating-to-6.0.0.md index f3f110e2b19..c9128936504 100644 --- a/docs/user-guide/migrating-to-6.0.0.md +++ b/docs/user-guide/migrating-to-6.0.0.md @@ -90,12 +90,12 @@ In rare cases (if you were relying on the previous behavior where `eslint:recomm Previously, ESLint loaded plugins relative to the location of the ESLint package itself. As a result, we suggested that users with global ESLint installations should also install plugins globally, and users with local ESLint installations should install plugins locally. However, due to a design bug, this strategy caused ESLint to randomly fail to load plugins and shareable configs under certain circumstances, particularly when using package management tools like [`lerna`](https://github.com/lerna/lerna) and [Yarn Plug n' Play](https://yarnpkg.com/lang/en/docs/pnp/). -As a rule of thumb: With ESLint v6, plugins should always be installed locally, even if ESLint was installed globally. More precisely, ESLint v6 always resolves plugins relative to the end user's project, and always resolves shareable configs and parsers relative to the location of the config file that imports them. - - +As a rule of thumb: With ESLint v6, plugins should always be installed locally, even if ESLint was installed globally. More precisely, ESLint v6 resolves plugins relative to the end user's project by default, and always resolves shareable configs and parsers relative to the location of the config file that imports them. **To address:** If you use a global installation of ESLint (e.g. installed with `npm install eslint --global`) along with plugins, you should install those plugins locally in the projects where you run ESLint. If your config file extends shareable configs and/or parsers, you should ensure that those packages are installed as dependencies of the project containing the config file. +If you use a config file located outside of a local project (with the `--config` flag), consider installing the plugins as dependencies of that config file, and setting the [`--resolve-plugins-relative-to`](./command-line-interface#--resolve-plugins-relative-to) flag to the location of the config file. + **Related issue(s):** [eslint/eslint#10125](https://github.com/eslint/eslint/issues/10125), [eslint/rfcs#7](https://github.com/eslint/rfcs/pull/7) ## The default parser now validates options more strictly diff --git a/lib/cli-engine.js b/lib/cli-engine.js index b97772133ed..d0d3bd5662f 100644 --- a/lib/cli-engine.js +++ b/lib/cli-engine.js @@ -73,6 +73,7 @@ const validFixTypes = new Set(["problem", "suggestion", "layout"]); * @property {string[]} rulePaths An array of directories to load custom rules from. * @property {boolean} reportUnusedDisableDirectives `true` adds reports for unused eslint-disable directives * @property {boolean} globInputPaths Set to false to skip glob resolution of input file paths to lint (default: true). If false, each input file paths is assumed to be a non-glob path to an existing file. + * @property {string} resolvePluginsRelativeTo The folder where plugins should be resolved from, defaulting to the CWD */ /** @@ -540,6 +541,7 @@ class CLIEngine { baseConfig: options.baseConfig || null, cliConfig: createConfigDataFromOptions(options), cwd: options.cwd, + resolvePluginsRelativeTo: options.resolvePluginsRelativeTo, rulePaths: options.rulePaths, specificConfigPath: options.configFile, useEslintrc: options.useEslintrc diff --git a/lib/cli-engine/cascading-config-array-factory.js b/lib/cli-engine/cascading-config-array-factory.js index 267ea96d486..27b2dd15b0e 100644 --- a/lib/cli-engine/cascading-config-array-factory.js +++ b/lib/cli-engine/cascading-config-array-factory.js @@ -177,13 +177,15 @@ class CascadingConfigArrayFactory { baseConfig: baseConfigData = null, cliConfig: cliConfigData = null, cwd = process.cwd(), + resolvePluginsRelativeTo = cwd, rulePaths = [], specificConfigPath = null, useEslintrc = true } = {}) { const configArrayFactory = new ConfigArrayFactory({ additionalPluginPool, - cwd + cwd, + resolvePluginsRelativeTo }); internalSlotsMap.set(this, { diff --git a/lib/cli-engine/config-array-factory.js b/lib/cli-engine/config-array-factory.js index 1ee5902f7b1..360ca0f4e9b 100644 --- a/lib/cli-engine/config-array-factory.js +++ b/lib/cli-engine/config-array-factory.js @@ -71,12 +71,14 @@ const configFilenames = [ * @typedef {Object} ConfigArrayFactoryOptions * @property {Map} [additionalPluginPool] The map for additional plugins. * @property {string} [cwd] The path to the current working directory. + * @property {string} [resolvePluginsRelativeTo] A path to the directory that plugins should be resolved from. Defaults to `cwd`. */ /** * @typedef {Object} ConfigArrayFactoryInternalSlots * @property {Map} additionalPluginPool The map for additional plugins. * @property {string} cwd The path to the current working directory. + * @property {string} resolvePluginsRelativeTo An absolute path the the directory that plugins should be resolved from. */ /** @type {WeakMap} */ @@ -340,9 +342,10 @@ class ConfigArrayFactory { */ constructor({ additionalPluginPool = new Map(), - cwd = process.cwd() + cwd = process.cwd(), + resolvePluginsRelativeTo = cwd } = {}) { - internalSlotsMap.set(this, { additionalPluginPool, cwd }); + internalSlotsMap.set(this, { additionalPluginPool, cwd, resolvePluginsRelativeTo: path.resolve(cwd, resolvePluginsRelativeTo) }); } /** @@ -791,7 +794,7 @@ class ConfigArrayFactory { _loadPlugin(name, importerPath, importerName) { debug("Loading plugin %j from %s", name, importerPath); - const { additionalPluginPool, cwd } = internalSlotsMap.get(this); + const { additionalPluginPool, resolvePluginsRelativeTo } = internalSlotsMap.get(this); const request = naming.normalizePackageName(name, "eslint-plugin"); const id = naming.getShorthandName(request, "eslint-plugin"); @@ -829,8 +832,8 @@ class ConfigArrayFactory { try { - // Resolve the plugin file relative to the project root. - const relativeTo = path.join(cwd, "__placeholder__.js"); + // Resolve the plugin file + const relativeTo = path.join(resolvePluginsRelativeTo, "__placeholder__.js"); const filePath = ModuleResolver.resolve(request, relativeTo); writeDebugLogForLoading(request, relativeTo, filePath); @@ -849,7 +852,7 @@ class ConfigArrayFactory { error.messageTemplate = "plugin-missing"; error.messageData = { pluginName: request, - pluginRootPath: cwd, + resolvePluginsRelativeTo, importerName }; } diff --git a/lib/cli.js b/lib/cli.js index 2b050e1c610..1bec4b3d941 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -66,7 +66,8 @@ function translateOptions(cliOptions) { fix: (cliOptions.fix || cliOptions.fixDryRun) && (cliOptions.quiet ? quietFixPredicate : true), fixTypes: cliOptions.fixType, allowInlineConfig: cliOptions.inlineConfig, - reportUnusedDisableDirectives: cliOptions.reportUnusedDisableDirectives + reportUnusedDisableDirectives: cliOptions.reportUnusedDisableDirectives, + resolvePluginsRelativeTo: cliOptions.resolvePluginsRelativeTo }; } diff --git a/lib/options.js b/lib/options.js index ee7357a296a..be4c09b8eab 100644 --- a/lib/options.js +++ b/lib/options.js @@ -64,6 +64,11 @@ module.exports = optionator({ type: "Object", description: "Specify parser options" }, + { + option: "resolve-plugins-relative-to", + type: "path::String", + description: "A folder where plugins should be resolved from, CWD by default" + }, { heading: "Specifying rules and plugins" }, diff --git a/messages/plugin-missing.txt b/messages/plugin-missing.txt index 92507d0dd0b..32e9f0ae5de 100644 --- a/messages/plugin-missing.txt +++ b/messages/plugin-missing.txt @@ -1,6 +1,6 @@ ESLint couldn't find the plugin "<%- pluginName %>". -(The package "<%- pluginName %>" was not found when loaded as a Node module from the directory "<%- pluginRootPath %>".) +(The package "<%- pluginName %>" was not found when loaded as a Node module from the directory "<%- resolvePluginsRelativeTo %>".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: diff --git a/tests/fixtures/plugins/node_modules/eslint-plugin-with-rules.js b/tests/fixtures/plugins/node_modules/eslint-plugin-with-rules.js new file mode 100644 index 00000000000..2c629b2b3e6 --- /dev/null +++ b/tests/fixtures/plugins/node_modules/eslint-plugin-with-rules.js @@ -0,0 +1,14 @@ +module.exports = { + rules: { + rule1: { + create(context) { + context.report({ + node: context.getSourceCode().ast, + message: "Rule report from plugin" + }); + + return {}; + } + } + } +} diff --git a/tests/lib/cli-engine.js b/tests/lib/cli-engine.js index ba2d85727a0..97d0d9e414a 100644 --- a/tests/lib/cli-engine.js +++ b/tests/lib/cli-engine.js @@ -2079,6 +2079,24 @@ describe("CLIEngine", () => { assert.strictEqual(report.results[0].messages.length, 2); assert.strictEqual(report.results[0].messages[0].ruleId, "test/example-rule"); }); + + it("should load plugins from the `loadPluginsRelativeTo` directory, if specified", () => { + engine = new CLIEngine({ + resolvePluginsRelativeTo: getFixturePath("plugins"), + baseConfig: { + plugins: ["with-rules"], + rules: { "with-rules/rule1": "error" } + }, + useEslintrc: false + }); + + const report = engine.executeOnText("foo"); + + assert.strictEqual(report.results.length, 1); + assert.strictEqual(report.results[0].messages.length, 1); + assert.strictEqual(report.results[0].messages[0].ruleId, "with-rules/rule1"); + assert.strictEqual(report.results[0].messages[0].message, "Rule report from plugin"); + }); }); describe("cache", () => { diff --git a/tests/lib/cli-engine/config-array-factory.js b/tests/lib/cli-engine/config-array-factory.js index 2d0b210967e..b54f2ffbcc2 100644 --- a/tests/lib/cli-engine/config-array-factory.js +++ b/tests/lib/cli-engine/config-array-factory.js @@ -1369,7 +1369,7 @@ describe("ConfigArrayFactory", () => { assert.strictEqual(err.messageTemplate, "plugin-missing"); assert.deepStrictEqual(err.messageData, { pluginName: "eslint-plugin-nonexistent-plugin", - pluginRootPath: process.cwd(), + resolvePluginsRelativeTo: process.cwd(), importerName: "whatever" }); return; @@ -1386,7 +1386,7 @@ describe("ConfigArrayFactory", () => { assert.strictEqual(err.messageTemplate, "plugin-missing"); assert.deepStrictEqual(err.messageData, { pluginName: "eslint-plugin-nonexistent-plugin", - pluginRootPath: process.cwd(), + resolvePluginsRelativeTo: process.cwd(), importerName: "whatever" }); return; @@ -2096,10 +2096,11 @@ describe("ConfigArrayFactory", () => { /** * Load a plugin. * @param {string} request A request to load a plugin. + * @param {ConfigArrayFactory} [configArrayFactory] The factory to use * @returns {Map} The loaded plugins. */ - function load(request) { - const config = factory.create({ plugins: [request] }); + function load(request, configArrayFactory = factory) { + const config = configArrayFactory.create({ plugins: [request] }); return new Map( Object @@ -2122,6 +2123,24 @@ describe("ConfigArrayFactory", () => { ); }); + it("should load a plugin when referenced by short name, even when using a custom loadPluginsRelativeTo value", () => { + const { ConfigArrayFactory: FactoryWithPluginsInSubdir } = defineConfigArrayFactoryWithInMemoryFileSystem({ + cwd: () => tempDir, + files: { + "subdir/node_modules/eslint-plugin-example/index.js": "exports.configs = { name: 'eslint-plugin-example' };" + } + }); + + const factoryWithCustomPluginPath = new FactoryWithPluginsInSubdir({ resolvePluginsRelativeTo: "subdir" }); + + const loadedPlugins = load("example", factoryWithCustomPluginPath); + + assertPluginDefinition( + loadedPlugins.get("example"), + { configs: { name: "eslint-plugin-example" } } + ); + }); + it("should load a plugin when referenced by long name", () => { const loadedPlugins = load("eslint-plugin-example"); diff --git a/tests/lib/cli.js b/tests/lib/cli.js index 1486d0ae621..54bf3867cdf 100644 --- a/tests/lib/cli.js +++ b/tests/lib/cli.js @@ -588,6 +588,16 @@ describe("cli", () => { }); + describe("when supplied with a plugin-loading path", () => { + it("should pass the option to CLIEngine", () => { + const examplePluginDirPath = "foo/bar"; + + verifyCLIEngineOpts(`--resolve-plugins-relative-to ${examplePluginDirPath} foo.js`, { + resolvePluginsRelativeTo: examplePluginDirPath + }); + }); + }); + describe("when given an parser name", () => { it("should exit with a fatal error if parser is invalid", () => { const filePath = getFixturePath("passing.js");