Repository: eslint/eslint (js)

Issue:
Proposal: support @scope for @scope/eslint-plugin
**Tell us about your environment**

* **ESLint Version:** v4.16.0
* **Node Version:** v9.2.0
* **npm Version:** v5.6.0

**What parser (default, Babel-ESLint, etc.) are you using?**

- default

**Please show your full configuration:**

<details open>
<summary>Configuration</summary>

<!-- Paste your configuration below: -->
```js
module.exports = {
  plugins: [
    '@scope'
  ],
  rules: {
    '@scope/rule': 'error'
  }
}
```

</details>

**What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.**

I run ESLint.

**What did you expect to happen?**

ESLint loads `@scope/eslint-plugin`.

**What actually happened? Please include the actual, raw output from ESLint.**

ESLint failed to load `eslint-plugin-@scope`.

```
Oops! Something went wrong! :(

ESLint: 4.16.0.
ESLint couldn't find the plugin "eslint-plugin-@scope". This can happen for a couple different reasons:

1. If ESLint is installed globally, then make sure eslint-plugin-@scope is also installed globally. A globally-installed ESLint cannot find a locally-installed plugin.

2. If ESLint is installed locally, then it's likely that the plugin isn't installed correctly. Try reinstalling by running the following:

    npm i eslint-plugin-@scope@latest --save-dev

If you still can't figure out the problem, please stop by https://gitter.im/eslint/eslint to chat with the team.
```

----

ESLint supports `@scope` for `@scope/eslint-config` since #3123, but doesn't support `@scope` for `@scope/eslint-plugin`.
It looks inconsistent.

I propose the feature, ESLint resolves `plugins: ["@scope"]` to `@scope/eslint-plugin`.


Pull request:
Breaking: support @scope shorthand in plugins (fixes #9903)
<!--
    ESLint adheres to the [JS Foundation Code of Conduct](https://js.foundation/community/code-of-conduct).
-->

**What is the purpose of this pull request? (put an "X" next to item)**

[ ] Documentation update
[ ] Bug fix ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/bug-report.md))
[ ] New rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-proposal.md))
[ ] Changes an existing rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-change-proposal.md))
[ ] Add autofixing to a rule
[ ] Add a CLI option
[X] Add something to the core
[ ] Other, please explain:

Fixes #9903.

**What changes did you make? (Give an overview)**

This is a reference implementation for #9903.
This PR changes the logic to normalize plugin names as same as shareable config's.
As the result, we can use `plugins: ["@scope"]` shorthand for `@scope/eslint-plugin` package.

**Is there anything you'd like reviewers to focus on?**

Nothing in particular.


Patch:
diff --git a/lib/config/plugins.js b/lib/config/plugins.js
index 756f9ff21..a47322705 100644
--- a/lib/config/plugins.js
+++ b/lib/config/plugins.js
@@ -11,12 +11,6 @@
 const debug = require("debug")("eslint:plugins");
 const naming = require("../util/naming");
 
-//------------------------------------------------------------------------------
-// Private
-//------------------------------------------------------------------------------
-
-const PLUGIN_NAME_PREFIX = "eslint-plugin-";
-
 //------------------------------------------------------------------------------
 // Public Interface
 //------------------------------------------------------------------------------
