From 5abbb0cadae5b1a516eb965be0c4524d1e893d3d Mon Sep 17 00:00:00 2001
From: John Daly <john.daly2@gmail.com>
Date: Mon, 8 Apr 2024 21:38:15 -0700
Subject: [PATCH] fix(linter): import/no-cycle ignore type-only imports (#2924)

A fix for the work done in: https://github.com/oxc-project/oxc/pull/2905

The existing code checks if _all_ import entries in a ModuleRecord are
type-only, when determining if the ModuleRecord should be skipped when
checking for cycles. This is incorrect.

To demonstrate the problem: Running the `no-cycles` rule, with
`ignoreTypes` enabled, on the following example code will cause a cycle
to be reported between Module A and Module B

```typescript
// Module A
import type { Bar } from './b';
import { Baz } from './c'

// Module B
import type { Foo } from './a';

// Module C
export const Baz = 'baz';
```

The reason this is happening is because the import to Module C in Module
A is causing the `was_imported_as_type` variable to evaluate to `false`,
since the Module C import is not type-only.

What we actually want to do is skip visiting Module B entirely, by
checking if its import request from Module A is type-only.

This PR fixes the logic so that only the import entries for the next
ModuleRecord are considered when determining `was_imported_as_type`.
---
 .../ts-depth-type-and-value-imports.ts          |  5 +++++
 .../import/cycles/typescript/ts-value.ts        |  1 +
 crates/oxc_linter/src/rules/import/no_cycle.rs  | 17 +++++++++++++----
 3 files changed, 19 insertions(+), 4 deletions(-)
 create mode 100644 crates/oxc_linter/fixtures/import/cycles/typescript/ts-depth-type-and-value-imports.ts
 create mode 100644 crates/oxc_linter/fixtures/import/cycles/typescript/ts-value.ts

diff --git a/crates/oxc_linter/fixtures/import/cycles/typescript/ts-depth-type-and-value-imports.ts b/crates/oxc_linter/fixtures/import/cycles/typescript/ts-depth-type-and-value-imports.ts
new file mode 100644
index 000000000..38056d729
--- /dev/null
+++ b/crates/oxc_linter/fixtures/import/cycles/typescript/ts-depth-type-and-value-imports.ts
@@ -0,0 +1,5 @@
+// @ts-ignore
+import { foo } from './ts-value';
+
+// @ts-ignore
+import type { foo as FooType } from '../depth-zero';
diff --git a/crates/oxc_linter/fixtures/import/cycles/typescript/ts-value.ts b/crates/oxc_linter/fixtures/import/cycles/typescript/ts-value.ts
new file mode 100644
index 000000000..3329a7d97
--- /dev/null
+++ b/crates/oxc_linter/fixtures/import/cycles/typescript/ts-value.ts
@@ -0,0 +1 @@
+export const foo = 'foo';
diff --git a/crates/oxc_linter/src/rules/import/no_cycle.rs b/crates/oxc_linter/src/rules/import/no_cycle.rs
index 87a3ca01b..268bb9e4f 100644
--- a/crates/oxc_linter/src/rules/import/no_cycle.rs
+++ b/crates/oxc_linter/src/rules/import/no_cycle.rs
@@ -147,10 +147,15 @@ impl NoCycle {
 
         for module_record_ref in &module_record.loaded_modules {
             let resolved_absolute_path = &module_record_ref.resolved_absolute_path;
-            let was_imported_as_type =
-                &module_record_ref.import_entries.iter().all(|entry| entry.is_type);
-            if self.ignore_types && *was_imported_as_type {
-                continue;
+            if self.ignore_types {
+                let was_imported_as_type = &module_record
+                    .import_entries
+                    .iter()
+                    .filter(|entry| entry.module_request.name() == module_record_ref.key())
+                    .all(|entry| entry.is_type);
+                if *was_imported_as_type {
+                    continue;
+                }
             }
             if !state.traversed.insert(resolved_absolute_path.clone()) {
                 continue;
@@ -229,6 +234,10 @@ fn test() {
             r#"import { foo } from "./typescript/ts-types-depth-two";"#,
             Some(json!([{"ignoreTypes":true}])),
         ),
+        (
+            r#"import { foo } from "./typescript/ts-depth-type-and-value-imports";"#,
+            Some(json!([{"ignoreTypes":true}])),
+        ),
         // Flow not supported
         // (r#"import { bar } from "./flow-types""#, None),
         // (r#"import { bar } from "./flow-types-only-importing-type""#, None),