From 4d7d7ab79551fa1bb2ff3d355d1bfff69d870faf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=20Erik=20St=C3=B8wer?= Date: Wed, 1 Nov 2017 22:43:46 +0100 Subject: [PATCH] Update: Resolve npm installed formatters (#5900) (#9464) --- docs/user-guide/command-line-interface.md | 11 ++ lib/cli-engine.js | 19 ++- lib/config/config-file.js | 51 +------- lib/config/plugins.js | 43 ++----- lib/util/naming.js | 112 ++++++++++++++++++ .../eslint-formatter-foo/index.js | 1 + .../eslint-formatter-bar/index.js | 1 + tests/lib/cli-engine.js | 36 ++++++ tests/lib/config/config-file.js | 20 ---- tests/lib/config/plugins.js | 31 ----- tests/lib/util/naming.js | 82 +++++++++++++ 11 files changed, 269 insertions(+), 138 deletions(-) create mode 100644 lib/util/naming.js create mode 100644 tests/fixtures/cli-engine/node_modules/@somenamespace/eslint-formatter-foo/index.js create mode 100644 tests/fixtures/cli-engine/node_modules/eslint-formatter-bar/index.js create mode 100644 tests/lib/util/naming.js diff --git a/docs/user-guide/command-line-interface.md b/docs/user-guide/command-line-interface.md index 3e6a5a31b23..a8f51cf6129 100644 --- a/docs/user-guide/command-line-interface.md +++ b/docs/user-guide/command-line-interface.md @@ -333,6 +333,17 @@ Example: eslint -f ./customformat.js file.js +An npm-installed formatter is resolved with or without `eslint-formatter-` prefix. + +Example: + + npm install eslint-formatter-pretty + + eslint -f pretty file.js + + // equivalent: + eslint -f eslint-formatter-pretty file.js + When specified, the given format is output to the console. If you'd like to save that output into a file, you can do so on the command line like so: eslint -f compact file.js > results.txt diff --git a/lib/cli-engine.js b/lib/cli-engine.js index 29ee9fef560..4d093b74268 100644 --- a/lib/cli-engine.js +++ b/lib/cli-engine.js @@ -26,10 +26,14 @@ const fs = require("fs"), validator = require("./config/config-validator"), stringify = require("json-stable-stringify"), hash = require("./util/hash"), + ModuleResolver = require("./util/module-resolver"), + naming = require("./util/naming"), pkg = require("../package.json"); const debug = require("debug")("eslint:cli-engine"); +const resolver = new ModuleResolver(); + //------------------------------------------------------------------------------ // Typedefs //------------------------------------------------------------------------------ @@ -670,15 +674,22 @@ class CLIEngine { // replace \ with / for Windows compatibility format = format.replace(/\\/g, "/"); + const cwd = this.options ? this.options.cwd : process.cwd(); + const namespace = naming.getNamespaceFromTerm(format); + let formatterPath; // if there's a slash, then it's a file - if (format.indexOf("/") > -1) { - const cwd = this.options ? this.options.cwd : process.cwd(); - + if (!namespace && format.indexOf("/") > -1) { formatterPath = path.resolve(cwd, format); } else { - formatterPath = `./formatters/${format}`; + try { + const npmFormat = naming.normalizePackageName(format, "eslint-formatter"); + + formatterPath = resolver.resolve(npmFormat, `${cwd}/node_modules`); + } catch (e) { + formatterPath = `./formatters/${format}`; + } } try { diff --git a/lib/config/config-file.js b/lib/config/config-file.js index 87412dd2a2d..4d391b3a1d7 100644 --- a/lib/config/config-file.js +++ b/lib/config/config-file.js @@ -13,8 +13,8 @@ const fs = require("fs"), path = require("path"), ConfigOps = require("./config-ops"), validator = require("./config-validator"), - pathUtil = require("../util/path-util"), ModuleResolver = require("../util/module-resolver"), + naming = require("../util/naming"), pathIsInside = require("path-is-inside"), stripComments = require("strip-json-comments"), stringify = require("json-stable-stringify"), @@ -436,50 +436,6 @@ function applyExtends(config, configContext, filePath, relativeTo) { return config; } -/** - * Brings package name to correct format based on prefix - * @param {string} name The name of the package. - * @param {string} prefix Can be either "eslint-plugin" or "eslint-config - * @returns {string} Normalized name of the package - * @private - */ -function normalizePackageName(name, prefix) { - - /* - * On Windows, name can come in with Windows slashes instead of Unix slashes. - * Normalize to Unix first to avoid errors later on. - * https://github.com/eslint/eslint/issues/5644 - */ - if (name.indexOf("\\") > -1) { - name = pathUtil.convertPathToPosix(name); - } - - if (name.charAt(0) === "@") { - - /* - * it's a scoped package - * package name is "eslint-config", or just a username - */ - const scopedPackageShortcutRegex = new RegExp(`^(@[^/]+)(?:/(?:${prefix})?)?$`), - scopedPackageNameRegex = new RegExp(`^${prefix}(-|$)`); - - if (scopedPackageShortcutRegex.test(name)) { - name = name.replace(scopedPackageShortcutRegex, `$1/${prefix}`); - } else if (!scopedPackageNameRegex.test(name.split("/")[1])) { - - /* - * for scoped packages, insert the eslint-config after the first / unless - * the path is already @scope/eslint or @scope/eslint-config-xxx - */ - name = name.replace(/^@([^/]+)\/(.*)$/, `@$1/${prefix}-$2`); - } - } else if (name.indexOf(`${prefix}-`) !== 0) { - name = `${prefix}-${name}`; - } - - return name; -} - /** * Resolves a configuration file path into the fully-formed path, whether filename * or package name. @@ -505,12 +461,12 @@ function resolve(filePath, relativeTo) { const pluginName = filePath.slice(7, filePath.lastIndexOf("/")); const configName = filePath.slice(filePath.lastIndexOf("/") + 1); - normalizedPackageName = normalizePackageName(pluginName, "eslint-plugin"); + normalizedPackageName = naming.normalizePackageName(pluginName, "eslint-plugin"); debug(`Attempting to resolve ${normalizedPackageName}`); filePath = resolver.resolve(normalizedPackageName, getLookupPath(relativeTo)); return { filePath, configName, configFullName }; } - normalizedPackageName = normalizePackageName(filePath, "eslint-config"); + normalizedPackageName = naming.normalizePackageName(filePath, "eslint-config"); debug(`Attempting to resolve ${normalizedPackageName}`); filePath = resolver.resolve(normalizedPackageName, getLookupPath(relativeTo)); return { filePath, configFullName: filePath }; @@ -614,7 +570,6 @@ module.exports = { resolve, write, applyExtends, - normalizePackageName, CONFIG_FILES, /** diff --git a/lib/config/plugins.js b/lib/config/plugins.js index 9884f360391..c509c48fe25 100644 --- a/lib/config/plugins.js +++ b/lib/config/plugins.js @@ -9,13 +9,13 @@ //------------------------------------------------------------------------------ const debug = require("debug")("eslint:plugins"); +const naming = require("../util/naming"); //------------------------------------------------------------------------------ // Private //------------------------------------------------------------------------------ -const PLUGIN_NAME_PREFIX = "eslint-plugin-", - NAMESPACE_REGEX = /^@.*\//i; +const PLUGIN_NAME_PREFIX = "eslint-plugin-"; //------------------------------------------------------------------------------ // Public Interface @@ -37,33 +37,6 @@ class Plugins { this._rules = rulesContext; } - /** - * Removes the prefix `eslint-plugin-` from a plugin name. - * @param {string} pluginName The name of the plugin which may have the prefix. - * @returns {string} The name of the plugin without prefix. - */ - static removePrefix(pluginName) { - return pluginName.startsWith(PLUGIN_NAME_PREFIX) ? pluginName.slice(PLUGIN_NAME_PREFIX.length) : pluginName; - } - - /** - * Gets the scope (namespace) of a plugin. - * @param {string} pluginName The name of the plugin which may have the prefix. - * @returns {string} The name of the plugins namepace if it has one. - */ - static getNamespace(pluginName) { - return pluginName.match(NAMESPACE_REGEX) ? pluginName.match(NAMESPACE_REGEX)[0] : ""; - } - - /** - * Removes the namespace from a plugin name. - * @param {string} pluginName The name of the plugin which may have the prefix. - * @returns {string} The name of the plugin without the namespace. - */ - static removeNamespace(pluginName) { - return pluginName.replace(NAMESPACE_REGEX, ""); - } - /** * Defines a plugin with a given name rather than loading from disk. * @param {string} pluginName The name of the plugin to load. @@ -71,9 +44,9 @@ class Plugins { * @returns {void} */ define(pluginName, plugin) { - const pluginNamespace = Plugins.getNamespace(pluginName), - pluginNameWithoutNamespace = Plugins.removeNamespace(pluginName), - pluginNameWithoutPrefix = Plugins.removePrefix(pluginNameWithoutNamespace), + const pluginNamespace = naming.getNamespaceFromTerm(pluginName), + pluginNameWithoutNamespace = naming.removeNamespaceFromTerm(pluginName), + pluginNameWithoutPrefix = naming.removePrefixFromTerm(PLUGIN_NAME_PREFIX, pluginNameWithoutNamespace), shortName = pluginNamespace + pluginNameWithoutPrefix; // load up environments and rules @@ -106,9 +79,9 @@ class Plugins { * @throws {Error} If the plugin cannot be loaded. */ load(pluginName) { - const pluginNamespace = Plugins.getNamespace(pluginName), - pluginNameWithoutNamespace = Plugins.removeNamespace(pluginName), - pluginNameWithoutPrefix = Plugins.removePrefix(pluginNameWithoutNamespace), + const pluginNamespace = naming.getNamespaceFromTerm(pluginName), + pluginNameWithoutNamespace = naming.removeNamespaceFromTerm(pluginName), + pluginNameWithoutPrefix = naming.removePrefixFromTerm(PLUGIN_NAME_PREFIX, pluginNameWithoutNamespace), shortName = pluginNamespace + pluginNameWithoutPrefix, longName = pluginNamespace + PLUGIN_NAME_PREFIX + pluginNameWithoutPrefix; let plugin = null; diff --git a/lib/util/naming.js b/lib/util/naming.js new file mode 100644 index 00000000000..dcac81bbd64 --- /dev/null +++ b/lib/util/naming.js @@ -0,0 +1,112 @@ +/** + * @fileoverview Common helpers for naming of plugins, formatters and configs + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const pathUtil = require("../util/path-util"); + +//------------------------------------------------------------------------------ +// Private +//------------------------------------------------------------------------------ + +const NAMESPACE_REGEX = /^@.*\//i; + +/** + * Brings package name to correct format based on prefix + * @param {string} name The name of the package. + * @param {string} prefix Can be either "eslint-plugin", "eslint-config" or "eslint-formatter" + * @returns {string} Normalized name of the package + * @private + */ +function normalizePackageName(name, prefix) { + + /** + * On Windows, name can come in with Windows slashes instead of Unix slashes. + * Normalize to Unix first to avoid errors later on. + * https://github.com/eslint/eslint/issues/5644 + */ + if (name.indexOf("\\") > -1) { + name = pathUtil.convertPathToPosix(name); + } + + if (name.charAt(0) === "@") { + + /** + * it's a scoped package + * package name is the prefix, or just a username + */ + const scopedPackageShortcutRegex = new RegExp(`^(@[^/]+)(?:/(?:${prefix})?)?$`), + scopedPackageNameRegex = new RegExp(`^${prefix}(-|$)`); + + if (scopedPackageShortcutRegex.test(name)) { + name = name.replace(scopedPackageShortcutRegex, `$1/${prefix}`); + } else if (!scopedPackageNameRegex.test(name.split("/")[1])) { + + /** + * for scoped packages, insert the prefix after the first / unless + * the path is already @scope/eslint or @scope/eslint-xxx-yyy + */ + name = name.replace(/^@([^/]+)\/(.*)$/, `@$1/${prefix}-$2`); + } + } else if (name.indexOf(`${prefix}-`) !== 0) { + name = `${prefix}-${name}`; + } + + return name; +} + +/** + * Removes the prefix from a term. + * @param {string} prefix The prefix to remove. + * @param {string} term The term which may have the prefix. + * @returns {string} The term without prefix. + */ +function removePrefixFromTerm(prefix, term) { + return term.startsWith(prefix) ? term.slice(prefix.length) : term; +} + +/** + * Adds a prefix to a term. + * @param {string} prefix The prefix to add. + * @param {string} term The term which may not have the prefix. + * @returns {string} The term with prefix. + */ +function addPrefixToTerm(prefix, term) { + return term.startsWith(prefix) ? term : `${prefix}${term}`; +} + +/** + * Gets the scope (namespace) of a term. + * @param {string} term The term which may have the namespace. + * @returns {string} The namepace of the term if it has one. + */ +function getNamespaceFromTerm(term) { + const match = term.match(NAMESPACE_REGEX); + + return match ? match[0] : ""; +} + +/** + * Removes the namespace from a term. + * @param {string} term The term which may have the namespace. + * @returns {string} The name of the plugin without the namespace. + */ +function removeNamespaceFromTerm(term) { + return term.replace(NAMESPACE_REGEX, ""); +} + +//------------------------------------------------------------------------------ +// Public Interface +//------------------------------------------------------------------------------ + +module.exports = { + normalizePackageName, + removePrefixFromTerm, + addPrefixToTerm, + getNamespaceFromTerm, + removeNamespaceFromTerm +}; diff --git a/tests/fixtures/cli-engine/node_modules/@somenamespace/eslint-formatter-foo/index.js b/tests/fixtures/cli-engine/node_modules/@somenamespace/eslint-formatter-foo/index.js new file mode 100644 index 00000000000..0a55f349ae7 --- /dev/null +++ b/tests/fixtures/cli-engine/node_modules/@somenamespace/eslint-formatter-foo/index.js @@ -0,0 +1 @@ +module.exports = function() {}; diff --git a/tests/fixtures/cli-engine/node_modules/eslint-formatter-bar/index.js b/tests/fixtures/cli-engine/node_modules/eslint-formatter-bar/index.js new file mode 100644 index 00000000000..0a55f349ae7 --- /dev/null +++ b/tests/fixtures/cli-engine/node_modules/eslint-formatter-bar/index.js @@ -0,0 +1 @@ +module.exports = function() {}; diff --git a/tests/lib/cli-engine.js b/tests/lib/cli-engine.js index 81d00a96e62..c924ba297d6 100644 --- a/tests/lib/cli-engine.js +++ b/tests/lib/cli-engine.js @@ -2689,6 +2689,42 @@ describe("CLIEngine", () => { assert.isFunction(formatter); }); + it("should return a function when a formatter prefixed with eslint-formatter is requested", () => { + const engine = new CLIEngine({ + cwd: getFixturePath("cli-engine") + }), + formatter = engine.getFormatter("bar"); + + assert.isFunction(formatter); + }); + + it("should return a function when a formatter is requested, also when the eslint-formatter prefix is included in the format argument", () => { + const engine = new CLIEngine({ + cwd: getFixturePath("cli-engine") + }), + formatter = engine.getFormatter("eslint-formatter-bar"); + + assert.isFunction(formatter); + }); + + it("should return a function when a formatter is requested within a scoped npm package", () => { + const engine = new CLIEngine({ + cwd: getFixturePath("cli-engine") + }), + formatter = engine.getFormatter("@somenamespace/foo"); + + assert.isFunction(formatter); + }); + + it("should return a function when a formatter is requested within a scoped npm package, also when the eslint-formatter prefix is included in the format argument", () => { + const engine = new CLIEngine({ + cwd: getFixturePath("cli-engine") + }), + formatter = engine.getFormatter("@somenamespace/eslint-formatter-foo"); + + assert.isFunction(formatter); + }); + it("should return null when a customer formatter doesn't exist", () => { const engine = new CLIEngine(), formatterPath = getFixturePath("formatters", "doesntexist.js"); diff --git a/tests/lib/config/config-file.js b/tests/lib/config/config-file.js index 4cdf22c9a85..91c6508d4a4 100644 --- a/tests/lib/config/config-file.js +++ b/tests/lib/config/config-file.js @@ -1175,26 +1175,6 @@ describe("ConfigFile", () => { }); - describe("normalizePackageName()", () => { - - leche.withData([ - ["foo", "eslint-config-foo"], - ["eslint-config-foo", "eslint-config-foo"], - ["@z/foo", "@z/eslint-config-foo"], - ["@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"] - ], (input, expected) => { - it(`should return ${expected} when passed ${input}`, () => { - const result = ConfigFile.normalizePackageName(input, "eslint-config"); - - assert.strictEqual(result, expected); - }); - }); - - }); - describe("write()", () => { let sandbox, diff --git a/tests/lib/config/plugins.js b/tests/lib/config/plugins.js index cbc6d9a4c7e..37b96c0480f 100644 --- a/tests/lib/config/plugins.js +++ b/tests/lib/config/plugins.js @@ -241,35 +241,4 @@ describe("Plugins", () => { }); }); - - describe("removePrefix()", () => { - it("should remove common prefix when passed a plugin name with a prefix", () => { - const pluginName = Plugins.removePrefix("eslint-plugin-test"); - - assert.strictEqual(pluginName, "test"); - }); - - it("should not modify plugin name when passed a plugin name without a prefix", () => { - const pluginName = Plugins.removePrefix("test"); - - assert.strictEqual(pluginName, "test"); - }); - }); - - describe("getNamespace()", () => { - it("should remove namepace when passed with namepace", () => { - const namespace = Plugins.getNamespace("@namepace/eslint-plugin-test"); - - assert.strictEqual(namespace, "@namepace/"); - }); - }); - - describe("removeNamespace()", () => { - it("should remove namepace when passed with namepace", () => { - const namespace = Plugins.removeNamespace("@namepace/eslint-plugin-test"); - - assert.strictEqual(namespace, "eslint-plugin-test"); - }); - }); - }); diff --git a/tests/lib/util/naming.js b/tests/lib/util/naming.js new file mode 100644 index 00000000000..e57f1d27ab1 --- /dev/null +++ b/tests/lib/util/naming.js @@ -0,0 +1,82 @@ +/** + * @fileoverview Tests for naming util + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const assert = require("chai").assert, + leche = require("leche"), + naming = require("../../../lib/util/naming"); + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +describe("naming", () => { + describe("normalizePackageName()", () => { + + leche.withData([ + ["foo", "eslint-config-foo"], + ["eslint-config-foo", "eslint-config-foo"], + ["@z/foo", "@z/eslint-config-foo"], + ["@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"] + ], (input, expected) => { + it(`should return ${expected} when passed ${input}`, () => { + const result = naming.normalizePackageName(input, "eslint-config"); + + assert.strictEqual(result, expected); + }); + }); + + }); + + describe("removePrefixFromTerm()", () => { + it("should remove prefix when passed a term with a prefix", () => { + const pluginName = naming.removePrefixFromTerm("eslint-plugin-", "eslint-plugin-test"); + + assert.strictEqual(pluginName, "test"); + }); + + it("should not modify term when passed a term without a prefix", () => { + const pluginName = naming.removePrefixFromTerm("eslint-plugin-", "test"); + + assert.strictEqual(pluginName, "test"); + }); + }); + + describe("addPrefixToTerm()", () => { + it("should add prefix when passed a term without a prefix", () => { + const pluginName = naming.addPrefixToTerm("eslint-plugin-", "test"); + + assert.strictEqual(pluginName, "eslint-plugin-test"); + }); + + it("should not modify term when passed a term with a prefix", () => { + const pluginName = naming.addPrefixToTerm("eslint-plugin-", "eslint-plugin-test"); + + assert.strictEqual(pluginName, "eslint-plugin-test"); + }); + }); + + describe("getNamespaceFromTerm()", () => { + it("should remove namepace when passed with namepace", () => { + const namespace = naming.getNamespaceFromTerm("@namepace/eslint-plugin-test"); + + assert.strictEqual(namespace, "@namepace/"); + }); + }); + + describe("removeNamespaceFromTerm()", () => { + it("should remove namepace when passed with namepace", () => { + const namespace = naming.removeNamespaceFromTerm("@namepace/eslint-plugin-test"); + + assert.strictEqual(namespace, "eslint-plugin-test"); + }); + }); +});