diff --git a/Cargo.lock b/Cargo.lock index b95f982..a22bc1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -150,7 +150,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.11", + "syn 2.0.13", ] [[package]] @@ -171,6 +171,7 @@ dependencies = [ "itertools", "jwalk", "path-clean", + "predicates", "rayon", "regex", "rusty-hook", @@ -321,6 +322,15 @@ dependencies = [ "rustc_version", ] +[[package]] +name = "float-cmp" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "98de4bbd547a563b716d8dfa9aad1cb19bfab00f4fa09a6a4ed21dbcf44ce9c4" +dependencies = [ + "num-traits", +] + [[package]] name = "fsio" version = "0.1.3" @@ -441,9 +451,9 @@ checksum = "99227334921fae1a979cf0bfdfcc6b3e5ce376ef57e16fb6fb3ea2ed6095f80c" [[package]] name = "linux-raw-sys" -version = "0.3.0" +version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cd550e73688e6d578f0ac2119e32b797a327631a42f9433e59d02e139c8df60d" +checksum = "d59d8c75012853d2e872fb56bc8a2e53718e2cafe1a4c823143141c6d90c322f" [[package]] name = "log" @@ -484,6 +494,12 @@ version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ab250442c86f1850815b5d268639dff018c0627022bc1940eb2d642ca1ce12f0" +[[package]] +name = "normalize-line-endings" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" + [[package]] name = "nu-ansi-term" version = "0.46.0" @@ -494,6 +510,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "num-traits" +version = "0.2.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "578ede34cf02f8924ab9447f50c28075b4d3e5b269972345e7e0372b38c6cdcd" +dependencies = [ + "autocfg", +] + [[package]] name = "num_cpus" version = "1.15.0" @@ -536,8 +561,11 @@ checksum = "c575290b64d24745b6c57a12a31465f0a66f3a4799686a6921526a33b0797965" dependencies = [ "anstyle", "difflib", + "float-cmp", "itertools", + "normalize-line-endings", "predicates-core", + "regex", ] [[package]] @@ -558,9 +586,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.54" +version = "1.0.55" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e472a104799c74b514a57226160104aa483546de37e839ec50e3c2e41dd87534" +checksum = "1d0dd4be24fcdcfeaa12a432d588dc59bbad6cad3510c67e74a2b6b2fc950564" dependencies = [ "unicode-ident", ] @@ -633,9 +661,9 @@ dependencies = [ [[package]] name = "rustix" -version = "0.37.5" +version = "0.37.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0e78cc525325c06b4a7ff02db283472f3c042b7ff0c391f96c6d5ac6f4f91b75" +checksum = "d097081ed288dfe45699b72f5b5d648e5f15d64d900c7080273baa20c16a6849" dependencies = [ "bitflags", "errno", @@ -692,7 +720,7 @@ checksum = "4c614d17805b093df4b147b51339e7e44bf05ef59fba1e45d83500bcfb4d8585" dependencies = [ "proc-macro2", "quote", - "syn 2.0.11", + "syn 2.0.13", ] [[package]] @@ -742,9 +770,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.11" +version = "2.0.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "21e3787bb71465627110e7d87ed4faaa36c1f61042ee67badb9e2ef173accc40" +checksum = "4c9da457c5285ac1f936ebd076af6dac17a61cfe7826f2076b4d015cf47bc8ec" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index 980e6f2..20056cd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,3 +24,4 @@ tracing-subscriber = { version = "0.3.16", features = ["env-filter"] } [dev-dependencies] assert_cmd = "2.0.10" rusty-hook = "^0.11.2" +predicates = "3.0.2" diff --git a/src/ext.rs b/src/error_stack_ext.rs similarity index 100% rename from src/ext.rs rename to src/error_stack_ext.rs diff --git a/src/main.rs b/src/main.rs index 56e18d4..4e7e971 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,4 +1,4 @@ -use ext::IntoContext; +use error_stack_ext::IntoContext; use ownership::Ownership; use crate::project::Project; @@ -13,7 +13,7 @@ use std::{ }; mod config; -mod ext; +mod error_stack_ext; mod ownership; mod project; @@ -85,7 +85,7 @@ impl Context for Error {} fn main() -> Result<(), Error> { install_logger(); - print_validation_errors_to_stdout(cli())?; + maybe_print_errors(cli())?; Ok(()) } @@ -99,14 +99,13 @@ fn cli() -> Result<(), Error> { let config_file = File::open(&config_path) .into_context(Error::Io) - .attach_printable(format!("{}", config_path.to_string_lossy()))?; + .attach_printable(format!("Can't open config file: {}", config_path.to_string_lossy()))?; let config = serde_yaml::from_reader(config_file).into_context(Error::Io)?; let ownership = Ownership::build(Project::build(&project_root, &codeowners_file_path, &config).change_context(Error::Io)?); - let command = args.command; - match command { + match args.command { Command::Validate => ownership.validate().into_context(Error::ValidationFailed)?, Command::Generate => { std::fs::write(codeowners_file_path, ownership.generate_file()).into_context(Error::Io)?; @@ -120,7 +119,7 @@ fn cli() -> Result<(), Error> { Ok(()) } -fn print_validation_errors_to_stdout(result: Result<(), Error>) -> Result<(), Error> { +fn maybe_print_errors(result: Result<(), Error>) -> Result<(), Error> { if let Err(error) = result { if let Some(validation_errors) = error.downcast_ref::() { println!("{}", validation_errors); diff --git a/src/ownership/validator.rs b/src/ownership/validator.rs index b35be6b..9e916c7 100644 --- a/src/ownership/validator.rs +++ b/src/ownership/validator.rs @@ -157,6 +157,8 @@ impl Error { impl Display for Errors { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let grouped_errors = self.0.iter().into_group_map_by(|error| error.error_category_message()); + let grouped_errors = Vec::from_iter(grouped_errors.iter()); + for (error_category_message, errors) in grouped_errors { write!(f, "\n{}", error_category_message)?; diff --git a/src/project.rs b/src/project.rs index ab4c22f..a70d324 100644 --- a/src/project.rs +++ b/src/project.rs @@ -13,7 +13,7 @@ use rayon::prelude::{IntoParallelIterator, ParallelIterator}; use regex::Regex; use tracing::{debug, instrument}; -use crate::{config::Config, ext::IntoContext}; +use crate::{config::Config, error_stack_ext::IntoContext}; use glob_match::glob_match; pub struct Project { @@ -131,7 +131,7 @@ impl Context for Error {} impl Project { #[instrument(level = "debug", skip_all)] pub fn build(base_path: &Path, codeowners_file_path: &Path, config: &Config) -> Result { - debug!("scanning project ({})", base_path.to_string_lossy()); + debug!(base_path = base_path.to_str(), "scanning project"); let mut owned_file_paths: Vec = Vec::new(); let mut packages: Vec = Vec::new(); diff --git a/tests/fixtures/invalid_project/.github/CODEOWNERS b/tests/fixtures/invalid_project/.github/CODEOWNERS new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/tests/fixtures/invalid_project/.github/CODEOWNERS @@ -0,0 +1 @@ + diff --git a/tests/fixtures/invalid_project/config/code_ownership.yml b/tests/fixtures/invalid_project/config/code_ownership.yml new file mode 100644 index 0000000..fa56397 --- /dev/null +++ b/tests/fixtures/invalid_project/config/code_ownership.yml @@ -0,0 +1,10 @@ +owned_globs: + - "**/*.{rb,tsx}" +ruby_package_paths: + - ruby/packages/**/* +javascript_package_paths: + - javascript/packages/** +team_file_glob: + - config/teams/**/*.yml +vendored_gems_path: gems +unowned_globs: diff --git a/tests/fixtures/invalid_project/config/teams/payments.yml b/tests/fixtures/invalid_project/config/teams/payments.yml new file mode 100644 index 0000000..af0c841 --- /dev/null +++ b/tests/fixtures/invalid_project/config/teams/payments.yml @@ -0,0 +1,5 @@ +name: Payments +github: + team: '@PaymentTeam' +owned_globs: + - ruby/app/payments/**/* diff --git a/tests/fixtures/invalid_project/config/teams/payroll.yml b/tests/fixtures/invalid_project/config/teams/payroll.yml new file mode 100644 index 0000000..8c66f53 --- /dev/null +++ b/tests/fixtures/invalid_project/config/teams/payroll.yml @@ -0,0 +1,9 @@ +name: Payroll +github: + team: '@PayrollTeam' +ruby: + owned_gems: + - payroll_calculator +javascript: + owned_packages: + - 'PayrollFlow' diff --git a/tests/fixtures/invalid_project/gems/payroll_calculator/calculator.rb b/tests/fixtures/invalid_project/gems/payroll_calculator/calculator.rb new file mode 100644 index 0000000..3505ddb --- /dev/null +++ b/tests/fixtures/invalid_project/gems/payroll_calculator/calculator.rb @@ -0,0 +1,6 @@ +# @team Payments +class PayrollCalculator + def calculate + 10_000 + end +end diff --git a/tests/fixtures/invalid_project/ruby/app/models/bank_account.rb b/tests/fixtures/invalid_project/ruby/app/models/bank_account.rb new file mode 100644 index 0000000..e57ee65 --- /dev/null +++ b/tests/fixtures/invalid_project/ruby/app/models/bank_account.rb @@ -0,0 +1,3 @@ +# @team Payments + +class BankAccount; end diff --git a/tests/fixtures/invalid_project/ruby/app/models/payroll.rb b/tests/fixtures/invalid_project/ruby/app/models/payroll.rb new file mode 100644 index 0000000..ac9c4ed --- /dev/null +++ b/tests/fixtures/invalid_project/ruby/app/models/payroll.rb @@ -0,0 +1,3 @@ +# @team Payroll + +class Payroll; end diff --git a/tests/fixtures/invalid_project/ruby/app/payments/nacha.rb b/tests/fixtures/invalid_project/ruby/app/payments/nacha.rb new file mode 100644 index 0000000..3503dfc --- /dev/null +++ b/tests/fixtures/invalid_project/ruby/app/payments/nacha.rb @@ -0,0 +1 @@ +class Nacha; end diff --git a/tests/fixtures/invalid_project/ruby/app/unowned.rb b/tests/fixtures/invalid_project/ruby/app/unowned.rb new file mode 100644 index 0000000..e69de29 diff --git a/tests/fixtures/invalid_project/ruby/packages/payroll_flow/package.yml b/tests/fixtures/invalid_project/ruby/packages/payroll_flow/package.yml new file mode 100644 index 0000000..270b730 --- /dev/null +++ b/tests/fixtures/invalid_project/ruby/packages/payroll_flow/package.yml @@ -0,0 +1,2 @@ +metadata: + owner: Payroll diff --git a/tests/fixtures/valid_project/.github/CODEOWNERS b/tests/fixtures/valid_project/.github/CODEOWNERS index a60f0a8..c91ffdc 100644 --- a/tests/fixtures/valid_project/.github/CODEOWNERS +++ b/tests/fixtures/valid_project/.github/CODEOWNERS @@ -13,7 +13,7 @@ /ruby/app/models/payroll.rb @PayrollTeam # Team-specific owned globs -/ruby/app/payments/**/* @PayrollTeam +/ruby/app/payments/**/* @PaymentsTeam # Owner metadata key in package.yml /ruby/packages/payroll_flow/**/** @PayrollTeam @@ -22,8 +22,8 @@ /javascript/packages/PayrollFlow/**/** @PayrollTeam # Team YML ownership -/config/teams/payments.yml @PayrollTeam -/config/teams/payroll.yml @PaymentsTeam +/config/teams/payments.yml @PaymentsTeam +/config/teams/payroll.yml @PayrollTeam # Team owned gems -/gems/payroll_calculator @PaymentsTeam +/gems/payroll_calculator @PayrollTeam diff --git a/tests/fixtures/valid_project/config/teams/payments.yml b/tests/fixtures/valid_project/config/teams/payments.yml index 5e18b66..2f01f29 100644 --- a/tests/fixtures/valid_project/config/teams/payments.yml +++ b/tests/fixtures/valid_project/config/teams/payments.yml @@ -1,5 +1,5 @@ -name: Payroll +name: Payments github: - team: '@PayrollTeam' + team: '@PaymentsTeam' owned_globs: - ruby/app/payments/**/* diff --git a/tests/fixtures/valid_project/config/teams/payroll.yml b/tests/fixtures/valid_project/config/teams/payroll.yml index 72b8466..8c66f53 100644 --- a/tests/fixtures/valid_project/config/teams/payroll.yml +++ b/tests/fixtures/valid_project/config/teams/payroll.yml @@ -1,6 +1,6 @@ -name: Payments +name: Payroll github: - team: '@PaymentsTeam' + team: '@PayrollTeam' ruby: owned_gems: - payroll_calculator diff --git a/tests/invalid_project_test.rs b/tests/invalid_project_test.rs new file mode 100644 index 0000000..ae14622 --- /dev/null +++ b/tests/invalid_project_test.rs @@ -0,0 +1,18 @@ +use assert_cmd::prelude::*; +use predicates::prelude::*; +use std::{error::Error, process::Command}; + +#[test] +fn test_validate() -> Result<(), Box> { + Command::cargo_bin("codeowners")? + .arg("--project-root") + .arg("tests/fixtures/invalid_project") + .arg("validate") + .assert() + .failure() + .stdout(predicate::str::contains("CODEOWNERS out of date. Run `codeownership generate` to update the CODEOWNERS file")) + .stdout(predicate::str::contains("Some files are missing ownership:\n- ruby/app/unowned.rb")) + .stdout(predicate::str::contains("Code ownership should only be defined for each file in one way. The following files have declared ownership in multiple ways.\n- gems/payroll_calculator/calculator.rb (owner: Payments, source: team_file_mapper)\n- gems/payroll_calculator/calculator.rb (owner: Payroll, source: team_gem_mapper)")); + + Ok(()) +}