check team validity (#7)

Author: mzruya

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

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());
+                }
             }
         }
 

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<Owner> },
     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<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();
 
@@ -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)],
         }
     }
 }