Repository: eslint/eslint
Pull request #9413 (2017-10-12 00:55:50)

Add support for custom messages to restricted imports
**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))
[X] 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
[ ] Add something to the core
[ ] Other, please explain:

**What rule do you want to change?**
`no-restricted-imports` and `no-restricted-modules` (am interested in feedback first before making the changes to `no-restricted-modules` as well)

**Does this change cause the rule to produce more or fewer warnings?**
Same amount of errors but adds more nuance to the error messaging

**How will the change be implemented? (New option, new default behavior, etc.)?**
Modification to allowed options. Anywhere you can pass in a path string, you can now also pass in an object with a `name` and `message` key where the `name` corresponds to a path and the `message` is the custom message you want appended to the default error message.

**Please provide some example code that this change will affect:**
All existing configurations will continue to work. However, the following will also be accepted:
```json
"no-restricted-imports": ["error", [{
  "name": "import-foo",
  "message": "Please use import-bar instead."
}]]
```

or like this:

```json
"no-restricted-imports": ["error", {
  "paths": [{
    "name": "import-foo",
    "message": "Please use import-bar instead."
  }]
}]
```

**What does the rule currently do for this code?**
errors on any restricted imports that are passed into the options.

**What will the rule do after it's changed?**
continue to error on any restricted imports that are passed into the options, but with more clarity on what the developer should do instead.

**What changes did you make? (Give an overview)**
Following the precedent set by no-restricted-globals, anywhere you can pass a path string, you can also pass an object with a "name" and "message" key. This custom message will be appended to any error message triggered by an import of the "name" value. Custom messages are not supported with patterns.

**Is there anything you'd like reviewers to focus on?**
See if this pattern (and naming scheme) is acceptable so as to do the same for `no-restricted-modules`

FYI @ljharb @yzimet



