Add no-dynamic-require rule (fixes #567) (#568)

CHANGELOG.md

@@ -5,6 +5,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 
 ## [Unreleased]
 ### Added
+- New rule [`no-dynamic-require`]: restrict deep package imports to specific folders. ([#567])
 - New rule [`no-internal-modules`]: restrict deep package imports to specific folders. ([#485], thanks [@spalger]!)
 - [`extensions`]: allow override of a chosen default with options object ([#555], thanks [@ljharb]!)
 
@@ -311,7 +312,9 @@ for info on changes for earlier releases.
 [`no-absolute-path`]: ./docs/rules/no-absolute-path.md
 [`max-dependencies`]: ./docs/rules/max-dependencies.md
 [`no-internal-modules`]: ./docs/rules/no-internal-modules.md
+[`no-dynamic-require`]: ./docs/rules/no-dynamic-require.md
 
+[#567]: https://github.com/benmosher/eslint-plugin-import/pull/567
 [#555]: https://github.com/benmosher/eslint-plugin-import/pull/555
 [#538]: https://github.com/benmosher/eslint-plugin-import/pull/538
 [#527]: https://github.com/benmosher/eslint-plugin-import/pull/527

README.md

@@ -20,13 +20,15 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
 * Ensure imported namespaces contain dereferenced properties as they are dereferenced. ([`namespace`])
 * Restrict which files can be imported in a given folder ([`no-restricted-paths`])
 * Forbid import of modules using absolute paths ([`no-absolute-path`])
+* Forbid `require()` calls with expressions ([`no-dynamic-require`])
 
 [`no-unresolved`]: ./docs/rules/no-unresolved.md
 [`named`]: ./docs/rules/named.md
 [`default`]: ./docs/rules/default.md
 [`namespace`]: ./docs/rules/namespace.md
 [`no-restricted-paths`]: ./docs/rules/no-restricted-paths.md
 [`no-absolute-path`]: ./docs/rules/no-absolute-path.md
+[`no-dynamic-require`]: ./docs/rules/no-dynamic-require.md
 
 **Helpful warnings:**
 

docs/rules/no-dynamic-require.md

@@ -0,0 +1,24 @@
+# Forbid `require()` calls with expressions
+
+The `require` method from CommonJS is used to import modules from different files. Unlike the ES6 `import` syntax, it can be given expressions that will be resolved at runtime. While this is sometimes necessary and useful, in most cases it isn't. Using expressions (for instance, concatenating a path and variable) as the argument makes it harder for tools to do static code analysis, or to find where in the codebase a module is used.
+
+This rule checks every call to `require()` that uses expressions for the module name argument.
+
+## Rule Details
+
+### Fail
+
+```js
+require(name);
+require('../' + name);
+require(`../${name}`);
+require(name());
+```
+
+### Pass
+
+```js
+require('../name');
+require('../name' + name);
+require(`../name`);
+```

src/index.js

@@ -24,6 +24,7 @@ export const rules = {
   'order': require('./rules/order'),
   'newline-after-import': require('./rules/newline-after-import'),
   'prefer-default-export': require('./rules/prefer-default-export'),
+  'no-dynamic-require': require('./rules/no-dynamic-require'),
 
   // metadata-based
   'no-deprecated': require('./rules/no-deprecated'),

src/rules/no-dynamic-require.js

@@ -0,0 +1,25 @@
+function isRequire(node) {
+  return node &&
+    node.callee &&
+    node.callee.type === 'Identifier' &&
+    node.callee.name === 'require' &&
+    node.arguments.length >= 1
+}
+
+function isStaticValue(arg) {
+  return arg.type === 'Literal' ||
+    (arg.type === 'TemplateLiteral' && arg.expressions.length === 0)
+}
+
+module.exports = function (context) {
+  return {
+    CallExpression(node) {
+      if (isRequire(node) && !isStaticValue(node.arguments[0])) {
+        context.report({
+          node,
+          message: 'Calls to require() should use string literals',
+        })
+      }
+    },
+  }
+}

tests/src/rules/no-dynamic-require.js

@@ -0,0 +1,49 @@
+import { test } from '../utils'
+
+import { RuleTester } from 'eslint'
+
+const ruleTester = new RuleTester()
+    , rule = require('rules/no-dynamic-require')
+
+const error = {
+  ruleId: 'no-dynamic-require',
+  message: 'Calls to require() should use string literals',
+}
+
+ruleTester.run('no-dynamic-require', rule, {
+  valid: [
+    test({ code: 'import _ from "lodash"'}),
+    test({ code: 'require("foo")'}),
+    test({ code: 'require(`foo`)'}),
+    test({ code: 'require("./foo")'}),
+    test({ code: 'require("@scope/foo")'}),
+    test({ code: 'require()'}),
+    test({ code: 'require("./foo", "bar" + "okay")'}),
+    test({ code: 'var foo = require("foo")'}),
+    test({ code: 'var foo = require(`foo`)'}),
+    test({ code: 'var foo = require("./foo")'}),
+    test({ code: 'var foo = require("@scope/foo")'}),
+  ],
+  invalid: [
+    test({
+      code: 'require("../" + name)',
+      errors: [error],
+    }),
+    test({
+      code: 'require(`../${name}`)',
+      errors: [error],
+    }),
+    test({
+      code: 'require(name)',
+      errors: [error],
+    }),
+    test({
+      code: 'require(name())',
+      errors: [error],
+    }),
+    test({
+      code: 'require(name + "foo", "bar")',
+      errors: [error],
+    }),
+  ],
+})