Sorting special characters feature parity (#51)

* bumping rust to 1.83.0

* sorting special characters

Author: perryqh

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "codeowners"
-version = "0.2.2"
+version = "0.2.3"
 edition = "2021"
 
 [profile.release]

Cargo.toml

@@ -1,4 +1,4 @@
 [toolchain]
-channel = "1.82.0"
+channel = "1.83.0"
 components = ["clippy", "rustfmt"]
 targets = ["x86_64-apple-darwin", "aarch64-apple-darwin", "x86_64-unknown-linux-gnu"]

rust-toolchain.toml

@@ -1,3 +1,5 @@
+use std::cmp::Ordering;
+
 use super::{Entry, Mapper};
 
 pub struct FileGenerator {
@@ -41,7 +43,159 @@ impl FileGenerator {
 
     fn to_sorted_lines(entries: &[Entry]) -> Vec<String> {
         let mut lines: Vec<String> = entries.iter().map(|entry| entry.to_row()).collect();
-        lines.sort();
+        lines.sort_by(|a, b| {
+            if let Some((prefix, _)) = a.split_once("**") {
+                if b.starts_with(prefix) {
+                    return Ordering::Less;
+                }
+            }
+            if let Some((prefix, _)) = b.split_once("**") {
+                if a.starts_with(prefix) {
+                    return Ordering::Greater;
+                }
+            }
+            a.cmp(b)
+        });
         lines
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_to_sorted_lines_with_special_characters() {
+        // The `(` character is less than `*` in the default sort order.
+        let mut illustrate_issue = vec!["*".to_string(), "(".to_string()];
+        illustrate_issue.sort();
+        assert_eq!(illustrate_issue, vec!["(".to_string(), "*".to_string()]);
+
+        let entries = vec![
+            Entry {
+                // Problem is when a dir name starts with `(`.
+                path: "directory/owner/(my_folder)/**/**".to_string(),
+                github_team: "@foo".to_string(),
+                team_name: "footeam".to_string(),
+                disabled: false,
+            },
+            Entry {
+                // Another example of a dir starting with `(`.
+                path: "directory/owner/(my_folder)/without_glob".to_string(),
+                github_team: "@zoo".to_string(),
+                team_name: "zooteam".to_string(),
+                disabled: false,
+            },
+            Entry {
+                // And is compared to a glob that starts with `*`.
+                path: "directory/owner/**".to_string(),
+                github_team: "@bar".to_string(),
+                team_name: "barteam".to_string(),
+                disabled: false,
+            },
+            Entry {
+                path: "directory/owner/my_folder/**".to_string(),
+                github_team: "@baz".to_string(),
+                team_name: "bazteam".to_string(),
+                disabled: false,
+            },
+            Entry {
+                path: "directory/**".to_string(),
+                github_team: "@bop".to_string(),
+                team_name: "bopteam".to_string(),
+                disabled: false,
+            },
+        ];
+        let sorted = FileGenerator::to_sorted_lines(&entries);
+        assert_eq!(
+            sorted,
+            vec![
+                "/directory/** @bop",
+                "/directory/owner/** @bar",
+                "/directory/owner/(my_folder)/**/** @foo",
+                "/directory/owner/(my_folder)/without_glob @zoo",
+                "/directory/owner/my_folder/** @baz"
+            ]
+        );
+    }
+
+    #[test]
+    fn test_basic_sorting() {
+        let entries = vec![
+            Entry {
+                path: "b_directory/owner/**".to_string(),
+                github_team: "@bar".to_string(),
+                team_name: "barteam".to_string(),
+                disabled: false,
+            },
+            Entry {
+                path: "a_directory/owner/**".to_string(),
+                github_team: "@foo".to_string(),
+                team_name: "footeam".to_string(),
+                disabled: false,
+            },
+        ];
+        let sorted = FileGenerator::to_sorted_lines(&entries);
+        assert_eq!(sorted, vec!["/a_directory/owner/** @foo", "/b_directory/owner/** @bar"]);
+    }
+
+    #[test]
+    fn test_sorting_with_mixed_case() {
+        let entries = vec![
+            Entry {
+                path: "directory/Owner/**".to_string(),
+                github_team: "@bar".to_string(),
+                team_name: "barteam".to_string(),
+                disabled: false,
+            },
+            Entry {
+                path: "directory/owner/**".to_string(),
+                github_team: "@foo".to_string(),
+                team_name: "footeam".to_string(),
+                disabled: false,
+            },
+        ];
+        let sorted = FileGenerator::to_sorted_lines(&entries);
+        assert_eq!(sorted, vec!["/directory/Owner/** @bar", "/directory/owner/** @foo"]);
+    }
+
+    #[test]
+    fn test_sorting_with_numbers() {
+        let entries = vec![
+            Entry {
+                path: "directory/owner1/**".to_string(),
+                github_team: "@foo".to_string(),
+                team_name: "footeam".to_string(),
+                disabled: false,
+            },
+            Entry {
+                path: "directory/owner2/**".to_string(),
+                github_team: "@bar".to_string(),
+                team_name: "barteam".to_string(),
+                disabled: false,
+            },
+        ];
+        let sorted = FileGenerator::to_sorted_lines(&entries);
+        assert_eq!(sorted, vec!["/directory/owner1/** @foo", "/directory/owner2/** @bar"]);
+    }
+
+    #[test]
+    fn test_sorting_with_special_characters() {
+        let entries = vec![
+            Entry {
+                path: "directory/owner-1/**".to_string(),
+                github_team: "@foo".to_string(),
+                team_name: "footeam".to_string(),
+                disabled: false,
+            },
+            Entry {
+                path: "directory/owner_2/**".to_string(),
+                github_team: "@bar".to_string(),
+                team_name: "barteam".to_string(),
+                disabled: false,
+            },
+        ];
+        let sorted = FileGenerator::to_sorted_lines(&entries);
+        assert_eq!(sorted, vec!["/directory/owner-1/** @foo", "/directory/owner_2/** @bar"]);
+    }
+}

src/ownership/file_generator.rs

@@ -12,7 +12,7 @@ pub struct FileOwnerFinder<'a> {
     pub owner_matchers: &'a [OwnerMatcher],
 }
 
-impl<'a> FileOwnerFinder<'a> {
+impl FileOwnerFinder<'_> {
     pub fn find(&self, relative_path: &Path) -> Vec<Owner> {
         let mut team_sources_map: HashMap<&TeamName, Vec<Source>> = HashMap::new();
         let mut directory_overrider = DirectoryOverrider::default();

src/ownership/file_owner_finder.rs

@@ -16,6 +16,8 @@
 /ruby/app/payments/**/* @PaymentsTeam
 
 # Owner in .codeowner
+/javascript/packages/items/**/** @PayrollTeam
+/javascript/packages/items/(special)/**/** @PaymentsTeam
 /ruby/app/payroll/**/** @PayrollTeam
 
 # Owner metadata key in package.yml

tests/fixtures/valid_project/.github/CODEOWNERS

@@ -0,0 +1 @@
+Payments

tests/fixtures/valid_project/javascript/packages/items/(special)/.codeowner

@@ -0,0 +1 @@
+Payroll

tests/fixtures/valid_project/javascript/packages/items/(special)/pay.ts

@@ -120,6 +120,7 @@ fn test_for_team() -> Result<(), Box<dyn Error>> {
             This team owns nothing in this category.
 
             ## Owner in .codeowner
+            /javascript/packages/items/**/**
             /ruby/app/payroll/**/**
 
             ## Owner metadata key in package.yml