Proposal to sort CODEOWNERS entries by specificity across all sections

src/ownership/codeowners_file_parser.rs

@@ -161,6 +161,9 @@ fn codeowner_sections(codeowners_file: &str) -> Result<Vec<Section>, Box<dyn Err
 
     if let Some(section_name) = current_section {
         sections.push(Section::new(section_name, current_lines));
+    } else if !current_lines.is_empty() {
+        // If no section headers were found, treat all lines as a single section
+        sections.push(Section::new("# Generated entries".to_string(), current_lines));
     }
 
     Ok(sections)
@@ -196,8 +199,12 @@ pub fn parse_for_team(team_name: String, codeowners_file: &str) -> Result<Vec<Te
                 }
             }
             team_line if team_line.ends_with(&team_name) => {
-                let section = current_section.as_mut().ok_or(error_message)?;
+                // If no section header exists, create a default one
+                if current_section.is_none() {
+                    current_section = Some(TeamOwnership::new("# Owned Files".to_string()));
+                }
 
+                let section = current_section.as_mut().ok_or(error_message)?;
                 let glob = line.split_once(' ').ok_or(error_message)?.0.to_string();
                 section.globs.push(glob);
             }
@@ -342,10 +349,20 @@ mod tests {
             /another/owned/path @Foo
         "};
 
-        let team_ownership = parse_for_team("@Foo".to_string(), codeownership_file);
-        assert!(
-            team_ownership
-                .is_err_and(|e| e.to_string() == "CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file")
+        let team_ownership = parse_for_team("@Foo".to_string(), codeownership_file)?;
+        // Now handles files without section headers by creating a default "# Owned Files" section
+        vecs_match(
+            &team_ownership,
+            &vec![
+                TeamOwnership {
+                    heading: "# First Section".to_string(),
+                    globs: vec!["/path/to/owned".to_string()],
+                },
+                TeamOwnership {
+                    heading: "# Owned Files".to_string(),
+                    globs: vec!["/another/owned/path".to_string()],
+                },
+            ],
         );
         Ok(())
     }

src/ownership/file_generator.rs

@@ -11,17 +11,16 @@ impl FileGenerator {
         let mut lines: Vec<String> = Vec::new();
         lines.append(&mut Self::disclaimer());
 
+        // Collect all entries from all mappers
+        let mut all_entries: Vec<Entry> = Vec::new();
         for mapper in &self.mappers {
-            if mapper.entries().is_empty() {
-                continue;
-            }
-
-            lines.push(format!("# {}", mapper.name()));
-            lines.append(&mut Self::to_sorted_lines(&mapper.entries()));
-            lines.push("".to_owned());
+            all_entries.extend(mapper.entries());
         }
 
-        lines.join("\n")
+        // Sort all entries together to ensure correct specificity ordering
+        lines.append(&mut Self::to_sorted_lines(&all_entries));
+
+        format!("{}\n", lines.join("\n"))
     }
 
     pub fn disclaimer() -> Vec<String> {
@@ -200,4 +199,34 @@ mod tests {
         let sorted = FileGenerator::to_sorted_lines(&entries);
         assert_eq!(sorted, vec!["/directory/owner-1/** @foo", "/directory/owner_2/** @bar"]);
     }
+
+    #[test]
+    fn test_specific_file_overrides_glob_pattern() {
+        // This tests the case where a specific file should override a general glob pattern.
+        // In CODEOWNERS, the last matching rule wins, so general patterns must come first.
+        let entries = vec![
+            Entry {
+                path: "js/packages/zp-constants/src/__generated__/global.ts".to_string(),
+                github_team: "@Gusto/dev-productivity-web".to_string(),
+                team_name: "dev-productivity-web".to_string(),
+                disabled: false,
+            },
+            Entry {
+                path: "js/packages/zp-constants/**/**".to_string(),
+                github_team: "@Gusto/dev-productivity-modularity-unowned".to_string(),
+                team_name: "dev-productivity-modularity-unowned".to_string(),
+                disabled: false,
+            },
+        ];
+        let sorted = FileGenerator::to_sorted_lines(&entries);
+        // The glob pattern should come FIRST, then the specific file
+        // This way the specific file rule overrides the general glob
+        assert_eq!(
+            sorted,
+            vec![
+                "/js/packages/zp-constants/**/** @Gusto/dev-productivity-modularity-unowned",
+                "/js/packages/zp-constants/src/__generated__/global.ts @Gusto/dev-productivity-web"
+            ]
+        );
+    }
 }

src/ownership/mapper.rs

@@ -24,7 +24,6 @@ pub use team_yml_mapper::TeamYmlMapper;
 use super::Entry;
 
 pub trait Mapper {
-    fn name(&self) -> String;
     fn entries(&self) -> Vec<Entry>;
     fn owner_matchers(&self) -> Vec<OwnerMatcher>;
 }

src/ownership/mapper/annotated_file_mapper.rs

@@ -61,10 +61,6 @@ impl Mapper for TeamFileMapper {
 
         vec![OwnerMatcher::ExactMatches(path_to_team, Source::AnnotatedFile)]
     }
-
-    fn name(&self) -> String {
-        "Annotations at the top of file".to_owned()
-    }
 }
 
 #[cfg(test)]

