Breaking: improve plugin resolving (refs eslint/rfcs#47) (#12922)

* Breaking: change relative paths with --config (refs eslint/rfcs#37) * update docs * Breaking: improve plugin resolving (refs eslint/rfcs#47) * replace getRules by getRulesForFile (refs eslint/rfcs#47) * replace rulesMeta by getRuleMeta (refs eslint/rfcs#47) * replace report.usedDeprecatedRules by report.results[].usedDeprecatedRules * Revert "replace report.usedDeprecatedRules by report.results[].usedDeprecatedRules" This reverts commit f3cc32f. * Revert "replace rulesMeta by getRuleMeta (refs eslint/rfcs#47)" This reverts commit 0d6afaf. * Revert "replace getRules by getRulesForFile (refs eslint/rfcs#47)" This reverts commit d29f613. * update docs * Update docs/user-guide/configuring.md Co-Authored-By: Kai Cataldo <kai@kaicataldo.com> * fix markdownlint error Co-authored-by: Kai Cataldo <kai@kaicataldo.com>
eslint · Mar 27, 2020 · 185982d · 185982d
1 parent 0c20bc0
commit 185982d
Show file tree

Hide file tree

Showing 8 changed files with 448 additions and 40 deletions.
diff --git a/docs/user-guide/configuring.md b/docs/user-guide/configuring.md
@@ -321,7 +321,10 @@ And in YAML:
     - eslint-plugin-plugin2
 ```
 
-**Note:** Plugins are resolved relative to the current working directory of the ESLint process. In other words, ESLint will load the same plugin as a user would obtain by running `require('eslint-plugin-pluginname')` in a Node REPL from their project root.
+**Notes:**
+
+1. Plugins are resolved relative to the config file. In other words, ESLint will load the plugin as a user would obtain by running `require('eslint-plugin-pluginname')` in the config file.
+2. Plugins in the base configuration (loaded by `extends` setting) are relative to the derived config file. For example, if `./.eslintrc` has `extends: ["foo"]` and the `eslint-config-foo` has `plugins: ["bar"]`, ESLint finds the `eslint-plugin-bar` from `./node_modules/` (rather than `./node_modules/eslint-config-foo/node_modules/`) or ancestor directories. Thus every plugin in the config file and base configurations is resolved uniquely.
 
 ### Naming Convention
 

diff --git a/lib/cli-engine/cascading-config-array-factory.js b/lib/cli-engine/cascading-config-array-factory.js
@@ -199,7 +199,7 @@ class CascadingConfigArrayFactory {
         cliConfig: cliConfigData = null,
         cwd = process.cwd(),
         ignorePath,
-        resolvePluginsRelativeTo = cwd,
+        resolvePluginsRelativeTo,
         rulePaths = [],
         specificConfigPath = null,
         useEslintrc = true

diff --git a/lib/cli-engine/config-array-factory.js b/lib/cli-engine/config-array-factory.js
@@ -90,7 +90,16 @@ const configFilenames = [
  * @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.
+ * @property {string | undefined} resolvePluginsRelativeTo An absolute path the the directory that plugins should be resolved from.
+ */
+
+/**
+ * @typedef {Object} ConfigArrayFactoryLoadingContext
+ * @property {string} filePath The path to the current configuration.
+ * @property {string} matchBasePath The base path to resolve relative paths in `overrides[].files`, `overrides[].excludedFiles`, and `ignorePatterns`.
+ * @property {string} name The name of the current configuration.
+ * @property {string} pluginBasePath The base path to resolve plugins.
+ * @property {"config" | "ignore" | "implicit-processor"} type The type of the current configuration. This is `"config"` in normal. This is `"ignore"` if it came from `.eslintignore`. This is `"implicit-processor"` if it came from legacy file-extension processors.
  */
 
 /**
@@ -337,15 +346,15 @@ function writeDebugLogForLoading(request, relativeTo, filePath) {
 
 /**
  * Create a new context with default values.
- * @param {string | undefined} cwd The current working directory.
+ * @param {ConfigArrayFactoryInternalSlots} slots The internal slots.
  * @param {"config" | "ignore" | "implicit-processor" | undefined} providedType The type of the current configuration. Default is `"config"`.
  * @param {string | undefined} providedName The name of the current configuration. Default is the relative path from `cwd` to `filePath`.
  * @param {string | undefined} providedFilePath The path to the current configuration. Default is empty string.
  * @param {string | undefined} providedMatchBasePath The type of the current configuration. Default is the directory of `filePath` or `cwd`.
  * @returns {ConfigArrayFactoryLoadingContext} The created context.
  */
 function createContext(
-    cwd,
+    { cwd, resolvePluginsRelativeTo },
     providedType,
     providedName,
     providedFilePath,
@@ -355,16 +364,20 @@ function createContext(
         ? path.resolve(cwd, providedFilePath)
         : "";
     const matchBasePath =
-        providedMatchBasePath ||
+        (providedMatchBasePath && path.resolve(cwd, providedMatchBasePath)) ||
         (filePath && path.dirname(filePath)) ||
         cwd;
     const name =
         providedName ||
         (filePath && path.relative(cwd, filePath)) ||
         "";
+    const pluginBasePath =
+        resolvePluginsRelativeTo ||
+        (filePath && path.dirname(filePath)) ||
+        cwd;
     const type = providedType || "config";
 
-    return { filePath, matchBasePath, name, type };
+    return { filePath, matchBasePath, name, pluginBasePath, type };
 }
 
 /**
@@ -399,12 +412,14 @@ class ConfigArrayFactory {
     constructor({
         additionalPluginPool = new Map(),
         cwd = process.cwd(),
-        resolvePluginsRelativeTo = cwd
+        resolvePluginsRelativeTo
     } = {}) {
         internalSlotsMap.set(this, {
             additionalPluginPool,
             cwd,
-            resolvePluginsRelativeTo: path.resolve(cwd, resolvePluginsRelativeTo)
+            resolvePluginsRelativeTo:
+                resolvePluginsRelativeTo &&
+                path.resolve(cwd, resolvePluginsRelativeTo)
         });
     }
 
@@ -422,8 +437,8 @@ class ConfigArrayFactory {
             return new ConfigArray();
         }
 
-        const { cwd } = internalSlotsMap.get(this);
-        const ctx = createContext(cwd, "config", name, filePath, basePath);
+        const slots = internalSlotsMap.get(this);
+        const ctx = createContext(slots, "config", name, filePath, basePath);
         const elements = this._normalizeConfigData(configData, ctx);
 
         return new ConfigArray(...elements);
@@ -438,8 +453,8 @@ class ConfigArrayFactory {
      * @returns {ConfigArray} Loaded config.
      */
     loadFile(filePath, { basePath, name } = {}) {
-        const { cwd } = internalSlotsMap.get(this);
-        const ctx = createContext(cwd, "config", name, filePath, basePath);
+        const slots = internalSlotsMap.get(this);
+        const ctx = createContext(slots, "config", name, filePath, basePath);
 
         return new ConfigArray(...this._loadConfigData(ctx));
     }
@@ -453,11 +468,11 @@ class ConfigArrayFactory {
      * @returns {ConfigArray} Loaded config. An empty `ConfigArray` if any config doesn't exist.
      */
     loadInDirectory(directoryPath, { basePath, name } = {}) {
-        const { cwd } = internalSlotsMap.get(this);
+        const slots = internalSlotsMap.get(this);
 
         for (const filename of configFilenames) {
             const ctx = createContext(
-                cwd,
+                slots,
                 "config",
                 name,
                 path.join(directoryPath, filename),
@@ -517,16 +532,15 @@ class ConfigArrayFactory {
      * @returns {ConfigArray} Loaded config. An empty `ConfigArray` if any config doesn't exist.
      */
     loadESLintIgnore(filePath) {
-        const { cwd } = internalSlotsMap.get(this);
-        const absolutePath = path.resolve(cwd, filePath);
-        const ignorePatterns = loadESLintIgnoreFile(absolutePath);
+        const slots = internalSlotsMap.get(this);
         const ctx = createContext(
-            cwd,
+            slots,
             "ignore",
             void 0,
-            absolutePath,
-            cwd
+            filePath,
+            slots.cwd
         );
+        const ignorePatterns = loadESLintIgnoreFile(ctx.filePath);
 
         return new ConfigArray(
             ...this._normalizeESLintIgnoreData(ignorePatterns, ctx)
@@ -538,9 +552,9 @@ class ConfigArrayFactory {
      * @returns {ConfigArray} Loaded config. An empty `ConfigArray` if any config doesn't exist.
      */
     loadDefaultESLintIgnore() {
-        const { cwd } = internalSlotsMap.get(this);
-        const eslintIgnorePath = path.resolve(cwd, ".eslintignore");
-        const packageJsonPath = path.resolve(cwd, "package.json");
+        const slots = internalSlotsMap.get(this);
+        const eslintIgnorePath = path.resolve(slots.cwd, ".eslintignore");
+        const packageJsonPath = path.resolve(slots.cwd, "package.json");
 
         if (fs.existsSync(eslintIgnorePath)) {
             return this.loadESLintIgnore(eslintIgnorePath);
@@ -553,11 +567,11 @@ class ConfigArrayFactory {
                     throw new Error("Package.json eslintIgnore property requires an array of paths");
                 }
                 const ctx = createContext(
-                    cwd,
+                    slots,
                     "ignore",
                     "eslintIgnore in package.json",
                     packageJsonPath,
-                    cwd
+                    slots.cwd
                 );
 
                 return new ConfigArray(
@@ -934,10 +948,10 @@ class ConfigArrayFactory {
     _loadPlugin(name, ctx) {
         debug("Loading plugin %j from %s", name, ctx.filePath);
 
-        const { additionalPluginPool, resolvePluginsRelativeTo } = internalSlotsMap.get(this);
+        const { additionalPluginPool } = internalSlotsMap.get(this);
         const request = naming.normalizePackageName(name, "eslint-plugin");
         const id = naming.getShorthandName(request, "eslint-plugin");
-        const relativeTo = path.join(resolvePluginsRelativeTo, "__placeholder__.js");
+        const relativeTo = path.join(ctx.pluginBasePath, "__placeholder__.js");
 
         if (name.match(/\s+/u)) {
             const error = Object.assign(
@@ -983,7 +997,7 @@ class ConfigArrayFactory {
                 error.messageTemplate = "plugin-missing";
                 error.messageData = {
                     pluginName: request,
-                    resolvePluginsRelativeTo,
+                    resolvePluginsRelativeTo: ctx.pluginBasePath,
                     importerName: ctx.name
                 };
             }

diff --git a/lib/cli-engine/config-array/config-array.js b/lib/cli-engine/config-array/config-array.js
@@ -156,6 +156,23 @@ function mergeWithoutOverwrite(target, source) {
     }
 }
 
+/**
+ * The error for plugin conflicts.
+ */
+class PluginConflictError extends Error {
+
+    /**
+     * Initialize this error object.
+     * @param {string} pluginId The plugin ID.
+     * @param {{filePath:string, importerName:string}[]} plugins The resolved plugins.
+     */
+    constructor(pluginId, plugins) {
+        super(`Plugin "${pluginId}" was conflicted between ${plugins.map(p => `"${p.importerName}"`).join(" and ")}.`);
+        this.messageTemplate = "plugin-conflict";
+        this.messageData = { pluginId, plugins };
+    }
+}
+
 /**
  * Merge plugins.
  * `target`'s definition is prior to `source`'s.
@@ -181,6 +198,17 @@ function mergePlugins(target, source) {
                 throw sourceValue.error;
             }
             target[key] = sourceValue;
+        } else if (sourceValue.filePath !== targetValue.filePath) {
+            throw new PluginConflictError(key, [
+                {
+                    filePath: targetValue.filePath,
+                    importerName: targetValue.importerName
+                },
+                {
+                    filePath: sourceValue.filePath,
+                    importerName: sourceValue.importerName
+                }
+            ]);
         }
     }
 }

diff --git a/messages/plugin-conflict.txt b/messages/plugin-conflict.txt
@@ -0,0 +1,7 @@
+ESLint couldn't determine the plugin "<%- pluginId %>" uniquely.
+<% for (const { filePath, importerName } of plugins) { %>
+- <%= filePath %> (loaded in "<%= importerName %>")<% } %>
+
+Please remove the "plugins" setting from either config or remove either plugin installation.
+
+If you still can't figure out the problem, please stop by https://gitter.im/eslint/eslint to chat with the team.
diff --git a/tests/lib/cli-engine/cascading-config-array-factory.js b/tests/lib/cli-engine/cascading-config-array-factory.js
@@ -938,7 +938,8 @@ describe("CascadingConfigArrayFactory", () => {
                     cliConfig: {
                         plugins: ["another-plugin"]
                     },
-                    cwd: getFixturePath("plugins")
+                    cwd: getFixturePath("plugins"),
+                    resolvePluginsRelativeTo: getFixturePath("plugins")
                 });
                 const file = getFixturePath("broken", "plugins", "console-wrong-quotes.js");
                 const expected = {