refactor: reduce code duplication for node sorter

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<Box<dyn Fn((usize, Option<Node<'a>>), (usize, Option<Node<'a>>), Program<'a>) -> std::cmp::Ordering>> {
+  fn get_node_sorter<'a>(parent_decl: Node, context: &Context<'a>) -> Option<NodeSorter> {
     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<Node<'a>>, context: &
 
   return items;
 
-  fn get_node_sorter<'a>(
-    group_kind: StmtGroupKind,
-    context: &Context<'a>,
-  ) -> Option<Box<dyn Fn((usize, Option<Node<'a>>), (usize, Option<Node<'a>>), Program<'a>) -> std::cmp::Ordering>> {
+  fn get_node_sorter<'a>(group_kind: StmtGroupKind, context: &Context<'a>) -> Option<NodeSorter> {
     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<Box<dyn Fn((usize, Option<Node<'a>>), (usize, Option<Node<'a>>), Program<'a>) -> std::cmp::Ordering>>,
+  node_sorter: Option<NodeSorter>,
 }
 
 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<Item = Option<Node<'a>>>,
-  sorter: Box<dyn Fn((usize, Option<Node<'a>>), (usize, Option<Node<'a>>), Program<'a>) -> std::cmp::Ordering>,
-  context: &mut Context<'a>,
-) -> utils::VecMap<usize> {
+fn get_sorted_indexes<'a: 'b, 'b>(nodes: impl Iterator<Item = Option<Node<'a>>>, sorter: NodeSorter, context: &mut Context<'a>) -> utils::VecMap<usize> {
   let mut nodes_with_indexes = nodes.enumerate().collect::<Vec<_>>();
-  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<Box<dyn Fn((usize, Option<Node<'a>>), (usize, Option<Node<'a>>), Program<'a>) -> std::cmp::Ordering>>,
+  node_sorter: Option<NodeSorter>,
 }
 
 fn gen_object_like_node<'a>(opts: GenObjectLikeNodeOptions<'a>, context: &mut Context<'a>) -> PrintItems {

src/generation/sorting/mod.rs

@@ -1,55 +1,26 @@
 mod module_specifiers;
 use module_specifiers::*;
 
+mod sorter;
+use sorter::*;
+
 use deno_ast::view::*;
 use deno_ast::SourceRange;
 use deno_ast::SourceRanged;
 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<Box<dyn Fn((usize, Option<Node<'a>>), (usize, Option<Node<'a>>), 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<NodeSorter> {
   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<ComparisonNode> {
   }
 }
 
-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<Node>) -> bool {
   matches!(node, Some(Node::ImportDecl(_) | Node::NamedExport(_) | Node::ExportAll(_)))
 }

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]

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<Node<'a>>), (b_index, b): (usize, Option<Node<'a>>), 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<T> CompareText for T
+where
+  T: Fn(&str, &str) -> Ordering,
+{
+  fn cmp_text(self, a: &str, b: &str) -> Ordering {
+    self(a, b)
+  }
+}