From 9638c0371f2105596ce1e41d8c27faae5f88d995 Mon Sep 17 00:00:00 2001 From: therewillbecode Date: Sun, 9 Mar 2025 10:11:55 +0000 Subject: [PATCH 1/4] Refactor if let into let else --- .../src/rules/eslint/no_duplicate_imports.rs | 137 +++++++++--------- 1 file changed, 69 insertions(+), 68 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs b/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs index 11d79468aa0f7..6c069b7480e55 100644 --- a/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs +++ b/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs @@ -153,88 +153,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, @@ -243,13 +225,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, + )); } } } From c96be0ec763450831653ac981c4646f066da3d4c Mon Sep 17 00:00:00 2001 From: therewillbecode Date: Sun, 9 Mar 2025 10:34:43 +0000 Subject: [PATCH 2/4] Document options for no-duplicate-imports --- .../src/rules/eslint/no_duplicate_imports.rs | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs b/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs index 6c069b7480e55..04e75057b7873 100644 --- a/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs +++ b/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs @@ -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'; @@ -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, From a5152993a4cbbb0cc894a771dddd34312d3ec69a Mon Sep 17 00:00:00 2001 From: therewillbecode Date: Sun, 9 Mar 2025 10:38:42 +0000 Subject: [PATCH 3/4] New line --- crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs b/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs index 04e75057b7873..18393de25bf5e 100644 --- a/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs +++ b/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs @@ -91,7 +91,7 @@ declare_oxc_lint!( /// import { merge, find } from 'module'; /// /// // cannot be merged with the above import - /// export * as something from 'module'; + /// export * as something from 'module'; /// /// // cannot be written differently /// export * from 'module'; From 585091639a7c1bbb7e70302b755e9f1d5017cabd Mon Sep 17 00:00:00 2001 From: therewillbecode Date: Sun, 9 Mar 2025 17:38:20 +0000 Subject: [PATCH 4/4] Better formatting --- crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs b/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs index 18393de25bf5e..07c8808bf336b 100644 --- a/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs +++ b/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs @@ -62,7 +62,7 @@ declare_oxc_lint!( /// /// ### Options /// - /// ### includeExports + /// #### includeExports /// /// `{ "includeExports": boolean }` ///