Merge pull request #204 from javierbrea/feat-no-external-custom-message

no-external rule custom error messages

.github/CONTRIBUTING.md

@@ -62,10 +62,7 @@ This document may also be subject to pull-requests or changes by contributors wh
 
 ### Git Commit Messages
 
-* Use the present tense ("Add feature" not "Added feature")
-* Use the imperative mood ("Move cursor to..." not "Moves cursor to...")
-* Limit the first line to 72 characters or less
-* Reference issues and pull requests liberally after the first line
+* Use [semmantic commit](https://gist.github.com/joshbuchea/6f47e86d2510bce28f8e7f42ae84c716)
 
 ### JavaScript Styleguide
 

CHANGELOG.md

@@ -11,6 +11,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Removed
 ### BREAKING CHANGES
 
+## [unreleased]
+
+### Added
+- feat(#170): Support custom messages in `no-external` rule config
+
+### Changed
+- docs: Minor changes to contributing guidelines
+- chore(deps): Update devDependencies
+
 ## [2.8.0] - 2021-11-28
 
 ### Added

README.md

@@ -259,9 +259,9 @@ Some rules require extra configuration, and it has to be defined in each specifi
 
 #### Main format of rules options
 
-The docs of each rule contains an specification of their own options, but __the main rules share the format in which the options have to be defined__. The format described here is valid for options of [`element-types`](docs/rules/element-types.md), [`external`](docs/rules/external.md) and [`entry-point`](docs/rules/entry-point.md) rules, __except the `message` property, which for the moment is only supported in the [`element-types`](docs/rules/element-types.md), [`entry-point`](docs/rules/entry-point.md) and [`no-private`](docs/rules/no-private.md) rules settings.
+The docs of each rule contains an specification of their own options, but __the main rules share the format in which the options have to be defined__. The format described here is valid for options of [`element-types`](docs/rules/element-types.md), [`external`](docs/rules/external.md) and [`entry-point`](docs/rules/entry-point.md) rules.
 
-Options set an "allow/disallow" value by default, and provide an array of rules. Each matching rule will override the default value and the value returned by previous matching rules. So, the final result of the options, once processed for each case, will be "allow" or "disallow", and this value will be applied by the plugin rule in the correspondant way, making it to produce an eslint error or not.
+Options set an `allow` or `disallow` value by default, and provide an array of rules. Each matching rule will override the default value and the value returned by previous matching rules. So, the final result of the options, once processed for each case, will be `allow` or `disallow`, and this value will be applied by the plugin rule in the correspondant way, making it to produce an eslint error or not.
 
 ```jsonc
 {
@@ -331,12 +331,20 @@ Available properties in error templates both from `file` or `dependency` are:
 
 * `type`: Element's type.
 * `internalPath`: File path being analyzed or imported. Relative to the element's root path.
+* `source`: Available only for `dependency`. The source of the `import` statement as it is in the code.
 * `parent`: If the element is child of another element, it is also available in this property, which contains correspondent `type`, `internalPath` and captured properties as well.
 * ...All captured properties are also available
 
-
 > Tip: Read ["Global settings"](#global-settings) for further info about how to capture values from elements.
 
+Some rules also provide extra information about the reported error. For example, `no-external` rules provides information about detected forbidden specifiers. This information is available using `${report.PROPERTY}`. Check each rule's documentation to know which report properties it provides:
+
+```jsonc
+{
+  "message": "Do not import ${report.specifiers} from ${dependency.source} in helpers"
+}
+```
+
 ##### Advanced example of a rule configuration
 
 Just to illustrate the high level of customization that the plugin supports, here is an example of advanced options for the `boundaries/element-types` rule based on the previous global `elements` settings example:

docs/rules/external.md

@@ -14,18 +14,20 @@ It checks `import` statements to external modules and allow or disallow them bas
 
 * `enabled`: for enabling the rule. 0=off, 1=warn, 2=error.
 * `default`: `allow` or `disallow`. If no one `rule` matches, the external dependency will be allowed or disallowed based on this value.
+* `message`: Custom message for the rule errors. Note that __the rule default message provides a lot of information about why the error was produced__, so you should define a custom message only if you are sure about what you are doing. Read ["error messages"](#error-messages) for further information.
 * `rules`: Rules to be processed in order to decide if the `import` statement has to be allowed or not.
   * `from`: `<element matchers>` If the file being analyzed matches with this, then the rule will be executed to know if it allows/disallows the `import`. If not, the rule is skipped.
   * `disallow`: `<external modules matchers>` If the element being imported matches with this, then the result of the rule will be "disallow", and the import will be notified as an `eslint` error (this value can be overwritten by a next rule returning "allow")
   * `allow`: `<external modules matchers>` If the element being imported matches with this, then the result of the rule will be "allow", and the import will not be notified as an `eslint` error (this value can be overwritten by a next rule returning "disallow")
+  * `message`: `<string>` Custom error message only for this rule. Read ["error messages"](#error-messages) for further info.
 
 ##### External modules matchers
 
 `allow` and `disallow` properties in the options should receive one or multiple patterns to match external libraries. Every single pattern can have next formats:
 
 * `<string>`. A [`micromatch` pattern](https://github.com/micromatch/micromatch) to match the name of the module.
 * `[<string>, <object>]`. An array containing a [`micromatch` pattern](https://github.com/micromatch/micromatch) as first element, and an options object, which can have next properties:
-  * `specifiers`: `<array>` Array of used specifiers when importing the library. Each specifier can be expressed also as a [`micromatch` pattern](https://github.com/micromatch/micromatch).
+  * `specifiers`: `<array>` Array of used specifiers when importing the library. Each specifier can be expressed also as a [`micromatch` pattern](https://github.com/micromatch/micromatch). Matching specifiers are available as `${report.specifiers}` when defining custom error messages.
 
 If `specifiers` option is provided, then it will only match if any of the specifiers is used in the `import` statement.
 
@@ -232,7 +234,16 @@ _Modules can import `useHistory` from `react-router-dom`:_
 import { useHistory } from 'react-router-dom'
 ```
 
+### Error messages
+
+This rule provides a lot of information about the specific option producing an error, so the user can have enough context to solve it.
+
+* If the error is produced because all imports are disallowed by default, and no rule is specificly allowing it, then the message provides information about the file and the external dependency: `No rule allows the usage of external module 'react' in elements of type 'helper'`.
+* If the error is produced by a specific option, then the message includes information about the option producing it: `Usage of external module 'react' is not allowed in elements of type 'helper' with elementName 'helper-a'. Disallowed in rule 2`
+* If the error is produced by a specific option including specifiers property, then the message includes information it: `Usage of 'useMemo, useEffect' from external module 'react' is not allowed in elements of type 'helper' with elementName 'helper-a'. Disallowed in rule 2`
+
+You can also configure a custom error message for changing this default behaviour, or even custom error messages only for a specific rule option. Read ["error messages"](../../README.md#error-messages) in the main docs for further info about how to configure messages.
+
 ## Further reading
 
 Read [how to configure the `boundaries/elements` setting](../../README.md#global-settings) to assign an element type to each project's file.
-

src/helpers/messages.js

@@ -80,10 +80,15 @@ function ruleElementMessage(elementPatterns, elementCapturedValues) {
 }
 
 function elementPropertiesToReplaceInTemplate(element) {
-  return { ...element.capturedValues, type: element.type, internalPath: element.internalPath };
+  return {
+    ...element.capturedValues,
+    type: element.type,
+    internalPath: element.internalPath,
+    source: element.source,
+  };
 }
 
-function customErrorMessage(message, file, dependency) {
+function customErrorMessage(message, file, dependency, report = {}) {
   let replacedMessage = replaceObjectValuesInTemplates(
     replaceObjectValuesInTemplates(message, elementPropertiesToReplaceInTemplate(file), "file"),
     elementPropertiesToReplaceInTemplate(dependency),
@@ -103,7 +108,7 @@ function customErrorMessage(message, file, dependency) {
       "dependency.parent"
     );
   }
-  return replacedMessage;
+  return replaceObjectValuesInTemplates(replacedMessage, report, "report");
 }
 
 function elementCapturedValuesMessage(capturedValues) {

src/helpers/validations.js

@@ -42,12 +42,15 @@ function elementsMatcherSchema(matcherOptions = DEFAULT_MATCHER_OPTIONS) {
   };
 }
 
-function rulesOptionsSchema(options) {
+function rulesOptionsSchema(options = {}) {
   const mainKey = rulesMainKey(options.rulesMainKey);
-  const schema = [
+  return [
     {
       type: "object",
       properties: {
+        message: {
+          type: "string",
+        },
         default: {
           type: "string",
           enum: ["allow", "disallow"],
@@ -60,6 +63,9 @@ function rulesOptionsSchema(options) {
               [mainKey]: elementsMatcherSchema(),
               allow: elementsMatcherSchema(options.targetMatcherOptions),
               disallow: elementsMatcherSchema(options.targetMatcherOptions),
+              message: {
+                type: "string",
+              },
             },
             additionalProperties: false,
             anyOf: [
@@ -79,17 +85,6 @@ function rulesOptionsSchema(options) {
       additionalProperties: false,
     },
   ];
-  if (options.customMessage) {
-    schema[0].properties.message = {
-      type: "string",
-    };
-  }
-  if (options.customRuleMessage) {
-    schema[0].properties.rules.items.properties.message = {
-      type: "string",
-    };
-  }
-  return schema;
 }
 
 function isValidElementTypesMatcher(matcher, settings) {

src/rules/element-types.js

@@ -42,10 +42,7 @@ module.exports = dependencyRule(
   {
     ruleName: RULE_ELEMENT_TYPES,
     description: `Check allowed dependencies between element types`,
-    schema: rulesOptionsSchema({
-      customMessage: true,
-      customRuleMessage: true,
-    }),
+    schema: rulesOptionsSchema(),
   },
   function ({ dependency, file, node, context, options }) {
     if (dependency.isLocal && !dependency.isIgnored && dependency.type && !dependency.isInternal) {

src/rules/entry-point.js

@@ -46,8 +46,6 @@ module.exports = dependencyRule(
     description: `Check entry point used for each element type`,
     schema: rulesOptionsSchema({
       rulesMainKey: "target",
-      customMessage: true,
-      customRuleMessage: true,
     }),
   },
   function ({ dependency, file, node, context, options }) {

src/rules/external.js

@@ -6,6 +6,7 @@ const dependencyRule = require("../rules-factories/dependency-rule");
 
 const { rulesOptionsSchema } = require("../helpers/validations");
 const { dependencyLocation, elementRulesAllowDependency } = require("../helpers/rules");
+const { customErrorMessage, ruleElementMessage, elementMessage } = require("../helpers/messages");
 
 function specifiersMatch(specifiers, options) {
   const importedSpecifiersNames = specifiers
@@ -44,6 +45,32 @@ function elementRulesAllowExternalDependency(element, dependency, options) {
   });
 }
 
+function errorMessage(ruleData, file, dependency) {
+  const ruleReport = ruleData.ruleReport;
+  if (ruleReport.message) {
+    return customErrorMessage(ruleReport.message, file, dependency, {
+      specifiers: ruleData.report && ruleData.report.join(", "),
+    });
+  }
+  if (ruleReport.isDefault) {
+    return `No rule allows the usage of external module '${
+      dependency.baseModule
+    }' in elements ${elementMessage(file)}`;
+  }
+
+  const fileReport = `is not allowed in ${ruleElementMessage(
+    ruleReport.element,
+    file.capturedValues
+  )}. Disallowed in rule ${ruleReport.index + 1}`;
+
+  if (ruleData.report) {
+    return `Usage of '${ruleData.report.join(", ")}' from external module '${
+      dependency.baseModule
+    }' ${fileReport}`;
+  }
+  return `Usage of external module '${dependency.baseModule}' ${fileReport}`;
+}
+
 module.exports = dependencyRule(
   {
     ruleName: RULE_EXTERNAL,
@@ -65,27 +92,17 @@ module.exports = dependencyRule(
   },
   function ({ dependency, file, node, context, options }) {
     if (dependency.isExternal) {
-      const isAllowed = elementRulesAllowExternalDependency(
+      const ruleData = elementRulesAllowExternalDependency(
         file,
         { ...dependency, specifiers: node.source.parent.specifiers },
         options
       );
-      if (!isAllowed.result) {
-        if (isAllowed.report) {
-          context.report({
-            message: `Usage of '${isAllowed.report.join(", ")}' from external module '${
-              dependency.baseModule
-            }' is not allowed in '${file.type}'`,
-            node: node,
-            ...dependencyLocation(node, context),
-          });
-        } else {
-          context.report({
-            message: `Usage of external module '${dependency.baseModule}' is not allowed in '${file.type}'`,
-            node: node,
-            ...dependencyLocation(node, context),
-          });
-        }
+      if (!ruleData.result) {
+        context.report({
+          message: errorMessage(ruleData, file, dependency),
+          node: node,
+          ...dependencyLocation(node, context),
+        });
       }
     }
   },

test/rules/docs-examples/external.spec.js

@@ -1,13 +1,11 @@
 const { EXTERNAL: RULE } = require("../../../src/constants/rules");
 const { SETTINGS, createRuleTester, pathResolvers } = require("../../support/helpers");
+const { externalNoRuleMessage } = require("../../support/messages");
 
 const rule = require(`../../../src/rules/${RULE}`);
 
 const { absoluteFilePath } = pathResolvers("docs-examples");
 
-const errorMessage = (elementType, dependencyName) =>
-  `Usage of external module '${dependencyName}' is not allowed in '${elementType}'`;
-
 const settings = SETTINGS.docsExamples;
 
 const options = [
@@ -96,7 +94,25 @@ ruleTester.run(RULE, rule, {
       options,
       errors: [
         {
-          message: errorMessage("helpers", "react"),
+          message: externalNoRuleMessage({
+            file: "'helpers' with category 'data' and elementName 'parse'",
+            dep: "react",
+          }),
+          type: "ImportDeclaration",
+        },
+      ],
+    },
+    // Helpers can't import specifier from react
+    {
+      filename: absoluteFilePath("helpers/data/parse.js"),
+      code: "import { useMemo } from 'react'",
+      options,
+      errors: [
+        {
+          message: externalNoRuleMessage({
+            file: "'helpers' with category 'data' and elementName 'parse'",
+            dep: "react",
+          }),
           type: "ImportDeclaration",
         },
       ],
@@ -108,7 +124,10 @@ ruleTester.run(RULE, rule, {
       options,
       errors: [
         {
-          message: errorMessage("components", "moment"),
+          message: externalNoRuleMessage({
+            file: "'components' with family 'atoms' and elementName 'atom-a'",
+            dep: "moment",
+          }),
           type: "ImportDeclaration",
         },
       ],
@@ -120,7 +139,8 @@ ruleTester.run(RULE, rule, {
       options,
       errors: [
         {
-          message: errorMessage("components", "@material-ui/icons"),
+          message:
+            "Usage of external module '@material-ui/icons' is not allowed in elements of type 'components' with family 'molecules'. Disallowed in rule 3",
           type: "ImportDeclaration",
         },
       ],
@@ -132,7 +152,10 @@ ruleTester.run(RULE, rule, {
       options,
       errors: [
         {
-          message: errorMessage("modules", "react-router-dom"),
+          message: externalNoRuleMessage({
+            file: "'modules' with elementName 'module-a'",
+            dep: "react-router-dom",
+          }),
           type: "ImportDeclaration",
         },
       ],