Skip to content

refactor(linter): improve docs and simplify code of eslint/no-duplicate-imports #11320

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
camc314 merged 1 commit into from
May 27, 2025
Merged

refactor(linter): improve docs and simplify code of eslint/no-duplicate-imports #11320

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
140 changes: 57 additions & 83 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 @@ -10,21 +10,29 @@ use crate::{
rule::Rule,
};

fn no_duplicate_imports_diagnostic(module_name: &str, span: Span, span2: Span) -> OxcDiagnostic {
fn no_duplicate_imports_diagnostic(
module_name: &str,
span: Span,
previous_span: Span,
) -> OxcDiagnostic {
OxcDiagnostic::warn(format!("'{module_name}' import is duplicated"))
.with_help("Merge the duplicated import into a single import statement")
.with_labels([
span.label("This import is duplicated"),
span2.label("Can be merged with this import"),
previous_span.label("Can be merged with this import"),
])
}

fn no_duplicate_exports_diagnostic(module_name: &str, span: Span, span2: Span) -> OxcDiagnostic {
fn no_duplicate_exports_diagnostic(
module_name: &str,
span: Span,
previous_span: Span,
) -> OxcDiagnostic {
OxcDiagnostic::warn(format!("'{module_name}' export is duplicated"))
.with_help("Merge the duplicated exports into a single export statement")
.with_labels([
span.label("This export is duplicated"),
span2.label("Can be merged with this"),
previous_span.label("Can be merged with this"),
])
}

Expand Down Expand Up @@ -65,21 +73,21 @@ declare_oxc_lint!(
///
/// #### includeExports
///
/// `{ "includeExports": boolean }`
/// `{ type: boolean, default: false }`
///
/// 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`
/// Examples of **incorrect** code when `includeExports` 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`
/// Examples of **correct** code when `includeExports` 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`.
Expand All @@ -100,7 +108,8 @@ declare_oxc_lint!(
NoDuplicateImports,
eslint,
style,
pending);
pending
);

#[derive(Debug, Clone, PartialEq)]
enum ImportType {
Expand All @@ -119,10 +128,10 @@ enum ModuleType {

impl Rule for NoDuplicateImports {
fn from_configuration(value: serde_json::Value) -> Self {
let Some(value) = value.get(0) else { return Self { include_exports: false } };
let value = value.get(0);
Self {
include_exports: value
.get("includeExports")
.and_then(|v| v.get("includeExports"))
.and_then(serde_json::Value::as_bool)
.unwrap_or(false),
}
Expand All @@ -132,62 +141,54 @@ impl Rule for NoDuplicateImports {
let module_record = ctx.module_record();
let mut import_map: FxHashMap<&CompactStr, Vec<(ImportType, Span, ModuleType)>> =
FxHashMap::default();
let mut current_span: Option<Span> = None;
let mut previous_span: Option<Span> = None;
let mut side_effect_import_map: FxHashMap<&CompactStr, Vec<Span>> = FxHashMap::default();

for entry in &module_record.import_entries {
let source = &entry.module_request.name;
let span = entry.module_request.span;

let same_statement = if let Some(curr_span) = current_span {
curr_span == span
} else {
current_span = Some(span);
true
};

let import_type = match &entry.import_name {
ImportImportName::Name(_) => ImportType::Named,
ImportImportName::NamespaceObject => ImportType::Namespace,
ImportImportName::Default(_) => ImportType::Default,
};

if let Some(existing) = import_map.get(source) {
let can_merge = can_merge_imports(&import_type, existing, same_statement);
if can_merge {
ctx.diagnostic(no_duplicate_imports_diagnostic(
source,
span,
existing.first().unwrap().1,
));
continue;
if previous_span != Some(span) {
previous_span = Some(span);

if let Some(existing) = import_map.get(source) {
if can_merge_imports(&import_type, existing) {
ctx.diagnostic(no_duplicate_imports_diagnostic(
source,
span,
existing.first().unwrap().1,
));
continue;
}
}
}

import_map.entry(source).or_default().push((import_type, span, ModuleType::Import));

if !same_statement {
current_span = Some(span);
}
}

for (source, requests) in &module_record.requested_modules {
for request in requests {
if request.is_import && module_record.import_entries.is_empty() {
side_effect_import_map.entry(source).or_default().push(request.span);
if module_record.import_entries.is_empty() {
for (source, requests) in &module_record.requested_modules {
for request in requests {
if request.is_import {
side_effect_import_map.entry(source).or_default().push(request.span);
}
}
}
}

for (source, spans) in &side_effect_import_map {
if spans.len() > 1 {
for span in spans {
let i = spans.iter().position(|s| s == span).unwrap();
if i > 0 {
for (source, spans) in &side_effect_import_map {
let mut spans_iter = spans.iter();
if let Some(first_span) = spans_iter.next() {
for following_span in spans_iter {
ctx.diagnostic(no_duplicate_imports_diagnostic(
source,
*span,
*spans.first().unwrap(),
*following_span,
*first_span,
));
}
}
Expand Down Expand Up @@ -266,7 +267,6 @@ impl Rule for NoDuplicateImports {
span,
existing.first().unwrap().1,
));
continue;
}

continue;
Expand Down Expand Up @@ -302,12 +302,7 @@ impl Rule for NoDuplicateImports {
fn can_merge_imports(
current_type: &ImportType,
existing: &[(ImportType, Span, ModuleType)],
same_statement: bool,
) -> bool {
if same_statement {
return false;
}

let namespace = existing.iter().find(|(t, _, _)| matches!(t, ImportType::Namespace));
let named = existing.iter().find(|(t, _, _)| matches!(t, ImportType::Named));
let default = existing.iter().find(|(t, _, _)| matches!(t, ImportType::Default));
Expand All @@ -316,46 +311,25 @@ fn can_merge_imports(
let has_named = named.is_some();
let has_default = default.is_some();

if matches!(current_type, ImportType::Named) && has_named {
return true;
}

if matches!(current_type, ImportType::Namespace) {
if has_named && has_default {
if let Some((_, named_span, _)) = named {
if let Some((_, default_span, _)) = default {
if named_span == default_span {
return false;
match current_type {
ImportType::Named => has_named,
ImportType::Namespace => {
if has_named && has_default {
if let Some((_, named_span, _)) = named {
if let Some((_, default_span, _)) = default {
if named_span == default_span {
return false;
}
}
}
}
}

if has_namespace {
return true;
}
if has_default && !same_statement {
return true;
has_namespace || has_default
}
ImportType::Default => has_default || has_namespace || has_named,
ImportType::SideEffect => !existing.is_empty(),
ImportType::AllButDefault => false,
}

if matches!(current_type, ImportType::Default) {
if has_default {
return true;
}
if has_named && !same_statement {
return true;
}
if has_namespace {
return true;
}
}

if matches!(current_type, ImportType::SideEffect) && !existing.is_empty() {
return true;
}

false
}

#[test]
Expand Down
Loading