src/ownership/mapper/directory_mapper.rs

@@ -55,10 +55,6 @@ impl Mapper for DirectoryMapper {
 
         owner_matchers
     }
-
-    fn name(&self) -> String {
-        "Owner in .codeowner".to_owned()
-    }
 }
 
 #[cfg(test)]

src/ownership/mapper/package_mapper.rs

@@ -31,10 +31,6 @@ impl Mapper for RubyPackageMapper {
     fn owner_matchers(&self) -> Vec<OwnerMatcher> {
         PackageMapper::build(self.project.clone()).owner_matchers(&PackageType::Ruby)
     }
-
-    fn name(&self) -> String {
-        "Owner metadata key in package.yml".to_owned()
-    }
 }
 
 impl JavascriptPackageMapper {
@@ -51,10 +47,6 @@ impl Mapper for JavascriptPackageMapper {
     fn owner_matchers(&self) -> Vec<OwnerMatcher> {
         PackageMapper::build(self.project.clone()).owner_matchers(&PackageType::Javascript)
     }
-
-    fn name(&self) -> String {
-        "Owner metadata key in package.json".to_owned()
-    }
 }
 
 impl PackageMapper {

src/ownership/mapper/team_gem_mapper.rs

@@ -57,10 +57,6 @@ impl Mapper for TeamGemMapper {
 
         owner_matchers
     }
-
-    fn name(&self) -> String {
-        "Team owned gems".to_owned()
-    }
 }
 
 #[cfg(test)]

src/ownership/mapper/team_glob_mapper.rs

@@ -49,10 +49,6 @@ impl Mapper for TeamGlobMapper {
 
         owner_matchers
     }
-
-    fn name(&self) -> String {
-        "Team-specific owned globs".to_owned()
-    }
 }
 
 #[cfg(test)]

src/ownership/mapper/team_yml_mapper.rs

@@ -42,10 +42,6 @@ impl Mapper for TeamYmlMapper {
 
         vec![OwnerMatcher::ExactMatches(path_to_team, Source::TeamYml)]
     }
-
-    fn name(&self) -> String {
-        "Team YML ownership".to_owned()
-    }
 }
 
 #[cfg(test)]

tests/fixtures/multiple-directory-owners/.github/CODEOWNERS

@@ -7,11 +7,8 @@
 # https://help.github.com/en/articles/about-code-owners
 
 
-# Owner in .codeowner
 /app/consumers/**/** @barteam
 /app/services/**/** @footeam
 /app/services/exciting/**/** @barteam
-
-# Team YML ownership
 /config/teams/bar.yml @barteam
 /config/teams/foo.yml @footeam

tests/fixtures/multiple-directory-owners/tests/fixtures/multiple-directory-owners/.github/CODEOWNERS

