Skip to content
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
41 changes: 20 additions & 21 deletions src/ownership/mapper/package_mapper.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand All@@ -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
Expand Down
31 changes: 18 additions & 13 deletions src/ownership/mapper/team_file_mapper.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -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(),
});
}
}
}

Expand All@@ -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());
}
}
}

Expand Down
55 changes: 55 additions & 0 deletions src/ownership/validator.rs
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
use core::fmt;
use std::collections::HashMap;
use std::collections::HashSet;
use std::fmt::Display;
use std::path::Path;

Expand DownExpand Up@@ -32,6 +33,7 @@ struct Owner {

#[derive(Debug)]
enum Error {
InvalidTeam { name: String, path: PathBuf },
FileWithoutOwner { path: PathBuf },
FileWithMultipleOwners { path: PathBuf, owners: Vec<Owner> },
CodeownershipFileIsStale,
Expand All@@ -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());

Expand All@@ -58,6 +63,54 @@ impl Validator {
}
}

fn validate_invalid_team(&self) -> Vec<Error> {
debug!("validating project");
let mut errors: Vec<Error> = 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<Error> {
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<Error> {
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<Error> {
let mut validation_errors = Vec::new();

Expand DownExpand Up@@ -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(),
}
}

Expand All@@ -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)],
}
}
}
Expand Down