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 2 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 @@ -274,14 +274,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 tempalte to source error strings from.
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
* @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 @@ -790,7 +791,7 @@ class ConfigArrayFactory {
});
}

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

/**
Expand All @@ -802,6 +803,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 @@ -822,7 +828,7 @@ class ConfigArrayFactory {
});
}

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

/**
Expand Down Expand Up @@ -855,7 +861,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
2 changes: 1 addition & 1 deletion lib/shared/naming.js
Expand Up @@ -43,7 +43,7 @@ function normalizePackageName(name, prefix) {
*/
normalizedName = normalizedName.replace(/^@([^/]+)\/(.*)$/u, `@$1/${prefix}-$2`);
}
} else if (!normalizedName.startsWith(`${prefix}-`)) {
} else if (!normalizedName.startsWith(`${prefix}-`) && normalizedName.indexOf(":") === -1) {
normalizedName = `${prefix}-${normalizedName}`;
}

Expand Down
9 changes: 9 additions & 0 deletions messages/plugin-invalid.txt
@@ -0,0 +1,9 @@
ESLint couldn't find the plugin "<%- pluginName %>".

(The package "<%- pluginName %>" was not found when loaded as a Node module from the directory "<%- resolvePluginsRelativeTo %>".)

It looks like this might not be a valid plugin name. Did you use the name of a shareable config instead of a plugin?
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved

The plugin "<%- pluginName %>" 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 @@ -1503,6 +1503,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 referenced for a plugin config is invalid", () => {
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
try {
applyExtends({
extends: "plugin:nonexistent-plugin",
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
rules: { eqeqeq: 2 }
});
} catch (err) {
assert.strictEqual(err.messageTemplate, "plugin-invalid");
assert.deepStrictEqual(err.messageData, {
configName: "plugin:nonexistent-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
3 changes: 2 additions & 1 deletion tests/lib/shared/naming.js
Expand Up @@ -25,7 +25,8 @@ describe("naming", () => {
["@z\\foo", "@z/eslint-config-foo"],
["@z\\foo\\bar.js", "@z/eslint-config-foo/bar.js"],
["@z/eslint-config", "@z/eslint-config"],
["@z/eslint-config-foo", "@z/eslint-config-foo"]
["@z/eslint-config-foo", "@z/eslint-config-foo"],
["plugin:invalid", "plugin:invalid"]
], (input, expected) => {
it(`should return ${expected} when passed ${input}`, () => {
const result = naming.normalizePackageName(input, "eslint-config");
Expand Down