From b02fbb655db1ebf7e4bc46dbe6f81548519706ef Mon Sep 17 00:00:00 2001 From: Maja Wichrowska Date: Fri, 13 Oct 2017 21:27:59 -0700 Subject: [PATCH] Update: custom messages for no-restricted-* (refs #8400) 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. --- docs/rules/no-restricted-imports.md | 22 +++++ docs/rules/no-restricted-modules.md | 22 +++++ lib/rules/no-restricted-imports.js | 103 +++++++++++++++++++---- lib/rules/no-restricted-modules.js | 99 ++++++++++++++++++---- tests/lib/rules/no-restricted-imports.js | 36 ++++++++ tests/lib/rules/no-restricted-modules.js | 36 ++++++++ 6 files changed, 286 insertions(+), 32 deletions(-) diff --git a/docs/rules/no-restricted-imports.md b/docs/rules/no-restricted-imports.md index 856b8f2cce4..420ce4e589c 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 e2f0dd2bf81..9a19e8ef5d9 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 c245f22a0a4..d46b098acee 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 3a9634de9e1..cd47975733d 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)) { diff --git a/tests/lib/rules/no-restricted-imports.js b/tests/lib/rules/no-restricted-imports.js index 444f3497921..82aa2a40aa5 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 26fa3e922b5..f89cbbd97ca 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" + }] }] });