mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
fix(linter/import/no-duplicates): distinguish import attributes (#26936)
Treat imports with different attribute keys or values as distinct in `import/no-duplicates`, without adding an option. This follows the upstream maintainer's guidance in https://github.com/import-js/eslint-plugin-import/issues/3290#issuecomment-5771071222; the current upstream release (2.32.0) and main branch still have the same false positive. Group imports by normalized attributes before applying the existing import-kind checks. Attribute order and quoted keys do not matter, and an empty attribute clause matches an absent clause. Regression tests cover the Bun macro example, custom attributes, genuine duplicates, and `prefer-inline` interactions. Fixes #26871.
This commit is contained in:
camc314 committed
1 parent
feb733b5a1
commit
cce28a0f72
2 files changed
+191
-69
No files matched your search
@@ -73,6 +73,7 @@ declare_oxc_lint!(
|
||||
/// ### What it does
|
||||
///
|
||||
/// Reports if a resolved path is imported more than once in the same module.
|
||||
/// Imports with different import attributes are treated as distinct modules.
|
||||
/// This helps avoid unnecessary duplicate imports and keeps the code clean.
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
@@ -151,88 +152,116 @@ impl Rule for NoDuplicates {
|
||||
.into_iter()
|
||||
.flat_map(|(_path, requested_modules)| requested_modules)
|
||||
.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<u8, Vec<&RequestedModule>> =
|
||||
FxHashMap::default();
|
||||
// Side-effect and top-level type-only imports cannot be merged directly. A runtime
|
||||
// import with specifiers (including inline type specifiers) can absorb either.
|
||||
let mut key0_side_effect_imports = Vec::new();
|
||||
let mut key0_type_only_imports = Vec::new();
|
||||
let mut key0_has_runtime_specifiers = false;
|
||||
for requested_module in requested_modules {
|
||||
let import_entries = module_record
|
||||
.import_entries
|
||||
.iter()
|
||||
.filter(|entry| entry.module_request.span == requested_module.span)
|
||||
let requested_modules = requested_modules.collect::<Vec<_>>();
|
||||
if requested_modules.len() <= 1 {
|
||||
continue;
|
||||
}
|
||||
|
||||
let declarations =
|
||||
import_declarations.get_or_insert_with(|| build_import_declarations(ctx));
|
||||
// Attribute order and key spelling do not affect module identity. An empty
|
||||
// clause is equivalent to having no attributes.
|
||||
let mut attribute_groups = FxHashMap::<_, Vec<_>>::default();
|
||||
for module in requested_modules {
|
||||
let mut attributes = declarations
|
||||
.get(&module.statement_span)
|
||||
.and_then(|decl| decl.with_clause.as_ref())
|
||||
.into_iter()
|
||||
.flat_map(|clause| &clause.with_entries)
|
||||
.map(|attribute| {
|
||||
(attribute.key.as_arena_str().as_str(), attribute.value.value.as_str())
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
if import_entries.is_empty() {
|
||||
let key = u8::from(requested_module.is_type && !self.prefer_inline);
|
||||
import_entries_maps.entry(key).or_default().push(requested_module);
|
||||
if key == 0 {
|
||||
attributes.sort_unstable();
|
||||
attribute_groups.entry(attributes).or_default().push(module);
|
||||
}
|
||||
|
||||
for (_, requested_modules) in
|
||||
attribute_groups.into_iter().sorted_by(|(a, _), (b, _)| a.cmp(b))
|
||||
{
|
||||
// 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<u8, Vec<&RequestedModule>> =
|
||||
FxHashMap::default();
|
||||
// Side-effect and top-level type-only imports cannot be merged directly. A runtime
|
||||
// import with specifiers (including inline type specifiers) can absorb either.
|
||||
let mut key0_side_effect_imports = Vec::new();
|
||||
let mut key0_type_only_imports = Vec::new();
|
||||
let mut key0_has_runtime_specifiers = false;
|
||||
for requested_module in requested_modules {
|
||||
let import_entries = module_record
|
||||
.import_entries
|
||||
.iter()
|
||||
.filter(|entry| entry.module_request.span == requested_module.span)
|
||||
.collect::<Vec<_>>();
|
||||
if import_entries.is_empty() {
|
||||
let key = u8::from(requested_module.is_type && !self.prefer_inline);
|
||||
import_entries_maps.entry(key).or_default().push(requested_module);
|
||||
if key == 0 {
|
||||
if requested_module.is_type {
|
||||
key0_type_only_imports.push(requested_module);
|
||||
} else {
|
||||
key0_side_effect_imports.push(requested_module);
|
||||
}
|
||||
}
|
||||
continue;
|
||||
}
|
||||
let mut flags = [true; 4];
|
||||
let mut pushed_to_key0 = false;
|
||||
for import_entry in import_entries {
|
||||
let key = if import_entry.is_type {
|
||||
match import_entry.import_name {
|
||||
ImportImportName::Name(_) => u8::from(!self.prefer_inline),
|
||||
ImportImportName::NamespaceObject => 2,
|
||||
ImportImportName::Default(_) => 3,
|
||||
}
|
||||
} else {
|
||||
match import_entry.import_name {
|
||||
ImportImportName::NamespaceObject => 2,
|
||||
_ => 0,
|
||||
}
|
||||
};
|
||||
|
||||
if flags[key as usize] {
|
||||
flags[key as usize] = false;
|
||||
import_entries_maps.entry(key).or_default().push(requested_module);
|
||||
if key == 0 {
|
||||
pushed_to_key0 = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
if pushed_to_key0 {
|
||||
if requested_module.is_type {
|
||||
key0_type_only_imports.push(requested_module);
|
||||
} else {
|
||||
key0_side_effect_imports.push(requested_module);
|
||||
key0_has_runtime_specifiers = true;
|
||||
}
|
||||
}
|
||||
continue;
|
||||
}
|
||||
let mut flags = [true; 4];
|
||||
let mut pushed_to_key0 = false;
|
||||
for import_entry in import_entries {
|
||||
let key = if import_entry.is_type {
|
||||
match import_entry.import_name {
|
||||
ImportImportName::Name(_) => u8::from(!self.prefer_inline),
|
||||
ImportImportName::NamespaceObject => 2,
|
||||
ImportImportName::Default(_) => 3,
|
||||
}
|
||||
} else {
|
||||
match import_entry.import_name {
|
||||
ImportImportName::NamespaceObject => 2,
|
||||
_ => 0,
|
||||
}
|
||||
};
|
||||
|
||||
if flags[key as usize] {
|
||||
flags[key as usize] = false;
|
||||
import_entries_maps.entry(key).or_default().push(requested_module);
|
||||
if key == 0 {
|
||||
pushed_to_key0 = true;
|
||||
}
|
||||
for i in 0..4 {
|
||||
if i == 0 && !key0_has_runtime_specifiers {
|
||||
check_duplicates(
|
||||
ctx,
|
||||
&mut import_declarations,
|
||||
self.prefer_inline,
|
||||
Some(&key0_side_effect_imports),
|
||||
);
|
||||
check_duplicates(
|
||||
ctx,
|
||||
&mut import_declarations,
|
||||
self.prefer_inline,
|
||||
Some(&key0_type_only_imports),
|
||||
);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
if pushed_to_key0 {
|
||||
if requested_module.is_type {
|
||||
key0_type_only_imports.push(requested_module);
|
||||
} else {
|
||||
key0_has_runtime_specifiers = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for i in 0..4 {
|
||||
if i == 0 && !key0_has_runtime_specifiers {
|
||||
check_duplicates(
|
||||
ctx,
|
||||
&mut import_declarations,
|
||||
self.prefer_inline,
|
||||
Some(&key0_side_effect_imports),
|
||||
import_entries_maps.get(&i),
|
||||
);
|
||||
check_duplicates(
|
||||
ctx,
|
||||
&mut import_declarations,
|
||||
self.prefer_inline,
|
||||
Some(&key0_type_only_imports),
|
||||
);
|
||||
continue;
|
||||
}
|
||||
check_duplicates(
|
||||
ctx,
|
||||
&mut import_declarations,
|
||||
self.prefer_inline,
|
||||
import_entries_maps.get(&i),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -690,6 +719,33 @@ fn test() {
|
||||
(r"import type {} from './foo'; import './foo';", None),
|
||||
(r"import type {} from './foo'; import './foo';", Some(json!([{ "preferInline": true }]))),
|
||||
(r"import { RouterModule, Routes } from '@angular/router';", None),
|
||||
(
|
||||
r#"import { compile } from "../src/macro.ts" with { type: "macro" };
|
||||
import { compile as compileNoMacro } from "../src/macro.ts";"#,
|
||||
None,
|
||||
),
|
||||
(
|
||||
r"import {x} from './foo' with { type: 'json' }; import {y} from './foo' with { type: 'css' };",
|
||||
None,
|
||||
),
|
||||
(
|
||||
r"import {x} from './foo' with { mode: 'one' }; import {y} from './foo' with { mode: 'two' };",
|
||||
None,
|
||||
),
|
||||
(
|
||||
r"import {x} from './foo' with { mode: 'one' }; import {y} from './foo' with { other: 'one' };",
|
||||
None,
|
||||
),
|
||||
(
|
||||
r"import {x} from './foo' with { type: 'json' }; import {y} from './foo' with { type: 'json', mode: 'one' };",
|
||||
None,
|
||||
),
|
||||
(r"import './foo' with { type: 'json' }; import './foo';", None),
|
||||
(r"import * as x from './foo' with { type: 'json' }; import * as y from './foo';", None),
|
||||
(
|
||||
r"import './foo'; import type { Foo } from './foo'; import { Bar } from './foo' with { type: 'macro' };",
|
||||
Some(json!([{ "prefer-inline": true }])),
|
||||
),
|
||||
];
|
||||
|
||||
let fail = vec![
|
||||
@@ -932,6 +988,24 @@ fn test() {
|
||||
r"import { type Foo } from './foo'; import './foo'",
|
||||
Some(json!([{ "preferInline": true }])),
|
||||
),
|
||||
(
|
||||
r"import {x} from './foo' with { type: 'json' }; import {y} from './foo' with { type: 'json' };",
|
||||
None,
|
||||
),
|
||||
(
|
||||
r"import {x} from './foo' with { type: 'json', mode: 'one' }; import {y} from './foo' with { 'mode': 'one', 'type': 'json' };",
|
||||
None,
|
||||
),
|
||||
(r"import {x} from './foo' with {}; import {y} from './foo';", None),
|
||||
(
|
||||
r"import {x} from './foo' with { type: 'json' }; import {y} from './foo' with { type: 'css' }; import {z} from './foo' with { type: 'json' };",
|
||||
None,
|
||||
),
|
||||
(r"import './foo' with { mode: 'one' }; import './foo' with { mode: 'one' };", None),
|
||||
(
|
||||
r"import * as x from './foo' with { mode: 'one' }; import * as y from './foo' with { mode: 'one' };",
|
||||
None,
|
||||
),
|
||||
];
|
||||
|
||||
let fix = vec![
|
||||
@@ -1042,8 +1116,8 @@ import type {Bar // comment
|
||||
),
|
||||
// Import attributes can change module semantics and must not be discarded by a merge.
|
||||
(
|
||||
r"import {x} from './foo' with { type: 'json' }; import {y} from './foo' with { type: 'css' }",
|
||||
r"import {x} from './foo' with { type: 'json' }; import {y} from './foo' with { type: 'css' }",
|
||||
r"import {x} from './foo' with { type: 'json' }; import {y} from './foo' with { type: 'json' }",
|
||||
r"import {x} from './foo' with { type: 'json' }; import {y} from './foo' with { type: 'json' }",
|
||||
None,
|
||||
),
|
||||
];
|
||||
|
||||
@@ -620,3 +620,51 @@ source: crates/oxc_linter/src/tester.rs
|
||||
· ╰── It is first imported here
|
||||
╰────
|
||||
help: Merge these imports into a single import statement
|
||||
|
||||
⚠ import(no-duplicates): Module './foo' is imported more than once in this file
|
||||
╭─[index.ts:1:17]
|
||||
1 │ import {x} from './foo' with { type: 'json' }; import {y} from './foo' with { type: 'json' };
|
||||
· ───┬─── ───────
|
||||
· ╰── It is first imported here
|
||||
╰────
|
||||
help: Merge these imports into a single import statement
|
||||
|
||||
⚠ import(no-duplicates): Module './foo' is imported more than once in this file
|
||||
╭─[index.ts:1:17]
|
||||
1 │ import {x} from './foo' with { type: 'json', mode: 'one' }; import {y} from './foo' with { 'mode': 'one', 'type': 'json' };
|
||||
· ───┬─── ───────
|
||||
· ╰── It is first imported here
|
||||
╰────
|
||||
help: Merge these imports into a single import statement
|
||||
|
||||
⚠ import(no-duplicates): Module './foo' is imported more than once in this file
|
||||
╭─[index.ts:1:17]
|
||||
1 │ import {x} from './foo' with {}; import {y} from './foo';
|
||||
· ───┬─── ───────
|
||||
· ╰── It is first imported here
|
||||
╰────
|
||||
help: Merge these imports into a single import statement
|
||||
|
||||
⚠ import(no-duplicates): Module './foo' is imported more than once in this file
|
||||
╭─[index.ts:1:17]
|
||||
1 │ import {x} from './foo' with { type: 'json' }; import {y} from './foo' with { type: 'css' }; import {z} from './foo' with { type: 'json' };
|
||||
· ───┬─── ───────
|
||||
· ╰── It is first imported here
|
||||
╰────
|
||||
help: Merge these imports into a single import statement
|
||||
|
||||
⚠ import(no-duplicates): Module './foo' is imported more than once in this file
|
||||
╭─[index.ts:1:8]
|
||||
1 │ import './foo' with { mode: 'one' }; import './foo' with { mode: 'one' };
|
||||
· ───┬─── ───────
|
||||
· ╰── It is first imported here
|
||||
╰────
|
||||
help: Merge these imports into a single import statement
|
||||
|
||||
⚠ import(no-duplicates): Module './foo' is imported more than once in this file
|
||||
╭─[index.ts:1:20]
|
||||
1 │ import * as x from './foo' with { mode: 'one' }; import * as y from './foo' with { mode: 'one' };
|
||||
· ───┬─── ───────
|
||||
· ╰── It is first imported here
|
||||
╰────
|
||||
help: Merge these imports into a single import statement
|
||||
Reference in new issue
Block a user