adding verify compare for file

Author: perryqh

prompts/owner-overrides.md

@@ -0,0 +1,31 @@
+# Feature Request
+
+## Owner overrides
+
+### Context
+
+Today, if more than one mapper claims ownership of a file, we return an error. There are limited exceptions for directory ownership where multiple directories may apply, and we conceptually pick the nearest directory.
+
+We have seen frequent confusion and repeated requests for a way to explicitly override ownership.
+
+### Proposal
+
+Allow overrides, resolving conflicts by choosing the most specific claim.
+
+#### Priority (most specific to least specific)
+
+Ownership for a file is determined by the _closest_ applicable claim:
+
+1. File annotations (inline annotations in the file)
+1. *Directory ownership (the nearest ancestor directory claim)
+1. Package ownership (`package.yml`, `package.json`)
+1. Team glob patterns
+
+If multiple teams match at the same priority level (e.g., multiple team glob patterns match the same path), this remains an error.
+
+#### Tooling to reduce confusion
+
+Update the `for-file` command to list all matching teams in descending priority, so users can see which owner would win and why.
+
+
+* If directory ownership is declared _above_ a package file, it would be lower priority

src/cli.rs

@@ -46,6 +46,9 @@ enum Command {
 
     #[clap(about = "Delete the cache file.", visible_alias = "d")]
     DeleteCache,
+
+    #[clap(about = "Compare the CODEOWNERS file to the for-file command.")]
+    VerifyCompareForFile,
 }
 
 /// A CLI to validate and generate Github's CODEOWNERS file.
@@ -113,6 +116,7 @@ pub fn cli() -> Result<RunResult, RunnerError> {
         Command::ForFile { name, fast: _ } => runner::for_file(&run_config, &name),
         Command::ForTeam { name } => runner::for_team(&run_config, &name),
         Command::DeleteCache => runner::delete_cache(&run_config),
+        Command::VerifyCompareForFile => runner::verify_compare_for_file(&run_config),
     };
 
     Ok(runner_result)

src/lib.rs

@@ -6,3 +6,4 @@ pub(crate) mod project;
 pub mod project_builder;
 pub mod project_file_builder;
 pub mod runner;
+pub mod verify;

src/runner.rs

@@ -132,6 +132,10 @@ pub fn delete_cache(run_config: &RunConfig) -> RunResult {
     run_with_runner(run_config, |runner| runner.delete_cache())
 }
 
+pub fn verify_compare_for_file(run_config: &RunConfig) -> RunResult {
+    run_with_runner(run_config, |runner| runner.verify_compare_for_file())
+}
+
 pub type Runnable = fn(Runner) -> RunResult;
 
 pub fn run_with_runner<F>(run_config: &RunConfig, runnable: F) -> RunResult
@@ -172,7 +176,7 @@ impl fmt::Display for Error {
     }
 }
 
-fn config_from_path(path: &PathBuf) -> Result<Config, Error> {
+pub(crate) fn config_from_path(path: &PathBuf) -> Result<Config, Error> {
     let config_file = File::open(path)
         .change_context(Error::Io(format!("Can't open config file: {}", &path.to_string_lossy())))
         .attach_printable(format!("Can't open config file: {}", &path.to_string_lossy()))?;
@@ -299,6 +303,10 @@ impl Runner {
             },
         }
     }
+
+    pub fn verify_compare_for_file(&self) -> RunResult {
+        crate::verify::verify_compare_for_file(&self.run_config, &self.cache)
+    }
 }
 
 #[cfg(test)]

src/verify.rs