jsf-clabot: [![CLA assistant check](https://cla.js.foundation/pull/badge/not_signed)](https://cla.js.foundation/eslint/eslint?pullRequest=9413) <br/>Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla.js.foundation/eslint/eslint?pullRequest=9413) before we can accept your contribution.<br/>
eslintbot: Thanks for the pull request, @majapw! I took a look to make sure it's ready for merging and found some changes are needed:


* The commit summary needs to begin with a tag (such as `Fix:` or `Update:`). Please check out our [guide](https://eslint.org/docs/developer-guide/contributing/pull-requests#step-2-make-your-changes) for how to properly format your commit summary and [update](https://eslint.org/docs/developer-guide/contributing/pull-requests#updating-the-commit-message) it on this pull request.


Can you please update the pull request to address these?

(More information can be found in our [pull request guide](https://eslint.org/docs/developer-guide/contributing/pull-requests).)
eslintbot: LGTM
not-an-aardvark: Thanks for the PR. Have you seen https://github.com/eslint/eslint/issues/8400? It looks like a similar proposal was accepted to add both `paths` and `patterns` options to `no-restricted-imports`, to allow for custom messages to be used.

I think it would be fine to just add the `paths` option in this PR, but I want to make sure this PR is compatible with the plan in https://github.com/eslint/eslint/issues/8400 so that we would still be able to add `patterns` later.
ljharb: The plan for "paths" is identical in this PR to #8400; I definitely think this could be merged right now, and in no way would it block #8400.
eslintbot: LGTM
majapw: @not-an-aardvark I think I've addressed your concerns (`hasOwnProperty` on the `restrictedPathMessages` object and add a `minLength` to the schema for the custom message). I've also duplicated the effort for `no-restricted-modules`. Please take another look! 
eslintbot: LGTM
majapw: @not-an-aardvark addressed!
kaicataldo: Let's make sure to update the issue after this lands so that it's clear that only custom messages for `patterns` are needed.
eslintbot: LGTM
majapw: @platinumazure I believe I've addressed your comments as well. 

Patch:
diff --git a/docs/developer-guide/development-environment.md b/docs/developer-guide/development-environment.md
index a6e44941f..afbab5641 100644
--- a/docs/developer-guide/development-environment.md
+++ b/docs/developer-guide/development-environment.md
@@ -57,11 +57,17 @@ npm test
 
 The testing takes a few minutes to complete. If any tests fail, that likely means one or more parts of the environment setup didn't complete correctly. The upstream tests always pass.
 
-## Build Scripts
+## Reference Information
+
+### Workflow
+
+Once you have your development environment installed, you can make and submit changes to the ESLint source files. Doing this successfully requires careful adherence to our [pull-request submission workflow](contributing/pull-requests.md).
+
+### Build Scripts
 
 ESLint has several build scripts that help with various parts of development.
 
-### npm test
+#### npm test
 
 The primary script to use is `npm test`, which does several things:
 
@@ -75,19 +81,19 @@ Be sure to run this after making changes and before sending a pull request with
 
 **Note:** The full code coverage report is output into `/coverage`.
 
-### npm run lint
+#### npm run lint
 
 Runs just the JavaScript and JSON linting on the repository
 
-### npm run browserify
+#### npm run browserify
 
 Generates `build/eslint.js`, a version of ESLint for use in the browser
 
-### npm run docs
+#### npm run docs
 
 Generates JSDoc documentation and places it into `/jsdoc`.
 
-### npm run profile
+#### npm run profile
 
 This command is used for intensive profiling of ESLint using Chrome Developer Tools. It starts a development server that runs through three profiles:
 
@@ -101,12 +107,3 @@ Your browser should automatically open to the page in question. When that happen
 1. Click on Profiles
 
 You should start to see profiles for each run show up on the left side. If not, reload the page in the browser. Once all three profiles have completed, they will be available for inspection.
-
-## Workflow
-
-Whenever you make changes to the ESLint source files, you'll need to run `npm test` to rerun the tests. The workflow is:
-
-1. Make changes
-2. Run `npm test` to run tests on the command line
-
-You'll have to do this each time you make a change. The tests are run automatically whenever a pull request is received, so make sure to verify your changes work before submitting them.
diff --git a/docs/rules/no-restricted-imports.md b/docs/rules/no-restricted-imports.md
index 856b8f2cc..420ce4e58 100644
--- a/docs/rules/no-restricted-imports.md
+++ b/docs/rules/no-restricted-imports.md
@@ -35,6 +35,28 @@ When using the object form, you can also specify an array of gitignore-style pat
 }]
 ```
 
+You may also specify a custom message for any paths you want to restrict as follows:
+
+```json
+"no-restricted-imports": ["error", [{
+  "name": "import-foo",
+  "message": "Please use import-bar instead."
+}]]
+```
+
+or like this:
+
+```json
+"no-restricted-imports": ["error", {
+  "paths": [{
+    "name": "import-foo",
+    "message": "Please use import-bar instead."
+  }]
+}]
+```
+
+The custom message will be appended to the default error message. Please note that you may not specify custom error messages for restricted patterns as a particular import may match more than one pattern.
+
 To restrict the use of all Node.js core imports (via https://github.com/nodejs/node/tree/master/lib):
 
 ```json
diff --git a/docs/rules/no-restricted-modules.md b/docs/rules/no-restricted-modules.md
index e2f0dd2bf..9a19e8ef5 100644
--- a/docs/rules/no-restricted-modules.md
+++ b/docs/rules/no-restricted-modules.md
@@ -26,6 +26,28 @@ For example, to restrict the use of all Node.js core modules (via https://github
 }
 ```
 
+You may also specify a custom message for any paths you want to restrict as follows:
+
+```json
+"no-restricted-modules": ["error", [{
+  "name": "foo-module",
+  "message": "Please use bar-module instead."
+}]]
+```
+
+or like this:
+
+```json
+"no-restricted-modules": ["error", {
+  "paths": [{
+    "name": "foo-module",
+    "message": "Please use bar-module instead."
+  }]
+}]
+```
+
+The custom message will be appended to the default error message. Please note that you may not specify custom error messages for restricted patterns as a particular module may match more than one pattern.
+
 Examples of **incorrect** code for this rule with sample `"fs", "cluster"` restricted modules:
 
 ```js
diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js
index c245f22a0..d46b098ac 100644
--- a/lib/rules/no-restricted-imports.js
+++ b/lib/rules/no-restricted-imports.js
@@ -4,6 +4,13 @@
  */
 "use strict";
 
+//------------------------------------------------------------------------------
+// Helpers
+//------------------------------------------------------------------------------
+
+const DEFAULT_MESSAGE_TEMPLATE = "'{{importName}}' import is restricted from being used.";
+const CUSTOM_MESSAGE_TEMPLATE = "'{{importName}}' import is restricted from being used. {{customMessage}}";
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -11,9 +18,29 @@
 const ignore = require("ignore");
 
 const arrayOfStrings = {
+    type: "array",
+    items: { type: "string" },
+    uniqueItems: true
+};
+
+const arrayOfStringsOrObjects = {
     type: "array",
     items: {
-        type: "string"
+        anyOf: [
+            { type: "string" },
+            {
+                type: "object",
+                properties: {
+                    name: { type: "string" },
+                    message: {
+                        type: "string",
+                        minLength: 1
+                    }
+                },
+                additionalProperties: false,
+                required: ["name"]
+            }
+        ]
     },
     uniqueItems: true
 };
@@ -28,17 +55,17 @@ module.exports = {
 
         schema: {
             anyOf: [
-                arrayOfStrings,
+                arrayOfStringsOrObjects,
                 {
                     type: "array",
-                    items: [{
+                    items: {
                         type: "object",
                         properties: {
-                            paths: arrayOfStrings,
+                            paths: arrayOfStringsOrObjects,
                             patterns: arrayOfStrings
                         },
                         additionalProperties: false
-                    }],
+                    },
                     additionalItems: false
                 }
             ]
@@ -47,35 +74,77 @@ module.exports = {
 
     create(context) {
         const options = Array.isArray(context.options) ? context.options : [];
-        const isStringArray = typeof options[0] !== "object";
-        const restrictedPaths = new Set(isStringArray ? context.options : options[0].paths || []);
-        const restrictedPatterns = isStringArray ? [] : options[0].patterns || [];
+        const isPathAndPatternsObject =
+            typeof options[0] === "object" &&
+            (options[0].hasOwnProperty("paths") || options[0].hasOwnProperty("patterns"));
+
+        const restrictedPaths = (isPathAndPatternsObject ? options[0].paths : context.options) || [];
+        const restrictedPatterns = (isPathAndPatternsObject ? options[0].patterns : []) || [];
+
+        const restrictedPathMessages = restrictedPaths.reduce((memo, importName) => {
+            if (typeof importName === "string") {
+                memo[importName] = null;
+            } else {
+                memo[importName.name] = importName.message;
+            }
+            return memo;
+        }, {});
 
         // if no imports are restricted we don"t need to check
-        if (restrictedPaths.size === 0 && restrictedPatterns.length === 0) {
+        if (Object.keys(restrictedPaths).length === 0 && restrictedPatterns.length === 0) {
             return {};
         }
 
         const ig = ignore().add(restrictedPatterns);
 
+        /**
+         * Report a restricted path.
+         * @param {node} node representing the restricted path reference
+         * @returns {void}
+         * @private
+         */
+        function reportPath(node) {
+            const importName = node.source.value.trim();
+            const customMessage = restrictedPathMessages[importName];
+            const message = customMessage
+                ? CUSTOM_MESSAGE_TEMPLATE
+                : DEFAULT_MESSAGE_TEMPLATE;
+
+            context.report({
+                node,
+                message,
+                data: {
+                    importName,
+                    customMessage
+                }
+            });
+        }
+
+        /**
+         * Check if the given name is a restricted path name.
+         * @param {string} name name of a variable
+         * @returns {boolean} whether the variable is a restricted path or not
+         * @private
+         */
+        function isRestrictedPath(name) {
+            return Object.prototype.hasOwnProperty.call(restrictedPathMessages, name);
+        }
+
         return {
             ImportDeclaration(node) {
                 if (node && node.source && node.source.value) {
-
                     const importName = node.source.value.trim();
 
-                    if (restrictedPaths.has(importName)) {
-                        context.report({
-                            node,
-                            message: "'{{importName}}' import is restricted from being used.",
-                            data: { importName }
-                        });
+                    if (isRestrictedPath(importName)) {
+                        reportPath(node);
                     }
                     if (restrictedPatterns.length > 0 && ig.ignores(importName)) {
                         context.report({
                             node,
                             message: "'{{importName}}' import is restricted from being used by a pattern.",
-                            data: { importName }
+                            data: {
+                                importName
+                            }
                         });
                     }
                 }
diff --git a/lib/rules/no-restricted-modules.js b/lib/rules/no-restricted-modules.js
index 3a9634de9..cd4797573 100644
--- a/lib/rules/no-restricted-modules.js
+++ b/lib/rules/no-restricted-modules.js
@@ -4,6 +4,13 @@
  */
 "use strict";
 
+//------------------------------------------------------------------------------
+// Helpers
+//------------------------------------------------------------------------------
+
+const DEFAULT_MESSAGE_TEMPLATE = "'{{moduleName}}' module is restricted from being used.";
+const CUSTOM_MESSAGE_TEMPLATE = "'{{moduleName}}' module is restricted from being used. {{customMessage}}";
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -11,9 +18,29 @@
 const ignore = require("ignore");
 
 const arrayOfStrings = {
+    type: "array",
+    items: { type: "string" },
+    uniqueItems: true
+};
+
+const arrayOfStringsOrObjects = {
     type: "array",
     items: {
-        type: "string"
+        anyOf: [
+            { type: "string" },
+            {
+                type: "object",
+                properties: {
+                    name: { type: "string" },
+                    message: {
+                        type: "string",
+                        minLength: 1
+                    }
+                },
+                additionalProperties: false,
+                required: ["name"]
+            }
+        ]
     },
     uniqueItems: true
 };
@@ -28,17 +55,17 @@ module.exports = {
 
         schema: {
             anyOf: [
-                arrayOfStrings,
+                arrayOfStringsOrObjects,
                 {
                     type: "array",
-                    items: [{
+                    items: {
                         type: "object",
                         properties: {
-                            paths: arrayOfStrings,
+                            paths: arrayOfStringsOrObjects,
                             patterns: arrayOfStrings
                         },
                         additionalProperties: false
-                    }],
+                    },
                     additionalItems: false
                 }
             ]
@@ -47,17 +74,30 @@ module.exports = {
 
     create(context) {
         const options = Array.isArray(context.options) ? context.options : [];
-        const isStringArray = typeof options[0] !== "object";
-        const restrictedPaths = new Set(isStringArray ? context.options : options[0].paths || []);
-        const restrictedPatterns = isStringArray ? [] : options[0].patterns || [];
+        const isPathAndPatternsObject =
+            typeof options[0] === "object" &&
+            (options[0].hasOwnProperty("paths") || options[0].hasOwnProperty("patterns"));
+
+        const restrictedPaths = (isPathAndPatternsObject ? options[0].paths : context.options) || [];
+        const restrictedPatterns = (isPathAndPatternsObject ? options[0].patterns : []) || [];
+
+        const restrictedPathMessages = restrictedPaths.reduce((memo, importName) => {
+            if (typeof importName === "string") {
+                memo[importName] = null;
+            } else {
+                memo[importName.name] = importName.message;
+            }
+            return memo;
+        }, {});
 
         // if no imports are restricted we don"t need to check
-        if (restrictedPaths.size === 0 && restrictedPatterns.length === 0) {
+        if (Object.keys(restrictedPaths).length === 0 && restrictedPatterns.length === 0) {
             return {};
         }
 
         const ig = ignore().add(restrictedPatterns);
 
+
         /**
          * Function to check if a node is a string literal.
          * @param {ASTNode} node The node to check.
@@ -76,6 +116,39 @@ module.exports = {
             return node.callee.type === "Identifier" && node.callee.name === "require";
         }
 
+        /**
+         * Report a restricted path.
+         * @param {node} node representing the restricted path reference
+         * @returns {void}
+         * @private
+         */
+        function reportPath(node) {
+            const moduleName = node.arguments[0].value.trim();
+            const customMessage = restrictedPathMessages[moduleName];
+            const message = customMessage
+                ? CUSTOM_MESSAGE_TEMPLATE
+                : DEFAULT_MESSAGE_TEMPLATE;
+
+            context.report({
+                node,
+                message,
+                data: {
+                    moduleName,
+                    customMessage
+                }
+            });
+        }
+
+        /**
+         * Check if the given name is a restricted path name
+         * @param {string} name name of a variable
+         * @returns {boolean} whether the variable is a restricted path or not
+         * @private
+         */
+        function isRestrictedPath(name) {
+            return Object.prototype.hasOwnProperty.call(restrictedPathMessages, name);
+        }
+
         return {
             CallExpression(node) {
                 if (isRequireCall(node)) {
@@ -85,12 +158,8 @@ module.exports = {
                         const moduleName = node.arguments[0].value.trim();
 
                         // check if argument value is in restricted modules array
-                        if (restrictedPaths.has(moduleName)) {
-                            context.report({
-                                node,
-                                message: "'{{moduleName}}' module is restricted from being used.",
-                                data: { moduleName }
-                            });
+                        if (isRestrictedPath(moduleName)) {
+                            reportPath(node);
                         }
 
                         if (restrictedPatterns.length > 0 && ig.ignores(moduleName)) {


Test patch:
diff --git a/tests/lib/rules/no-restricted-imports.js b/tests/lib/rules/no-restricted-imports.js
index 444f34979..82aa2a40a 100644
--- a/tests/lib/rules/no-restricted-imports.js
+++ b/tests/lib/rules/no-restricted-imports.js
@@ -61,5 +61,41 @@ ruleTester.run("no-restricted-imports", rule, {
         code: "import withGitignores from \"foo/bar\";",
         options: [{ patterns: ["foo/*", "!foo/baz"] }],
         errors: [{ message: "'foo/bar' import is restricted from being used by a pattern.", type: "ImportDeclaration" }]
+    }, {
+        code: "import withGitignores from \"foo\";",
+        options: [{
+            name: "foo",
+            message: "Please import from 'bar' instead."
+        }],
+        errors: [{
+            message: "'foo' import is restricted from being used. Please import from 'bar' instead.",
+            type: "ImportDeclaration"
+        }]
+    }, {
+        code: "import withGitignores from \"bar\";",
+        options: [
+            "foo",
+            {
+                name: "bar",
+                message: "Please import from 'baz' instead."
+            },
+            "baz"
+        ],
+        errors: [{
+            message: "'bar' import is restricted from being used. Please import from 'baz' instead.",
+            type: "ImportDeclaration"
+        }]
+    }, {
+        code: "import withGitignores from \"foo\";",
+        options: [{
+            paths: [{
+                name: "foo",
+                message: "Please import from 'bar' instead."
+            }]
+        }],
+        errors: [{
+            message: "'foo' import is restricted from being used. Please import from 'bar' instead.",
+            type: "ImportDeclaration"
+        }]
     }]
 });
diff --git a/tests/lib/rules/no-restricted-modules.js b/tests/lib/rules/no-restricted-modules.js
index 26fa3e922..f89cbbd97 100644
--- a/tests/lib/rules/no-restricted-modules.js
+++ b/tests/lib/rules/no-restricted-modules.js
@@ -60,5 +60,41 @@ ruleTester.run("no-restricted-modules", rule, {
         code: "var withGitignores = require(\"foo/bar\");",
         options: [{ patterns: ["foo/*", "!foo/baz"], paths: ["foo"] }],
         errors: [{ message: "'foo/bar' module is restricted from being used by a pattern.", type: "CallExpression" }]
+    }, {
+        code: "var withGitignores = require(\"foo\");",
+        options: [{
+            name: "foo",
+            message: "Please use 'bar' module instead."
+        }],
+        errors: [{
+            message: "'foo' module is restricted from being used. Please use 'bar' module instead.",
+            type: "CallExpression"
+        }]
+    }, {
+        code: "var withGitignores = require(\"bar\");",
+        options: [
+            "foo",
+            {
+                name: "bar",
+                message: "Please use 'baz' module instead."
+            },
+            "baz"
+        ],
+        errors: [{
+            message: "'bar' module is restricted from being used. Please use 'baz' module instead.",
+            type: "CallExpression"
+        }]
+    }, {
+        code: "var withGitignores = require(\"foo\");",
+        options: [{
+            paths: [{
+                name: "foo",
+                message: "Please use 'bar' module instead."
+            }]
+        }],
+        errors: [{
+            message: "'foo' module is restricted from being used. Please use 'bar' module instead.",
+            type: "CallExpression"
+        }]
     }]
 });