mirror of
https://github.com/danbulant/oxc
synced 2026-05-19 04:08:41 +00:00
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'`
This commit is contained in:
DonIsaac
parent
23f88b3e18
commit
a5de2309bc
2 changed files with 58 additions and 34 deletions
|
|
@ -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);
|
||||
// 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
|
||||
let requested_modules = group
|
||||
.into_iter()
|
||||
.flat_map(|(_path, requested_modules)| requested_modules)
|
||||
.filter(|requested_module| requested_module.is_import())
|
||||
.into_group_map_by(|requested_module| {
|
||||
// We should early return if there is no type import
|
||||
if !has_type_import {
|
||||
return 0;
|
||||
.filter(|requested_module| requested_module.is_import());
|
||||
// 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 mut import_entries_maps: FxHashMap<i8, Vec<&RequestedModule>> =
|
||||
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 {
|
||||
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
|
||||
});
|
||||
import_entries_maps.entry(key).or_default().push(requested_module);
|
||||
}
|
||||
}
|
||||
|
||||
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 { y } from './foo'; import * as ns 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),
|
||||
// TypeScript
|
||||
(r"import type { x } from './foo'; import y from './foo'", None),
|
||||
(r"import type x from './foo'; import type y from './bar'", None),
|
||||
|
|
|
|||
|
|
@ -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'
|
||||
|
|
|
|||
Loading…
Reference in a new issue