From 3ebe664c9efb6be7663a0ea0a612b9c7fa8b5a9f Mon Sep 17 00:00:00 2001 From: Balthild Date: Thu, 19 Sep 2024 13:45:40 -0500 Subject: [PATCH] refactor: reduce code duplication for node sorter --- src/generation/generate.rs | 22 ++----- src/generation/sorting/mod.rs | 58 +++---------------- src/generation/sorting/module_specifiers.rs | 32 ++++++----- src/generation/sorting/sorter.rs | 64 +++++++++++++++++++++ 4 files changed, 95 insertions(+), 81 deletions(-) create mode 100644 src/generation/sorting/sorter.rs diff --git a/src/generation/generate.rs b/src/generation/generate.rs index 236c22e6..25ed792b 100644 --- a/src/generation/generate.rs +++ b/src/generation/generate.rs @@ -1539,10 +1539,7 @@ fn gen_named_import_or_export_specifiers<'a>(opts: GenNamedImportOrExportSpecifi } } - fn get_node_sorter<'a>( - parent_decl: Node, - context: &Context<'a>, - ) -> Option>), (usize, Option>), Program<'a>) -> std::cmp::Ordering>> { + fn get_node_sorter<'a>(parent_decl: Node, context: &Context<'a>) -> Option { match parent_decl { Node::NamedExport(_) => get_node_sorter_from_order( context.config.export_declaration_sort_named_exports, @@ -7105,10 +7102,7 @@ fn gen_statements<'a>(inner_range: SourceRange, stmts: Vec>, context: & return items; - fn get_node_sorter<'a>( - group_kind: StmtGroupKind, - context: &Context<'a>, - ) -> Option>), (usize, Option>), Program<'a>) -> std::cmp::Ordering>> { + fn get_node_sorter<'a>(group_kind: StmtGroupKind, context: &Context<'a>) -> Option { match group_kind { StmtGroupKind::Imports => get_node_sorter_from_order(context.config.module_sort_import_declarations, NamedTypeImportsExportsOrder::None), StmtGroupKind::Exports => get_node_sorter_from_order(context.config.module_sort_export_declarations, NamedTypeImportsExportsOrder::None), @@ -7600,7 +7594,7 @@ struct GenSeparatedValuesParams<'a> { single_line_options: ir_helpers::SingleLineOptions, multi_line_options: ir_helpers::MultiLineOptions, force_possible_newline_at_start: bool, - node_sorter: Option>), (usize, Option>), Program<'a>) -> std::cmp::Ordering>>, + node_sorter: Option, } enum NodeOrSeparator<'a> { @@ -7733,13 +7727,9 @@ fn gen_separated_values_with_result<'a>(opts: GenSeparatedValuesParams<'a>, cont ) } -fn get_sorted_indexes<'a: 'b, 'b>( - nodes: impl Iterator>>, - sorter: Box>), (usize, Option>), Program<'a>) -> std::cmp::Ordering>, - context: &mut Context<'a>, -) -> utils::VecMap { +fn get_sorted_indexes<'a: 'b, 'b>(nodes: impl Iterator>>, sorter: NodeSorter, context: &mut Context<'a>) -> utils::VecMap { let mut nodes_with_indexes = nodes.enumerate().collect::>(); - nodes_with_indexes.sort_unstable_by(|a, b| sorter((a.0, a.1), (b.0, b.1), context.program)); + nodes_with_indexes.sort_unstable_by(|a, b| sorter.compare((a.0, a.1), (b.0, b.1), context.program)); let mut old_to_new_index = utils::VecMap::with_capacity(nodes_with_indexes.len()); for (new_index, old_index) in nodes_with_indexes.into_iter().map(|(index, _)| index).enumerate() { @@ -7995,7 +7985,7 @@ struct GenObjectLikeNodeOptions<'a> { force_multi_line: bool, surround_single_line_with_spaces: bool, allow_blank_lines: bool, - node_sorter: Option>), (usize, Option>), Program<'a>) -> std::cmp::Ordering>>, + node_sorter: Option, } fn gen_object_like_node<'a>(opts: GenObjectLikeNodeOptions<'a>, context: &mut Context<'a>) -> PrintItems { diff --git a/src/generation/sorting/mod.rs b/src/generation/sorting/mod.rs index 59a5b79a..5951c9dd 100644 --- a/src/generation/sorting/mod.rs +++ b/src/generation/sorting/mod.rs @@ -1,6 +1,9 @@ mod module_specifiers; use module_specifiers::*; +mod sorter; +use sorter::*; + use deno_ast::view::*; use deno_ast::SourceRange; use deno_ast::SourceRanged; @@ -8,48 +11,16 @@ use std::cmp::Ordering; use crate::configuration::*; +pub use sorter::NodeSorter; + // very rough... this should be improved to not allocate so much // and be cleaner -pub fn get_node_sorter_from_order<'a>( - order: SortOrder, - named_type_imports_exports_order: NamedTypeImportsExportsOrder, -) -> Option>), (usize, Option>), Program<'a>) -> Ordering>> { - // todo: how to reduce code duplication here? +pub fn get_node_sorter_from_order(order: SortOrder, named_type_imports_exports_order: NamedTypeImportsExportsOrder) -> Option { match order { SortOrder::Maintain => None, - SortOrder::CaseInsensitive => Some(Box::new(move |(a_index, a), (b_index, b), program| { - let result = if is_import_or_export_declaration(&a) { - cmp_optional_nodes(a, b, program, named_type_imports_exports_order, |a, b, module| { - cmp_module_specifiers(a.text_fast(module), b.text_fast(module), cmp_text_case_insensitive) - }) - } else { - cmp_optional_nodes(a, b, program, named_type_imports_exports_order, |a, b, module| { - cmp_text_case_insensitive(a.text_fast(module), b.text_fast(module)) - }) - }; - if result == Ordering::Equal { - a_index.cmp(&b_index) - } else { - result - } - })), - SortOrder::CaseSensitive => Some(Box::new(move |(a_index, a), (b_index, b), program| { - let result = if is_import_or_export_declaration(&a) { - cmp_optional_nodes(a, b, program, named_type_imports_exports_order, |a, b, module| { - cmp_module_specifiers(a.text_fast(module), b.text_fast(module), cmp_text_case_sensitive) - }) - } else { - cmp_optional_nodes(a, b, program, named_type_imports_exports_order, |a, b, module| { - cmp_text_case_sensitive(a.text_fast(module), b.text_fast(module)) - }) - }; - if result == Ordering::Equal { - a_index.cmp(&b_index) - } else { - result - } - })), + SortOrder::CaseSensitive => Some(NodeSorter::new(NodeSorterMode::CaseSensitive, named_type_imports_exports_order)), + SortOrder::CaseInsensitive => Some(NodeSorter::new(NodeSorterMode::CaseInsensitive, named_type_imports_exports_order)), } } @@ -173,19 +144,6 @@ fn get_comparison_nodes(node: Node) -> Vec { } } -fn cmp_text_case_sensitive(a: &str, b: &str) -> Ordering { - a.cmp(b) -} - -fn cmp_text_case_insensitive(a: &str, b: &str) -> Ordering { - let case_insensitive_result = a.to_lowercase().cmp(&b.to_lowercase()); - if case_insensitive_result == Ordering::Equal { - cmp_text_case_sensitive(a, b) - } else { - case_insensitive_result - } -} - fn is_import_or_export_declaration(node: &Option) -> bool { matches!(node, Some(Node::ImportDecl(_) | Node::NamedExport(_) | Node::ExportAll(_))) } diff --git a/src/generation/sorting/module_specifiers.rs b/src/generation/sorting/module_specifiers.rs index a621db07..da9c4dc4 100644 --- a/src/generation/sorting/module_specifiers.rs +++ b/src/generation/sorting/module_specifiers.rs @@ -1,12 +1,14 @@ use std::cmp::Ordering; -pub fn cmp_module_specifiers(a: &str, b: &str, cmp_text: impl Fn(&str, &str) -> Ordering) -> Ordering { +use super::CompareText; + +pub fn cmp_module_specifiers(a: &str, b: &str, cmp: impl CompareText) -> Ordering { let a_info = get_module_specifier_info(a); let b_info = get_module_specifier_info(b); match &a_info { ModuleSpecifierInfo::Absolute { text: a_text } => match b_info { - ModuleSpecifierInfo::Absolute { text: b_text } => cmp_text(a_text, b_text), + ModuleSpecifierInfo::Absolute { text: b_text } => cmp.cmp_text(a_text, b_text), ModuleSpecifierInfo::Relative { .. } => Ordering::Less, }, ModuleSpecifierInfo::Relative { @@ -20,7 +22,7 @@ pub fn cmp_module_specifiers(a: &str, b: &str, cmp_text: impl Fn(&str, &str) -> } => match a_relative_count.cmp(b_relative_count) { Ordering::Greater => Ordering::Less, Ordering::Less => Ordering::Greater, - Ordering::Equal => cmp_text(a_folder_text, b_folder_text), + Ordering::Equal => cmp.cmp_text(a_folder_text, b_folder_text), }, }, } @@ -66,18 +68,18 @@ mod test { #[test] fn it_should_compare_module_specifiers() { - assert_eq!(cmp_module_specifiers("''", "'test'", |a, b| a.cmp(b)), Ordering::Less); - assert_eq!(cmp_module_specifiers("'a'", "'b'", |a, b| a.cmp(b)), Ordering::Less); - assert_eq!(cmp_module_specifiers("'b'", "'a'", |a, b| a.cmp(b)), Ordering::Greater); - assert_eq!(cmp_module_specifiers("'a'", "'a'", |a, b| a.cmp(b)), Ordering::Equal); - assert_eq!(cmp_module_specifiers("'a'", "'./a'", |a, b| a.cmp(b)), Ordering::Less); - assert_eq!(cmp_module_specifiers("'./a'", "'a'", |a, b| a.cmp(b)), Ordering::Greater); - assert_eq!(cmp_module_specifiers("'./a'", "'./a'", |a, b| a.cmp(b)), Ordering::Equal); - assert_eq!(cmp_module_specifiers("'../a'", "'./a'", |a, b| a.cmp(b)), Ordering::Less); - assert_eq!(cmp_module_specifiers("'./a'", "'../a'", |a, b| a.cmp(b)), Ordering::Greater); - assert_eq!(cmp_module_specifiers("'../../a'", "'../a'", |a, b| a.cmp(b)), Ordering::Less); - assert_eq!(cmp_module_specifiers("'../a'", "'../../a'", |a, b| a.cmp(b)), Ordering::Greater); - assert_eq!(cmp_module_specifiers("'..'", "'test'", |a, b| a.cmp(b)), Ordering::Greater); + assert_eq!(cmp_module_specifiers("''", "'test'", str::cmp), Ordering::Less); + assert_eq!(cmp_module_specifiers("'a'", "'b'", str::cmp), Ordering::Less); + assert_eq!(cmp_module_specifiers("'b'", "'a'", str::cmp), Ordering::Greater); + assert_eq!(cmp_module_specifiers("'a'", "'a'", str::cmp), Ordering::Equal); + assert_eq!(cmp_module_specifiers("'a'", "'./a'", str::cmp), Ordering::Less); + assert_eq!(cmp_module_specifiers("'./a'", "'a'", str::cmp), Ordering::Greater); + assert_eq!(cmp_module_specifiers("'./a'", "'./a'", str::cmp), Ordering::Equal); + assert_eq!(cmp_module_specifiers("'../a'", "'./a'", str::cmp), Ordering::Less); + assert_eq!(cmp_module_specifiers("'./a'", "'../a'", str::cmp), Ordering::Greater); + assert_eq!(cmp_module_specifiers("'../../a'", "'../a'", str::cmp), Ordering::Less); + assert_eq!(cmp_module_specifiers("'../a'", "'../../a'", str::cmp), Ordering::Greater); + assert_eq!(cmp_module_specifiers("'..'", "'test'", str::cmp), Ordering::Greater); } #[test] diff --git a/src/generation/sorting/sorter.rs b/src/generation/sorting/sorter.rs new file mode 100644 index 00000000..c330c975 --- /dev/null +++ b/src/generation/sorting/sorter.rs @@ -0,0 +1,64 @@ +use deno_ast::view::*; +use deno_ast::SourceRanged; +use std::cmp::Ordering; + +use super::cmp_module_specifiers; +use super::cmp_optional_nodes; +use super::is_import_or_export_declaration; +use super::NamedTypeImportsExportsOrder; + +pub struct NodeSorter { + mode: NodeSorterMode, + order: NamedTypeImportsExportsOrder, +} + +pub enum NodeSorterMode { + CaseInsensitive, + CaseSensitive, +} + +impl NodeSorter { + pub fn new(mode: NodeSorterMode, order: NamedTypeImportsExportsOrder) -> Self { + Self { mode, order } + } + + pub fn compare<'a>(&self, (a_index, a): (usize, Option>), (b_index, b): (usize, Option>), program: Program<'a>) -> Ordering { + let result = if is_import_or_export_declaration(&a) { + cmp_optional_nodes(a, b, program, self.order, |a, b, module| { + cmp_module_specifiers(a.text_fast(module), b.text_fast(module), self) + }) + } else { + cmp_optional_nodes(a, b, program, self.order, |a, b, module| { + self.cmp_text(a.text_fast(module), b.text_fast(module)) + }) + }; + + if result == Ordering::Equal { + a_index.cmp(&b_index) + } else { + result + } + } +} + +pub trait CompareText { + fn cmp_text(self, a: &str, b: &str) -> Ordering; +} + +impl CompareText for &NodeSorter { + fn cmp_text(self, a: &str, b: &str) -> Ordering { + match self.mode { + NodeSorterMode::CaseInsensitive => a.to_lowercase().cmp(&b.to_lowercase()).then_with(|| a.cmp(b)), + NodeSorterMode::CaseSensitive => a.cmp(b), + } + } +} + +impl CompareText for T +where + T: Fn(&str, &str) -> Ordering, +{ + fn cmp_text(self, a: &str, b: &str) -> Ordering { + self(a, b) + } +}