Fix command handling

jackboberg · jackboberg · commit f430034f56a2 · 2024-11-07T18:23:29.000+01:00
diff --git a/src/main.rs b/src/main.rs
@@ -138,13 +138,19 @@ fn cli() -> Result<(), Error> {
                 }
             }
         }
-        Command::ForTeam { name } => {
-            let team_ownership = ownership.for_team(&name);
-            match team_ownership {
-                Ok(output) => println!("{:?}", output),
-                Err(err) => eprintln!("{}", err),
+        Command::ForTeam { name } => match ownership.for_team(&name) {
+            Ok(team_ownerships) => {
+                println!("# Code Ownership Report for `{}` Team", name);
+                for team_ownership in team_ownerships {
+                    println!("\n#{}", team_ownership.heading);
+                    match team_ownership.globs.len() {
+                        0 => println!("This team owns nothing in this category."),
+                        _ => println!("{}", team_ownership.globs.join("\n")),
+                    }
+                }
             }
-        }
+            Err(err) => eprintln!("{}", err),
+        },
     }
 
     Ok(())
diff --git a/src/ownership.rs b/src/ownership.rs
@@ -1,6 +1,7 @@
 use file_owner_finder::FileOwnerFinder;
 use mapper::{OwnerMatcher, Source, TeamName};
 use std::{
+    error::Error,
     fmt::{self, Display},
     path::Path,
     sync::Arc,
@@ -38,6 +39,15 @@ pub struct TeamOwnership {
     pub globs: Vec<String>,
 }
 
+impl TeamOwnership {
+    fn new(heading: String) -> Self {
+        Self {
+            heading,
+            ..Default::default()
+        }
+    }
+}
+
 impl Display for FileOwner {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let sources = self
@@ -128,7 +138,7 @@ impl Ownership {
     }
 
     #[instrument(level = "debug", skip_all)]
-    pub fn for_team(&self, team_name: &str) -> Result<Vec<TeamOwnership>, Box<dyn std::error::Error>> {
+    pub fn for_team(&self, team_name: &str) -> Result<Vec<TeamOwnership>, Box<dyn Error>> {
         info!("getting team ownership for {}", team_name);
         let team = self.project.get_team(team_name).ok_or("Team not found")?;
 
@@ -155,33 +165,37 @@ impl Ownership {
     }
 }
 
-fn parse_for_team(team_name: String, codeowners_file: &str) -> Result<Vec<TeamOwnership>, Box<dyn std::error::Error>> {
+fn parse_for_team(team_name: String, codeowners_file: &str) -> Result<Vec<TeamOwnership>, Box<dyn Error>> {
     let mut output = vec![];
-    let mut current_section = TeamOwnership::default();
+    let mut current_section: Option<TeamOwnership> = None;
     let input: String = codeowners_file.replace(&FileGenerator::disclaimer().join("\n"), "");
 
     for line in input.trim_start().lines() {
         match line {
             comment if comment.starts_with("#") => {
-                current_section.heading = comment.to_string();
+                current_section = Some(TeamOwnership::new(comment.to_string()));
             }
             "" => {
-                if current_section != TeamOwnership::default() {
-                    output.push(current_section.clone());
-                    current_section = TeamOwnership::default();
+                if let Some(cs) = current_section {
+                    output.push(cs.clone());
+                    current_section = None;
                 }
             }
-            team_line if team_line.ends_with(&team_name) => {
-                let v: Vec<&str> = team_line.split(' ').collect();
-                let s = v.first().ok_or("malformed team line")?;
+            team_line if team_line.ends_with(&team_name) => match current_section {
+                Some(ref mut cs) => {
+                    let v: Vec<&str> = team_line.split(' ').collect();
+                    let s = v.first().ok_or("Malformed team line")?;
 
-                current_section.globs.push(s.to_string());
-            }
+                    cs.globs.push(s.to_string());
+                }
+                _ => return Err(Box::from("Missing preceding section header")),
+            },
             _ => {}
         }
     }
-    if current_section != TeamOwnership::default() {
-        output.push(current_section.clone());
+
+    if let Some(cs) = current_section {
+        output.push(cs.clone());
     }
 
     Ok(output)
@@ -193,7 +207,7 @@ mod tests {
     use crate::common_test::tests::{build_ownership_with_all_mappers, vecs_match};
 
     #[test]
-    fn test_for_file_owner() -> Result<(), Box<dyn std::error::Error>> {
+    fn test_for_file_owner() -> Result<(), Box<dyn Error>> {
         let ownership = build_ownership_with_all_mappers()?;
         let file_owners = ownership.for_file("app/consumers/directory_owned.rb").unwrap();
         assert_eq!(file_owners.len(), 1);
@@ -203,31 +217,31 @@ mod tests {
     }
 
     #[test]
-    fn test_for_file_no_owner() -> Result<(), Box<dyn std::error::Error>> {
+    fn test_for_file_no_owner() -> Result<(), Box<dyn Error>> {
         let ownership = build_ownership_with_all_mappers()?;
         let file_owners = ownership.for_file("app/madeup/foo.rb").unwrap();
         assert_eq!(file_owners.len(), 0);
         Ok(())
     }
 
     #[test]
-    fn test_for_team_not_found() -> Result<(), Box<dyn std::error::Error>> {
+    fn test_for_team() -> Result<(), Box<dyn Error>> {
         let ownership = build_ownership_with_all_mappers()?;
-        let team_ownership = ownership.for_team("Nope");
-        assert!(team_ownership.is_err(), "Team not found");
+        let team_ownership = ownership.for_team("Bar");
+        assert!(team_ownership.is_ok());
         Ok(())
     }
 
     #[test]
-    fn test_for_team() -> Result<(), Box<dyn std::error::Error>> {
+    fn test_for_team_not_found() -> Result<(), Box<dyn Error>> {
         let ownership = build_ownership_with_all_mappers()?;
-        let team_ownership = ownership.for_team("Bar");
-        assert!(team_ownership.is_ok());
+        let team_ownership = ownership.for_team("Nope");
+        assert!(team_ownership.is_err(), "Team not found");
         Ok(())
     }
 
     #[test]
-    fn test_parse_for_team_trims_header() -> Result<(), Box<dyn std::error::Error>> {
+    fn test_parse_for_team_trims_header() -> Result<(), Box<dyn Error>> {
         let codeownership_file = r#"
 # STOP! - DO NOT EDIT THIS FILE MANUALLY
 # This file was automatically generated by "bin/codeownership validate".
@@ -240,29 +254,29 @@ mod tests {
 
 "#;
 
-        let team_ownership = parse_for_team("@Bar".to_string(), &codeownership_file)?;
+        let team_ownership = parse_for_team("@Bar".to_string(), codeownership_file)?;
         assert!(team_ownership.is_empty());
         Ok(())
     }
 
     #[test]
-    fn test_parse_for_team_includes_owned_globs() -> Result<(), Box<dyn std::error::Error>> {
+    fn test_parse_for_team_includes_owned_globs() -> Result<(), Box<dyn Error>> {
         let codeownership_file = r#"
 # First Section
-/path/to/owned/**/* @Foo
-/path/to/not/owned/**/* @Bar
+/path/to/owned @Foo
+/path/to/not/owned @Bar
 
 # Last Section
 /another/owned/path @Foo
 "#;
 
-        let team_ownership = parse_for_team("@Foo".to_string(), &codeownership_file)?;
+        let team_ownership = parse_for_team("@Foo".to_string(), codeownership_file)?;
         vecs_match(
             &team_ownership,
             &vec![
                 TeamOwnership {
                     heading: "# First Section".to_string(),
-                    globs: vec!["/path/to/owned/**/*".to_string()],
+                    globs: vec!["/path/to/owned".to_string()],
                 },
                 TeamOwnership {
                     heading: "# Last Section".to_string(),
@@ -274,14 +288,14 @@ mod tests {
     }
 
     #[test]
-    fn test_parse_for_team_with_partial_team_match() -> Result<(), Box<dyn std::error::Error>> {
+    fn test_parse_for_team_with_partial_team_match() -> Result<(), Box<dyn Error>> {
         let codeownership_file = r#"
 # First Section
 /path/to/owned @Foo
 /path/to/not/owned @FooBar
 "#;
 
-        let team_ownership = parse_for_team("@Foo".to_string(), &codeownership_file)?;
+        let team_ownership = parse_for_team("@Foo".to_string(), codeownership_file)?;
         vecs_match(
             &team_ownership,
             &vec![TeamOwnership {
@@ -293,7 +307,7 @@ mod tests {
     }
 
     #[test]
-    fn test_parse_for_team_with_trailing_newlines() -> Result<(), Box<dyn std::error::Error>> {
+    fn test_parse_for_team_with_trailing_newlines() -> Result<(), Box<dyn Error>> {
         let codeownership_file = r#"
 # First Section
 /path/to/owned @Foo
@@ -305,7 +319,7 @@ mod tests {
 
 "#;
 
-        let team_ownership = parse_for_team("@Foo".to_string(), &codeownership_file)?;
+        let team_ownership = parse_for_team("@Foo".to_string(), codeownership_file)?;
         vecs_match(
             &team_ownership,
             &vec![
@@ -323,12 +337,12 @@ mod tests {
     }
 
     #[test]
-    fn test_parse_for_team_without_trailing_newline() -> Result<(), Box<dyn std::error::Error>> {
+    fn test_parse_for_team_without_trailing_newline() -> Result<(), Box<dyn Error>> {
         let codeownership_file = r#"
 # First Section
 /path/to/owned @Foo"#;
 
-        let team_ownership = parse_for_team("@Foo".to_string(), &codeownership_file)?;
+        let team_ownership = parse_for_team("@Foo".to_string(), codeownership_file)?;
         vecs_match(
             &team_ownership,
             &vec![TeamOwnership {
@@ -338,4 +352,19 @@ mod tests {
         );
         Ok(())
     }
+
+    #[test]
+    fn test_parse_for_team_with_missing_section_header() -> Result<(), Box<dyn Error>> {
+        let codeownership_file = r#"
+# First Section
+/path/to/owned @Foo
+
+/another/owned/path @Foo
+"#;
+
+        let team_ownership = parse_for_team("@Foo".to_string(), codeownership_file);
+        dbg!(&team_ownership);
+        assert!(team_ownership.is_err());
+        Ok(())
+    }
 }
diff --git a/tests/valid_project_test.rs b/tests/valid_project_test.rs
@@ -58,6 +58,7 @@ fn test_for_team() -> Result<(), Box<dyn Error>> {
 /ruby/app/models/payroll.rb
 
 ## Team-specific owned globs
+This team owns nothing in this category.
 
 ## Owner in .codeowner
 /ruby/app/payroll/**/**
@@ -73,7 +74,6 @@ fn test_for_team() -> Result<(), Box<dyn Error>> {
 
 ## Team owned gems
 /gems/payroll_calculator/**/**
-
 "#
     .trim_start();
 

Original file line number	Diff line number	Diff line change
`@@ -138,13 +138,19 @@ fn cli() -> Result<(), Error> {`
`138`	`138`	`}`
`139`	`139`	`}`
`140`	`140`	`}`
`141`		`- Command::ForTeam { name } => {`
`142`		`- let team_ownership = ownership.for_team(&name);`
`143`		`- match team_ownership {`
`144`		`- Ok(output) => println!("{:?}", output),`
`145`		`- Err(err) => eprintln!("{}", err),`
	`141`	`+ Command::ForTeam { name } => match ownership.for_team(&name) {`
	`142`	`+ Ok(team_ownerships) => {`
	`143`	+ println!("# Code Ownership Report for `{}` Team", name);
	`144`	`+ for team_ownership in team_ownerships {`
	`145`	`+ println!("\n#{}", team_ownership.heading);`
	`146`	`+ match team_ownership.globs.len() {`
	`147`	`+ 0 => println!("This team owns nothing in this category."),`
	`148`	`+ _ => println!("{}", team_ownership.globs.join("\n")),`
	`149`	`+ }`
	`150`	`+ }`
`146`	`151`	`}`
`147`		`- }`
	`152`	`+ Err(err) => eprintln!("{}", err),`
	`153`	`+ },`
`148`	`154`	`}`
`149`	`155`
`150`	`156`	`Ok(())`