@@ -0,0 +1,90 @@
+use std::path::Path;
+
+use crate::{
+    cache::Cache,
+    config::Config,
+    ownership::for_file_fast::find_file_owners,
+    project::Project,
+    project_builder::ProjectBuilder,
+    runner::{RunConfig, RunResult, config_from_path, team_for_file_from_codeowners},
+};
+
+pub fn verify_compare_for_file(run_config: &RunConfig, cache: &Cache) -> RunResult {
+    match do_verify_compare_for_file(run_config, cache) {
+        Ok(mismatches) if mismatches.is_empty() => RunResult {
+            info_messages: vec!["Success! All files match between CODEOWNERS and for-file command.".to_string()],
+            ..Default::default()
+        },
+        Ok(mismatches) => RunResult {
+            validation_errors: mismatches,
+            ..Default::default()
+        },
+        Err(err) => RunResult {
+            io_errors: vec![err],
+            ..Default::default()
+        },
+    }
+}
+
+fn do_verify_compare_for_file(run_config: &RunConfig, cache: &Cache) -> Result<Vec<String>, String> {
+    let config = load_config(run_config)?;
+    let project = build_project(&config, run_config, cache)?;
+
+    let mut mismatches: Vec<String> = Vec::new();
+    for file in &project.files {
+        let (codeowners_team, fast_display) = owners_for_file(&file.path, run_config, &config)?;
+        let codeowners_display = codeowners_team.clone().unwrap_or_else(|| "Unowned".to_string());
+        if !is_match(codeowners_team.as_deref(), &fast_display) {
+            mismatches.push(format_mismatch(&project, &file.path, &codeowners_display, &fast_display));
+        }
+    }
+
+    Ok(mismatches)
+}
+
+fn load_config(run_config: &RunConfig) -> Result<Config, String> {
+    config_from_path(&run_config.config_path).map_err(|e| e.to_string())
+}
+
+fn build_project(config: &Config, run_config: &RunConfig, cache: &Cache) -> Result<Project, String> {
+    let mut project_builder = ProjectBuilder::new(
+        config,
+        run_config.project_root.clone(),
+        run_config.codeowners_file_path.clone(),
+        cache,
+    );
+    project_builder.build().map_err(|e| e.to_string())
+}
+
+fn owners_for_file(path: &Path, run_config: &RunConfig, config: &Config) -> Result<(Option<String>, String), String> {
+    let file_path_str = path.to_string_lossy().to_string();
+
+    let codeowners_team = team_for_file_from_codeowners(run_config, &file_path_str)
+        .map_err(|e| e.to_string())?
+        .map(|t| t.name);
+
+    let fast_owners = find_file_owners(&run_config.project_root, config, Path::new(&file_path_str))?;
+    let fast_display = match fast_owners.len() {
+        0 => "Unowned".to_string(),
+        1 => fast_owners[0].team.name.clone(),
+        _ => {
+            let names: Vec<String> = fast_owners.into_iter().map(|fo| fo.team.name).collect();
+            format!("Multiple: {}", names.join(", "))
+        }
+    };
+
+    Ok((codeowners_team, fast_display))
+}
+
+fn is_match(codeowners_team: Option<&str>, fast_display: &str) -> bool {
+    match (codeowners_team, fast_display) {
+        (None, "Unowned") => true,
+        (Some(t), fd) if fd == t => true,
+        _ => false,
+    }
+}
+
+fn format_mismatch(project: &Project, file_path: &Path, codeowners_display: &str, fast_display: &str) -> String {
+    let rel = project.relative_path(file_path).to_string_lossy().to_string();
+    format!("- {}: CODEOWNERS={} fast={}", rel, codeowners_display, fast_display)
+}

tests/fixtures/valid_project_with_overrides/tmp/cache/codeowners/project-file-cache.json

@@ -36,6 +36,22 @@ fn test_generate() -> Result<(), Box<dyn Error>> {
     Ok(())
 }
 
+#[test]
+fn test_verify_compare_for_file() -> Result<(), Box<dyn Error>> {
+    Command::cargo_bin("codeowners")?
+        .arg("--project-root")
+        .arg("tests/fixtures/valid_project")
+        .arg("--no-cache")
+        .arg("verify-compare-for-file")
+        .assert()
+        .success()
+        .stdout(predicate::eq(indoc! {"
+            Success! All files match between CODEOWNERS and for-file command.
+        "}));
+
+    Ok(())
+}
+
 #[test]
 fn test_for_file() -> Result<(), Box<dyn Error>> {
     Command::cargo_bin("codeowners")?