Update: Resolve npm installed formatters (eslint#5900)

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

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,28 @@ 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 {
+                    let npmFormat = naming.removeNamespaceFromTerm(format);
+
+                    npmFormat = naming.addPrefixToTerm("eslint-formatter-", npmFormat);
+
+                    if (namespace) {
+                        npmFormat = naming.addNamespaceToTerm(namespace, npmFormat);
+                    }
+
+                    formatterPath = resolver.resolve(npmFormat, `${cwd}/node_modules`);
+                } catch (e) {
+                    formatterPath = `./formatters/${format}`;
+                }
             }
 
             try {

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,43 +37,16 @@ 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.
      * @param {Object} plugin The plugin object.
      * @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;

lib/util/naming.js

@@ -0,0 +1,70 @@
+/**
+ * @fileoverview Common helpers naming of plugins and formatters
+ */
+"use strict";
+
+//------------------------------------------------------------------------------
+// Private
+//------------------------------------------------------------------------------
+
+const NAMESPACE_REGEX = /^@.*\//i;
+
+/**
+ * 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}`;
+}
+
+/**
+ * Adds a namespace before a term
+ * @param {string} namespace The namespace to add before the term
+ * @param {string} term The term which may have a namespace
+ * @returns {string} The term with namespace
+ */
+function addNamespaceToTerm(namespace, term) {
+    return term.match(NAMESPACE_REGEX) ? term : `${namespace}${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) {
+    return term.match(NAMESPACE_REGEX) ? term.match(NAMESPACE_REGEX)[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 = {
+    removePrefixFromTerm,
+    addPrefixToTerm,
+    addNamespaceToTerm,
+    getNamespaceFromTerm,
+    removeNamespaceFromTerm
+};

tests/lib/cli-engine.js

@@ -2685,6 +2685,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");

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");
-        });
-    });
-
 });

tests/lib/util/naming.js

@@ -0,0 +1,75 @@
+/**
+ * @fileoverview Tests for naming util
+ */
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const assert = require("chai").assert,
+    naming = require("../../../lib/util/naming");
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+describe("naming", () => {
+    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("addNamespaceToTerm", () => {
+        it("should add namespace when passed a term without a namespace", () => {
+            const pluginName = naming.addNamespaceToTerm("@namespace/", "eslint-plugin-test");
+
+            assert.strictEqual(pluginName, "@namespace/eslint-plugin-test");
+        });
+
+        it("should not modify term when passed a term with a namespace", () => {
+            const pluginName = naming.addNamespaceToTerm("@othernamespace/", "@namespace/eslint-plugin-test");
+
+            assert.strictEqual(pluginName, "@namespace/eslint-plugin-test");
+        });
+    });
+
+    describe("getNamespace()", () => {
+        it("should remove namepace when passed with namepace", () => {
+            const namespace = naming.getNamespaceFromTerm("@namepace/eslint-plugin-test");
+
+            assert.strictEqual(namespace, "@namepace/");
+        });
+    });
+
+    describe("removeNamespace()", () => {
+        it("should remove namepace when passed with namepace", () => {
+            const namespace = naming.removeNamespaceFromTerm("@namepace/eslint-plugin-test");
+
+            assert.strictEqual(namespace, "eslint-plugin-test");
+        });
+    });
+});