Repository: import-js/eslint-plugin-import
Pull request #2511 (2022-07-27 07:06:49)

[Fix] `no-cycle`: add ExportNamedDeclaration statements to dependencies
This PR is a fix proposal for #2461. It adds `ExportNamedDeclaration`  to dependencies in the same way as `ImportDelcaration` does. I'm not familiar with the logic here, so please review carefully.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 66389999..d54c4266 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 885801fb..bbf43cf0 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) {


Test patch:
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 00000000..df509fa5
--- /dev/null
+++ b/tests/files/cycles/es6/depth-one-reexport.js
@@ -0,0 +1,1 @@
+export { foo } from "../depth-zero";
diff --git a/tests/src/rules/no-cycle.js b/tests/src/rules/no-cycle.js
index ad29292c..233cae61 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 }],