Implement new rule: no-internal-modules (#485)

* Implement new rule: no-reaching-inside

* normalize path segments all path segments to '/'

* point to minimatch/glob docs to define glob syntax

* refactor no-reaching-inside, allow now points to importable modules/files

* add test to verify that "no-reaching/allow" can limit the depth of matches

* code style improvements

* rename to no-internal-modules

* remove extra test executor

Author: spalger

docs/rules/no-internal-modules.md

@@ -0,0 +1,64 @@
+# no-internal-modules
+
+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.
+
+### Examples
+
+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", {
+      "allow": [ "**/actions/*", "source-map-support/*" ]
+    } ]
+  }
+}
+```
+
+The following patterns are considered problems:
+
+```js
+/**
+ *  in my-project/entry.js
+ */
+
+import { settings } from './app/index'; // Reaching to "./app/index" is not allowed
+import userReducer from './reducer/user'; // Reaching to "./reducer/user" is not allowed
+import configureStore from './redux/configureStore'; // Reaching to "./redux/configureStore" is not allowed
+```
+
+The following patterns are NOT considered problems:
+
+```js
+/**
+ *  in my-project/entry.js
+ */
+
+import 'source-map-support/register';
+import { settings } from '../app';
+import getUser from '../actions/getUser';
+```

src/index.js

@@ -8,6 +8,7 @@ export const rules = {
   'no-mutable-exports': require('./rules/no-mutable-exports'),
   'extensions': require('./rules/extensions'),
   'no-restricted-paths': require('./rules/no-restricted-paths'),
+  'no-internal-modules': require('./rules/no-internal-modules'),
 
   'no-named-as-default': require('./rules/no-named-as-default'),
   'no-named-as-default-member': require('./rules/no-named-as-default-member'),

src/rules/no-internal-modules.js

@@ -0,0 +1,92 @@
+import find from 'lodash.find'
+import minimatch from 'minimatch'
+
+import resolve from '../core/resolve'
+import importType from '../core/importType'
+import isStaticRequire from '../core/staticRequire'
+
+module.exports = 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 !!find(allowRegexps, re => re.test(importPath))
+  }
+
+  // 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)
+      .split('/')
+      .reduce((acc, step) => {
+        if (!step || step === '.') {
+          return acc
+        } else if (step === '..') {
+          return acc.slice(0, -1)
+        } else {
+          return acc.concat(step)
+        }
+      }, [])
+
+    if (steps.length <= 1) return false
+
+    // before trying to resolve, see if the raw import (with relative
+    // segments resolved) matches an allowed pattern
+    const justSteps = steps.join('/')
+    if (reachingAllowed(justSteps) || reachingAllowed(`/${justSteps}`)) return false
+
+    // 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 || reachingAllowed(normalizeSep(resolved))) return false
+
+    // this import was not allowed by the allowed paths, and reaches
+    // so it is a violation
+    return true
+  }
+
+  function checkImportForReaching(importPath, node) {
+    const potentialViolationTypes = ['parent', 'index', 'sibling', 'external', 'internal']
+    if (potentialViolationTypes.indexOf(importType(importPath, context)) !== -1 &&
+      isReachViolation(importPath)
+    ) {
+      context.report({
+        node,
+        message: `Reaching to "${importPath}" is not allowed.`,
+      })
+    }
+  }
+
+  return {
+    ImportDeclaration(node) {
+      checkImportForReaching(node.source.value, node.source)
+    },
+    CallExpression(node) {
+      if (isStaticRequire(node)) {
+        const [ firstArgument ] = node.arguments
+        checkImportForReaching(firstArgument.value, firstArgument)
+      }
+    },
+  }
+}
+
+module.exports.schema = [
+  {
+    type: 'object',
+    properties: {
+      allow: {
+        type: 'array',
+        items: {
+          type: 'string',
+        },
+      },
+    },
+    additionalProperties: false,
+  },
+]

tests/files/internal-modules/api/service/index.js

@@ -9,6 +9,7 @@
   },
   "dependencies": {
     "@scope/core": "^1.0.0",
+    "jquery": "^3.1.0",
     "lodash.cond": "^4.3.0",
     "pkg-up": "^1.0.0"
   },

tests/files/internal-modules/plugins/plugin.js

@@ -0,0 +1,115 @@
+import { RuleTester } from 'eslint'
+import rule from 'rules/no-internal-modules'
+
+import { test, testFilePath } from '../utils'
+
+const ruleTester = new RuleTester()
+
+ruleTester.run('no-internal-modules', rule, {
+  valid: [
+    test({
+      code: 'import a from "./plugin2"',
+      filename: testFilePath('./internal-modules/plugins/plugin.js'),
+      options: [],
+    }),
+    test({
+      code: 'const a = require("./plugin2")',
+      filename: testFilePath('./internal-modules/plugins/plugin.js'),
+    }),
+    test({
+      code: 'const a = require("./plugin2/")',
+      filename: testFilePath('./internal-modules/plugins/plugin.js'),
+    }),
+    test({
+      code: 'const dynamic = "./plugin2/"; const a = require(dynamic)',
+      filename: testFilePath('./internal-modules/plugins/plugin.js'),
+    }),
+    test({
+      code: 'import b from "./internal.js"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/index.js'),
+    }),
+    test({
+      code: 'import get from "lodash.get"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/index.js'),
+    }),
+    test({
+      code: 'import b from "../../api/service"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/internal.js'),
+      options: [ {
+        allow: [ '**/api/*' ],
+      } ],
+    }),
+    test({
+      code: 'import "jquery/dist/jquery"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/internal.js'),
+      options: [ {
+        allow: [ 'jquery/dist/*' ],
+      } ],
+    }),
+    test({
+      code: 'import "./app/index.js";\nimport "./app/index"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/internal.js'),
+      options: [ {
+        allow: [ '**/index{.js,}' ],
+      } ],
+    }),
+  ],
+
+  invalid: [
+    test({
+      code: 'import "./plugin2/index.js";\nimport "./plugin2/app/index"',
+      filename: testFilePath('./internal-modules/plugins/plugin.js'),
+      options: [ {
+        allow: [ '*/index.js' ],
+      } ],
+      errors: [ {
+        message: 'Reaching to "./plugin2/app/index" is not allowed.',
+        line: 2,
+        column: 8,
+      } ],
+    }),
+    test({
+      code: 'import "./app/index.js"',
+      filename: testFilePath('./internal-modules/plugins/plugin2/internal.js'),
+      errors: [ {
+        message: 'Reaching to "./app/index.js" is not allowed.',
+        line: 1,
+        column: 8,
+      } ],
+    }),
+    test({
+      code: 'import b from "./plugin2/internal"',
+      filename: testFilePath('./internal-modules/plugins/plugin.js'),
+      errors: [ {
+        message: 'Reaching to "./plugin2/internal" is not allowed.',
+        line: 1,
+        column: 15,
+      } ],
+    }),
+    test({
+      code: 'import a from "../api/service/index"',
+      filename: testFilePath('./internal-modules/plugins/plugin.js'),
+      options: [ {
+        allow: [ '**/internal-modules/*' ],
+      } ],
+      errors: [
+        {
+          message: 'Reaching to "../api/service/index" is not allowed.',
+          line: 1,
+          column: 15,
+        },
+      ],
+    }),
+    test({
+      code: 'import get from "debug/node"',
+      filename: testFilePath('./internal-modules/plugins/plugin.js'),
+      errors: [
+        {
+          message: 'Reaching to "debug/node" is not allowed.',
+          line: 1,
+          column: 17,
+        },
+      ],
+    }),
+  ],
+})