Improve code owner "source" descriptions (#36)

* providing location of package file

* improving glob source descriptions

Author: perryqh

src/ownership.rs

@@ -42,7 +42,7 @@ impl Display for FileOwner {
             .join(", ");
         write!(
             f,
-            "Team: {}\nTeam YML: {}\nSource(s): {}",
+            "Team: {}\nTeam YML: {}\nDescription: {}",
             self.team_name, self.team_config_file_path, sources
         )
     }

src/ownership/mapper.rs

@@ -34,20 +34,22 @@ pub enum Source {
     Directory(String),
     TeamFile,
     TeamGem,
-    TeamGlob,
+    TeamGlob(String),
     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"),
+            Source::Directory(path) => write!(f, "Owner specified in `{}/.codeowner`", path),
+            Source::TeamFile => write!(f, "Owner annotation at the top of the file"),
+            Source::TeamGem => write!(f, "Owner specified in Team YML's `owned_gems`"),
+            Source::TeamGlob(glob) => write!(f, "Owner specified in Team YML as an owned_glob `{}`", glob),
+            Source::Package(package_path, glob) => {
+                write!(f, "Owner defined in `{}` with implicity owned glob: `{}`", package_path, glob)
+            }
+            Source::TeamYml => write!(f, "Teams own their configuration files"),
         }
     }
 }
@@ -131,14 +133,20 @@ mod tests {
 
     #[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/**/**)"
+            Source::Directory("packs/bam".to_string()).to_string(),
+            "Owner specified in `packs/bam/.codeowner`"
+        );
+        assert_eq!(Source::TeamFile.to_string(), "Owner annotation at the top of the file");
+        assert_eq!(Source::TeamGem.to_string(), "Owner specified in Team YML's `owned_gems`");
+        assert_eq!(
+            Source::TeamGlob("a/glob/**".to_string()).to_string(),
+            "Owner specified in Team YML as an owned_glob `a/glob/**`"
+        );
+        assert_eq!(
+            Source::Package("packs/bam/packag.yml".to_string(), "packs/bam/**/**".to_string()).to_string(),
+            "Owner defined in `packs/bam/packag.yml` with implicity owned glob: `packs/bam/**/**`"
         );
-        assert_eq!(Source::TeamYml.to_string(), "TeamYmlMapper");
+        assert_eq!(Source::TeamYml.to_string(), "Teams own their configuration files");
     }
 }

src/ownership/mapper/package_mapper.rs

@@ -104,7 +104,7 @@ impl PackageMapper {
                 owner_matchers.push(OwnerMatcher::Glob {
                     glob: format!("{}/**/**", package_root),
                     team_name: team.name.to_owned(),
-                    source: Source::Package(package_type.to_string(), format!("{}/**/**", package_root)),
+                    source: Source::Package(package.path.to_string_lossy().to_string(), format!("{}/**/**", package_root)),
                 });
             }
         }
@@ -201,12 +201,12 @@ mod tests {
                 OwnerMatcher::Glob {
                     glob: "packs/bam/**/**".to_owned(),
                     team_name: "Bam".to_owned(),
-                    source: Source::Package("Ruby".to_owned(), "packs/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("Ruby".to_owned(), "packs/foo/**/**".to_owned()),
+                    source: Source::Package("packs/foo/package.yml".to_owned(), "packs/foo/**/**".to_owned()),
                 },
             ],
         );

src/ownership/mapper/team_glob_mapper.rs

@@ -40,7 +40,7 @@ impl Mapper for TeamGlobMapper {
                 owner_matchers.push(OwnerMatcher::Glob {
                     glob: owned_glob.clone(),
                     team_name: team.github_team.clone(),
-                    source: Source::TeamGlob,
+                    source: Source::TeamGlob(owned_glob.clone()),
                 })
             }
         }
@@ -85,7 +85,7 @@ mod tests {
             &vec![OwnerMatcher::Glob {
                 glob: "packs/bar/**".to_owned(),
                 team_name: "@Baz".to_owned(),
-                source: Source::TeamGlob,
+                source: Source::TeamGlob("packs/bar/**".to_owned()),
             }],
         );
         Ok(())

tests/invalid_project_test.rs

@@ -10,10 +10,30 @@ fn test_validate() -> Result<(), Box<dyn Error>> {
         .arg("validate")
         .assert()
         .failure()
-        .stdout(predicate::str::contains("CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file"))
-        .stdout(predicate::str::contains("Some files are missing ownership\n- ruby/app/models/blockchain.rb\n- ruby/app/unowned.rb"))
-        .stdout(predicate::str::contains("Found invalid team annotations\n- ruby/app/models/blockchain.rb is referencing an invalid team - 'Web3'"))
-        .stdout(predicate::str::contains("Code ownership should only be defined for each file in one way. The following files have declared ownership in multiple ways\n- gems/payroll_calculator/calculator.rb (owner: Payments, source: TeamFileMapper)\n- gems/payroll_calculator/calculator.rb (owner: Payroll, source: TeamGemMapper)"));
+        .stdout(predicate::str::contains(
+            "CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file",
+        ))
+        .stdout(predicate::str::contains(
+            "Some files are missing ownership\n- ruby/app/models/blockchain.rb\n- ruby/app/unowned.rb",
+        ))
+        .stdout(predicate::str::contains(
+            "Found invalid team annotations\n- ruby/app/models/blockchain.rb is referencing an invalid team - 'Web3'",
+        ))
+        .stdout(predicate::str::contains(
+            "Code ownership should only be defined for each file in one way. The following files have declared ownership in multiple ways",
+        ))
+        .stdout(predicate::str::contains(
+            "gems/payroll_calculator/calculator.rb (owner: Payments, source: Owner annotation at the top of the file)",
+        ))
+        .stdout(predicate::str::contains(
+            "gems/payroll_calculator/calculator.rb (owner: Payroll, source: Owner specified in Team YML's `owned_gems`)",
+        ))
+        .stdout(predicate::str::contains(
+            "ruby/app/services/multi_owned.rb (owner: Payments, source: Owner annotation at the top of the file)",
+        ))
+        .stdout(predicate::str::contains(
+            "ruby/app/services/multi_owned.rb (owner: Payroll, source: Owner specified in `ruby/app/services/.codeowner`",
+        ));
 
     Ok(())
 }
@@ -45,10 +65,12 @@ fn test_for_file_multiple_owners() -> Result<(), Box<dyn Error>> {
         .stdout(predicate::str::contains("Error: file is owned by multiple teams!"))
         .stdout(predicate::str::contains("Team: Payments"))
         .stdout(predicate::str::contains("Team YML: config/teams/payments.yml"))
-        .stdout(predicate::str::contains("Source(s): TeamFileMapper"))
+        .stdout(predicate::str::contains("Description: Owner annotation at the top of the file"))
         .stdout(predicate::str::contains("Team: Payroll"))
         .stdout(predicate::str::contains("Team YML: config/teams/payroll.yml"))
-        .stdout(predicate::str::contains("Source(s): DirectoryMapper(ruby/app/services)"));
+        .stdout(predicate::str::contains(
+            "Description: Owner specified in `ruby/app/services/.codeowner`",
+        ));
 
     Ok(())
 }