Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Corrected notice for invalid (:) plugin names #13473

Merged
merged 7 commits into from Sep 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 11 additions & 5 deletions lib/cli-engine/config-array-factory.js
Expand Up @@ -286,14 +286,15 @@ function loadESLintIgnoreFile(filePath) {
* Creates an error to notify about a missing config to extend from.
* @param {string} configName The name of the missing config.
* @param {string} importerName The name of the config that imported the missing config
* @param {string} messageTemplate The text template to source error strings from.
* @returns {Error} The error object to throw
* @private
*/
function configMissingError(configName, importerName) {
function configInvalidError(configName, importerName, messageTemplate) {
return Object.assign(
new Error(`Failed to load config "${configName}" to extend from.`),
{
messageTemplate: "extend-config-missing",
messageTemplate,
messageData: { configName, importerName }
}
);
Expand Down Expand Up @@ -802,7 +803,7 @@ class ConfigArrayFactory {
});
}

throw configMissingError(extendName, ctx.name);
throw configInvalidError(extendName, ctx.name, "extend-config-missing");
}

/**
Expand All @@ -814,6 +815,11 @@ class ConfigArrayFactory {
*/
_loadExtendedPluginConfig(extendName, ctx) {
const slashIndex = extendName.lastIndexOf("/");

if (slashIndex === -1) {
throw configInvalidError(extendName, ctx.filePath, "plugin-invalid");
}

const pluginName = extendName.slice("plugin:".length, slashIndex);
const configName = extendName.slice(slashIndex + 1);

Expand All @@ -834,7 +840,7 @@ class ConfigArrayFactory {
});
}

throw plugin.error || configMissingError(extendName, ctx.filePath);
throw plugin.error || configInvalidError(extendName, ctx.filePath, "extend-config-missing");
}

/**
Expand Down Expand Up @@ -867,7 +873,7 @@ class ConfigArrayFactory {
} catch (error) {
/* istanbul ignore else */
if (error && error.code === "MODULE_NOT_FOUND") {
throw configMissingError(extendName, ctx.filePath);
throw configInvalidError(extendName, ctx.filePath, "extend-config-missing");
}
throw error;
}
Expand Down
8 changes: 8 additions & 0 deletions messages/plugin-invalid.txt
@@ -0,0 +1,8 @@
"<%- configName %>" is invalid syntax for a config specifier.

* If your intention is to extend from a configuration exported from the plugin, add the configuration name after a slash: e.g. "<%- configName %>/myConfig".
* If this is the name of a shareable config instead of a plugin, remove the "plugin:" prefix: i.e. "<%- configName.slice("plugin:".length) %>".

"<%- configName %>" was referenced from the config file in "<%- importerName %>".

If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
17 changes: 17 additions & 0 deletions tests/lib/cli-engine/config-array-factory.js
Expand Up @@ -1515,6 +1515,23 @@ describe("ConfigArrayFactory", () => {
}, /Failed to load config "plugin:invalid-config\/bar" to extend from./u);
});

it("should throw an error with a message template when a plugin config specifier is missing config name", () => {
try {
applyExtends({
extends: "plugin:some-plugin",
rules: { eqeqeq: 2 }
});
} catch (err) {
assert.strictEqual(err.messageTemplate, "plugin-invalid");
assert.deepStrictEqual(err.messageData, {
configName: "plugin:some-plugin",
importerName: path.join(process.cwd(), "whatever")
});
return;
}
assert.fail("Expected to throw an error");
});

it("should throw an error with a message template when a plugin referenced for a plugin config is not found", () => {
try {
applyExtends({
Expand Down