fix(linter/import): import/no-duplicates handles namespace imports correctly (#6694)

Closes #6660

- Value (i.e. not `import type`) namespace imports are now grouped correctly
- When grouping, consider _all_ import entries, not just the first one. Fixes a false negative when inlined `type` imports are used, e.g. `import { a, type b } from 'foo'`

crates/oxc_linter/src/rules/import/no_duplicates.rs

@@ -5,6 +5,7 @@ use oxc_diagnostics::{LabeledSpan, OxcDiagnostic};
 use oxc_macros::declare_oxc_lint;
 use oxc_span::Span;
 use oxc_syntax::module_record::{ImportImportName, RequestedModule};
+use rustc_hash::FxHashMap;
 
 use crate::{context::LintContext, rule::Rule};
 
@@ -55,11 +56,20 @@ declare_oxc_lint!(
     /// ```javascript
     /// import { foo } from './module';
     /// import { bar } from './module';
+    ///
+    /// import a from './module';
+    /// import { b } from './module';
     /// ```
     ///
     /// Examples of **correct** code for this rule:
-    /// ```javascript
+    /// ```typescript
     /// import { foo, bar } from './module';
+    ///
+    /// import * as a from 'foo'; // separate statements for namespace imports
+    /// import { b } from 'foo';
+    ///
+    /// import { c } from 'foo';      // separate type imports, unless
+    /// import type { d } from 'foo'; // `preferInline` is true
     /// ```
     NoDuplicates,
     suspicious
@@ -91,33 +101,39 @@ impl Rule for NoDuplicates {
             .chunk_by(|r| r.0.clone());
 
         for (_path, group) in &groups {
-            let has_type_import = module_record.import_entries.iter().any(|entry| entry.is_type);
+            let requested_modules = group
-            // When prefer_inline is false, 0 is value, 1 is type named, 2 is type namespace and 3 is type default
-            // When prefer_inline is true, 0 is value and type named, 2 is type namespace and 3 is type default
-            let import_entries_maps = group
                 .into_iter()
                 .flat_map(|(_path, requested_modules)| requested_modules)
-                .filter(|requested_module| requested_module.is_import())
+                .filter(|requested_module| requested_module.is_import());
-                .into_group_map_by(|requested_module| {
+            // When prefer_inline is false, 0 is value, 1 is type named, 2 is type namespace and 3 is type default
-                    // We should early return if there is no type import
+            // When prefer_inline is true, 0 is value and type named, 2 is type // namespace and 3 is type default
-                    if !has_type_import {
+            let mut import_entries_maps: FxHashMap<i8, Vec<&RequestedModule>> =
-                        return 0;
+                FxHashMap::default();
+            for requested_module in requested_modules {
+                let imports = module_record
+                    .import_entries
+                    .iter()
+                    .filter(|entry| entry.module_request.span() == requested_module.span())
+                    .collect::<Vec<_>>();
+                if imports.is_empty() {
+                    import_entries_maps.entry(0).or_default().push(requested_module);
+                }
+                for imports in imports {
+                    let key = if imports.is_type {
+                        match imports.import_name {
+                            ImportImportName::Name(_) => i8::from(!self.prefer_inline),
+                            ImportImportName::NamespaceObject => 2,
+                            ImportImportName::Default(_) => 3,
+                        }
+                    } else {
+                        match imports.import_name {
+                            ImportImportName::NamespaceObject => 2,
+                            _ => 0,
+                        }
                     };
-                    for entry in &module_record.import_entries {
+                    import_entries_maps.entry(key).or_default().push(requested_module);
-                        if entry.module_request.span() != requested_module.span() {
+                }
-                            continue;
+            }
-                        }
-
-                        if entry.is_type {
-                            return match entry.import_name {
-                                ImportImportName::Name(_) => i8::from(!self.prefer_inline),
-                                ImportImportName::NamespaceObject => 2,
-                                ImportImportName::Default(_) => 3,
-                            };
-                        }
-                    }
-                    0
-                });
 
             for i in 0..4 {
                 check_duplicates(ctx, import_entries_maps.get(&i));
@@ -147,13 +163,13 @@ fn test() {
         (r#"import "./malformed.js""#, None),
         (r"import { x } from './foo'; import { y } from './bar'", None),
         (r#"import foo from "234artaf"; import { shoop } from "234q25ad""#, None),
-        // r#"import { x } from './foo'; import type { y } from './foo'"#,
+        (r"import { x } from './foo'; import type { y } from './foo'", None),
         // TODO: considerQueryString
         // r#"import x from './bar?optionX'; import y from './bar?optionY';"#,
         (r"import x from './foo'; import y from './bar';", None),
         // TODO: separate namespace
-        // (r"import * as ns from './foo'; import { y } from './foo'", None),
+        (r"import * as ns from './foo'; import { y } from './foo'", None),
-        // (r"import { y } from './foo'; import * as ns from './foo'", None),
+        (r"import { y } from './foo'; import * as ns from './foo'", None),
         // TypeScript
         (r"import type { x } from './foo'; import y from './foo'", None),
         (r"import type x from './foo'; import type y from './bar'", None),

crates/oxc_linter/src/snapshots/no_duplicates.snap

@@ -209,7 +209,7 @@ source: crates/oxc_linter/src/tester.rs
   ⚠ eslint-plugin-import(no-duplicates): Module './foo' is imported more than once in this file
    ╭─[index.ts:1:8]
  1 │ import './foo'; import def, {x} from './foo'
-   ·        ───┬───                       ───────
+   ·        ───┬───                       ────────
    ·           ╰── It is first imported here
    ╰────
   help: Merge these imports into a single import statement
@@ -249,7 +249,7 @@ source: crates/oxc_linter/src/tester.rs
   ⚠ eslint-plugin-import(no-duplicates): Module './foo' is imported more than once in this file
    ╭─[index.ts:1:17]
  1 │ import {x} from './foo'; import def, {y} from './foo'
-   ·                 ───┬───                       ───────
+   ·                 ───┬───                       ────────
    ·                    ╰── It is first imported here
    ╰────
   help: Merge these imports into a single import statement
@@ -263,17 +263,17 @@ source: crates/oxc_linter/src/tester.rs
   help: Merge these imports into a single import statement
 
   ⚠ eslint-plugin-import(no-duplicates): Module './foo' is imported more than once in this file
-   ╭─[index.ts:1:21]
+   ╭─[index.ts:1:46]
  1 │ import * as ns from './foo'; import {x} from './foo'; import {y} from './foo'
-   ·                     ───┬───                  ───────                  ───────
+   ·                                              ───┬───                  ───────
-   ·                        ╰── It is first imported here
+   ·                                                 ╰── It is first imported here
    ╰────
   help: Merge these imports into a single import statement
 
   ⚠ eslint-plugin-import(no-duplicates): Module './foo' is imported more than once in this file
    ╭─[index.ts:1:17]
  1 │ import {x} from './foo'; import * as ns from './foo'; import {y} from './foo'; import './foo'
-   ·                 ───┬───                      ───────                  ───────         ───────
+   ·                 ───┬───                                               ───────         ───────
    ·                    ╰── It is first imported here
    ╰────
   help: Merge these imports into a single import statement
@@ -540,6 +540,14 @@ source: crates/oxc_linter/src/tester.rs
    ╰────
   help: Merge these imports into a single import statement
 
+  ⚠ eslint-plugin-import(no-duplicates): Module './foo' is imported more than once in this file
+   ╭─[index.ts:1:38]
+ 1 │ import {AValue, type x, BValue} from './foo'; import {type y} from './foo'
+   ·                                      ───┬────
+   ·                                         ╰── It is first imported here
+   ╰────
+  help: Merge these imports into a single import statement
+
   ⚠ eslint-plugin-import(no-duplicates): Module './foo' is imported more than once in this file
    ╭─[index.ts:1:38]
  1 │ import {AValue, type x, BValue} from './foo'; import {type y} from './foo'