Skip to content

Commit

Permalink
[Fix] order: ignore non-module-level requires
Browse files Browse the repository at this point in the history
Fixes #1936.
  • Loading branch information
golopot authored and ljharb committed Nov 10, 2020
1 parent 6cd1fe1 commit 957092a
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 23 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -8,6 +8,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel

### Fixed
- [`export`]/TypeScript: properly detect export specifiers as children of a TS module block ([#1889], thanks [@andreubotella])
- [`order`]: ignore non-module-level requires ([#1940], thanks [@golopot])

## [2.22.1] - 2020-09-27
### Fixed
Expand Down Expand Up @@ -735,6 +736,7 @@ for info on changes for earlier releases.

[`memo-parser`]: ./memo-parser/README.md

[#1940]: https://github.com/benmosher/eslint-plugin-import/pull/1940
[#1889]: https://github.com/benmosher/eslint-plugin-import/pull/1889
[#1878]: https://github.com/benmosher/eslint-plugin-import/pull/1878
[#1854]: https://github.com/benmosher/eslint-plugin-import/issues/1854
Expand Down
38 changes: 16 additions & 22 deletions src/rules/order.js
Expand Up @@ -333,9 +333,21 @@ function registerNode(context, importEntry, ranks, imported, excludedImportTypes
}
}

function isInVariableDeclarator(node) {
return node &&
(node.type === 'VariableDeclarator' || isInVariableDeclarator(node.parent))
function isModuleLevelRequire(node) {
let n = node
// Handle cases like `const baz = require('foo').bar.baz`
// and `const foo = require('foo')()`
while (
(n.parent.type === 'MemberExpression' && n.parent.object === n) ||
(n.parent.type === 'CallExpression' && n.parent.callee === n)
) {
n = n.parent
}
return (
n.parent.type === 'VariableDeclarator' &&
n.parent.parent.type === 'VariableDeclaration' &&
n.parent.parent.parent.type === 'Program'
)
}

const types = ['builtin', 'external', 'internal', 'unknown', 'parent', 'sibling', 'index', 'object']
Expand Down Expand Up @@ -583,14 +595,6 @@ module.exports = {
}
}
let imported = []
let level = 0

function incrementLevel() {
level++
}
function decrementLevel() {
level--
}

return {
ImportDeclaration: function handleImports(node) {
Expand Down Expand Up @@ -641,7 +645,7 @@ module.exports = {
)
},
CallExpression: function handleRequires(node) {
if (level !== 0 || !isStaticRequire(node) || !isInVariableDeclarator(node.parent)) {
if (!isStaticRequire(node) || !isModuleLevelRequire(node)) {
return
}
const name = node.arguments[0].value
Expand Down Expand Up @@ -671,16 +675,6 @@ module.exports = {

imported = []
},
FunctionDeclaration: incrementLevel,
FunctionExpression: incrementLevel,
ArrowFunctionExpression: incrementLevel,
BlockStatement: incrementLevel,
ObjectExpression: incrementLevel,
'FunctionDeclaration:exit': decrementLevel,
'FunctionExpression:exit': decrementLevel,
'ArrowFunctionExpression:exit': decrementLevel,
'BlockStatement:exit': decrementLevel,
'ObjectExpression:exit': decrementLevel,
}
},
}
14 changes: 13 additions & 1 deletion tests/src/rules/order.js
Expand Up @@ -74,7 +74,7 @@ ruleTester.run('order', rule, {
var result = add(1, 2);
var _ = require('lodash');`,
}),
// Ignore requires that are not at the top-level
// Ignore requires that are not at the top-level #1
test({
code: `
var index = require('./');
Expand All @@ -86,6 +86,18 @@ ruleTester.run('order', rule, {
require('fs');
}`,
}),
// Ignore requires that are not at the top-level #2
test({
code: `
const foo = [
require('./foo'),
require('fs'),
]`,
}),
// Ignore requires in template literal (#1936)
test({
code: "const foo = `${require('./a')} ${require('fs')}`",
}),
// Ignore unknown/invalid cases
test({
code: `
Expand Down

0 comments on commit 957092a

Please sign in to comment.