diff --git a/src/ownership/mapper/package_mapper.rs b/src/ownership/mapper/package_mapper.rs index 0b715ca..9b9b2ba 100644 --- a/src/ownership/mapper/package_mapper.rs +++ b/src/ownership/mapper/package_mapper.rs @@ -70,19 +70,19 @@ impl PackageMapper { for package in self.project.packages.iter().filter(|package| &package.package_type == package_type) { let package_root = package.package_root().to_string_lossy(); - let team = team_by_name - .get(&package.owner) - .unwrap_or_else(|| panic!("Couldn't find team {}", package.owner)); + let team = team_by_name.get(&package.owner); - if team.avoid_ownership { - continue; - } + if let Some(team) = team { + if team.avoid_ownership { + continue; + } - entries.push(Entry { - path: format!("{}/**/**", package_root), - github_team: team.github_team.to_owned(), - team_name: team.name.to_owned(), - }); + entries.push(Entry { + path: format!("{}/**/**", package_root), + github_team: team.github_team.to_owned(), + team_name: team.name.to_owned(), + }); + } } entries @@ -101,16 +101,15 @@ impl PackageMapper { for package in packages { let package_root = package.package_root().to_string_lossy(); - - let team = team_by_name - .get(&package.owner) - .unwrap_or_else(|| panic!("Couldn't find team {}", package.owner)); - - owner_matchers.push(OwnerMatcher::Glob { - glob: format!("{}/**/**", package_root), - team_name: team.name.to_owned(), - source: format!("package_mapper ({:?} glob: {}/**/**)", &package_type, package_root), - }); + 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: format!("package_mapper ({:?} glob: {}/**/**)", &package_type, package_root), + }); + } } owner_matchers diff --git a/src/ownership/mapper/team_file_mapper.rs b/src/ownership/mapper/team_file_mapper.rs index 9b41d6f..56c7c9b 100644 --- a/src/ownership/mapper/team_file_mapper.rs +++ b/src/ownership/mapper/team_file_mapper.rs @@ -23,18 +23,21 @@ impl Mapper for TeamFileMapper { for owned_file in &self.project.files { if let Some(ref owner) = owned_file.owner { - let team = team_by_name.get(owner).unwrap_or_else(|| panic!("Couldn't find team {}", owner)); - if team.avoid_ownership { - continue; - } + let team = team_by_name.get(owner); + + if let Some(team) = team { + if team.avoid_ownership { + continue; + } - let relative_path = self.project.relative_path(&owned_file.path); + let relative_path = self.project.relative_path(&owned_file.path); - entries.push(Entry { - path: relative_path.to_string_lossy().to_string(), - github_team: team.github_team.to_owned(), - team_name: team.name.to_owned(), - }); + entries.push(Entry { + path: relative_path.to_string_lossy().to_string(), + github_team: team.github_team.to_owned(), + team_name: team.name.to_owned(), + }); + } } } @@ -48,10 +51,12 @@ impl Mapper for TeamFileMapper { for owned_file in &self.project.files { if let Some(ref owner) = owned_file.owner { - let team = team_by_name.get(owner).unwrap_or_else(|| panic!("Couldn't find team {}", owner)); - let relative_path = self.project.relative_path(&owned_file.path); + let team = team_by_name.get(owner); - path_to_team.insert(relative_path.to_owned(), team.name.clone()); + if let Some(team) = team { + let relative_path = self.project.relative_path(&owned_file.path); + path_to_team.insert(relative_path.to_owned(), team.name.clone()); + } } } diff --git a/src/ownership/validator.rs b/src/ownership/validator.rs index e05c26d..c76247c 100644 --- a/src/ownership/validator.rs +++ b/src/ownership/validator.rs @@ -1,5 +1,6 @@ use core::fmt; use std::collections::HashMap; +use std::collections::HashSet; use std::fmt::Display; use std::path::Path; @@ -32,6 +33,7 @@ struct Owner { #[derive(Debug)] enum Error { + InvalidTeam { name: String, path: PathBuf }, FileWithoutOwner { path: PathBuf }, FileWithMultipleOwners { path: PathBuf, owners: Vec }, CodeownershipFileIsStale, @@ -45,6 +47,9 @@ impl Validator { pub fn validate(&self) -> Result<(), Errors> { let mut validation_errors = Vec::new(); + debug!("validate_invalid_team"); + validation_errors.append(&mut self.validate_invalid_team()); + debug!("validate_file_ownership"); validation_errors.append(&mut self.validate_file_ownership()); @@ -58,6 +63,54 @@ impl Validator { } } + fn validate_invalid_team(&self) -> Vec { + debug!("validating project"); + let mut errors: Vec = Vec::new(); + + let team_names: HashSet<&String> = self.project.teams.iter().map(|team| &team.name).collect(); + + errors.append(&mut self.invalid_team_annotation(&team_names)); + errors.append(&mut self.invalid_package_ownership(&team_names)); + + errors + } + + fn invalid_team_annotation(&self, team_names: &HashSet<&String>) -> Vec { + self.project + .files + .par_iter() + .flat_map(|file| { + if let Some(owner) = &file.owner { + if !team_names.contains(owner) { + return Some(Error::InvalidTeam { + name: owner.clone(), + path: file.path.clone(), + }); + } + } + + None + }) + .collect() + } + + fn invalid_package_ownership(&self, team_names: &HashSet<&String>) -> Vec { + self.project + .packages + .iter() + .flat_map(|package| { + if !team_names.contains(&package.owner) { + Some(Error::InvalidTeam { + name: package.owner.clone(), + path: package.path.clone(), + }) + } else { + None + } + }) + .collect() + } + fn validate_file_ownership(&self) -> Vec { let mut validation_errors = Vec::new(); @@ -135,6 +188,7 @@ impl Error { Error::CodeownershipFileIsStale => { "CODEOWNERS out of date. Run `codeownership generate` to update the CODEOWNERS file".to_owned() } + Error::InvalidTeam { name: _, path: _ } => "Found invalid team annotaitons.".to_owned(), } } @@ -152,6 +206,7 @@ impl Error { }) .collect_vec(), Error::CodeownershipFileIsStale => vec![], + Error::InvalidTeam { name, path } => vec![format!("- {} is referencing an invalid team - '{}'", path.to_string_lossy(), name)], } } }