[New] no-internal-modules: Add forbid option

CHANGELOG.md

@@ -8,6 +8,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 
 ### Added
 - [`no-commonjs`]: Also detect require calls with expressionless template literals: ``` require(`x`) ``` ([#1958], thanks [@FloEdelmann])
+- [`no-internal-modules`]: Add `forbid` option ([#1846], thanks [@guillaumewuip])
 
 ### Fixed
 - [`export`]/TypeScript: properly detect export specifiers as children of a TS module block ([#1889], thanks [@andreubotella])
@@ -765,6 +766,7 @@ for info on changes for earlier releases.
 [#1878]: https://github.com/benmosher/eslint-plugin-import/pull/1878
 [#1854]: https://github.com/benmosher/eslint-plugin-import/issues/1854
 [#1848]: https://github.com/benmosher/eslint-plugin-import/pull/1848
+[#1846]: https://github.com/benmosher/eslint-plugin-import/pull/1846
 [#1841]: https://github.com/benmosher/eslint-plugin-import/issues/1841
 [#1836]: https://github.com/benmosher/eslint-plugin-import/pull/1836
 [#1835]: https://github.com/benmosher/eslint-plugin-import/pull/1835
@@ -1321,4 +1323,5 @@ for info on changes for earlier releases.
 [@MatthiasKunnen]: https://github.com/MatthiasKunnen
 [@paztis]: https://github.com/paztis
 [@FloEdelmann]: https://github.com/FloEdelmann
-[@bicstone]: https://github.com/bicstone
+[@bicstone]: https://github.com/bicstone
+[@guillaumewuip]: https://github.com/guillaumewuip

docs/rules/no-internal-modules.md

@@ -4,7 +4,10 @@ Use this rule to prevent importing the submodules of other modules.
 
 ## Rule Details
 
-This rule has one option, `allow` which is an array of [minimatch/glob patterns](https://github.com/isaacs/node-glob#glob-primer) patterns that whitelist paths and import statements that can be imported with reaching.
+This rule has two mutally exclusive options that are arrays of [minimatch/glob patterns](https://github.com/isaacs/node-glob#glob-primer) patterns:
+
+- `allow` that include paths and import statements that can be imported with reaching.
+- `forbid` that exclude paths and import statements that can be imported with reaching.
 
 ### Examples
 
@@ -33,7 +36,7 @@ And the .eslintrc file:
   ...
   "rules": {
     "import/no-internal-modules": [ "error", {
-      "allow": [ "**/actions/*", "source-map-support/*" ]
+      "allow": [ "**/actions/*", "source-map-support/*" ],
     } ]
   }
 }
@@ -68,3 +71,62 @@ import getUser from '../actions/getUser';
 export * from 'source-map-support/register';
 export { settings } from '../app';
 ```
+
+Given the following folder structure:
+
+```
+my-project
+├── actions
+│   └── getUser.js
+│   └── updateUser.js
+├── reducer
+│   └── index.js
+│   └── user.js
+├── redux
+│   └── index.js
+│   └── configureStore.js
+└── app
+│   └── index.js
+│   └── settings.js
+└── entry.js
+```
+
+And the .eslintrc file:
+```
+{
+  ...
+  "rules": {
+    "import/no-internal-modules": [ "error", {
+      "forbid": [ "**/actions/*", "source-map-support/*" ],
+    } ]
+  }
+}
+```
+
+The following patterns are considered problems:
+
+```js
+/**
+ *  in my-project/entry.js
+ */
+
+import 'source-map-support/register';
+import getUser from '../actions/getUser';
+
+export * from 'source-map-support/register';
+export getUser from '../actions/getUser';
+```
+
+The following patterns are NOT considered problems:
+
+```js
+/**
+ *  in my-project/entry.js
+ */
+
+import 'source-map-support';
+import { getUser } from '../actions';
+
+export * from 'source-map-support';
+export { getUser } from '../actions';
+```

src/rules/no-internal-modules.js

@@ -14,38 +14,49 @@ module.exports = {
 
     schema: [
       {
-        type: 'object',
-        properties: {
-          allow: {
-            type: 'array',
-            items: {
-              type: 'string',
+        oneOf: [
+          {
+            type: 'object',
+            properties: {
+              allow: {
+                type: 'array',
+                items: {
+                  type: 'string',
+                },
+              },
             },
+            additionalProperties: false,
           },
-        },
-        additionalProperties: false,
+          {
+            type: 'object',
+            properties: {
+              forbid: {
+                type: 'array',
+                items: {
+                  type: 'string',
+                },
+              },
+            },
+            additionalProperties: false,
+          },
+        ],
       },
     ],
   },
 
   create: function noReachingInside(context) {
     const options = context.options[0] || {};
     const allowRegexps = (options.allow || []).map(p => minimatch.makeRe(p));
-
-    // test if reaching to this destination is allowed
-    function reachingAllowed(importPath) {
-      return allowRegexps.some(re => re.test(importPath));
-    }
+    const forbidRegexps = (options.forbid || []).map(p => minimatch.makeRe(p));
 
     // minimatch patterns are expected to use / path separators, like import
     // statements, so normalize paths to use the same
     function normalizeSep(somePath) {
       return somePath.split('\\').join('/');
     }
 
-    // find a directory that is being reached into, but which shouldn't be
-    function isReachViolation(importPath) {
-      const steps = normalizeSep(importPath)
+    function toSteps(somePath) {
+      return  normalizeSep(somePath)
         .split('/')
         .reduce((acc, step) => {
           if (!step || step === '.') {
@@ -56,6 +67,20 @@ module.exports = {
             return acc.concat(step);
           }
         }, []);
+    }
+
+    // test if reaching to this destination is allowed
+    function reachingAllowed(importPath) {
+      return allowRegexps.some(re => re.test(importPath));
+    }
+
+    // test if reaching to this destination is forbidden
+    function reachingForbidden(importPath) {
+      return forbidRegexps.some(re => re.test(importPath));
+    }
+
+    function isAllowViolation(importPath) {
+      const steps = toSteps(importPath);
 
       const nonScopeSteps = steps.filter(step => step.indexOf('@') !== 0);
       if (nonScopeSteps.length <= 1) return false;
@@ -75,6 +100,27 @@ module.exports = {
       return true;
     }
 
+    function isForbidViolation(importPath) {
+      const steps = toSteps(importPath);
+
+      // before trying to resolve, see if the raw import (with relative
+      // segments resolved) matches a forbidden pattern
+      const justSteps = steps.join('/');
+
+      if (reachingForbidden(justSteps) || reachingForbidden(`/${justSteps}`)) return true;
+
+      // if the import statement doesn't match directly, try to match the
+      // resolved path if the import is resolvable
+      const resolved = resolve(importPath, context);
+      if (resolved && reachingForbidden(normalizeSep(resolved))) return true;
+
+      // this import was not forbidden by the forbidden paths so it is not a violation
+      return false;
+    }
+
+    // find a directory that is being reached into, but which shouldn't be
+    const isReachViolation = options.forbid ? isForbidViolation : isAllowViolation;
+
     function checkImportForReaching(importPath, node) {
       const potentialViolationTypes = ['parent', 'index', 'sibling', 'external', 'internal'];
       if (potentialViolationTypes.indexOf(importType(importPath, context)) !== -1 &&

tests/src/rules/no-internal-modules.js

@@ -59,6 +59,34 @@ ruleTester.run('no-internal-modules', rule, {
         allow: [ '**/index{.js,}' ],
       } ],
     }),
+    test({
+      code: 'import a from "./plugin2/thing"',
+      filename: testFilePath('./internal-modules/plugins/plugin.js'),
+      options: [ {
+        forbid: [ '**/api/*' ],
+      } ],
+    }),
+    test({
+      code: 'const a = require("./plugin2/thing")',
+      filename: testFilePath('./internal-modules/plugins/plugin.js'),
+      options: [ {
+        forbid: [ '**/api/*' ],
+      } ],
+    }),
+    test({
+      code: 'import b from "app/a"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/internal.js'),
+      options: [ {
+        forbid: [ 'app/**/**' ],
+      } ],
+    }),
+    test({
+      code: 'import b from "@org/package"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/internal.js'),
+      options: [ {
+        forbid: [ '@org/package/*' ],
+      } ],
+    }),
     // exports
     test({
       code: 'export {a} from "./internal.js"',
@@ -114,6 +142,34 @@ ruleTester.run('no-internal-modules', rule, {
         parser: parser,
       }),
     ]),
+    test({
+      code: 'export * from "./plugin2/thing"',
+      filename: testFilePath('./internal-modules/plugins/plugin.js'),
+      options: [ {
+        forbid: [ '**/api/*' ],
+      } ],
+    }),
+    test({
+      code: 'export * from "app/a"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/internal.js'),
+      options: [ {
+        forbid: [ 'app/**/**' ],
+      } ],
+    }),
+    test({
+      code: 'export { b } from "@org/package"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/internal.js'),
+      options: [ {
+        forbid: [ '@org/package/*' ],
+      } ],
+    }),
+    test({
+      code: 'export * from "./app/index.js";\nexport * from "./app/index"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/internal.js'),
+      options: [ {
+        forbid: [ '**/index.ts' ],
+      } ],
+    }),
   ],
 
   invalid: [
@@ -184,6 +240,70 @@ ruleTester.run('no-internal-modules', rule, {
         },
       ],
     }),
+    test({
+      code: 'import "./app/index.js"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/internal.js'),
+      options: [ {
+        forbid: [ '*/app/*' ],
+      } ],
+      errors: [ {
+        message: 'Reaching to "./app/index.js" is not allowed.',
+        line: 1,
+        column: 8,
+      } ],
+    }),
+    test({
+      code: 'import b from "@org/package"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/internal.js'),
+      options: [ {
+        forbid: [ '@org/**' ],
+      } ],
+      errors: [ {
+        message: 'Reaching to "@org/package" is not allowed.',
+        line: 1,
+        column: 15,
+      } ],
+    }),
+    test({
+      code: 'import b from "app/a/b"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/internal.js'),
+      options: [ {
+        forbid: [ 'app/**/**' ],
+      } ],
+      errors: [ {
+        message: 'Reaching to "app/a/b" is not allowed.',
+        line: 1,
+        column: 15,
+      } ],
+    }),
+    test({
+      code: 'import get from "lodash.get"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/index.js'),
+      options: [ {
+        forbid: [ 'lodash.*' ],
+      } ],
+      errors: [ {
+        message: 'Reaching to "lodash.get" is not allowed.',
+        line: 1,
+        column: 17,
+      } ],
+    }),
+    test({
+      code: 'import "./app/index.js";\nimport "./app/index"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/internal.js'),
+      options: [ {
+        forbid: [ '**/index{.js,}' ],
+      } ],
+      errors: [ {
+        message: 'Reaching to "./app/index.js" is not allowed.',
+        line: 1,
+        column: 8,
+      }, {
+        message: 'Reaching to "./app/index" is not allowed.',
+        line: 2,
+        column: 8,
+      } ],
+    }),
     // exports
     test({
       code: 'export * from "./plugin2/index.js";\nexport * from "./plugin2/app/index"',
@@ -251,5 +371,33 @@ ruleTester.run('no-internal-modules', rule, {
         },
       ],
     }),
+    test({
+      code: 'export * from "./plugin2/thing"',
+      filename: testFilePath('./internal-modules/plugins/plugin.js'),
+      options: [ {
+        forbid: [ '**/plugin2/*' ],
+      } ],
+      errors: [
+        {
+          message: 'Reaching to "./plugin2/thing" is not allowed.',
+          line: 1,
+          column: 15,
+        },
+      ],
+    }),
+    test({
+      code: 'export * from "app/a"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/internal.js'),
+      options: [ {
+        forbid: [ '**' ],
+      } ],
+      errors: [
+        {
+          message: 'Reaching to "app/a" is not allowed.',
+          line: 1,
+          column: 15,
+        },
+      ],
+    }),
   ],
 });