Skip to content

Commit

Permalink
[Fix] no-import-module-exports: avoid false positives with a shadow…
Browse files Browse the repository at this point in the history
…ed `module` or `exports`

Fixes #2297
  • Loading branch information
ljharb committed Nov 9, 2021
1 parent add650a commit 332d3c8
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 3 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -9,6 +9,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
### Fixed
- [`extensions`]: ignore unresolveable type-only imports ([#2270], [#2271], [@jablko])
- `importType`: fix `isExternalModule` calculation ([#2282], [@mx-bernhard])
- [`no-import-module-exports`]: avoid false positives with a shadowed `module` or `exports` ([#2297], [@ljharb])

### Changed
- [Docs] [`order`]: add type to the default groups ([#2272], [@charpeni])
Expand Down Expand Up @@ -939,6 +940,7 @@ for info on changes for earlier releases.

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

[#2297]: https://github.com/import-js/eslint-plugin-import/pull/2297
[#2287]: https://github.com/import-js/eslint-plugin-import/pull/2287
[#2282]: https://github.com/import-js/eslint-plugin-import/pull/2282
[#2279]: https://github.com/import-js/eslint-plugin-import/pull/2279
Expand Down
13 changes: 10 additions & 3 deletions src/rules/no-import-module-exports.js
Expand Up @@ -13,6 +13,12 @@ function getEntryPoint(context) {
}
}

function findScope(context, identifier) {
const scopeManager = context.getSourceCode().scopeManager;

return scopeManager.scopes.slice().reverse().find((scope) => scope.variables.some(variable => variable.identifiers.some((node) => node.name === identifier)));
}

module.exports = {
meta: {
type: 'problem',
Expand Down Expand Up @@ -43,10 +49,11 @@ module.exports = {
const isEntryPoint = entryPoint === fileName;
const isIdentifier = node.object.type === 'Identifier';
const hasKeywords = (/^(module|exports)$/).test(node.object.name);
const isException = options.exceptions &&
options.exceptions.some(glob => minimatch(fileName, glob));
const objectScope = hasKeywords && findScope(context, node.object.name);
const hasCJSExportReference = hasKeywords && (!objectScope || objectScope.type === 'module');
const isException = !!options.exceptions && options.exceptions.some(glob => minimatch(fileName, glob));

if (isIdentifier && hasKeywords && !isEntryPoint && !isException) {
if (isIdentifier && hasCJSExportReference && !isEntryPoint && !isException) {
importDeclarations.forEach(importDeclaration => {
context.report({
node: importDeclaration,
Expand Down
50 changes: 50 additions & 0 deletions tests/src/rules/no-import-module-exports.js
Expand Up @@ -64,6 +64,56 @@ ruleTester.run('no-import-module-exports', rule, {
`,
filename: path.join(process.cwd(), 'tests/files/missing-entrypoint/cli.js'),
}),
test({
code: `
import fs from 'fs/promises';
const subscriptions = new Map();
export default async (client) => {
/**
* loads all modules and their subscriptions
*/
const modules = await fs.readdir('./src/modules');
await Promise.all(
modules.map(async (moduleName) => {
// Loads the module
const module = await import(\`./modules/\${moduleName}/module.js\`);
// skips the module, in case it is disabled.
if (module.enabled) {
// Loads each of it's subscriptions into their according list.
module.subscriptions.forEach((fun, event) => {
if (!subscriptions.has(event)) {
subscriptions.set(event, []);
}
subscriptions.get(event).push(fun);
});
}
})
);
/**
* Setting up all events.
* binds all events inside the subscriptions map to call all functions provided
*/
subscriptions.forEach((funs, event) => {
client.on(event, (...args) => {
funs.forEach(async (fun) => {
try {
await fun(client, ...args);
} catch (e) {
client.emit('error', e);
}
});
});
});
};
`,
parserOptions: {
ecmaVersion: 2020,
},
}),
],
invalid: [
test({
Expand Down

0 comments on commit 332d3c8

Please sign in to comment.