Skip to content

Commit

Permalink
Fix: wrong 'plugin-missing' error on Node.js 12 (fixes #11720) (#11722)
Browse files Browse the repository at this point in the history
  • Loading branch information
mysticatea authored and not-an-aardvark committed May 16, 2019
1 parent 67c671f commit 7c8e86b
Show file tree
Hide file tree
Showing 13 changed files with 169 additions and 55 deletions.
107 changes: 54 additions & 53 deletions lib/cli-engine/config-array-factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -427,28 +427,22 @@ class ConfigArrayFactory {
_loadConfigDataInDirectory(directoryPath, name) {
for (const filename of configFilenames) {
const filePath = path.join(directoryPath, filename);
const originalDebugEnabled = debug.enabled;
let configData;

// Make silent temporary because of too verbose.
debug.enabled = false;
try {
configData = loadConfigFile(filePath);
} catch (error) {
if (
error.code !== "ENOENT" &&
error.code !== "MODULE_NOT_FOUND" &&
error.code !== "ESLINT_CONFIG_FIELD_NOT_FOUND"
) {
throw error;
if (fs.existsSync(filePath)) {
let configData;

try {
configData = loadConfigFile(filePath);
} catch (error) {
if (!error || error.code !== "ESLINT_CONFIG_FIELD_NOT_FOUND") {
throw error;
}
}
} finally {
debug.enabled = originalDebugEnabled;
}

if (configData) {
debug(`Config file found: ${filePath}`);
return this._normalizeConfigData(configData, filePath, name);
if (configData) {
debug(`Config file found: ${filePath}`);
return this._normalizeConfigData(configData, filePath, name);
}
}
}

Expand Down Expand Up @@ -695,20 +689,20 @@ class ConfigArrayFactory {
);
}

try {
const filePath = ModuleResolver.resolve(request, relativeTo);
let filePath;

writeDebugLogForLoading(request, relativeTo, filePath);

return this._loadConfigData(filePath, `${importerName} » ${request}`);
try {
filePath = ModuleResolver.resolve(request, relativeTo);
} catch (error) {
/* istanbul ignore next */
if (!error || error.code !== "MODULE_NOT_FOUND") {
throw error;
/* istanbul ignore else */
if (error && error.code === "MODULE_NOT_FOUND") {
throw configMissingError(extendName);
}
throw error;
}

throw configMissingError(extendName);
writeDebugLogForLoading(request, relativeTo, filePath);
return this._loadConfigData(filePath, `${importerName} » ${request}`);
}

/**
Expand Down Expand Up @@ -797,6 +791,7 @@ class ConfigArrayFactory {
const { additionalPluginPool, resolvePluginsRelativeTo } = internalSlotsMap.get(this);
const request = naming.normalizePackageName(name, "eslint-plugin");
const id = naming.getShorthandName(request, "eslint-plugin");
const relativeTo = path.join(resolvePluginsRelativeTo, "__placeholder__.js");

if (name.match(/\s+/u)) {
const error = Object.assign(
Expand Down Expand Up @@ -830,41 +825,47 @@ class ConfigArrayFactory {
});
}

try {

// Resolve the plugin file
const relativeTo = path.join(resolvePluginsRelativeTo, "__placeholder__.js");
const filePath = ModuleResolver.resolve(request, relativeTo);

writeDebugLogForLoading(request, relativeTo, filePath);
let filePath;
let error;

return new ConfigDependency({
definition: normalizePlugin(require(filePath)),
filePath,
id,
importerName,
importerPath
});
} catch (error) {
debug("Failed to load plugin '%s' declared in '%s'.", name, importerName);

if (error && error.code === "MODULE_NOT_FOUND" && error.message.includes(request)) {
try {
filePath = ModuleResolver.resolve(request, relativeTo);
} catch (resolveError) {
error = resolveError;
/* istanbul ignore else */
if (error && error.code === "MODULE_NOT_FOUND") {
error.messageTemplate = "plugin-missing";
error.messageData = {
pluginName: request,
resolvePluginsRelativeTo,
importerName
};
}
error.message = `Failed to load plugin '${name}' declared in '${importerName}': ${error.message}`;
}

return new ConfigDependency({
error,
id,
importerName,
importerPath
});
if (filePath) {
try {
writeDebugLogForLoading(request, relativeTo, filePath);
return new ConfigDependency({
definition: normalizePlugin(require(filePath)),
filePath,
id,
importerName,
importerPath
});
} catch (loadError) {
error = loadError;
}
}

debug("Failed to load plugin '%s' declared in '%s'.", name, importerName);
error.message = `Failed to load plugin '${name}' declared in '${importerName}': ${error.message}`;
return new ConfigDependency({
error,
id,
importerName,
importerPath
});
}

/**
Expand Down
10 changes: 8 additions & 2 deletions lib/util/relative-module-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,14 @@ module.exports = {
try {
return createRequire(relativeToPath).resolve(moduleName);
} catch (error) {
if (error && error.code === "MODULE_NOT_FOUND" && error.message.includes(moduleName)) {
error.message += ` relative to '${relativeToPath}'`;
if (
typeof error === "object" &&
error !== null &&
error.code === "MODULE_NOT_FOUND" &&
!error.requireStack &&
error.message.includes(moduleName)
) {
error.message += `\nRequire stack:\n- ${relativeToPath}`;
}
throw error;
}
Expand Down
1 change: 1 addition & 0 deletions tests/fixtures/module-not-found/.eslintrc.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
root: true
1 change: 1 addition & 0 deletions tests/fixtures/module-not-found/extends-js/.eslintrc.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
extends: nonexistent-config
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
extends: plugin:nonexistent-plugin/foo

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions tests/fixtures/module-not-found/plugins/.eslintrc.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
plugins: [nonexistent-plugin]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
require("eslint/lib/util/glob-utils")
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
extends: throw
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
extends: plugin:throw/foo
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
plugins: [throw]
97 changes: 97 additions & 0 deletions tests/lib/cli-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -3031,6 +3031,103 @@ describe("CLIEngine", () => {
assert.deepStrictEqual(results[0].output, "fixed;");
});
});

describe("MODULE_NOT_FOUND error handling", () => {
const cwd = getFixturePath("module-not-found");

beforeEach(() => {
engine = new CLIEngine({ cwd });
});

it("should throw an error with a message template when 'extends' property has a non-existence JavaScript config.", () => {
try {
engine.executeOnText("test", "extends-js/test.js");
} catch (err) {
assert.strictEqual(err.messageTemplate, "extend-config-missing");
assert.deepStrictEqual(err.messageData, {
configName: "nonexistent-config"
});
return;
}
assert.fail("Expected to throw an error");
});

it("should throw an error with a message template when 'extends' property has a non-existence plugin config.", () => {
try {
engine.executeOnText("test", "extends-plugin/test.js");
} catch (err) {
assert.strictEqual(err.code, "MODULE_NOT_FOUND");
assert.strictEqual(err.messageTemplate, "plugin-missing");
assert.deepStrictEqual(err.messageData, {
importerName: `extends-plugin${path.sep}.eslintrc.yml`,
pluginName: "eslint-plugin-nonexistent-plugin",
resolvePluginsRelativeTo: cwd
});
return;
}
assert.fail("Expected to throw an error");
});

it("should throw an error with a message template when 'plugins' property has a non-existence plugin.", () => {
try {
engine.executeOnText("test", "plugins/test.js");
} catch (err) {
assert.strictEqual(err.code, "MODULE_NOT_FOUND");
assert.strictEqual(err.messageTemplate, "plugin-missing");
assert.deepStrictEqual(err.messageData, {
importerName: `plugins${path.sep}.eslintrc.yml`,
pluginName: "eslint-plugin-nonexistent-plugin",
resolvePluginsRelativeTo: cwd
});
return;
}
assert.fail("Expected to throw an error");
});

it("should throw an error with no message template when a JavaScript config threw a 'MODULE_NOT_FOUND' error.", () => {
try {
engine.executeOnText("test", "throw-in-config-itself/test.js");
} catch (err) {
assert.strictEqual(err.code, "MODULE_NOT_FOUND");
assert.strictEqual(err.messageTemplate, void 0);
return;
}
assert.fail("Expected to throw an error");
});

it("should throw an error with no message template when 'extends' property has a JavaScript config that throws a 'MODULE_NOT_FOUND' error.", () => {
try {
engine.executeOnText("test", "throw-in-extends-js/test.js");
} catch (err) {
assert.strictEqual(err.code, "MODULE_NOT_FOUND");
assert.strictEqual(err.messageTemplate, void 0);
return;
}
assert.fail("Expected to throw an error");
});

it("should throw an error with no message template when 'extends' property has a plugin config that throws a 'MODULE_NOT_FOUND' error.", () => {
try {
engine.executeOnText("test", "throw-in-extends-plugin/test.js");
} catch (err) {
assert.strictEqual(err.code, "MODULE_NOT_FOUND");
assert.strictEqual(err.messageTemplate, void 0);
return;
}
assert.fail("Expected to throw an error");
});

it("should throw an error with no message template when 'plugins' property has a plugin config that throws a 'MODULE_NOT_FOUND' error.", () => {
try {
engine.executeOnText("test", "throw-in-plugins/test.js");
} catch (err) {
assert.strictEqual(err.code, "MODULE_NOT_FOUND");
assert.strictEqual(err.messageTemplate, void 0);
return;
}
assert.fail("Expected to throw an error");
});
});
});

describe("getConfigForFile", () => {
Expand Down

0 comments on commit 7c8e86b

Please sign in to comment.