@@ -44,10 +38,8 @@ class Plugins {
      * @returns {void}
      */
     define(pluginName, plugin) {
-        const pluginNamespace = naming.getNamespaceFromTerm(pluginName),
-            pluginNameWithoutNamespace = naming.removeNamespaceFromTerm(pluginName),
-            pluginNameWithoutPrefix = naming.removePrefixFromTerm(PLUGIN_NAME_PREFIX, pluginNameWithoutNamespace),
-            shortName = pluginNamespace + pluginNameWithoutPrefix;
+        const longName = naming.normalizePackageName(pluginName, "eslint-plugin");
+        const shortName = naming.getShorthandName(longName, "eslint-plugin");
 
         // load up environments and rules
         this._plugins[shortName] = plugin;
@@ -79,11 +71,8 @@ class Plugins {
      * @throws {Error} If the plugin cannot be loaded.
      */
     load(pluginName) {
-        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;
+        const longName = naming.normalizePackageName(pluginName, "eslint-plugin");
+        const shortName = naming.getShorthandName(longName, "eslint-plugin");
         let plugin = null;
 
         if (pluginName.match(/\s+/)) {
diff --git a/lib/formatters/html-template-message.html b/lib/formatters/html-template-message.html
index 66f49ff49..bc3535020 100644
--- a/lib/formatters/html-template-message.html
+++ b/lib/formatters/html-template-message.html
@@ -3,6 +3,6 @@
     <td class="clr-<%= severityNumber %>"><%= severityName %></td>
     <td><%- message %></td>
     <td>
-        <a href="https://eslint.org/docs/rules/<%= ruleId %>" target="_blank"><%= ruleId %></a>
+        <a href="https://eslint.org/docs/rules/<%= ruleId %>" target="_blank" rel="noopener noreferrer"><%= ruleId %></a>
     </td>
 </tr>
diff --git a/lib/linter.js b/lib/linter.js
index aeb558785..2d46b4bd8 100644
--- a/lib/linter.js
+++ b/lib/linter.js
@@ -1008,15 +1008,26 @@ module.exports = class Linter {
 
         const configuredRules = Object.assign({}, config.rules, commentDirectives.configuredRules);
 
-        const lintingProblems = runRules(
-            sourceCode,
-            configuredRules,
-            ruleId => this.rules.get(ruleId),
-            parserOptions,
-            parserName,
-            settings,
-            options.filename
-        );
+        let lintingProblems;
+
+        try {
+            lintingProblems = runRules(
+                sourceCode,
+                configuredRules,
+                ruleId => this.rules.get(ruleId),
+                parserOptions,
+                parserName,
+                settings,
+                options.filename
+            );
+        } catch (err) {
+            debug("An error occurred while traversing");
+            debug("Filename:", options.filename);
+            debug("Parser Options:", parserOptions);
+            debug("Parser Path:", parserName);
+            debug("Settings:", settings);
+            throw err;
+        }
 
         return applyDisableDirectives({
             directives: commentDirectives.disableDirectives,
diff --git a/lib/util/glob-util.js b/lib/util/glob-util.js
index d687aa746..b91ec53e6 100644
--- a/lib/util/glob-util.js
+++ b/lib/util/glob-util.js
@@ -106,7 +106,7 @@ const dotfilesPattern = /(?:(?:^\.)|(?:[/\\]\.))[^/\\.].*/;
 function listFilesToProcess(globPatterns, providedOptions) {
     const options = providedOptions || { ignore: true };
     const files = [];
-    const added = {};
+    const addedFilenames = new Set();
 
     const cwd = options.cwd || process.cwd();
 
@@ -117,41 +117,32 @@ function listFilesToProcess(globPatterns, providedOptions) {
 
     /**
      * Executes the linter on a file defined by the `filename`. Skips
-     * unsupported file extensions and any files that are already linted.
+     * any files that are already linted.
      * @param {string} filename The file to be processed
-     * @param {boolean} shouldWarnIgnored Whether or not a report should be made if
-     *                                    the file is ignored
+     * @param {boolean} isDirectPath True if the file
+     * was provided as a direct path (as opposed to being resolved from a glob)
      * @param {IgnoredPaths} ignoredPaths An instance of IgnoredPaths
      * @returns {void}
      */
-    function addFile(filename, shouldWarnIgnored, ignoredPaths) {
-        let ignored = false;
-        let isSilentlyIgnored;
-
-        if (ignoredPaths.contains(filename, "default")) {
-            ignored = (options.ignore !== false) && shouldWarnIgnored;
-            isSilentlyIgnored = !shouldWarnIgnored;
+    function addFile(filename, isDirectPath, ignoredPaths) {
+        if (addedFilenames.has(filename)) {
+            return;
         }
 
-        if (options.ignore !== false) {
-            if (ignoredPaths.contains(filename, "custom")) {
-                if (shouldWarnIgnored) {
-                    ignored = true;
-                } else {
-                    isSilentlyIgnored = true;
-                }
-            }
-        }
+        const shouldProcessCustomIgnores = options.ignore !== false;
+        const shouldLintIgnoredDirectPaths = options.ignore === false;
+        const fileMatchesIgnorePatterns = ignoredPaths.contains(filename, "default") ||
+            (shouldProcessCustomIgnores && ignoredPaths.contains(filename, "custom"));
 
-        if (isSilentlyIgnored && !ignored) {
-            return;
+        if (fileMatchesIgnorePatterns && isDirectPath && !shouldLintIgnoredDirectPaths) {
+            files.push({ filename, ignored: true });
+            addedFilenames.add(filename);
         }
 
-        if (added[filename]) {
-            return;
+        if (!fileMatchesIgnorePatterns || (isDirectPath && shouldLintIgnoredDirectPaths)) {
+            files.push({ filename, ignored: false });
+            addedFilenames.add(filename);
         }
-        files.push({ filename, ignored });
-        added[filename] = true;
     }
 
     debug("Creating list of files to process.");
diff --git a/lib/util/naming.js b/lib/util/naming.js
index c5ff429aa..a5d6b9d79 100644
--- a/lib/util/naming.js
+++ b/lib/util/naming.js
@@ -61,23 +61,28 @@ function normalizePackageName(name, prefix) {
 }
 
 /**
- * Removes the prefix from a term.
+ * Removes the prefix from a fullname.
+ * @param {string} fullname The term which may have the prefix.
  * @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;
-}
+function getShorthandName(fullname, prefix) {
+    if (fullname[0] === "@") {
+        let matchResult = new RegExp(`^(@[^/]+)/${prefix}$`).exec(fullname);
 
-/**
- * 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}`;
+        if (matchResult) {
+            return matchResult[1];
+        }
+
+        matchResult = new RegExp(`^(@[^/]+)/${prefix}-(.+)$`).exec(fullname);
+        if (matchResult) {
+            return `${matchResult[1]}/${matchResult[2]}`;
+        }
+    } else if (fullname.startsWith(`${prefix}-`)) {
+        return fullname.slice(prefix.length + 1);
+    }
+
+    return fullname;
 }
 
 /**
@@ -91,23 +96,12 @@ function getNamespaceFromTerm(term) {
     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
+    getShorthandName,
+    getNamespaceFromTerm
 };


Test patch:
diff --git a/tests/fixtures/plugin-shorthand/basic/.eslintrc.json b/tests/fixtures/plugin-shorthand/basic/.eslintrc.json
new file mode 100644
index 000000000..f74335450
--- /dev/null
+++ b/tests/fixtures/plugin-shorthand/basic/.eslintrc.json
@@ -0,0 +1,7 @@
+{
+    "root": true,
+    "plugins": ["@scope"],
+    "rules": {
+        "@scope/rule": "error"
+    }
+}
diff --git a/tests/fixtures/plugin-shorthand/basic/node_modules/@scope/eslint-plugin/index.js b/tests/fixtures/plugin-shorthand/basic/node_modules/@scope/eslint-plugin/index.js
new file mode 100644
index 000000000..54bb28c30
--- /dev/null
+++ b/tests/fixtures/plugin-shorthand/basic/node_modules/@scope/eslint-plugin/index.js
@@ -0,0 +1,22 @@
+"use strict"
+
+module.exports = {
+    configs: {
+        recommended: {
+            plugins: ["@scope"],
+            rules: { "@scope/rule": "error" }
+        }
+    },
+    rules: {
+        rule: {
+            create(context) {
+                return {
+                    Program(node) {
+                        const message = "OK"
+                        context.report({ node, message })
+                    }
+                }
+            }
+        }
+    }
+}
diff --git a/tests/fixtures/plugin-shorthand/extends/.eslintrc.json b/tests/fixtures/plugin-shorthand/extends/.eslintrc.json
new file mode 100644
index 000000000..f392715a8
--- /dev/null
+++ b/tests/fixtures/plugin-shorthand/extends/.eslintrc.json
@@ -0,0 +1,4 @@
+{
+    "root": true,
+    "extends": ["plugin:@scope/recommended"]
+}
diff --git a/tests/fixtures/plugin-shorthand/extends/node_modules/@scope/eslint-plugin/index.js b/tests/fixtures/plugin-shorthand/extends/node_modules/@scope/eslint-plugin/index.js
new file mode 100644
index 000000000..9dcc4756b
--- /dev/null
+++ b/tests/fixtures/plugin-shorthand/extends/node_modules/@scope/eslint-plugin/index.js
@@ -0,0 +1,22 @@
+"use strict"
+
+module.exports = {
+    configs: {
+        recommended: {
+            plugins: ["@scope"],
+            rules: { "@scope/rule": "error" }
+        }
+    },
+    rules: {
+        rule: {
+            create(context) {
+                return {
+                    Program(node) {
+                        const message = "OK"
+                        context.report({node, message})
+                    }
+                }
+            }
+        }
+    }
+}
diff --git a/tests/lib/cli-engine.js b/tests/lib/cli-engine.js
index 9ed25a9ef..54f018da0 100644
--- a/tests/lib/cli-engine.js
+++ b/tests/lib/cli-engine.js
@@ -557,6 +557,44 @@ describe("CLIEngine", () => {
             assert.strictEqual(report.results[0].messages[0].message, expectedMsg);
         });
 
+        // @scope for @scope/eslint-plugin
+        describe("(plugin shorthand)", () => {
+            const Module = require("module");
+            let originalFindPath = null;
+
+            /* eslint-disable no-underscore-dangle */
+            before(() => {
+                originalFindPath = Module._findPath;
+                Module._findPath = function(id) {
+                    if (id === "@scope/eslint-plugin") {
+                        return path.resolve(__dirname, "../fixtures/plugin-shorthand/basic/node_modules/@scope/eslint-plugin/index.js");
+                    }
+                    return originalFindPath.apply(this, arguments);
+                };
+            });
+            after(() => {
+                Module._findPath = originalFindPath;
+            });
+            /* eslint-enable no-underscore-dangle */
+
+            it("should resolve 'plugins:[\"@scope\"]' to 'node_modules/@scope/eslint-plugin'.", () => {
+                engine = new CLIEngine({ cwd: getFixturePath("plugin-shorthand/basic") });
+                const report = engine.executeOnText("var x = 0", "index.js").results[0];
+
+                assert.strictEqual(report.filePath, getFixturePath("plugin-shorthand/basic/index.js"));
+                assert.strictEqual(report.messages[0].ruleId, "@scope/rule");
+                assert.strictEqual(report.messages[0].message, "OK");
+            });
+
+            it("should resolve 'extends:[\"plugin:@scope/recommended\"]' to 'node_modules/@scope/eslint-plugin'.", () => {
+                engine = new CLIEngine({ cwd: getFixturePath("plugin-shorthand/extends") });
+                const report = engine.executeOnText("var x = 0", "index.js").results[0];
+
+                assert.strictEqual(report.filePath, getFixturePath("plugin-shorthand/extends/index.js"));
+                assert.strictEqual(report.messages[0].ruleId, "@scope/rule");
+                assert.strictEqual(report.messages[0].message, "OK");
+            });
+        });
     });
 
     describe("executeOnFiles()", () => {
diff --git a/tests/lib/linter.js b/tests/lib/linter.js
index 9e9a04dbb..80301cbc8 100644
--- a/tests/lib/linter.js
+++ b/tests/lib/linter.js
@@ -98,7 +98,7 @@ describe("Linter", () => {
             }));
 
             assert.throws(() => {
-                linter.verify(code, config, filename, true);
+                linter.verify(code, config, filename);
             }, "Intentional error.");
         });
 
@@ -4362,6 +4362,4 @@ describe("Linter", () => {
             });
         });
     }
-
-
 });
diff --git a/tests/lib/util/naming.js b/tests/lib/util/naming.js
index e57f1d27a..e98067018 100644
--- a/tests/lib/util/naming.js
+++ b/tests/lib/util/naming.js
@@ -36,32 +36,23 @@ describe("naming", () => {
 
     });
 
-    describe("removePrefixFromTerm()", () => {
-        it("should remove prefix when passed a term with a prefix", () => {
-            const pluginName = naming.removePrefixFromTerm("eslint-plugin-", "eslint-plugin-test");
+    describe("getShorthandName()", () => {
 
-            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");
+        leche.withData([
+            ["foo", "foo"],
+            ["eslint-config-foo", "foo"],
+            ["@z", "@z"],
+            ["@z/eslint-config", "@z"],
+            ["@z/foo", "@z/foo"],
+            ["@z/eslint-config-foo", "@z/foo"]
+        ], (input, expected) => {
+            it(`should return ${expected} when passed ${input}`, () => {
+                const result = naming.getShorthandName(input, "eslint-config");
 
-            assert.strictEqual(pluginName, "eslint-plugin-test");
+                assert.strictEqual(result, expected);
+            });
         });
 
-        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()", () => {
@@ -71,12 +62,4 @@ describe("naming", () => {
             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");
-        });
-    });
 });