Skip to content

refactor(linter): improve eslint/no-duplicate-imports #9627

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Boshen merged 4 commits into from
Mar 10, 2025
Merged

refactor(linter): improve eslint/no-duplicate-imports #9627

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9638c03
Refactor if let into let else
therewillbecode Mar 9, 2025
c96be0e
Document options for no-duplicate-imports
therewillbecode Mar 9, 2025
a515299
New line
therewillbecode Mar 9, 2025
5850916
Better formatting
therewillbecode Mar 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
183 changes: 113 additions & 70 deletions crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,19 @@ pub struct NoDuplicateImports {

declare_oxc_lint!(
/// ### What it does
/// Disallow duplicate module imports
///
/// Disallow duplicate module imports.
///
/// ### Why is this bad?
/// Using a single import statement per module will make the code clearer because you can see everything being imported from that module on one line.
/// Using a single import statement per module will make the code clearer because you can see
/// everything being imported from that module on one line.
///
/// ### Examples
///
/// Examples of **incorrect** code for this rule:
///
/// In the following example the module import on line 1 is repeated on line 3. These can be
/// combined to make the list of imports more succinct.
/// ```js
/// import { merge } from 'module';
/// import something from 'another-module';
Expand All @@ -54,6 +59,43 @@ declare_oxc_lint!(
/// import { merge, find } from 'module';
/// import something from 'another-module';
/// ```
///
/// ### Options
///
/// #### includeExports
///
/// `{ "includeExports": boolean }`
///
/// When `true` this rule will also look at exports to see if there is both a re-export of a
/// module as in `export ... from 'module'` and also a standard import statement for the same
/// module. This would count as a rule violation because there are in a sense two statements
/// importing from the same module.
///
/// Examples of **incorrect** when this rule is set to `true`
/// ```js
/// import { merge } from 'module';
///
/// export { find } from 'module'; // re-export which is an import and an export.
/// ```
///
/// Examples of **correct** when this rule is set to `true`
///
/// If re-exporting from an imported module, you should add the imports to the
/// `import` statement, and export that directly, not use `export ... from`.
/// ```js
/// import { merge } from "lodash-es";
/// export { merge as lodashMerge }
/// ```
///
/// ```js
/// import { merge, find } from 'module';
///
/// // cannot be merged with the above import
/// export * as something from 'module';
///
/// // cannot be written differently
/// export * from 'module';
/// ```
NoDuplicateImports,
eslint,
style,
Expand Down Expand Up @@ -153,88 +195,70 @@ impl Rule for NoDuplicateImports {

if self.include_exports {
for entry in &module_record.star_export_entries {
if let Some(module_request) = &entry.module_request {
let source = &module_request.name;
let span = entry.span;

if entry.import_name.is_all_but_default() {
if let Some(existing) = import_map.get(source) {
if existing
.iter()
.any(|(t, _, _)| matches!(t, ImportType::AllButDefault))
{
ctx.diagnostic(no_duplicate_exports_diagnostic(
source,
span,
existing.first().unwrap().1,
));
continue;
}
}
if let Some(existing) = side_effect_import_map.get(source) {
let Some(module_request) = &entry.module_request else {
continue;
};
let source = &module_request.name;
let span = entry.span;

if entry.import_name.is_all_but_default() {
if let Some(existing) = import_map.get(source) {
if existing.iter().any(|(t, _, _)| matches!(t, ImportType::AllButDefault)) {
ctx.diagnostic(no_duplicate_exports_diagnostic(
source,
span,
*existing.first().unwrap(),
existing.first().unwrap().1,
));
continue;
}
import_map.entry(source).or_default().push((
ImportType::AllButDefault,
}
if let Some(existing) = side_effect_import_map.get(source) {
ctx.diagnostic(no_duplicate_exports_diagnostic(
source,
span,
ModuleType::Export,
*existing.first().unwrap(),
));
continue;
}
if let Some(existing) = import_map.get(source) {
if existing.iter().any(|(t, _, _)| {
matches!(t, ImportType::Named | ImportType::SideEffect)
}) {
ctx.diagnostic(no_duplicate_exports_diagnostic(
source,
span,
existing.first().unwrap().1,
));
continue;
}
}

import_map.entry(source).or_default().push((
ImportType::SideEffect,
ImportType::AllButDefault,
span,
ModuleType::Export,
));
continue;
}
if let Some(existing) = import_map.get(source) {
if existing
.iter()
.any(|(t, _, _)| matches!(t, ImportType::Named | ImportType::SideEffect))
{
ctx.diagnostic(no_duplicate_exports_diagnostic(
source,
span,
existing.first().unwrap().1,
));
continue;
}
}

import_map.entry(source).or_default().push((
ImportType::SideEffect,
span,
ModuleType::Export,
));
}

for entry in &module_record.indirect_export_entries {
if let Some(module_request) = &entry.module_request {
let source = &module_request.name;
let span = entry.span;

if let Some(existing) = import_map.get(source) {
if entry.import_name == ExportImportName::All {
if existing.iter().any(|(t, _, _)| {
matches!(t, ImportType::Default | ImportType::Namespace)
}) {
ctx.diagnostic(no_duplicate_exports_diagnostic(
source,
span,
existing.first().unwrap().1,
));
continue;
}

continue;
}
let Some(module_request) = &entry.module_request else {
continue;
};
let source = &module_request.name;
let span = entry.span;

if existing.iter().any(|(t, _, module_type)| {
(matches!(
t,
ImportType::Named | ImportType::SideEffect | ImportType::Default
) && *module_type == ModuleType::Export)
|| (matches!(t, ImportType::Default)
&& *module_type == ModuleType::Import)
if let Some(existing) = import_map.get(source) {
if entry.import_name == ExportImportName::All {
if existing.iter().any(|(t, _, _)| {
matches!(t, ImportType::Default | ImportType::Namespace)
}) {
ctx.diagnostic(no_duplicate_exports_diagnostic(
source,
Expand All @@ -243,13 +267,32 @@ impl Rule for NoDuplicateImports {
));
continue;
}

continue;
}

if existing.iter().any(|(t, _, module_type)| {
(matches!(
t,
ImportType::Named | ImportType::SideEffect | ImportType::Default
) && *module_type == ModuleType::Export)
|| (matches!(t, ImportType::Default)
&& *module_type == ModuleType::Import)
}) {
ctx.diagnostic(no_duplicate_exports_diagnostic(
source,
span,
existing.first().unwrap().1,
));
continue;
}
import_map.entry(source).or_default().push((
ImportType::Named,
span,
ModuleType::Export,
));
}

import_map.entry(source).or_default().push((
ImportType::Named,
span,
ModuleType::Export,
));
}
}
}
Expand Down