diff --git a/CHANGELOG.md b/CHANGELOG.md index 74561f836..e52231bb7 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index 97194dcfe..83e71665a 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ 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 @@ -27,6 +28,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a [`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:** diff --git a/docs/rules/no-dynamic-require.md b/docs/rules/no-dynamic-require.md new file mode 100644 index 000000000..3f9764878 --- /dev/null +++ b/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`); +``` diff --git a/src/index.js b/src/index.js index 406d53ee5..2583e89a6 100644 --- a/src/index.js +++ b/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'), diff --git a/src/rules/no-dynamic-require.js b/src/rules/no-dynamic-require.js new file mode 100644 index 000000000..8a0efae6c --- /dev/null +++ b/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', + }) + } + }, + } +} diff --git a/tests/src/rules/no-dynamic-require.js b/tests/src/rules/no-dynamic-require.js new file mode 100644 index 000000000..8793d0dd8 --- /dev/null +++ b/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], + }), + ], +})