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 eslint/rfcs#18)
eslint · May 11, 2019 · 21dd211 · 21dd211
1 parent 1a3a88d
commit 21dd211
Show file tree

Hide file tree

Showing 13 changed files with 99 additions and 16 deletions.
diff --git 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
@@ -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
@@ -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.
-
-<!-- FIXME: Update this section with workarounds for global eslint/global config file setups if https://github.com/eslint/rfcs/pull/18 is accepted -->
+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)
 
 ## <a name="espree-validation"></a> The default parser now validates options more strictly

diff --git 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
@@ -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
@@ -71,12 +71,14 @@ const configFilenames = [
  * @typedef {Object} ConfigArrayFactoryOptions
  * @property {Map<string,Plugin>} [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<string,Plugin>} 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<ConfigArrayFactory, ConfigArrayFactoryInternalSlots>} */
@@ -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
@@ -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
@@ -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
@@ -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
diff --git 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
@@ -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<string,Object>} 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
@@ -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");