From 93b6d92fb1807858aa87ca5907ad3b0bbf092420 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 28 Sep 2026 10:38:03 -0700 Subject: [PATCH 1/7] Add allow_unowned_files to permit files with no owner code_ownership#140: a repo that assigns owners to only some files had no way to pass `validate` except `unowned_globs: ['**/*']`, and files matched by unowned_globs are never read, so that also switched off their `@team` annotations. With `allow_unowned_files: true`, a file in owned_globs that no mechanism assigns an owner is no longer an error, in both `validate` and `validate `. Every mechanism, file annotations included, still applies to those files, so the annotations keep working. It defaults to false, and nothing else changes: unowned_globs files are still never read, which is what keeps large third-party trees cheap. Making annotations work inside unowned_globs instead would have meant opening every file those globs match. Measured on a synthetic tree of tracked vendored files, that made a cold `generate` about 7x slower at 100k files and 11x at 300k. --- README.md | 5 +- src/config.rs | 21 ++++ src/ownership.rs | 1 + src/ownership/file_owner_resolver.rs | 1 + src/ownership/validator.rs | 5 +- src/project.rs | 2 + src/project_builder.rs | 1 + src/runner.rs | 1 + tests/allow_unowned_files_test.rs | 98 +++++++++++++++++++ .../allow_unowned_files/.github/CODEOWNERS | 14 +++ .../allow_unowned_files/app/annotated.rb | 2 + .../allow_unowned_files/app/unowned.rb | 1 + .../config/code_ownership.yml | 4 + .../allow_unowned_files/config/teams/foo.yml | 4 + 14 files changed, 157 insertions(+), 3 deletions(-) create mode 100644 tests/allow_unowned_files_test.rs create mode 100644 tests/fixtures/allow_unowned_files/.github/CODEOWNERS create mode 100644 tests/fixtures/allow_unowned_files/app/annotated.rb create mode 100644 tests/fixtures/allow_unowned_files/app/unowned.rb create mode 100644 tests/fixtures/allow_unowned_files/config/code_ownership.yml create mode 100644 tests/fixtures/allow_unowned_files/config/teams/foo.yml diff --git a/README.md b/README.md index ebbafcf..904827e 100644 --- a/README.md +++ b/README.md @@ -209,11 +209,12 @@ codeowners gv --no-cache - `ruby_package_paths` (default: `['packs/**/*', 'components/**']`) - `js_package_paths` / `javascript_package_paths` (default: `['frontend/**/*']`) - `team_file_glob` (default: `['config/teams/**/*.yml']`) -- `unowned_globs` (default: `['frontend/**/node_modules/**/*', 'frontend/**/__generated__/**/*']`) +- `unowned_globs` (default: `['frontend/**/node_modules/**/*', 'frontend/**/__generated__/**/*']`): Files matched here don't need an owner, and they're never read, so file annotations in them are ignored. Use this for third-party or generated code. - `vendored_gems_path` (default: `'vendored/'`) - `cache_directory` (default: `'tmp/cache/codeowners'`) - `ignore_dirs` (default includes: `.git`, `node_modules`, `tmp`, etc.) - `executable_name` (default: `'codeowners'`): Customize the command name shown in validation error messages. Useful when using `codeowners-rs` via wrappers like the [code_ownership](https://github.com/rubyatscale/code_ownership) Ruby gem. +- `allow_unowned_files` (default: `false`): When `true`, files in `owned_globs` that no mechanism assigns an owner are not a validation error. Every ownership mechanism, including file annotations, still applies to them. Use this when only some of your code has owners; unlike putting files in `unowned_globs`, their annotations keep working. Example configuration with custom executable name: @@ -238,7 +239,7 @@ By default, cache is stored under `tmp/cache/codeowners` relative to the project 1. Only one mechanism defines ownership for any file. 2. All referenced teams are valid. -3. All files in `owned_globs` are owned, unless matched by `unowned_globs`. +3. All files in `owned_globs` are owned, unless matched by `unowned_globs` or `allow_unowned_files` is set. 4. The generated `CODEOWNERS` file is up to date. Exit status is non-zero on errors. diff --git a/src/config.rs b/src/config.rs index 620185f..3674525 100644 --- a/src/config.rs +++ b/src/config.rs @@ -31,6 +31,9 @@ pub struct Config { #[serde(default = "default_codeowners_path")] pub codeowners_path: String, + + #[serde(default)] + pub allow_unowned_files: bool, } #[allow(dead_code)] @@ -168,6 +171,24 @@ mod tests { let config_file = File::open(&config_path)?; let config: Config = serde_yaml::from_reader(config_file)?; assert_eq!(config.executable_name, "codeowners generate"); + assert!(!config.allow_unowned_files); + Ok(()) + } + + #[test] + fn test_parse_config_with_allow_unowned_files() -> Result<(), Box> { + let temp_dir = tempdir()?; + let config_path = temp_dir.path().join("config.yml"); + let config_str = indoc! {" + --- + owned_globs: + - \"**/*.rb\" + allow_unowned_files: true + "}; + fs::write(&config_path, config_str)?; + let config_file = File::open(&config_path)?; + let config: Config = serde_yaml::from_reader(config_file)?; + assert!(config.allow_unowned_files); Ok(()) } diff --git a/src/ownership.rs b/src/ownership.rs index c0422e4..0d0ac2d 100644 --- a/src/ownership.rs +++ b/src/ownership.rs @@ -126,6 +126,7 @@ impl Ownership { mappers: self.codeowners_file_mappers(), }, executable_name: self.project.executable_name.clone(), + allow_unowned_files: self.project.allow_unowned_files, }; validator.validate() diff --git a/src/ownership/file_owner_resolver.rs b/src/ownership/file_owner_resolver.rs index cc7f9fa..3276a9d 100644 --- a/src/ownership/file_owner_resolver.rs +++ b/src/ownership/file_owner_resolver.rs @@ -377,6 +377,7 @@ mod tests { ignore_dirs: vec![], executable_name: "codeowners".to_string(), codeowners_path: ".github".to_string(), + allow_unowned_files: false, } } diff --git a/src/ownership/validator.rs b/src/ownership/validator.rs index 3f5339c..af8ddac 100644 --- a/src/ownership/validator.rs +++ b/src/ownership/validator.rs @@ -22,6 +22,7 @@ pub struct Validator { pub mappers: Vec>, pub file_generator: FileGenerator, pub executable_name: String, + pub allow_unowned_files: bool, } #[derive(Debug)] @@ -172,7 +173,9 @@ impl Validator { let relative_path = self.project.relative_path(&file.path).to_owned(); if owners.is_empty() { - validation_errors.push(Error::FileWithoutOwner { path: relative_path }) + if !self.allow_unowned_files { + validation_errors.push(Error::FileWithoutOwner { path: relative_path }) + } } else if owners.len() > 1 { validation_errors.push(Error::FileWithMultipleOwners { path: relative_path, diff --git a/src/project.rs b/src/project.rs index 12b09a6..1176f62 100644 --- a/src/project.rs +++ b/src/project.rs @@ -18,6 +18,7 @@ pub struct Project { pub directory_codeowner_files: Vec, pub teams_by_name: HashMap, pub executable_name: String, + pub allow_unowned_files: bool, } #[derive(Clone, Debug)] @@ -222,6 +223,7 @@ mod tests { directory_codeowner_files: vec![], teams_by_name: HashMap::new(), executable_name: "codeowners generate".to_string(), + allow_unowned_files: false, }; let map = project.vendored_gem_by_name(); diff --git a/src/project_builder.rs b/src/project_builder.rs index e0d06ee..f5ed2a0 100644 --- a/src/project_builder.rs +++ b/src/project_builder.rs @@ -325,6 +325,7 @@ impl<'a> ProjectBuilder<'a> { directory_codeowner_files: directory_codeowners, teams_by_name, executable_name: self.config.executable_name.clone(), + allow_unowned_files: self.config.allow_unowned_files, }) } } diff --git a/src/runner.rs b/src/runner.rs index 5562979..a72a44e 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -168,6 +168,7 @@ impl Runner { for file_path in filtered_paths { match team_for_file_from_codeowners(&self.run_config, &file_path) { Ok(Some(_)) => {} + Ok(None) if self.config.allow_unowned_files => {} Ok(None) => unowned_files.push(file_path), Err(err) => io_errors.push(format!("{}: {}", file_path, err)), } diff --git a/tests/allow_unowned_files_test.rs b/tests/allow_unowned_files_test.rs new file mode 100644 index 0000000..6196d08 --- /dev/null +++ b/tests/allow_unowned_files_test.rs @@ -0,0 +1,98 @@ +use assert_cmd::prelude::*; +use indoc::indoc; +use predicates::prelude::*; +use std::{error::Error, fs, path::Path, process::Command}; + +mod common; +use common::{OutputStream, git_add_all_files, run_codeowners, setup_fixture_repo}; + +const FIXTURE: &str = "tests/fixtures/allow_unowned_files"; + +#[test] +fn test_validate_passes_with_unowned_files() -> Result<(), Box> { + run_codeowners("allow_unowned_files", &["validate"], true, OutputStream::Stdout, predicate::eq(""))?; + Ok(()) +} + +#[test] +fn test_generate_and_validate_passes_with_unowned_files() -> Result<(), Box> { + run_codeowners("allow_unowned_files", &["gv"], true, OutputStream::Stdout, predicate::eq(""))?; + Ok(()) +} + +#[test] +fn test_validate_files_passes_for_an_unowned_file() -> Result<(), Box> { + run_codeowners( + "allow_unowned_files", + &["validate", "app/unowned.rb"], + true, + OutputStream::Stdout, + predicate::eq(""), + )?; + Ok(()) +} + +#[test] +fn test_annotations_still_assign_owners() -> Result<(), Box> { + run_codeowners( + "allow_unowned_files", + &["for-file", "app/annotated.rb"], + true, + OutputStream::Stdout, + predicate::eq(indoc! {" + Team: Foo + Github Team: @FooTeam + Team YML: config/teams/foo.yml + Description: + - Owner annotation at the top of the file + "}), + )?; + Ok(()) +} + +#[test] +fn test_unowned_file_is_reported_as_unowned() -> Result<(), Box> { + run_codeowners( + "allow_unowned_files", + &["for-file", "app/unowned.rb"], + true, + OutputStream::Stdout, + predicate::str::contains("Team: Unowned"), + )?; + Ok(()) +} + +fn run_without_the_flag(args: &[&str]) -> Result> { + let temp_dir = setup_fixture_repo(Path::new(FIXTURE)); + let project_root = temp_dir.path(); + fs::write( + project_root.join("config/code_ownership.yml"), + "---\nowned_globs:\n - \"{app,config}/**/*.rb\"\n", + )?; + git_add_all_files(project_root); + let assert = Command::cargo_bin("codeowners")? + .arg("--project-root") + .arg(project_root) + .arg("--no-cache") + .args(args) + .assert(); + Ok(assert) +} + +#[test] +fn test_validate_reports_unowned_files_without_the_flag() -> Result<(), Box> { + run_without_the_flag(&["validate"])? + .failure() + .stdout(predicate::str::contains("Some files are missing ownership")) + .stdout(predicate::str::contains("- app/unowned.rb")); + Ok(()) +} + +#[test] +fn test_validate_files_reports_an_unowned_file_without_the_flag() -> Result<(), Box> { + run_without_the_flag(&["validate", "app/unowned.rb"])? + .failure() + .stdout(predicate::str::contains("Unowned files detected:")) + .stdout(predicate::str::contains("app/unowned.rb")); + Ok(()) +} diff --git a/tests/fixtures/allow_unowned_files/.github/CODEOWNERS b/tests/fixtures/allow_unowned_files/.github/CODEOWNERS new file mode 100644 index 0000000..0667b79 --- /dev/null +++ b/tests/fixtures/allow_unowned_files/.github/CODEOWNERS @@ -0,0 +1,14 @@ +# STOP! - DO NOT EDIT THIS FILE MANUALLY +# This file was automatically generated by "bin/codeownership validate". +# +# CODEOWNERS is used for GitHub to suggest code/file owners to various GitHub +# teams. This is useful when developers create Pull Requests since the +# code/file owner is notified. Reference GitHub docs for more details: +# https://help.github.com/en/articles/about-code-owners + + +# Team YML ownership +/config/teams/foo.yml @FooTeam + +# Annotations at the top of file +/app/annotated.rb @FooTeam diff --git a/tests/fixtures/allow_unowned_files/app/annotated.rb b/tests/fixtures/allow_unowned_files/app/annotated.rb new file mode 100644 index 0000000..cfab2e8 --- /dev/null +++ b/tests/fixtures/allow_unowned_files/app/annotated.rb @@ -0,0 +1,2 @@ +# @team Foo +puts 'annotated' diff --git a/tests/fixtures/allow_unowned_files/app/unowned.rb b/tests/fixtures/allow_unowned_files/app/unowned.rb new file mode 100644 index 0000000..8eb0b57 --- /dev/null +++ b/tests/fixtures/allow_unowned_files/app/unowned.rb @@ -0,0 +1 @@ +puts 'no owner' diff --git a/tests/fixtures/allow_unowned_files/config/code_ownership.yml b/tests/fixtures/allow_unowned_files/config/code_ownership.yml new file mode 100644 index 0000000..d765f38 --- /dev/null +++ b/tests/fixtures/allow_unowned_files/config/code_ownership.yml @@ -0,0 +1,4 @@ +--- +owned_globs: + - "{app,config}/**/*.rb" +allow_unowned_files: true diff --git a/tests/fixtures/allow_unowned_files/config/teams/foo.yml b/tests/fixtures/allow_unowned_files/config/teams/foo.yml new file mode 100644 index 0000000..3b226c9 --- /dev/null +++ b/tests/fixtures/allow_unowned_files/config/teams/foo.yml @@ -0,0 +1,4 @@ +--- +name: Foo +github: + team: '@FooTeam' From 0b91d2964006a0c3a35d52b6ad0153a51f1ffdaf Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 28 Sep 2026 13:01:06 -0700 Subject: [PATCH 2/7] Take globs in allow_unowned_globs rather than a boolean A boolean could only exempt every file at once, while a glob list can also scope the exemption, e.g. `**/deprecated/**/*`, and `['**/*']` still covers code_ownership#140's case. It matches how owned_globs and unowned_globs are configured. The difference from unowned_globs is that files here are still read, so annotations and every other mechanism still apply; they just don't have to have an owner. The globs are only checked for files that have no owner, using the same glob_match used for owned_globs on every walked file. --- README.md | 4 +- src/config.rs | 11 +- src/ownership.rs | 2 +- src/ownership/file_owner_resolver.rs | 2 +- src/ownership/validator.rs | 5 +- src/project.rs | 4 +- src/project_builder.rs | 4 +- src/runner.rs | 21 +-- tests/allow_unowned_files_test.rs | 98 -------------- tests/allow_unowned_globs_test.rs | 120 ++++++++++++++++++ .../config/code_ownership.yml | 4 - .../.github/CODEOWNERS | 1 + .../app/annotated.rb | 0 .../app/deprecated/annotated_old.rb | 2 + .../app/deprecated/old.rb} | 0 .../config/code_ownership.yml | 5 + .../config/teams/foo.yml | 0 17 files changed, 157 insertions(+), 126 deletions(-) delete mode 100644 tests/allow_unowned_files_test.rs create mode 100644 tests/allow_unowned_globs_test.rs delete mode 100644 tests/fixtures/allow_unowned_files/config/code_ownership.yml rename tests/fixtures/{allow_unowned_files => allow_unowned_globs}/.github/CODEOWNERS (92%) rename tests/fixtures/{allow_unowned_files => allow_unowned_globs}/app/annotated.rb (100%) create mode 100644 tests/fixtures/allow_unowned_globs/app/deprecated/annotated_old.rb rename tests/fixtures/{allow_unowned_files/app/unowned.rb => allow_unowned_globs/app/deprecated/old.rb} (100%) create mode 100644 tests/fixtures/allow_unowned_globs/config/code_ownership.yml rename tests/fixtures/{allow_unowned_files => allow_unowned_globs}/config/teams/foo.yml (100%) diff --git a/README.md b/README.md index 904827e..9a08729 100644 --- a/README.md +++ b/README.md @@ -214,7 +214,7 @@ codeowners gv --no-cache - `cache_directory` (default: `'tmp/cache/codeowners'`) - `ignore_dirs` (default includes: `.git`, `node_modules`, `tmp`, etc.) - `executable_name` (default: `'codeowners'`): Customize the command name shown in validation error messages. Useful when using `codeowners-rs` via wrappers like the [code_ownership](https://github.com/rubyatscale/code_ownership) Ruby gem. -- `allow_unowned_files` (default: `false`): When `true`, files in `owned_globs` that no mechanism assigns an owner are not a validation error. Every ownership mechanism, including file annotations, still applies to them. Use this when only some of your code has owners; unlike putting files in `unowned_globs`, their annotations keep working. +- `allow_unowned_globs` (default: `[]`): Files in `owned_globs` that match these globs don't need an owner, so leaving them unowned isn't a validation error. Unlike `unowned_globs`, they're still read and every ownership mechanism, including file annotations, still applies to them. Use `['**/*']` if only some of your code has owners, or scope it, e.g. `['**/deprecated/**/*']`. Example configuration with custom executable name: @@ -239,7 +239,7 @@ By default, cache is stored under `tmp/cache/codeowners` relative to the project 1. Only one mechanism defines ownership for any file. 2. All referenced teams are valid. -3. All files in `owned_globs` are owned, unless matched by `unowned_globs` or `allow_unowned_files` is set. +3. All files in `owned_globs` are owned, unless matched by `unowned_globs` or `allow_unowned_globs`. 4. The generated `CODEOWNERS` file is up to date. Exit status is non-zero on errors. diff --git a/src/config.rs b/src/config.rs index 3674525..232b48d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -33,7 +33,7 @@ pub struct Config { pub codeowners_path: String, #[serde(default)] - pub allow_unowned_files: bool, + pub allow_unowned_globs: Vec, } #[allow(dead_code)] @@ -171,24 +171,25 @@ mod tests { let config_file = File::open(&config_path)?; let config: Config = serde_yaml::from_reader(config_file)?; assert_eq!(config.executable_name, "codeowners generate"); - assert!(!config.allow_unowned_files); + assert!(config.allow_unowned_globs.is_empty()); Ok(()) } #[test] - fn test_parse_config_with_allow_unowned_files() -> Result<(), Box> { + fn test_parse_config_with_allow_unowned_globs() -> Result<(), Box> { let temp_dir = tempdir()?; let config_path = temp_dir.path().join("config.yml"); let config_str = indoc! {" --- owned_globs: - \"**/*.rb\" - allow_unowned_files: true + allow_unowned_globs: + - \"**/deprecated/**/*\" "}; fs::write(&config_path, config_str)?; let config_file = File::open(&config_path)?; let config: Config = serde_yaml::from_reader(config_file)?; - assert!(config.allow_unowned_files); + assert_eq!(config.allow_unowned_globs, vec!["**/deprecated/**/*"]); Ok(()) } diff --git a/src/ownership.rs b/src/ownership.rs index 0d0ac2d..41c5e42 100644 --- a/src/ownership.rs +++ b/src/ownership.rs @@ -126,7 +126,7 @@ impl Ownership { mappers: self.codeowners_file_mappers(), }, executable_name: self.project.executable_name.clone(), - allow_unowned_files: self.project.allow_unowned_files, + allow_unowned_globs: self.project.allow_unowned_globs.clone(), }; validator.validate() diff --git a/src/ownership/file_owner_resolver.rs b/src/ownership/file_owner_resolver.rs index 3276a9d..ab6d2ec 100644 --- a/src/ownership/file_owner_resolver.rs +++ b/src/ownership/file_owner_resolver.rs @@ -377,7 +377,7 @@ mod tests { ignore_dirs: vec![], executable_name: "codeowners".to_string(), codeowners_path: ".github".to_string(), - allow_unowned_files: false, + allow_unowned_globs: vec![], } } diff --git a/src/ownership/validator.rs b/src/ownership/validator.rs index af8ddac..70a59c4 100644 --- a/src/ownership/validator.rs +++ b/src/ownership/validator.rs @@ -1,4 +1,5 @@ use crate::project::{Project, ProjectFile}; +use crate::project_builder::matches_globs; use core::fmt; use std::collections::HashSet; use std::fmt::Display; @@ -22,7 +23,7 @@ pub struct Validator { pub mappers: Vec>, pub file_generator: FileGenerator, pub executable_name: String, - pub allow_unowned_files: bool, + pub allow_unowned_globs: Vec, } #[derive(Debug)] @@ -173,7 +174,7 @@ impl Validator { let relative_path = self.project.relative_path(&file.path).to_owned(); if owners.is_empty() { - if !self.allow_unowned_files { + if !matches_globs(&relative_path, &self.allow_unowned_globs) { validation_errors.push(Error::FileWithoutOwner { path: relative_path }) } } else if owners.len() > 1 { diff --git a/src/project.rs b/src/project.rs index 1176f62..3f21b7c 100644 --- a/src/project.rs +++ b/src/project.rs @@ -18,7 +18,7 @@ pub struct Project { pub directory_codeowner_files: Vec, pub teams_by_name: HashMap, pub executable_name: String, - pub allow_unowned_files: bool, + pub allow_unowned_globs: Vec, } #[derive(Clone, Debug)] @@ -223,7 +223,7 @@ mod tests { directory_codeowner_files: vec![], teams_by_name: HashMap::new(), executable_name: "codeowners generate".to_string(), - allow_unowned_files: false, + allow_unowned_globs: vec![], }; let map = project.vendored_gem_by_name(); diff --git a/src/project_builder.rs b/src/project_builder.rs index f5ed2a0..34c4d81 100644 --- a/src/project_builder.rs +++ b/src/project_builder.rs @@ -325,12 +325,12 @@ impl<'a> ProjectBuilder<'a> { directory_codeowner_files: directory_codeowners, teams_by_name, executable_name: self.config.executable_name.clone(), - allow_unowned_files: self.config.allow_unowned_files, + allow_unowned_globs: self.config.allow_unowned_globs.clone(), }) } } -fn matches_globs(path: &Path, globs: &[String]) -> bool { +pub(crate) fn matches_globs(path: &Path, globs: &[String]) -> bool { match path.to_str() { Some(s) => globs.iter().any(|glob| glob_match(glob, s)), None => false, diff --git a/src/runner.rs b/src/runner.rs index a72a44e..2e7abe0 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -146,21 +146,24 @@ impl Runner { let mut unowned_files = Vec::new(); let mut io_errors = Vec::new(); + let relative_to_root = |file_path: &str| { + let path = Path::new(file_path); + if path.is_absolute() { + path.strip_prefix(&self.run_config.project_root).unwrap_or(path).to_path_buf() + } else { + path.to_path_buf() + } + }; + // Filter files based on owned_globs and unowned_globs configuration // Only validate files that match owned_globs and don't match unowned_globs let filtered_paths: Vec = file_paths .into_iter() .filter(|file_path| { - // Convert to relative path for glob matching - let path = Path::new(file_path); - let relative_path = if path.is_absolute() { - path.strip_prefix(&self.run_config.project_root).unwrap_or(path) - } else { - path - }; + let relative_path = relative_to_root(file_path); // Mirror the filtering applied by ProjectBuilder when walking the project - matches_globs(relative_path, &self.config.owned_globs) && !matches_globs(relative_path, &self.config.unowned_globs) + matches_globs(&relative_path, &self.config.owned_globs) && !matches_globs(&relative_path, &self.config.unowned_globs) }) .collect(); @@ -168,7 +171,7 @@ impl Runner { for file_path in filtered_paths { match team_for_file_from_codeowners(&self.run_config, &file_path) { Ok(Some(_)) => {} - Ok(None) if self.config.allow_unowned_files => {} + Ok(None) if matches_globs(&relative_to_root(&file_path), &self.config.allow_unowned_globs) => {} Ok(None) => unowned_files.push(file_path), Err(err) => io_errors.push(format!("{}: {}", file_path, err)), } diff --git a/tests/allow_unowned_files_test.rs b/tests/allow_unowned_files_test.rs deleted file mode 100644 index 6196d08..0000000 --- a/tests/allow_unowned_files_test.rs +++ /dev/null @@ -1,98 +0,0 @@ -use assert_cmd::prelude::*; -use indoc::indoc; -use predicates::prelude::*; -use std::{error::Error, fs, path::Path, process::Command}; - -mod common; -use common::{OutputStream, git_add_all_files, run_codeowners, setup_fixture_repo}; - -const FIXTURE: &str = "tests/fixtures/allow_unowned_files"; - -#[test] -fn test_validate_passes_with_unowned_files() -> Result<(), Box> { - run_codeowners("allow_unowned_files", &["validate"], true, OutputStream::Stdout, predicate::eq(""))?; - Ok(()) -} - -#[test] -fn test_generate_and_validate_passes_with_unowned_files() -> Result<(), Box> { - run_codeowners("allow_unowned_files", &["gv"], true, OutputStream::Stdout, predicate::eq(""))?; - Ok(()) -} - -#[test] -fn test_validate_files_passes_for_an_unowned_file() -> Result<(), Box> { - run_codeowners( - "allow_unowned_files", - &["validate", "app/unowned.rb"], - true, - OutputStream::Stdout, - predicate::eq(""), - )?; - Ok(()) -} - -#[test] -fn test_annotations_still_assign_owners() -> Result<(), Box> { - run_codeowners( - "allow_unowned_files", - &["for-file", "app/annotated.rb"], - true, - OutputStream::Stdout, - predicate::eq(indoc! {" - Team: Foo - Github Team: @FooTeam - Team YML: config/teams/foo.yml - Description: - - Owner annotation at the top of the file - "}), - )?; - Ok(()) -} - -#[test] -fn test_unowned_file_is_reported_as_unowned() -> Result<(), Box> { - run_codeowners( - "allow_unowned_files", - &["for-file", "app/unowned.rb"], - true, - OutputStream::Stdout, - predicate::str::contains("Team: Unowned"), - )?; - Ok(()) -} - -fn run_without_the_flag(args: &[&str]) -> Result> { - let temp_dir = setup_fixture_repo(Path::new(FIXTURE)); - let project_root = temp_dir.path(); - fs::write( - project_root.join("config/code_ownership.yml"), - "---\nowned_globs:\n - \"{app,config}/**/*.rb\"\n", - )?; - git_add_all_files(project_root); - let assert = Command::cargo_bin("codeowners")? - .arg("--project-root") - .arg(project_root) - .arg("--no-cache") - .args(args) - .assert(); - Ok(assert) -} - -#[test] -fn test_validate_reports_unowned_files_without_the_flag() -> Result<(), Box> { - run_without_the_flag(&["validate"])? - .failure() - .stdout(predicate::str::contains("Some files are missing ownership")) - .stdout(predicate::str::contains("- app/unowned.rb")); - Ok(()) -} - -#[test] -fn test_validate_files_reports_an_unowned_file_without_the_flag() -> Result<(), Box> { - run_without_the_flag(&["validate", "app/unowned.rb"])? - .failure() - .stdout(predicate::str::contains("Unowned files detected:")) - .stdout(predicate::str::contains("app/unowned.rb")); - Ok(()) -} diff --git a/tests/allow_unowned_globs_test.rs b/tests/allow_unowned_globs_test.rs new file mode 100644 index 0000000..240c950 --- /dev/null +++ b/tests/allow_unowned_globs_test.rs @@ -0,0 +1,120 @@ +use assert_cmd::prelude::*; +use indoc::indoc; +use predicates::prelude::*; +use std::{error::Error, fs, path::Path, process::Command}; + +mod common; +use common::{OutputStream, git_add_all_files, run_codeowners, setup_fixture_repo}; + +const FIXTURE: &str = "tests/fixtures/allow_unowned_globs"; + +#[test] +fn test_validate_passes_with_unowned_files_in_allowed_globs() -> Result<(), Box> { + run_codeowners("allow_unowned_globs", &["validate"], true, OutputStream::Stdout, predicate::eq(""))?; + Ok(()) +} + +#[test] +fn test_generate_and_validate_passes_with_unowned_files_in_allowed_globs() -> Result<(), Box> { + run_codeowners("allow_unowned_globs", &["gv"], true, OutputStream::Stdout, predicate::eq(""))?; + Ok(()) +} + +#[test] +fn test_validate_files_passes_for_an_unowned_file_in_an_allowed_glob() -> Result<(), Box> { + run_codeowners( + "allow_unowned_globs", + &["validate", "app/deprecated/old.rb"], + true, + OutputStream::Stdout, + predicate::eq(""), + )?; + Ok(()) +} + +#[test] +fn test_annotations_still_assign_owners_in_allowed_globs() -> Result<(), Box> { + run_codeowners( + "allow_unowned_globs", + &["for-file", "app/deprecated/annotated_old.rb"], + true, + OutputStream::Stdout, + predicate::eq(indoc! {" + Team: Foo + Github Team: @FooTeam + Team YML: config/teams/foo.yml + Description: + - Owner annotation at the top of the file + "}), + )?; + Ok(()) +} + +#[test] +fn test_unowned_file_in_an_allowed_glob_is_reported_as_unowned() -> Result<(), Box> { + run_codeowners( + "allow_unowned_globs", + &["for-file", "app/deprecated/old.rb"], + true, + OutputStream::Stdout, + predicate::str::contains("Team: Unowned"), + )?; + Ok(()) +} + +fn run_on_modified_fixture( + modify: impl FnOnce(&Path) -> std::io::Result<()>, + args: &[&str], +) -> Result> { + let temp_dir = setup_fixture_repo(Path::new(FIXTURE)); + let project_root = temp_dir.path(); + modify(project_root)?; + git_add_all_files(project_root); + let assert = Command::cargo_bin("codeowners")? + .arg("--project-root") + .arg(project_root) + .arg("--no-cache") + .args(args) + .assert(); + Ok(assert) +} + +fn add_unowned_file_outside_allowed_globs(project_root: &Path) -> std::io::Result<()> { + fs::write(project_root.join("app/stray.rb"), "puts 'no owner'\n") +} + +#[test] +fn test_validate_reports_unowned_files_outside_allowed_globs() -> Result<(), Box> { + run_on_modified_fixture(add_unowned_file_outside_allowed_globs, &["validate"])? + .failure() + .stdout(predicate::str::contains("Some files are missing ownership")) + .stdout(predicate::str::contains("- app/stray.rb")) + .stdout(predicate::str::contains("app/deprecated/old.rb").not()); + Ok(()) +} + +#[test] +fn test_validate_files_reports_an_unowned_file_outside_allowed_globs() -> Result<(), Box> { + run_on_modified_fixture(add_unowned_file_outside_allowed_globs, &["validate", "app/stray.rb"])? + .failure() + .stdout(predicate::str::contains("Unowned files detected:")) + .stdout(predicate::str::contains("app/stray.rb")); + Ok(()) +} + +#[test] +fn test_validate_reports_unowned_files_without_allowed_globs() -> Result<(), Box> { + run_on_modified_fixture( + |project_root| { + fs::write( + project_root.join("config/code_ownership.yml"), + "---\nowned_globs:\n - \"{app,config}/**/*.rb\"\n", + ) + }, + &["validate"], + )? + .failure() + .stdout(predicate::str::contains("Some files are missing ownership")) + .stdout(predicate::str::contains("- app/deprecated/old.rb")); + Ok(()) +} diff --git a/tests/fixtures/allow_unowned_files/config/code_ownership.yml b/tests/fixtures/allow_unowned_files/config/code_ownership.yml deleted file mode 100644 index d765f38..0000000 --- a/tests/fixtures/allow_unowned_files/config/code_ownership.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -owned_globs: - - "{app,config}/**/*.rb" -allow_unowned_files: true diff --git a/tests/fixtures/allow_unowned_files/.github/CODEOWNERS b/tests/fixtures/allow_unowned_globs/.github/CODEOWNERS similarity index 92% rename from tests/fixtures/allow_unowned_files/.github/CODEOWNERS rename to tests/fixtures/allow_unowned_globs/.github/CODEOWNERS index 0667b79..b117ec9 100644 --- a/tests/fixtures/allow_unowned_files/.github/CODEOWNERS +++ b/tests/fixtures/allow_unowned_globs/.github/CODEOWNERS @@ -12,3 +12,4 @@ # Annotations at the top of file /app/annotated.rb @FooTeam +/app/deprecated/annotated_old.rb @FooTeam diff --git a/tests/fixtures/allow_unowned_files/app/annotated.rb b/tests/fixtures/allow_unowned_globs/app/annotated.rb similarity index 100% rename from tests/fixtures/allow_unowned_files/app/annotated.rb rename to tests/fixtures/allow_unowned_globs/app/annotated.rb diff --git a/tests/fixtures/allow_unowned_globs/app/deprecated/annotated_old.rb b/tests/fixtures/allow_unowned_globs/app/deprecated/annotated_old.rb new file mode 100644 index 0000000..ca506d0 --- /dev/null +++ b/tests/fixtures/allow_unowned_globs/app/deprecated/annotated_old.rb @@ -0,0 +1,2 @@ +# @team Foo +puts 'annotated, in an allowed glob' diff --git a/tests/fixtures/allow_unowned_files/app/unowned.rb b/tests/fixtures/allow_unowned_globs/app/deprecated/old.rb similarity index 100% rename from tests/fixtures/allow_unowned_files/app/unowned.rb rename to tests/fixtures/allow_unowned_globs/app/deprecated/old.rb diff --git a/tests/fixtures/allow_unowned_globs/config/code_ownership.yml b/tests/fixtures/allow_unowned_globs/config/code_ownership.yml new file mode 100644 index 0000000..8fd3663 --- /dev/null +++ b/tests/fixtures/allow_unowned_globs/config/code_ownership.yml @@ -0,0 +1,5 @@ +--- +owned_globs: + - "{app,config}/**/*.rb" +allow_unowned_globs: + - "app/deprecated/**/*" diff --git a/tests/fixtures/allow_unowned_files/config/teams/foo.yml b/tests/fixtures/allow_unowned_globs/config/teams/foo.yml similarity index 100% rename from tests/fixtures/allow_unowned_files/config/teams/foo.yml rename to tests/fixtures/allow_unowned_globs/config/teams/foo.yml From 2060252e77552c50ca02c153c2c31dd5104f046c Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 28 Sep 2026 13:08:41 -0700 Subject: [PATCH 3/7] Keep glob matching local to the validator and reuse relative_to_buf The validator imported matches_globs from project_builder, which made project_builder's copy crate-public just to share a one-line helper. It now has its own private copy, like runner.rs and file_owner_resolver.rs, and project_builder's copy is private again. validate_files' relative-path closure now calls path_utils::relative_to_buf instead of hand-rolling it. Also pin that `allow_unowned_globs: ['**/*']`, the README's way to allow every unowned file, covers files at the project root, for both `validate` and `validate `. --- src/ownership/validator.rs | 9 ++++++++- src/project_builder.rs | 2 +- src/runner.rs | 9 +-------- tests/allow_unowned_globs_test.rs | 19 +++++++++++++++++++ 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/ownership/validator.rs b/src/ownership/validator.rs index 70a59c4..4d5e1ab 100644 --- a/src/ownership/validator.rs +++ b/src/ownership/validator.rs @@ -1,11 +1,11 @@ use crate::project::{Project, ProjectFile}; -use crate::project_builder::matches_globs; use core::fmt; use std::collections::HashSet; use std::fmt::Display; use std::path::{Path, PathBuf}; use std::sync::Arc; +use fast_glob::glob_match; use itertools::Itertools; use rayon::prelude::IntoParallelRefIterator; use rayon::prelude::ParallelIterator; @@ -341,6 +341,13 @@ impl Display for Errors { impl core::error::Error for Errors {} +fn matches_globs(path: &Path, globs: &[String]) -> bool { + match path.to_str() { + Some(s) => globs.iter().any(|glob| glob_match(glob, s)), + None => false, + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/project_builder.rs b/src/project_builder.rs index 34c4d81..1b1a52a 100644 --- a/src/project_builder.rs +++ b/src/project_builder.rs @@ -330,7 +330,7 @@ impl<'a> ProjectBuilder<'a> { } } -pub(crate) fn matches_globs(path: &Path, globs: &[String]) -> bool { +fn matches_globs(path: &Path, globs: &[String]) -> bool { match path.to_str() { Some(s) => globs.iter().any(|glob| glob_match(glob, s)), None => false, diff --git a/src/runner.rs b/src/runner.rs index 2e7abe0..e29f010 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -146,14 +146,7 @@ impl Runner { let mut unowned_files = Vec::new(); let mut io_errors = Vec::new(); - let relative_to_root = |file_path: &str| { - let path = Path::new(file_path); - if path.is_absolute() { - path.strip_prefix(&self.run_config.project_root).unwrap_or(path).to_path_buf() - } else { - path.to_path_buf() - } - }; + let relative_to_root = |file_path: &str| crate::path_utils::relative_to_buf(&self.run_config.project_root, Path::new(file_path)); // Filter files based on owned_globs and unowned_globs configuration // Only validate files that match owned_globs and don't match unowned_globs diff --git a/tests/allow_unowned_globs_test.rs b/tests/allow_unowned_globs_test.rs index 240c950..1ec16b8 100644 --- a/tests/allow_unowned_globs_test.rs +++ b/tests/allow_unowned_globs_test.rs @@ -118,3 +118,22 @@ fn test_validate_reports_unowned_files_without_allowed_globs() -> Result<(), Box .stdout(predicate::str::contains("- app/deprecated/old.rb")); Ok(()) } + +fn allow_every_unowned_file_with_one_at_the_project_root(project_root: &Path) -> std::io::Result<()> { + fs::write(project_root.join("root.rb"), "puts 'no owner'\n")?; + fs::write( + project_root.join("config/code_ownership.yml"), + "---\nowned_globs:\n - \"**/*.rb\"\nallow_unowned_globs:\n - \"**/*\"\n", + ) +} + +#[test] +fn test_double_star_glob_allows_unowned_files_at_the_project_root() -> Result<(), Box> { + run_on_modified_fixture(allow_every_unowned_file_with_one_at_the_project_root, &["validate"])? + .success() + .stdout(predicate::eq("")); + run_on_modified_fixture(allow_every_unowned_file_with_one_at_the_project_root, &["validate", "root.rb"])? + .success() + .stdout(predicate::eq("")); + Ok(()) +} From b694dda422b965ab8b630288e778757215689c27 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 28 Sep 2026 13:17:41 -0700 Subject: [PATCH 4/7] Share one matches_globs in path_utils runner.rs, project_builder.rs and the validator each had an identical private matches_globs. They now all use path_utils::matches_globs, next to relative_to and relative_to_buf, with a unit test that `**/*` matches both root-level and nested paths. --- src/ownership/validator.rs | 9 +-------- src/path_utils.rs | 15 +++++++++++++++ src/project_builder.rs | 10 ++-------- src/runner.rs | 11 ++--------- 4 files changed, 20 insertions(+), 25 deletions(-) diff --git a/src/ownership/validator.rs b/src/ownership/validator.rs index 4d5e1ab..cd807bf 100644 --- a/src/ownership/validator.rs +++ b/src/ownership/validator.rs @@ -1,3 +1,4 @@ +use crate::path_utils::matches_globs; use crate::project::{Project, ProjectFile}; use core::fmt; use std::collections::HashSet; @@ -5,7 +6,6 @@ use std::fmt::Display; use std::path::{Path, PathBuf}; use std::sync::Arc; -use fast_glob::glob_match; use itertools::Itertools; use rayon::prelude::IntoParallelRefIterator; use rayon::prelude::ParallelIterator; @@ -341,13 +341,6 @@ impl Display for Errors { impl core::error::Error for Errors {} -fn matches_globs(path: &Path, globs: &[String]) -> bool { - match path.to_str() { - Some(s) => globs.iter().any(|glob| glob_match(glob, s)), - None => false, - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/src/path_utils.rs b/src/path_utils.rs index 230b1d1..5fa95ca 100644 --- a/src/path_utils.rs +++ b/src/path_utils.rs @@ -10,10 +10,25 @@ pub fn relative_to_buf(root: &Path, path: &Path) -> PathBuf { relative_to(root, path).to_path_buf() } +/// Returns true if `path` matches any of the provided glob patterns. +pub fn matches_globs(path: &Path, globs: &[String]) -> bool { + match path.to_str() { + Some(s) => globs.iter().any(|glob| fast_glob::glob_match(glob, s)), + None => false, + } +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn matches_globs_double_star_matches_root_level_and_nested_files() { + let globs = vec!["**/*".to_string()]; + assert!(matches_globs(Path::new("root.rb"), &globs)); + assert!(matches_globs(Path::new("app/models/user.rb"), &globs)); + } + #[test] fn relative_to_returns_relative_when_under_root() { let root = Path::new("/a/b"); diff --git a/src/project_builder.rs b/src/project_builder.rs index 1b1a52a..b0223b8 100644 --- a/src/project_builder.rs +++ b/src/project_builder.rs @@ -5,11 +5,11 @@ use std::{ }; use error_stack::{Report, ResultExt}; -use fast_glob::glob_match; use ignore::{DirEntry, WalkBuilder, WalkParallel, WalkState}; use rayon::iter::{IntoParallelIterator, ParallelIterator}; use tracing::instrument; +use crate::path_utils::matches_globs; use crate::{ cache::Cache, config::Config, @@ -330,13 +330,6 @@ impl<'a> ProjectBuilder<'a> { } } -fn matches_globs(path: &Path, globs: &[String]) -> bool { - match path.to_str() { - Some(s) => globs.iter().any(|glob| glob_match(glob, s)), - None => false, - } -} - pub(crate) fn ruby_package_owner(path: &Path) -> Result, Report> { let file = File::open(path).change_context(Error::Io)?; let deserializer: deserializers::RubyPackage = serde_yaml::from_reader(file).change_context(Error::SerdeYaml)?; @@ -366,6 +359,7 @@ fn javascript_package_owner(path: &Path) -> Result, Report #[cfg(test)] mod tests { use super::*; + use fast_glob::glob_match; const OWNED_GLOB: &str = "{app,components,config,frontend,lib,packs,spec,danger,script}/**/*.{rb,arb,erb,rake,js,jsx,ts,tsx}"; diff --git a/src/runner.rs b/src/runner.rs index e29f010..fd2b585 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -1,8 +1,9 @@ use std::path::{Path, PathBuf}; use std::process::Command; +use crate::path_utils::matches_globs; + use error_stack::{Report, ResultExt}; -use fast_glob::glob_match; use serde::Serialize; use tracing::debug_span; @@ -439,14 +440,6 @@ impl RunResult { } } -/// Returns true if `path` matches any of the provided glob patterns. -fn matches_globs(path: &Path, globs: &[String]) -> bool { - match path.to_str() { - Some(s) => globs.iter().any(|glob| glob_match(glob, s)), - None => false, - } -} - #[cfg(test)] mod tests { use super::*; From 9edb0efb973588dd13ea6a5cdf23f787d9136aa8 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 28 Sep 2026 14:06:55 -0700 Subject: [PATCH 5/7] Test allow_unowned_globs with absolute paths and the checks it keeps Nothing tested that `validate ` matched allow_unowned_globs against the project-relative path. Matching the raw argument instead passed the whole suite, because the only test passed a path that was already relative. A test with an absolute path now covers it. validate_files also relativizes each path once, carrying it through the owned_globs filter, instead of twice per file. Two more tests pin the README's claim that only the missing-owner check is waived: a file in an allowed glob still fails when it has more than one owner or names an invalid team. The validator now reads allow_unowned_globs from its project instead of keeping a copy, matches_globs is pub(crate) so it doesn't become public API, and its imports sit with the other crate imports. --- src/ownership.rs | 1 - src/ownership/validator.rs | 3 +- src/path_utils.rs | 2 +- src/project_builder.rs | 2 +- src/runner.rs | 23 +++++++------- tests/allow_unowned_globs_test.rs | 53 +++++++++++++++++++++++++++++++ 6 files changed, 67 insertions(+), 17 deletions(-) diff --git a/src/ownership.rs b/src/ownership.rs index 41c5e42..c0422e4 100644 --- a/src/ownership.rs +++ b/src/ownership.rs @@ -126,7 +126,6 @@ impl Ownership { mappers: self.codeowners_file_mappers(), }, executable_name: self.project.executable_name.clone(), - allow_unowned_globs: self.project.allow_unowned_globs.clone(), }; validator.validate() diff --git a/src/ownership/validator.rs b/src/ownership/validator.rs index cd807bf..e796572 100644 --- a/src/ownership/validator.rs +++ b/src/ownership/validator.rs @@ -23,7 +23,6 @@ pub struct Validator { pub mappers: Vec>, pub file_generator: FileGenerator, pub executable_name: String, - pub allow_unowned_globs: Vec, } #[derive(Debug)] @@ -174,7 +173,7 @@ impl Validator { let relative_path = self.project.relative_path(&file.path).to_owned(); if owners.is_empty() { - if !matches_globs(&relative_path, &self.allow_unowned_globs) { + if !matches_globs(&relative_path, &self.project.allow_unowned_globs) { validation_errors.push(Error::FileWithoutOwner { path: relative_path }) } } else if owners.len() > 1 { diff --git a/src/path_utils.rs b/src/path_utils.rs index 5fa95ca..6c42be8 100644 --- a/src/path_utils.rs +++ b/src/path_utils.rs @@ -11,7 +11,7 @@ pub fn relative_to_buf(root: &Path, path: &Path) -> PathBuf { } /// Returns true if `path` matches any of the provided glob patterns. -pub fn matches_globs(path: &Path, globs: &[String]) -> bool { +pub(crate) fn matches_globs(path: &Path, globs: &[String]) -> bool { match path.to_str() { Some(s) => globs.iter().any(|glob| fast_glob::glob_match(glob, s)), None => false, diff --git a/src/project_builder.rs b/src/project_builder.rs index b0223b8..34fc37e 100644 --- a/src/project_builder.rs +++ b/src/project_builder.rs @@ -9,10 +9,10 @@ use ignore::{DirEntry, WalkBuilder, WalkParallel, WalkState}; use rayon::iter::{IntoParallelIterator, ParallelIterator}; use tracing::instrument; -use crate::path_utils::matches_globs; use crate::{ cache::Cache, config::Config, + path_utils::matches_globs, project::{DirectoryCodeownersFile, Error, Package, PackageType, Project, ProjectFile, Team, VendoredGem, deserializers}, project_file_builder::ProjectFileBuilder, tracked_files, diff --git a/src/runner.rs b/src/runner.rs index fd2b585..b9c5ce4 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -1,8 +1,6 @@ use std::path::{Path, PathBuf}; use std::process::Command; -use crate::path_utils::matches_globs; - use error_stack::{Report, ResultExt}; use serde::Serialize; use tracing::debug_span; @@ -11,6 +9,7 @@ use crate::{ cache::{Cache, Caching, file::GlobalCache, noop::NoopCache}, config::Config, ownership::{FileOwner, Ownership}, + path_utils::{matches_globs, relative_to_buf}, project_builder::ProjectBuilder, }; @@ -147,25 +146,25 @@ impl Runner { let mut unowned_files = Vec::new(); let mut io_errors = Vec::new(); - let relative_to_root = |file_path: &str| crate::path_utils::relative_to_buf(&self.run_config.project_root, Path::new(file_path)); - // Filter files based on owned_globs and unowned_globs configuration // Only validate files that match owned_globs and don't match unowned_globs - let filtered_paths: Vec = file_paths + let filtered_paths: Vec<(String, PathBuf)> = file_paths .into_iter() - .filter(|file_path| { - let relative_path = relative_to_root(file_path); - - // Mirror the filtering applied by ProjectBuilder when walking the project - matches_globs(&relative_path, &self.config.owned_globs) && !matches_globs(&relative_path, &self.config.unowned_globs) + .map(|file_path| { + let relative_path = relative_to_buf(&self.run_config.project_root, Path::new(&file_path)); + (file_path, relative_path) + }) + // Mirror the filtering applied by ProjectBuilder when walking the project + .filter(|(_, relative_path)| { + matches_globs(relative_path, &self.config.owned_globs) && !matches_globs(relative_path, &self.config.unowned_globs) }) .collect(); debug_span!("per_file_query").in_scope(|| { - for file_path in filtered_paths { + for (file_path, relative_path) in filtered_paths { match team_for_file_from_codeowners(&self.run_config, &file_path) { Ok(Some(_)) => {} - Ok(None) if matches_globs(&relative_to_root(&file_path), &self.config.allow_unowned_globs) => {} + Ok(None) if matches_globs(&relative_path, &self.config.allow_unowned_globs) => {} Ok(None) => unowned_files.push(file_path), Err(err) => io_errors.push(format!("{}: {}", file_path, err)), } diff --git a/tests/allow_unowned_globs_test.rs b/tests/allow_unowned_globs_test.rs index 1ec16b8..78c877b 100644 --- a/tests/allow_unowned_globs_test.rs +++ b/tests/allow_unowned_globs_test.rs @@ -32,6 +32,26 @@ fn test_validate_files_passes_for_an_unowned_file_in_an_allowed_glob() -> Result Ok(()) } +#[test] +fn test_validate_files_passes_for_an_absolute_path_in_an_allowed_glob() -> Result<(), Box> { + let temp_dir = setup_fixture_repo(Path::new(FIXTURE)); + let project_root = temp_dir.path(); + git_add_all_files(project_root); + + let file_absolute_path = project_root.join("app/deprecated/old.rb").canonicalize()?; + + Command::cargo_bin("codeowners")? + .arg("--project-root") + .arg(project_root) + .arg("--no-cache") + .arg("validate") + .arg(file_absolute_path) + .assert() + .success() + .stdout(predicate::eq("")); + Ok(()) +} + #[test] fn test_annotations_still_assign_owners_in_allowed_globs() -> Result<(), Box> { run_codeowners( @@ -102,6 +122,39 @@ fn test_validate_files_reports_an_unowned_file_outside_allowed_globs() -> Result Ok(()) } +fn add_a_second_team_owning_the_allowed_glob(project_root: &Path) -> std::io::Result<()> { + fs::write( + project_root.join("config/teams/bar.yml"), + "---\nname: Bar\ngithub:\n team: '@BarTeam'\nowned_globs:\n - app/deprecated/**/*\n", + ) +} + +#[test] +fn test_validate_reports_multiple_owners_in_allowed_globs() -> Result<(), Box> { + run_on_modified_fixture(add_a_second_team_owning_the_allowed_glob, &["validate"])? + .failure() + .stdout(predicate::str::contains( + "Code ownership should only be defined for each file in one way.", + )) + .stdout(predicate::str::contains("app/deprecated/annotated_old.rb\n owner:")); + Ok(()) +} + +fn add_an_annotation_naming_an_unknown_team_in_an_allowed_glob(project_root: &Path) -> std::io::Result<()> { + fs::write(project_root.join("app/deprecated/typo.rb"), "# @team Typo\nputs 'typo'\n") +} + +#[test] +fn test_validate_reports_invalid_teams_in_allowed_globs() -> Result<(), Box> { + run_on_modified_fixture(add_an_annotation_naming_an_unknown_team_in_an_allowed_glob, &["validate"])? + .failure() + .stdout(predicate::str::contains("Found invalid team references")) + .stdout(predicate::str::contains( + "- app/deprecated/typo.rb is referencing an invalid team - 'Typo'", + )); + Ok(()) +} + #[test] fn test_validate_reports_unowned_files_without_allowed_globs() -> Result<(), Box> { run_on_modified_fixture( From 31cf459261edc62841d7f9eb580646d667d9f785 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 28 Sep 2026 14:08:29 -0700 Subject: [PATCH 6/7] Explain how allow_unowned_globs differs from unowned_globs The README said unowned_globs files are never read, but `for-file` reads the top of the file before ignoring its annotation. It also implied that no ownership mechanism applies to unowned_globs files, when directory, package and team-glob rules still cover them through CODEOWNERS. The real differences are that annotations count and that the other checks still run. It now also says that unowned_globs takes precedence, and how to move off `unowned_globs: ['**/*']`, the setup code_ownership#140 started from, where adding the new key alone changes nothing. The Getting Started example moves a hand-picked app file from unowned_globs to allow_unowned_globs, since unowned_globs is meant for third-party and generated code. --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 9a08729..13c7703 100644 --- a/README.md +++ b/README.md @@ -66,8 +66,9 @@ cargo install --git https://github.com/rubyatscale/codeowners-rs codeowners js_package_paths: [] unowned_globs: - db/**/* - - app/services/some_file1.rb - frontend/javascripts/**/__generated__/**/* + allow_unowned_globs: + - app/services/some_file1.rb ``` 2. **Declare Teams** @@ -209,12 +210,12 @@ codeowners gv --no-cache - `ruby_package_paths` (default: `['packs/**/*', 'components/**']`) - `js_package_paths` / `javascript_package_paths` (default: `['frontend/**/*']`) - `team_file_glob` (default: `['config/teams/**/*.yml']`) -- `unowned_globs` (default: `['frontend/**/node_modules/**/*', 'frontend/**/__generated__/**/*']`): Files matched here don't need an owner, and they're never read, so file annotations in them are ignored. Use this for third-party or generated code. +- `unowned_globs` (default: `['frontend/**/node_modules/**/*', 'frontend/**/__generated__/**/*']`): Files matched here are left out of the project: they don't need an owner, validation doesn't check them, and file annotations in them are ignored. Use this for third-party or generated code. - `vendored_gems_path` (default: `'vendored/'`) - `cache_directory` (default: `'tmp/cache/codeowners'`) - `ignore_dirs` (default includes: `.git`, `node_modules`, `tmp`, etc.) - `executable_name` (default: `'codeowners'`): Customize the command name shown in validation error messages. Useful when using `codeowners-rs` via wrappers like the [code_ownership](https://github.com/rubyatscale/code_ownership) Ruby gem. -- `allow_unowned_globs` (default: `[]`): Files in `owned_globs` that match these globs don't need an owner, so leaving them unowned isn't a validation error. Unlike `unowned_globs`, they're still read and every ownership mechanism, including file annotations, still applies to them. Use `['**/*']` if only some of your code has owners, or scope it, e.g. `['**/deprecated/**/*']`. +- `allow_unowned_globs` (default: `[]`): Files in `owned_globs` that match these globs don't need an owner, so leaving them unowned isn't a validation error. Unlike `unowned_globs`, file annotations in them still count, and every other check still runs, so a file there can't have more than one owner or name an invalid team. Use `['**/*']` if only some of your code has owners, or scope it, e.g. `['**/deprecated/**/*']`. `unowned_globs` takes precedence, so a file that matches both is left out. If you use `unowned_globs: ['**/*']` only so that files can go unowned, replace it with `allow_unowned_globs: ['**/*']` to make annotations work. Those files then get the other checks too, which may report problems that were hidden before. Example configuration with custom executable name: From 62ed8d7d31540165f6678943951776162478919d Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 28 Sep 2026 14:09:47 -0700 Subject: [PATCH 7/7] Bump version to 0.6.0 A minor bump. Since 0.5.0: - `allow_unowned_globs` (#135) is a new config key. It defaults to `[]`, so existing configs behave as before, but it adds a `pub` field to `Config`, which breaks downstream code that builds a `Config` with a struct literal. - Test git no longer follows a hook's GIT_DIR into this repo (#133). This only affects the test suite. --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9be5280..9b07a78 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -168,7 +168,7 @@ checksum = "b94f61472cee1439c0b966b47e3aca9ae07e45d070759512cd390ea2bebc6675" [[package]] name = "codeowners" -version = "0.5.0" +version = "0.6.0" dependencies = [ "assert_cmd", "clap", diff --git a/Cargo.toml b/Cargo.toml index d95b654..b352a71 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "codeowners" -version = "0.5.0" +version = "0.6.0" edition = "2024" [profile.release]