Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
mysticatea and kaicataldo committed Mar 27, 2020
1 parent 0c20bc0 commit 185982d
Show file tree
Hide file tree
Showing 8 changed files with 448 additions and 40 deletions.
5 changes: 4 additions & 1 deletion docs/user-guide/configuring.md
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion lib/cli-engine/cascading-config-array-factory.js
Expand Up @@ -199,7 +199,7 @@ class CascadingConfigArrayFactory {
cliConfig: cliConfigData = null,
cwd = process.cwd(),
ignorePath,
resolvePluginsRelativeTo = cwd,
resolvePluginsRelativeTo,
rulePaths = [],
specificConfigPath = null,
useEslintrc = true
Expand Down
68 changes: 41 additions & 27 deletions lib/cli-engine/config-array-factory.js
Expand Up @@ -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.
*/

/**
Expand Down Expand Up @@ -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,
Expand All @@ -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 };
}

/**
Expand Down Expand Up @@ -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)
});
}

Expand All @@ -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);
Expand All @@ -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));
}
Expand All @@ -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),
Expand Down Expand Up @@ -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)
Expand All @@ -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);
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -983,7 +997,7 @@ class ConfigArrayFactory {
error.messageTemplate = "plugin-missing";
error.messageData = {
pluginName: request,
resolvePluginsRelativeTo,
resolvePluginsRelativeTo: ctx.pluginBasePath,
importerName: ctx.name
};
}
Expand Down
28 changes: 28 additions & 0 deletions lib/cli-engine/config-array/config-array.js
Expand Up @@ -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.
Expand All @@ -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
}
]);
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions 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.
3 changes: 2 additions & 1 deletion tests/lib/cli-engine/cascading-config-array-factory.js
Expand Up @@ -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 = {
Expand Down

0 comments on commit 185982d

Please sign in to comment.