Skip to content

Unowned team globs #58

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "codeowners"
version = "0.2.3"
version = "0.2.4"
edition = "2024"

[profile.release]
Expand Down
7 changes: 7 additions & 0 deletions src/common_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,13 @@ pub mod tests {
})
}

pub fn build_ownership_with_subtracted_globs_team_glob_codeowners() -> Result<Ownership, Box<dyn Error>> {
ownership!(TestProjectFile {
relative_path: "config/teams/baz.yml".to_owned(),
content: "name: Baz\ngithub:\n team: \"@Baz\"\n members:\n - Baz member\nowned_globs:\n - \"packs/bar/**\"\nunowned_globs:\n - \"packs/bar/excluded/**\"\n".to_owned(),
})
}

pub fn build_ownership_with_team_yml_codeowners() -> Result<Ownership, Box<dyn Error>> {
let temp_dir = tempdir()?;

Expand Down
134 changes: 115 additions & 19 deletions src/ownership/mapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,56 @@ impl Source {
#[derive(Debug, PartialEq)]
pub enum OwnerMatcher {
ExactMatches(HashMap<PathBuf, TeamName>, Source),
Glob { glob: String, team_name: TeamName, source: Source },
Glob {
glob: String,
subtracted_globs: Vec<String>,
team_name: TeamName,
source: Source,
},
}

impl OwnerMatcher {
pub fn new_glob_with_candidate_subtracted_globs(
glob: String,
candidate_subtracted_globs: &[String],
team_name: TeamName,
source: Source,
) -> Self {
let subtracted_globs = candidate_subtracted_globs
.iter()
.filter(|candidate_subtracted_glob| {
glob_match(candidate_subtracted_glob, &glob) || glob_match(&glob, candidate_subtracted_glob)
})
.cloned()
.collect();
OwnerMatcher::Glob {
glob,
subtracted_globs,
team_name,
source,
}
}

pub fn new_glob(glob: String, team_name: TeamName, source: Source) -> Self {
OwnerMatcher::Glob {
glob,
subtracted_globs: vec![],
team_name,
source,
}
}

pub fn owner_for(&self, relative_path: &Path) -> (Option<&TeamName>, &Source) {
match self {
OwnerMatcher::Glob { glob, team_name, source } => {
if glob_match(glob, relative_path.to_str().unwrap()) {
(Some(team_name), source)
} else {
(None, source)
}
}
OwnerMatcher::Glob {
glob,
subtracted_globs,
team_name,
source,
} => relative_path
.to_str()
.filter(|path| glob_match(glob, path) && !subtracted_globs.iter().any(|subtracted| glob_match(subtracted, path)))
.map_or((None, source), |_| (Some(team_name), source)),
OwnerMatcher::ExactMatches(path_to_team, source) => (path_to_team.get(relative_path), source),
}
}
Expand All @@ -89,14 +126,15 @@ impl OwnerMatcher {
mod tests {
use super::*;

fn assert_owner_for(glob: &str, relative_path: &str, expect_match: bool) {
fn assert_owner_for(glob: &str, subtracted_globs: &[&str], relative_path: &str, expect_match: bool) {
let source = Source::Directory("packs/bam".to_string());
let team_name = "team1".to_string();
let owner_matcher = OwnerMatcher::Glob {
glob: glob.to_string(),
team_name: team_name.clone(),
source: source.clone(),
};
let owner_matcher = OwnerMatcher::new_glob_with_candidate_subtracted_globs(
glob.to_string(),
&subtracted_globs.iter().map(|s| s.to_string()).collect::<Vec<String>>(),
team_name.clone(),
source.clone(),
);
let response = owner_matcher.owner_for(Path::new(relative_path));
if expect_match {
assert_eq!(response, (Some(&team_name), &source));
Expand All @@ -107,26 +145,50 @@ mod tests {

#[test]
fn owner_for_without_brackets_in_glob() {
assert_owner_for("packs/bam/**/**", "packs/bam/app/components/sidebar.jsx", true);
assert_owner_for("packs/bam/**/**", "packs/baz/app/components/sidebar.jsx", false);
assert_owner_for("packs/bam/**/**", "packs/bam/app/[components]/gadgets/sidebar.jsx", true);
assert_owner_for("packs/bam/**/**", "packs/bam/app/sidebar_[component].jsx", true);
assert_owner_for("packs/bam/**/**", &[], "packs/bam/app/components/sidebar.jsx", true);
assert_owner_for("packs/bam/**/**", &[], "packs/baz/app/components/sidebar.jsx", false);
assert_owner_for("packs/bam/**/**", &[], "packs/bam/app/[components]/gadgets/sidebar.jsx", true);
assert_owner_for("packs/bam/**/**", &[], "packs/bam/app/sidebar_[component].jsx", true);
assert_owner_for(
"packs/bam/**/**",
&["packs/bam/app/excluded/**"],
"packs/bam/app/excluded/sidebar_[component].jsx",
false,
);
}

#[test]
fn subtracted_globs() {
assert_owner_for(
"packs/bam/**/**",
&["packs/bam/app/excluded/**"],
"packs/bam/app/excluded/some_file.rb",
false,
);
assert_owner_for(
"packs/bam/**/**",
&["packs/bam/app/excluded/**"],
"packs/bam/app/not_excluded/some_file.rb",
true,
);
}

#[test]
fn owner_for_with_brackets_in_glob() {
assert_owner_for(
"packs/bam/app/\\[components\\]/**/**",
&[],
"packs/bam/app/[components]/gadgets/sidebar.jsx",
true,
);
assert_owner_for("packs/\\[bam\\]/**/**", "packs/[bam]/app/components/sidebar.jsx", true);
assert_owner_for("packs/\\[bam\\]/**/**", &[], "packs/[bam]/app/components/sidebar.jsx", true);
}

#[test]
fn owner_for_with_multiple_brackets_in_glob() {
assert_owner_for(
"packs/\\[bam\\]/bar/\\[foo\\]/**/**",
&[],
"packs/[bam]/bar/[foo]/app/components/sidebar.jsx",
true,
);
Expand All @@ -150,4 +212,38 @@ mod tests {
);
assert_eq!(Source::TeamYml.to_string(), "Teams own their configuration files");
}

#[test]
fn test_new_glob_with_candidate_subtracted_globs() {
assert_new_glob_with_candidate_subtracted_globs("packs/bam/**/**", &[], &[]);
assert_new_glob_with_candidate_subtracted_globs("packs/bam/**/**", &["packs/bam/app/**/**"], &["packs/bam/app/**/**"]);
assert_new_glob_with_candidate_subtracted_globs(
"packs/bam/**/**",
&["packs/bam/app/an/exceptional/path/it.rb"],
&["packs/bam/app/an/exceptional/path/it.rb"],
);
assert_new_glob_with_candidate_subtracted_globs("packs/bam/**/**", &["packs/bam.rb"], &[]);
assert_new_glob_with_candidate_subtracted_globs("packs/bam/**/**", &["packs/nope/app/**/**"], &[]);
assert_new_glob_with_candidate_subtracted_globs("packs/**", &["packs/yep/app/**/**"], &["packs/yep/app/**/**"]);
assert_new_glob_with_candidate_subtracted_globs("packs/foo.yml", &["packs/foo/**/**"], &[]);
}

fn assert_new_glob_with_candidate_subtracted_globs(
glob: &str,
candidate_subtracted_globs: &[&str],
expected_subtracted_globs: &[&str],
) {
let owner_matcher = OwnerMatcher::new_glob_with_candidate_subtracted_globs(
glob.to_string(),
&candidate_subtracted_globs.iter().map(|s| s.to_string()).collect::<Vec<String>>(),
"team1".to_string(),
Source::TeamGlob(glob.to_string()),
);

if let OwnerMatcher::Glob { subtracted_globs, .. } = owner_matcher {
assert_eq!(subtracted_globs, expected_subtracted_globs);
} else {
panic!("Expected a Glob matcher");
}
}
}
60 changes: 30 additions & 30 deletions src/ownership/mapper/directory_mapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ impl Mapper for DirectoryMapper {
let mut owner_matchers = Vec::new();

for file in &self.project.directory_codeowner_files {
owner_matchers.push(OwnerMatcher::Glob {
glob: format!("{}/**/**", escape_brackets(&file.directory_root().to_string_lossy())),
team_name: file.owner.to_owned(),
source: Source::Directory(file.directory_root().to_string_lossy().to_string()),
});
owner_matchers.push(OwnerMatcher::new_glob(
format!("{}/**/**", escape_brackets(&file.directory_root().to_string_lossy())),
file.owner.to_owned(),
Source::Directory(file.directory_root().to_string_lossy().to_string()),
));
}

owner_matchers
Expand Down Expand Up @@ -125,21 +125,21 @@ mod tests {
vecs_match(
&mapper.owner_matchers(),
&vec![
OwnerMatcher::Glob {
glob: "app/consumers/**/**".to_owned(),
team_name: "Bar".to_owned(),
source: Source::Directory("app/consumers".to_string()),
},
OwnerMatcher::Glob {
glob: "app/services/**/**".to_owned(),
team_name: "Foo".to_owned(),
source: Source::Directory("app/services".to_owned()),
},
OwnerMatcher::Glob {
glob: "app/services/exciting/**/**".to_owned(),
team_name: "Bar".to_owned(),
source: Source::Directory("app/services/exciting".to_owned()),
},
OwnerMatcher::new_glob(
"app/consumers/**/**".to_owned(),
"Bar".to_owned(),
Source::Directory("app/consumers".to_string()),
),
OwnerMatcher::new_glob(
"app/services/**/**".to_owned(),
"Foo".to_owned(),
Source::Directory("app/services".to_owned()),
),
OwnerMatcher::new_glob(
"app/services/exciting/**/**".to_owned(),
"Bar".to_owned(),
Source::Directory("app/services/exciting".to_owned()),
),
],
);
Ok(())
Expand All @@ -152,16 +152,16 @@ mod tests {
vecs_match(
&mapper.owner_matchers(),
&vec![
OwnerMatcher::Glob {
glob: "app/\\[consumers\\]/**/**".to_string(),
team_name: "Bar".to_string(),
source: Source::Directory("app/[consumers]".to_string()),
},
OwnerMatcher::Glob {
glob: "app/\\[consumers\\]/deep/nesting/\\[nestdir\\]/**/**".to_string(),
team_name: "Foo".to_string(),
source: Source::Directory("app/[consumers]/deep/nesting/[nestdir]".to_string()),
},
OwnerMatcher::new_glob(
"app/\\[consumers\\]/**/**".to_string(),
"Bar".to_string(),
Source::Directory("app/[consumers]".to_string()),
),
OwnerMatcher::new_glob(
"app/\\[consumers\\]/deep/nesting/\\[nestdir\\]/**/**".to_string(),
"Foo".to_string(),
Source::Directory("app/[consumers]/deep/nesting/[nestdir]".to_string()),
),
],
);
Ok(())
Expand Down
30 changes: 15 additions & 15 deletions src/ownership/mapper/package_mapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ impl PackageMapper {
let team = team_by_name.get(&package.owner);

if let Some(team) = team {
owner_matchers.push(OwnerMatcher::Glob {
glob: format!("{}/**/**", package_root),
team_name: team.name.to_owned(),
source: Source::Package(package.path.to_string_lossy().to_string(), format!("{}/**/**", package_root)),
});
owner_matchers.push(OwnerMatcher::new_glob(
format!("{}/**/**", package_root),
team.name.to_owned(),
Source::Package(package.path.to_string_lossy().to_string(), format!("{}/**/**", package_root)),
));
}
}

Expand Down Expand Up @@ -198,16 +198,16 @@ mod tests {
vecs_match(
&mapper.owner_matchers(&PackageType::Ruby),
&vec![
OwnerMatcher::Glob {
glob: "packs/bam/**/**".to_owned(),
team_name: "Bam".to_owned(),
source: Source::Package("packs/bam/package.yml".to_owned(), "packs/bam/**/**".to_owned()),
},
OwnerMatcher::Glob {
glob: "packs/foo/**/**".to_owned(),
team_name: "Baz".to_owned(),
source: Source::Package("packs/foo/package.yml".to_owned(), "packs/foo/**/**".to_owned()),
},
OwnerMatcher::new_glob(
"packs/bam/**/**".to_owned(),
"Bam".to_owned(),
Source::Package("packs/bam/package.yml".to_owned(), "packs/bam/**/**".to_owned()),
),
OwnerMatcher::new_glob(
"packs/foo/**/**".to_owned(),
"Baz".to_owned(),
Source::Package("packs/foo/package.yml".to_owned(), "packs/foo/**/**".to_owned()),
),
],
);
Ok(())
Expand Down
20 changes: 10 additions & 10 deletions src/ownership/mapper/team_gem_mapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ impl Mapper for TeamGemMapper {
let vendored_gem = vendored_gem_by_name.get(owned_gem);

if let Some(vendored_gem) = vendored_gem {
owner_matchers.push(OwnerMatcher::Glob {
glob: format!("{}/**/*", self.project.relative_path(&vendored_gem.path).to_string_lossy()),
team_name: team.name.clone(),
source: Source::TeamGem,
});
owner_matchers.push(OwnerMatcher::new_glob(
format!("{}/**/*", self.project.relative_path(&vendored_gem.path).to_string_lossy()),
team.name.clone(),
Source::TeamGem,
));
}
}
}
Expand Down Expand Up @@ -92,11 +92,11 @@ mod tests {
let mapper = TeamGemMapper::build(ownership.project.clone());
vecs_match(
&mapper.owner_matchers(),
&vec![OwnerMatcher::Glob {
glob: "gems/globbing/**/*".to_owned(),
team_name: "Bam".to_owned(),
source: Source::TeamGem,
}],
&vec![OwnerMatcher::new_glob(
"gems/globbing/**/*".to_owned(),
"Bam".to_owned(),
Source::TeamGem,
)],
);
Ok(())
}
Expand Down
Loading