@@ -0,0 +1,8 @@
+# STOP! - DO NOT EDIT THIS FILE MANUALLY
+# This file was automatically generated by "bin/codeownership validate".
+#
+# CODEOWNERS is used for GitHub to suggest code/file owners to various GitHub
+# teams. This is useful when developers create Pull Requests since the
+# code/file owner is notified. Reference GitHub docs for more details:
+# https://help.github.com/en/articles/about-code-owners
+

tests/fixtures/valid_project/.github/CODEOWNERS

@@ -7,35 +7,22 @@
 # https://help.github.com/en/articles/about-code-owners
 
 
-# Annotations at the top of file
+/config/teams/payments.yml @PaymentsTeam
+/config/teams/payroll.yml @PayrollTeam
+/config/teams/ux.yml @UX
+/gems/payroll_calculator/**/** @PayrollTeam
+/gems/pets/**/** @UX
+/javascript/packages/PayrollFlow/**/** @PayrollTeam
 /javascript/packages/PayrollFlow/index.tsx @PayrollTeam
+/javascript/packages/items/**/** @PayrollTeam
+/javascript/packages/items/(special)/**/** @PaymentsTeam
 /javascript/packages/list/page-admin.tsx @PaymentsTeam
 /ruby/app/models/bank_account.rb @PaymentsTeam
 /ruby/app/models/payroll.rb @PayrollTeam
-/ruby/app/views/foos/edit.erb @PayrollTeam
-/ruby/app/views/foos/index.html.erb @UX
-/ruby/app/views/foos/new.html.erb @PayrollTeam
-
-# Team-specific owned globs
 /ruby/app/payments/**/* @PaymentsTeam
-
-# Owner in .codeowner
-/javascript/packages/items/**/** @PayrollTeam
-/javascript/packages/items/(special)/**/** @PaymentsTeam
 /ruby/app/payments/foo/**/** @PayrollTeam
 /ruby/app/payroll/**/** @PayrollTeam
-
-# Owner metadata key in package.yml
+/ruby/app/views/foos/edit.erb @PayrollTeam
+/ruby/app/views/foos/index.html.erb @UX
+/ruby/app/views/foos/new.html.erb @PayrollTeam
 /ruby/packages/payroll_flow/**/** @PayrollTeam
-
-# Owner metadata key in package.json
-/javascript/packages/PayrollFlow/**/** @PayrollTeam
-
-# Team YML ownership
-/config/teams/payments.yml @PaymentsTeam
-/config/teams/payroll.yml @PayrollTeam
-/config/teams/ux.yml @UX
-
-# Team owned gems
-/gems/payroll_calculator/**/** @PayrollTeam
-/gems/pets/**/** @UX

tests/valid_project_test.rs

@@ -299,31 +299,18 @@ fn test_for_team() -> Result<(), Box<dyn Error>> {
         predicate::eq(indoc! {"
             # Code Ownership Report for `Payroll` Team
 
-            ## Annotations at the top of file
+            ## Owned Files
+            /config/teams/payroll.yml
+            /gems/payroll_calculator/**/**
+            /javascript/packages/PayrollFlow/**/**
             /javascript/packages/PayrollFlow/index.tsx
-            /ruby/app/models/payroll.rb
-            /ruby/app/views/foos/edit.erb
-            /ruby/app/views/foos/new.html.erb
-
-            ## Team-specific owned globs
-            This team owns nothing in this category.
-
-            ## Owner in .codeowner
             /javascript/packages/items/**/**
+            /ruby/app/models/payroll.rb
             /ruby/app/payments/foo/**/**
             /ruby/app/payroll/**/**
-
-            ## Owner metadata key in package.yml
+            /ruby/app/views/foos/edit.erb
+            /ruby/app/views/foos/new.html.erb
             /ruby/packages/payroll_flow/**/**
-
-            ## Owner metadata key in package.json
-            /javascript/packages/PayrollFlow/**/**
-
-            ## Team YML ownership
-            /config/teams/payroll.yml
-
-            ## Team owned gems
-            /gems/payroll_calculator/**/**
         "}),
     )?;