feat: absolute module specifiers are now sorted alphabetically instead of based on folder path like relative specifiers

Closes #270

Author: dsherret

src/parsing/sorting/module_specifiers.rs

@@ -4,62 +4,70 @@ pub fn cmp_module_specifiers(a: &str, b: &str, cmp_text: impl Fn(&str, &str) ->
     let a_info = get_module_specifier_info(a);
     let b_info = get_module_specifier_info(b);
 
-    return match a_info.kind {
-        ModuleSpecifierKind::Absolute => {
-            match b_info.kind {
-                ModuleSpecifierKind::Absolute => compare_folder_items(&a_info, &b_info, cmp_text),
-                ModuleSpecifierKind::Relative(_) => Ordering::Less,
+    return match &a_info {
+        ModuleSpecifierInfo::Absolute {
+            text: a_text,
+        } => {
+            match b_info {
+                ModuleSpecifierInfo::Absolute { text: b_text } => cmp_text(a_text, b_text),
+                ModuleSpecifierInfo::Relative { .. } => Ordering::Less,
             }
         },
-        ModuleSpecifierKind::Relative(a_relative_count) => {
-            match b_info.kind {
-                ModuleSpecifierKind::Absolute => Ordering::Greater,
-                ModuleSpecifierKind::Relative(b_relative_count) => {
+        ModuleSpecifierInfo::Relative {
+            relative_count: a_relative_count,
+            folder_items: a_folder_items,
+         } => {
+            match &b_info {
+                ModuleSpecifierInfo::Absolute { .. } => Ordering::Greater,
+                ModuleSpecifierInfo::Relative {
+                    relative_count: b_relative_count,
+                    folder_items: b_folder_items,
+                } => {
                     match a_relative_count.cmp(&b_relative_count) {
                         Ordering::Greater => Ordering::Less,
                         Ordering::Less => Ordering::Greater,
-                        Ordering::Equal => compare_folder_items(&a_info, &b_info, cmp_text),
+                        Ordering::Equal => compare_folder_items(a_folder_items, b_folder_items, cmp_text),
                     }
                 },
             }
         }
     };
 
-    fn compare_folder_items(a_info: &ModuleSpecifierInfo, b_info: &ModuleSpecifierInfo, cmp_text: impl Fn(&str, &str) -> Ordering) -> Ordering {
-        let max_len = std::cmp::min(a_info.folder_items.len(), b_info.folder_items.len());
+    fn compare_folder_items(a_folder_items: &Vec<&'_ str>, b_folder_items: &Vec<&'_ str>, cmp_text: impl Fn(&str, &str) -> Ordering) -> Ordering {
+        let max_len = std::cmp::min(a_folder_items.len(), b_folder_items.len());
         // compare the common items except for the potential file name at the end
         for i in 0..max_len - 1 {
-            let a_text = a_info.folder_items[i];
-            let b_text = b_info.folder_items[i];
+            let a_text = a_folder_items[i];
+            let b_text = b_folder_items[i];
             let ordering = cmp_text(a_text, b_text);
             if ordering != Ordering::Equal {
                 return ordering;
             }
         }
 
-        if a_info.folder_items.len() != b_info.folder_items.len() {
+        if a_folder_items.len() != b_folder_items.len() {
             // the import that has a folder name should appear before the one with a file name
-            if a_info.folder_items.len() > b_info.folder_items.len() {
+            if a_folder_items.len() > b_folder_items.len() {
                 Ordering::Less
             } else {
                 Ordering::Greater
             }
         } else {
             // compare the file names
-            cmp_text(a_info.folder_items.last().unwrap(), b_info.folder_items.last().unwrap())
+            cmp_text(a_folder_items.last().unwrap(), b_folder_items.last().unwrap())
         }
     }
 }
 
-struct ModuleSpecifierInfo<'a> {
-    kind: ModuleSpecifierKind,
-    folder_items: Vec<&'a str>,
-}
-
 #[derive(Debug, PartialEq)]
-enum ModuleSpecifierKind {
-    Absolute,
-    Relative(usize),
+enum ModuleSpecifierInfo<'a> {
+    Absolute {
+        text: &'a str,
+    },
+    Relative {
+        relative_count: usize,
+        folder_items: Vec<&'a str>,
+    },
 }
 
 fn get_module_specifier_info<'a>(text: &'a str) -> ModuleSpecifierInfo<'a> {
@@ -81,14 +89,13 @@ fn get_module_specifier_info<'a>(text: &'a str) -> ModuleSpecifierInfo<'a> {
             start_index += part.len() + next_slash_width;
         }
 
-        ModuleSpecifierInfo {
+        ModuleSpecifierInfo::Relative {
+            relative_count,
             folder_items: no_quotes_text[start_index..].split('/').collect(),
-            kind: ModuleSpecifierKind::Relative(relative_count),
         }
     } else {
-        ModuleSpecifierInfo {
-            folder_items: no_quotes_text.split('/').collect(),
-            kind: ModuleSpecifierKind::Absolute,
+        ModuleSpecifierInfo::Absolute {
+            text: no_quotes_text,
         }
     }
 }
@@ -116,39 +123,50 @@ mod test {
     #[test]
     fn it_should_get_module_specifier_info_when_empty() {
         let result = get_module_specifier_info("''");
-        assert_eq!(result.folder_items, vec![""]);
-        assert_eq!(result.kind, ModuleSpecifierKind::Absolute);
+        assert_eq!(result, ModuleSpecifierInfo::Absolute {
+            text: "",
+        });
     }
 
     #[test]
     fn it_should_get_module_specifier_info_for_absolute() {
         // double quotes
         let result = get_module_specifier_info(r#""testing/asdf""#);
-        assert_eq!(result.folder_items, vec!["testing", "asdf"]);
-        assert_eq!(result.kind, ModuleSpecifierKind::Absolute);
+        assert_eq!(result, ModuleSpecifierInfo::Absolute {
+            text: "testing/asdf",
+        });
 
         // single quotes
         let result = get_module_specifier_info("'testing'");
-        assert_eq!(result.folder_items, vec!["testing"]);
-        assert_eq!(result.kind, ModuleSpecifierKind::Absolute);
+        assert_eq!(result, ModuleSpecifierInfo::Absolute {
+            text: "testing",
+        });
     }
 
     #[test]
     fn it_should_get_module_specifier_info_for_relative() {
         let result = get_module_specifier_info("'./a'");
-        assert_eq!(result.folder_items, vec!["a"]);
-        assert_eq!(result.kind, ModuleSpecifierKind::Relative(0));
+        assert_eq!(result, ModuleSpecifierInfo::Relative {
+            relative_count: 0,
+            folder_items: vec!["a"],
+        });
 
         let result = get_module_specifier_info("'./../testing-test/t'");
-        assert_eq!(result.folder_items, vec!["testing-test", "t"]);
-        assert_eq!(result.kind, ModuleSpecifierKind::Relative(1));
+        assert_eq!(result, ModuleSpecifierInfo::Relative {
+            relative_count: 1,
+            folder_items: vec!["testing-test", "t"],
+        });
 
         let result = get_module_specifier_info("'../asdf'");
-        assert_eq!(result.folder_items, vec!["asdf"]);
-        assert_eq!(result.kind, ModuleSpecifierKind::Relative(1));
+        assert_eq!(result, ModuleSpecifierInfo::Relative {
+            relative_count: 1,
+            folder_items: vec!["asdf"],
+        });
 
         let result = get_module_specifier_info("'../../../test'");
-        assert_eq!(result.folder_items, vec!["test"]);
-        assert_eq!(result.kind, ModuleSpecifierKind::Relative(3));
+        assert_eq!(result, ModuleSpecifierInfo::Relative {
+            relative_count: 3,
+            folder_items: vec!["test"],
+        });
     }
 }

tests/specs/file/sorting/Statements_SortOrder.txt

@@ -94,12 +94,30 @@ import { a } from "test";
 
 import { a } from "test";
 
-== should put folders above files ==
+== should sort straight alphabetical for absolute paths or import map/package names ==
 import * as t from "https://deno.land/x/a.ts";
 import * as u from "https://deno.land/x/sub-dir/a.ts";
-import * as u from "https://deno.land/x/sub-dir/other/a.ts";
+import * as v from "https://deno.land/x/sub-dir/other/a.ts";
+
+import "package";
+import "package/other";
+import "package/abc/xyz";
 
 [expect]
-import * as u from "https://deno.land/x/sub-dir/other/a.ts";
-import * as u from "https://deno.land/x/sub-dir/a.ts";
 import * as t from "https://deno.land/x/a.ts";
+import * as u from "https://deno.land/x/sub-dir/a.ts";
+import * as v from "https://deno.land/x/sub-dir/other/a.ts";
+
+import "package";
+import "package/abc/xyz";
+import "package/other";
+
+== should put package names in alphabetical order regardless of directories ==
+import "package";
+import "package/asdf/test";
+import "package/other";
+
+[expect]
+import "package";
+import "package/asdf/test";
+import "package/other";