Informative for-file error message (#33)

* introducing source enum

* removing redundant postfix

* friendly for-file error message

Author: perryqh

src/common_test.rs

@@ -286,4 +286,27 @@ team_file_glob:
             }
         )
     }
+
+    pub fn vecs_match<T: PartialEq + std::fmt::Debug>(a: &Vec<T>, b: &Vec<T>) {
+        // First check lengths match
+        assert_eq!(a.len(), b.len(), "Vectors have different lengths");
+
+        // Check each element in a exists in b
+        for elem_a in a {
+            assert!(
+                b.contains(elem_a),
+                "Element {:?} from first vector not found in second vector",
+                elem_a
+            );
+        }
+
+        // Check each element in b exists in a
+        for elem_b in b {
+            assert!(
+                a.contains(elem_b),
+                "Element {:?} from second vector not found in first vector",
+                elem_b
+            );
+        }
+    }
 }

src/ownership.rs

@@ -1,5 +1,5 @@
 use file_owner_finder::FileOwnerFinder;
-use mapper::{OwnerMatcher, TeamName};
+use mapper::{OwnerMatcher, Source, TeamName};
 use std::{
     fmt::{self, Display},
     path::Path,
@@ -9,7 +9,7 @@ use tracing::{info, instrument};
 
 mod file_generator;
 mod file_owner_finder;
-mod mapper;
+pub(crate) mod mapper;
 mod validator;
 
 use crate::{ownership::mapper::DirectoryMapper, project::Project};
@@ -29,11 +29,22 @@ pub struct Ownership {
 pub struct FileOwner {
     pub team_name: TeamName,
     pub team_config_file_path: String,
+    pub sources: Vec<Source>,
 }
 
 impl Display for FileOwner {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "Team: {}\nTeam YML: {}", self.team_name, self.team_config_file_path)
+        let sources = self
+            .sources
+            .iter()
+            .map(|source| source.to_string())
+            .collect::<Vec<String>>()
+            .join(", ");
+        write!(
+            f,
+            "Team: {}\nTeam YML: {}\nSource(s): {}",
+            self.team_name, self.team_config_file_path, sources
+        )
     }
 }
 
@@ -42,6 +53,7 @@ impl Default for FileOwner {
         Self {
             team_name: "Unowned".to_string(),
             team_config_file_path: "Unowned".to_string(),
+            sources: vec![],
         }
     }
 }
@@ -102,6 +114,7 @@ impl Ownership {
                         .path
                         .strip_prefix(&self.project.base_path)
                         .map_or_else(|_| String::new(), |p| p.to_string_lossy().to_string()),
+                    sources: owner.sources.clone(),
                 },
                 None => FileOwner::default(),
             })

src/ownership/file_owner_finder.rs

@@ -1,6 +1,6 @@
 use std::{collections::HashMap, path::Path};
 
-use super::mapper::{directory_mapper::is_directory_mapper_source, OwnerMatcher, Source, TeamName};
+use super::mapper::{OwnerMatcher, Source, TeamName};
 
 #[derive(Debug)]
 pub struct Owner {
@@ -21,10 +21,13 @@ impl<'a> FileOwnerFinder<'a> {
             let (owner, source) = owner_matcher.owner_for(relative_path);
 
             if let Some(team_name) = owner {
-                if is_directory_mapper_source(source) {
-                    directory_overrider.process(team_name, source);
-                } else {
-                    team_sources_map.entry(team_name).or_default().push(source.clone());
+                match source {
+                    Source::Directory(_) => {
+                        directory_overrider.process(team_name, source);
+                    }
+                    _ => {
+                        team_sources_map.entry(team_name).or_default().push(source.clone());
+                    }
                 }
             }
         }
@@ -74,20 +77,20 @@ mod tests {
         let mut directory_overrider = DirectoryOverrider::default();
         assert_eq!(directory_overrider.specific_directory_owner(), None);
         let team_name_1 = "team1".to_string();
-        let source_1 = "src/**".to_string();
+        let source_1 = Source::Directory("src/**".to_string());
         directory_overrider.process(&team_name_1, &source_1);
         assert_eq!(directory_overrider.specific_directory_owner(), Some((&team_name_1, &source_1)));
 
         let team_name_longest = "team2".to_string();
-        let source_longest = "source/subdir/**".to_string();
+        let source_longest = Source::Directory("source/subdir/**".to_string());
         directory_overrider.process(&team_name_longest, &source_longest);
         assert_eq!(
             directory_overrider.specific_directory_owner(),
             Some((&team_name_longest, &source_longest))
         );
 
         let team_name_3 = "team3".to_string();
-        let source_3 = "source/**".to_string();
+        let source_3 = Source::Directory("source/**".to_string());
         directory_overrider.process(&team_name_3, &source_3);
         assert_eq!(
             directory_overrider.specific_directory_owner(),

src/ownership/mapper.rs

@@ -1,6 +1,7 @@
 use glob_match::glob_match;
 use std::{
     collections::HashMap,
+    fmt::{self, Display},
     path::{Path, PathBuf},
 };
 
@@ -27,7 +28,38 @@ pub trait Mapper {
     fn owner_matchers(&self) -> Vec<OwnerMatcher>;
 }
 pub type TeamName = String;
-pub type Source = String;
+
+#[derive(Debug, PartialEq, Clone)]
+pub enum Source {
+    Directory(String),
+    TeamFile,
+    TeamGem,
+    TeamGlob,
+    Package(String, String),
+    TeamYml,
+}
+
+impl Display for Source {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Source::Directory(path) => write!(f, "DirectoryMapper({})", path),
+            Source::TeamFile => write!(f, "TeamFileMapper"),
+            Source::TeamGem => write!(f, "TeamGemMapper"),
+            Source::TeamGlob => write!(f, "TeamGlobMapper"),
+            Source::Package(file_type, path) => write!(f, "PackageMapper({}, glob: {})", file_type, path),
+            Source::TeamYml => write!(f, "TeamYmlMapper"),
+        }
+    }
+}
+
+impl Source {
+    pub fn len(&self) -> usize {
+        match self {
+            Source::Directory(path) => path.matches('/').count(),
+            _ => 0,
+        }
+    }
+}
 
 #[derive(Debug, PartialEq)]
 pub enum OwnerMatcher {
@@ -55,7 +87,7 @@ mod tests {
     use super::*;
 
     fn assert_owner_for(glob: &str, relative_path: &str, expect_match: bool) {
-        let source = "directory_mapper (\"packs/bam\")".to_string();
+        let source = Source::Directory("packs/bam".to_string());
         let team_name = "team1".to_string();
         let owner_matcher = OwnerMatcher::Glob {
             glob: glob.to_string(),
@@ -96,4 +128,17 @@ mod tests {
             true,
         );
     }
+
+    #[test]
+    fn display_source() {
+        assert_eq!(Source::Directory("packs/bam".to_string()).to_string(), "DirectoryMapper(packs/bam)");
+        assert_eq!(Source::TeamFile.to_string(), "TeamFileMapper");
+        assert_eq!(Source::TeamGem.to_string(), "TeamGemMapper");
+        assert_eq!(Source::TeamGlob.to_string(), "TeamGlobMapper");
+        assert_eq!(
+            Source::Package("Ruby".to_string(), "packs/bam/**/**".to_string()).to_string(),
+            "PackageMapper(Ruby, glob: packs/bam/**/**)"
+        );
+        assert_eq!(Source::TeamYml.to_string(), "TeamYmlMapper");
+    }
 }