diff --git a/Cargo.lock b/Cargo.lock index ac8b2d9..69a49f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -162,7 +162,7 @@ checksum = "1462739cb27611015575c0c11df5df7601141071f07518d56fcc1be504cbec97" [[package]] name = "codeowners" -version = "0.2.2" +version = "0.2.3" dependencies = [ "assert_cmd", "clap", diff --git a/Cargo.toml b/Cargo.toml index 2d97658..dbb6fa0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "codeowners" -version = "0.2.2" +version = "0.2.3" edition = "2021" [profile.release] diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 2a718e8..6b1db70 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.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"] diff --git a/src/ownership/file_generator.rs b/src/ownership/file_generator.rs index b4097e1..f2b8efb 100644 --- a/src/ownership/file_generator.rs +++ b/src/ownership/file_generator.rs @@ -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 { let mut lines: Vec = 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"]); + } +} diff --git a/src/ownership/file_owner_finder.rs b/src/ownership/file_owner_finder.rs index 39d9280..502294a 100644 --- a/src/ownership/file_owner_finder.rs +++ b/src/ownership/file_owner_finder.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 { let mut team_sources_map: HashMap<&TeamName, Vec> = HashMap::new(); let mut directory_overrider = DirectoryOverrider::default(); diff --git a/tests/fixtures/valid_project/.github/CODEOWNERS b/tests/fixtures/valid_project/.github/CODEOWNERS index c7785fe..38f4292 100644 --- a/tests/fixtures/valid_project/.github/CODEOWNERS +++ b/tests/fixtures/valid_project/.github/CODEOWNERS @@ -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 diff --git a/tests/fixtures/valid_project/javascript/packages/items/(special)/.codeowner b/tests/fixtures/valid_project/javascript/packages/items/(special)/.codeowner new file mode 100644 index 0000000..a8040e7 --- /dev/null +++ b/tests/fixtures/valid_project/javascript/packages/items/(special)/.codeowner @@ -0,0 +1 @@ +Payments \ No newline at end of file diff --git a/tests/fixtures/valid_project/javascript/packages/items/(special)/pay.ts b/tests/fixtures/valid_project/javascript/packages/items/(special)/pay.ts new file mode 100644 index 0000000..e69de29 diff --git a/tests/fixtures/valid_project/javascript/packages/items/.codeowner b/tests/fixtures/valid_project/javascript/packages/items/.codeowner new file mode 100644 index 0000000..4dc904f --- /dev/null +++ b/tests/fixtures/valid_project/javascript/packages/items/.codeowner @@ -0,0 +1 @@ +Payroll \ No newline at end of file diff --git a/tests/fixtures/valid_project/javascript/packages/items/item.ts b/tests/fixtures/valid_project/javascript/packages/items/item.ts new file mode 100644 index 0000000..e69de29 diff --git a/tests/valid_project_test.rs b/tests/valid_project_test.rs index a781476..497c4a1 100644 --- a/tests/valid_project_test.rs +++ b/tests/valid_project_test.rs @@ -120,6 +120,7 @@ fn test_for_team() -> Result<(), Box> { This team owns nothing in this category. ## Owner in .codeowner + /javascript/packages/items/**/** /ruby/app/payroll/**/** ## Owner metadata key in package.yml