From 116af3192ae9ab428dc332bec4a0107aa7290553 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Wed, 27 Jul 2022 08:35:52 +0200 Subject: [PATCH] [Fix] `no-cycle`: add ExportNamedDeclaration statements to dependencies Fixes #2461 --- CHANGELOG.md | 3 ++ src/ExportMap.js | 42 +++++++++++--------- tests/files/cycles/es6/depth-one-reexport.js | 1 + tests/src/rules/no-cycle.js | 5 +++ 4 files changed, 32 insertions(+), 19 deletions(-) create mode 100644 tests/files/cycles/es6/depth-one-reexport.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 66389999d..d54c4266b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange - [`no-restricted-paths`]: use `Minimatch.match` instead of `minimatch` to comply with Windows Native paths ([#2466], thanks [@AdriAt360]) - [`order`]: require with member expression could not be fixed if alphabetize.order was used ([#2490], thanks [@msvab]) - [`order`]: leave more space in rankings for consecutive path groups ([#2506], thanks [@Pearce-Ropion]) +- [`no-cycle`]: add ExportNamedDeclaration statements to dependencies ([#2511], thanks [@BenoitZugmeyer]) ### Changed - [Tests] `named`: Run all TypeScript test ([#2427], thanks [@ProdigySim]) @@ -1001,6 +1002,7 @@ for info on changes for earlier releases. [`memo-parser`]: ./memo-parser/README.md [#2531]: https://github.com/import-js/eslint-plugin-import/pull/2531 +[#2511]: https://github.com/import-js/eslint-plugin-import/pull/2511 [#2506]: https://github.com/import-js/eslint-plugin-import/pull/2506 [#2503]: https://github.com/import-js/eslint-plugin-import/pull/2503 [#2490]: https://github.com/import-js/eslint-plugin-import/pull/2490 @@ -1529,6 +1531,7 @@ for info on changes for earlier releases. [@beatrizrezener]: https://github.com/beatrizrezener [@benmosher]: https://github.com/benmosher [@benmunro]: https://github.com/benmunro +[@BenoitZugmeyer]: https://github.com/BenoitZugmeyer [@bicstone]: https://github.com/bicstone [@Blasz]: https://github.com/Blasz [@bmish]: https://github.com/bmish diff --git a/src/ExportMap.js b/src/ExportMap.js index 885801fbb..bbf43cf03 100644 --- a/src/ExportMap.js +++ b/src/ExportMap.js @@ -501,6 +501,26 @@ ExportMap.parse = function (path, content, context) { m.reexports.set(s.exported.name, { local, getImport: () => resolveImport(nsource) }); } + function captureDependencyWithSpecifiers(n) { + // import type { Foo } (TS and Flow) + const declarationIsType = n.importKind === 'type'; + // import './foo' or import {} from './foo' (both 0 specifiers) is a side effect and + // shouldn't be considered to be just importing types + let specifiersOnlyImportingTypes = n.specifiers.length > 0; + const importedSpecifiers = new Set(); + n.specifiers.forEach(specifier => { + if (specifier.type === 'ImportSpecifier') { + importedSpecifiers.add(specifier.imported.name || specifier.imported.value); + } else if (supportedImportTypes.has(specifier.type)) { + importedSpecifiers.add(specifier.type); + } + + // import { type Foo } (Flow) + specifiersOnlyImportingTypes = specifiersOnlyImportingTypes && specifier.importKind === 'type'; + }); + captureDependency(n, declarationIsType || specifiersOnlyImportingTypes, importedSpecifiers); + } + function captureDependency({ source }, isOnlyImportingTypes, importedSpecifiers = new Set()) { if (source == null) return null; @@ -587,25 +607,7 @@ ExportMap.parse = function (path, content, context) { // capture namespaces in case of later export if (n.type === 'ImportDeclaration') { - // import type { Foo } (TS and Flow) - const declarationIsType = n.importKind === 'type'; - // import './foo' or import {} from './foo' (both 0 specifiers) is a side effect and - // shouldn't be considered to be just importing types - let specifiersOnlyImportingTypes = n.specifiers.length; - const importedSpecifiers = new Set(); - n.specifiers.forEach(specifier => { - if (supportedImportTypes.has(specifier.type)) { - importedSpecifiers.add(specifier.type); - } - if (specifier.type === 'ImportSpecifier') { - importedSpecifiers.add(specifier.imported.name || specifier.imported.value); - } - - // import { type Foo } (Flow) - specifiersOnlyImportingTypes = - specifiersOnlyImportingTypes && specifier.importKind === 'type'; - }); - captureDependency(n, declarationIsType || specifiersOnlyImportingTypes, importedSpecifiers); + captureDependencyWithSpecifiers(n); const ns = n.specifiers.find(s => s.type === 'ImportNamespaceSpecifier'); if (ns) { @@ -615,6 +617,8 @@ ExportMap.parse = function (path, content, context) { } if (n.type === 'ExportNamedDeclaration') { + captureDependencyWithSpecifiers(n); + // capture declaration if (n.declaration != null) { switch (n.declaration.type) { diff --git a/tests/files/cycles/es6/depth-one-reexport.js b/tests/files/cycles/es6/depth-one-reexport.js new file mode 100644 index 000000000..df509fa51 --- /dev/null +++ b/tests/files/cycles/es6/depth-one-reexport.js @@ -0,0 +1 @@ +export { foo } from "../depth-zero"; diff --git a/tests/src/rules/no-cycle.js b/tests/src/rules/no-cycle.js index ad29292c2..233cae613 100644 --- a/tests/src/rules/no-cycle.js +++ b/tests/src/rules/no-cycle.js @@ -166,6 +166,11 @@ ruleTester.run('no-cycle', rule, { errors: [error(`Dependency cycle detected.`)], options: [{ ...opts, amd: true }], }), + test({ + code: `import { foo } from "./${testDialect}/depth-one-reexport"`, + options: [{ ...opts }], + errors: [error(`Dependency cycle detected.`)], + }), test({ code: `import { foo } from "./${testDialect}/depth-two"`, options: [{ ...opts }],