diff --git a/src/cli.rs b/src/cli.rs index 1e72bce..cb56ac6 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -38,12 +38,17 @@ enum Command { about = "Validate the validity of the CODEOWNERS file. A validation failure will exit with a failure code and a detailed output of the validation errors.", visible_alias = "v" )] - Validate, + Validate { + #[arg(help = "Optional list of files to validate ownership for (fast mode for git hooks)")] + files: Vec, + }, #[clap(about = "Chains both `generate` and `validate` commands.", visible_alias = "gv")] GenerateAndValidate { #[arg(long, short, default_value = "false", help = "Skip staging the CODEOWNERS file")] skip_stage: bool, + #[arg(help = "Optional list of files to validate ownership for (fast mode for git hooks)")] + files: Vec, }, #[clap(about = "Delete the cache file.", visible_alias = "d")] @@ -112,9 +117,9 @@ pub fn cli() -> Result { }; let runner_result = match args.command { - Command::Validate => runner::validate(&run_config, vec![]), + Command::Validate { files } => runner::validate(&run_config, files), Command::Generate { skip_stage } => runner::generate(&run_config, !skip_stage), - Command::GenerateAndValidate { skip_stage } => runner::generate_and_validate(&run_config, vec![], !skip_stage), + Command::GenerateAndValidate { files, skip_stage } => runner::generate_and_validate(&run_config, files, !skip_stage), Command::ForFile { name, from_codeowners, diff --git a/src/runner.rs b/src/runner.rs index abf573d..4d64750 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -91,7 +91,15 @@ impl Runner { }) } - pub fn validate(&self) -> RunResult { + pub fn validate(&self, file_paths: Vec) -> RunResult { + if file_paths.is_empty() { + self.validate_all() + } else { + self.validate_files(file_paths) + } + } + + fn validate_all(&self) -> RunResult { match self.ownership.validate() { Ok(_) => RunResult::default(), Err(err) => RunResult { @@ -101,6 +109,40 @@ impl Runner { } } + fn validate_files(&self, file_paths: Vec) -> RunResult { + let mut unowned_files = Vec::new(); + let mut io_errors = Vec::new(); + + for file_path in file_paths { + match team_for_file_from_codeowners(&self.run_config, &file_path) { + Ok(Some(_)) => {} + Ok(None) => unowned_files.push(file_path), + Err(err) => io_errors.push(format!("{}: {}", file_path, err)), + } + } + + if !unowned_files.is_empty() { + let validation_errors = std::iter::once("Unowned files detected:".to_string()) + .chain(unowned_files.into_iter().map(|file| format!(" {}", file))) + .collect(); + + return RunResult { + validation_errors, + io_errors, + ..Default::default() + }; + } + + if !io_errors.is_empty() { + return RunResult { + io_errors, + ..Default::default() + }; + } + + RunResult::default() + } + pub fn generate(&self, git_stage: bool) -> RunResult { let content = self.ownership.generate_file(); if let Some(parent) = &self.run_config.codeowners_file_path.parent() { @@ -120,12 +162,12 @@ impl Runner { } } - pub fn generate_and_validate(&self, git_stage: bool) -> RunResult { + pub fn generate_and_validate(&self, file_paths: Vec, git_stage: bool) -> RunResult { let run_result = self.generate(git_stage); if run_result.has_errors() { return run_result; } - self.validate() + self.validate(file_paths) } fn git_stage(&self) { diff --git a/src/runner/api.rs b/src/runner/api.rs index acaf8ae..9a88512 100644 --- a/src/runner/api.rs +++ b/src/runner/api.rs @@ -16,16 +16,16 @@ pub fn for_team(run_config: &RunConfig, team_name: &str) -> RunResult { run(run_config, |runner| runner.for_team(team_name)) } -pub fn validate(run_config: &RunConfig, _file_paths: Vec) -> RunResult { - run(run_config, |runner| runner.validate()) +pub fn validate(run_config: &RunConfig, file_paths: Vec) -> RunResult { + run(run_config, |runner| runner.validate(file_paths)) } pub fn generate(run_config: &RunConfig, git_stage: bool) -> RunResult { run(run_config, |runner| runner.generate(git_stage)) } -pub fn generate_and_validate(run_config: &RunConfig, _file_paths: Vec, git_stage: bool) -> RunResult { - run(run_config, |runner| runner.generate_and_validate(git_stage)) +pub fn generate_and_validate(run_config: &RunConfig, file_paths: Vec, git_stage: bool) -> RunResult { + run(run_config, |runner| runner.generate_and_validate(file_paths, git_stage)) } pub fn delete_cache(run_config: &RunConfig) -> RunResult { diff --git a/tests/validate_files_test.rs b/tests/validate_files_test.rs new file mode 100644 index 0000000..e1a2c7e --- /dev/null +++ b/tests/validate_files_test.rs @@ -0,0 +1,144 @@ +use assert_cmd::prelude::*; +use predicates::prelude::*; +use std::{error::Error, process::Command}; + +mod common; + +use common::*; + +#[test] +fn test_validate_with_owned_files() -> Result<(), Box> { + run_codeowners( + "valid_project", + &["validate", "ruby/app/models/payroll.rb", "ruby/app/models/bank_account.rb"], + true, + OutputStream::Stdout, + predicate::eq(""), + )?; + + Ok(()) +} + +#[test] +fn test_validate_with_unowned_file() -> Result<(), Box> { + run_codeowners( + "valid_project", + &["validate", "ruby/app/unowned.rb"], + false, + OutputStream::Stdout, + predicate::str::contains("ruby/app/unowned.rb").and(predicate::str::contains("Unowned")), + )?; + + Ok(()) +} + +#[test] +fn test_validate_with_mixed_files() -> Result<(), Box> { + run_codeowners( + "valid_project", + &["validate", "ruby/app/models/payroll.rb", "ruby/app/unowned.rb"], + false, + OutputStream::Stdout, + predicate::str::contains("ruby/app/unowned.rb").and(predicate::str::contains("Unowned")), + )?; + + Ok(()) +} + +#[test] +fn test_validate_with_no_files() -> Result<(), Box> { + // Existing behavior - validates entire project + run_codeowners("valid_project", &["validate"], true, OutputStream::Stdout, predicate::eq(""))?; + + Ok(()) +} + +#[test] +fn test_generate_and_validate_with_owned_files() -> Result<(), Box> { + let fixture_root = std::path::Path::new("tests/fixtures/valid_project"); + let temp_dir = setup_fixture_repo(fixture_root); + let project_root = temp_dir.path(); + git_add_all_files(project_root); + + let codeowners_path = project_root.join("tmp/CODEOWNERS"); + + Command::cargo_bin("codeowners")? + .arg("--project-root") + .arg(project_root) + .arg("--codeowners-file-path") + .arg(&codeowners_path) + .arg("--no-cache") + .arg("generate-and-validate") + .arg("ruby/app/models/payroll.rb") + .arg("ruby/app/models/bank_account.rb") + .assert() + .success(); + + Ok(()) +} + +#[test] +fn test_generate_and_validate_with_unowned_file() -> Result<(), Box> { + let fixture_root = std::path::Path::new("tests/fixtures/valid_project"); + let temp_dir = setup_fixture_repo(fixture_root); + let project_root = temp_dir.path(); + git_add_all_files(project_root); + + let codeowners_path = project_root.join("tmp/CODEOWNERS"); + + Command::cargo_bin("codeowners")? + .arg("--project-root") + .arg(project_root) + .arg("--codeowners-file-path") + .arg(&codeowners_path) + .arg("--no-cache") + .arg("generate-and-validate") + .arg("ruby/app/unowned.rb") + .assert() + .failure() + .stdout(predicate::str::contains("ruby/app/unowned.rb")) + .stdout(predicate::str::contains("Unowned")); + + Ok(()) +} + +#[test] +fn test_validate_with_absolute_path() -> Result<(), Box> { + let fixture_root = std::path::Path::new("tests/fixtures/valid_project"); + let temp_dir = setup_fixture_repo(fixture_root); + let project_root = temp_dir.path(); + git_add_all_files(project_root); + + let file_absolute_path = project_root.join("ruby/app/models/payroll.rb").canonicalize()?; + + Command::cargo_bin("codeowners")? + .arg("--project-root") + .arg(project_root) + .arg("--no-cache") + .arg("validate") + .arg(file_absolute_path.to_str().unwrap()) + .assert() + .success(); + + Ok(()) +} + +#[test] +fn test_validate_only_checks_codeowners_file() -> Result<(), Box> { + // This test demonstrates that `validate` with files only checks the CODEOWNERS file + // It does NOT check file annotations or other ownership sources + // + // If a file has an annotation but is missing from CODEOWNERS, `validate` will report it as unowned + // This is why `generate-and-validate` should be used for accuracy + + // ruby/app/models/bank_account.rb has @team Payments annotation and is in CODEOWNERS + run_codeowners( + "valid_project", + &["validate", "ruby/app/models/bank_account.rb"], + true, + OutputStream::Stdout, + predicate::eq(""), + )?; + + Ok(()) +}