From 79014ffb1d8a510aded2a494add160e6558e8981 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Sun, 1 Dec 2024 14:49:04 +0000 Subject: [PATCH] refactor(syntax): clean up `ModuleRecord` (#7568) --- .../src/rules/eslint/no_duplicate_imports.rs | 4 +- .../oxc_linter/src/rules/import/no_cycle.rs | 2 +- .../src/rules/import/no_duplicates.rs | 6 +- .../src/rules/import/no_self_import.rs | 2 +- .../src/rules/vitest/no_import_node_test.rs | 4 +- crates/oxc_syntax/src/module_record.rs | 71 +++++++++---------- 6 files changed, 41 insertions(+), 48 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 37d19351f3868..69a994d34520a 100644 --- a/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs +++ b/crates/oxc_linter/src/rules/eslint/no_duplicate_imports.rs @@ -129,8 +129,8 @@ impl Rule for NoDuplicateImports { 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 request.is_import && module_record.import_entries.is_empty() { + side_effect_import_map.entry(source).or_default().push(request.span); } } } diff --git a/crates/oxc_linter/src/rules/import/no_cycle.rs b/crates/oxc_linter/src/rules/import/no_cycle.rs index 7f4c0d65ed0a4..1d5a744253c68 100644 --- a/crates/oxc_linter/src/rules/import/no_cycle.rs +++ b/crates/oxc_linter/src/rules/import/no_cycle.rs @@ -170,7 +170,7 @@ impl Rule for NoCycle { }); if visitor_result.result { - let span = module_record.requested_modules[&stack[0].0][0].span(); + let span = module_record.requested_modules[&stack[0].0][0].span; let help = stack .iter() .map(|(specifier, path)| { diff --git a/crates/oxc_linter/src/rules/import/no_duplicates.rs b/crates/oxc_linter/src/rules/import/no_duplicates.rs index b33caac862117..b538290a92e31 100644 --- a/crates/oxc_linter/src/rules/import/no_duplicates.rs +++ b/crates/oxc_linter/src/rules/import/no_duplicates.rs @@ -107,7 +107,7 @@ impl Rule for NoDuplicates { let requested_modules = group .into_iter() .flat_map(|(_path, requested_modules)| requested_modules) - .filter(|requested_module| requested_module.is_import()); + .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> = @@ -116,7 +116,7 @@ impl Rule for NoDuplicates { let imports = module_record .import_entries .iter() - .filter(|entry| entry.module_request.span() == requested_module.span()) + .filter(|entry| entry.module_request.span() == requested_module.span) .collect::>(); if imports.is_empty() { import_entries_maps.entry(0).or_default().push(requested_module); @@ -154,7 +154,7 @@ impl Rule for NoDuplicates { fn check_duplicates(ctx: &LintContext, requested_modules: Option<&Vec<&RequestedModule>>) { if let Some(requested_modules) = requested_modules { if requested_modules.len() > 1 { - let mut labels = requested_modules.iter().map(|m| m.span()); + let mut labels = requested_modules.iter().map(|m| m.span); let first = labels.next().unwrap(); // we know there is at least one let module_name = ctx.source_range(first).trim_matches('\'').trim_matches('"'); ctx.diagnostic(no_duplicates_diagnostic(module_name, first, labels)); diff --git a/crates/oxc_linter/src/rules/import/no_self_import.rs b/crates/oxc_linter/src/rules/import/no_self_import.rs index 921c57dd9a3eb..576381cb0a804 100644 --- a/crates/oxc_linter/src/rules/import/no_self_import.rs +++ b/crates/oxc_linter/src/rules/import/no_self_import.rs @@ -50,7 +50,7 @@ impl Rule for NoSelfImport { }; if remote_module_record_ref.value().resolved_absolute_path == *resolved_absolute_path { for requested_module in requested_modules { - ctx.diagnostic(no_self_import_diagnostic(requested_module.span())); + ctx.diagnostic(no_self_import_diagnostic(requested_module.span)); } } } diff --git a/crates/oxc_linter/src/rules/vitest/no_import_node_test.rs b/crates/oxc_linter/src/rules/vitest/no_import_node_test.rs index 4e95cbb9cb09c..0cb5a6258c103 100644 --- a/crates/oxc_linter/src/rules/vitest/no_import_node_test.rs +++ b/crates/oxc_linter/src/rules/vitest/no_import_node_test.rs @@ -56,8 +56,8 @@ impl Rule for NoImportNodeTest { if let Some(node_test_module) = module_record.requested_modules.get("node:test") { if let Some(requested_module) = node_test_module.first() { - ctx.diagnostic_with_fix(no_import_node_test(requested_module.span()), |fixer| { - fixer.replace(requested_module.span(), "\"vitest\"") + ctx.diagnostic_with_fix(no_import_node_test(requested_module.span), |fixer| { + fixer.replace(requested_module.span, "\"vitest\"") }); } } diff --git a/crates/oxc_syntax/src/module_record.rs b/crates/oxc_syntax/src/module_record.rs index f1f6d91d3afcf..d674aa7a319de 100644 --- a/crates/oxc_syntax/src/module_record.rs +++ b/crates/oxc_syntax/src/module_record.rs @@ -1,7 +1,4 @@ //! [ECMAScript Module Record](https://tc39.es/ecma262/#sec-abstract-module-records) -#![allow(missing_docs)] // fixme - -use std::fmt; use oxc_allocator::{Allocator, Vec}; use oxc_span::{Atom, Span}; @@ -15,6 +12,7 @@ use rustc_hash::FxHashMap; /// See /// * /// * +#[derive(Debug)] pub struct ModuleRecord<'a> { /// This module has no import / export statements pub not_esm: bool, @@ -74,6 +72,7 @@ pub struct ModuleRecord<'a> { } impl<'a> ModuleRecord<'a> { + /// Constructor pub fn new(allocator: &'a Allocator) -> Self { Self { not_esm: true, @@ -91,30 +90,18 @@ impl<'a> ModuleRecord<'a> { } } -impl fmt::Debug for ModuleRecord<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("ModuleRecord") - .field("not_esm", &self.not_esm) - .field("import_entries", &self.import_entries) - .field("local_export_entries", &self.local_export_entries) - .field("indirect_export_entries", &self.indirect_export_entries) - .field("star_export_entries", &self.star_export_entries) - .field("exported_bindings", &self.exported_bindings) - .field("exported_bindings_duplicated", &self.exported_bindings_duplicated) - .field("exported_bindings_from_star_export", &self.exported_bindings_from_star_export) - .field("export_default", &self.export_default) - .field("export_default_duplicated", &self.export_default_duplicated) - .finish() - } -} - +/// Name and Span #[derive(Debug, Clone, PartialEq, Eq)] pub struct NameSpan<'a> { + /// Name pub name: Atom<'a>, + + /// Span pub span: Span, } impl<'a> NameSpan<'a> { + /// Constructor pub fn new(name: Atom<'a>, span: Span) -> Self { Self { name, span } } @@ -193,16 +180,21 @@ pub struct ImportEntry<'a> { /// `ImportName` For `ImportEntry` #[derive(Debug, Clone, PartialEq, Eq)] pub enum ImportImportName<'a> { + /// Name Name(NameSpan<'a>), + /// Namespace Object NamespaceObject, + /// Default Default(Span), } impl ImportImportName<'_> { + /// Is `default` pub fn is_default(&self) -> bool { matches!(self, Self::Default(_)) } + /// Is namespace pub fn is_namespace_object(&self) -> bool { matches!(self, Self::NamespaceObject) } @@ -253,6 +245,7 @@ pub struct ExportEntry<'a> { /// `ImportName` for `ExportEntry` #[derive(Debug, Default, Clone, PartialEq, Eq)] pub enum ExportImportName<'a> { + /// Name Name(NameSpan<'a>), /// all is used for export * as ns from "mod" declarations. All, @@ -263,11 +256,14 @@ pub enum ExportImportName<'a> { Null, } +/// Export Import Name impl ExportImportName<'_> { + /// Is all pub fn is_all(&self) -> bool { matches!(self, Self::All) } + /// Is all but default pub fn is_all_but_default(&self) -> bool { matches!(self, Self::AllButDefault) } @@ -276,8 +272,11 @@ impl ExportImportName<'_> { /// `ExportName` for `ExportEntry` #[derive(Debug, Default, Clone, PartialEq, Eq)] pub enum ExportExportName<'a> { + /// Name Name(NameSpan<'a>), + /// Default Default(Span), + /// Null #[default] Null, } @@ -306,9 +305,11 @@ impl ExportExportName<'_> { /// `LocalName` for `ExportEntry` #[derive(Debug, Default, Clone, PartialEq, Eq)] pub enum ExportLocalName<'a> { + /// Name Name(NameSpan<'a>), /// `export default name_span` Default(NameSpan<'a>), + /// Null #[default] Null, } @@ -333,22 +334,11 @@ impl<'a> ExportLocalName<'a> { } } +/// RequestedModule #[derive(Debug, Clone, Copy)] pub struct RequestedModule { - span: Span, - is_type: bool, - /// is_import is true if the module is requested by an import statement. - is_import: bool, -} - -impl RequestedModule { - pub fn new(span: Span, is_type: bool, is_import: bool) -> Self { - Self { span, is_type, is_import } - } - - pub fn span(&self) -> Span { - self.span - } + /// Span + pub span: Span, /// `true` if a `type` modifier was used in the import statement. /// @@ -358,13 +348,16 @@ impl RequestedModule { /// import { type bar } from "bar"; // false, `type` is on specifier /// import { baz } from "baz"; // false, no `type` modifier /// ``` - pub fn is_type(&self) -> bool { - self.is_type - } + pub is_type: bool, /// `true` if the module is requested by an import statement. - pub fn is_import(&self) -> bool { - self.is_import + pub is_import: bool, +} + +impl RequestedModule { + /// Constructor + pub fn new(span: Span, is_type: bool, is_import: bool) -> Self { + Self { span, is_type, is_import } } }