Skip to content

Commit e682520

Browse files
martinemdeclaude
andcommitted
fix: sort CODEOWNERS entries by specificity across all sections
Ensure more specific file patterns override general globs by sorting all entries together instead of grouping by section. Remove section headers from output and handle CODEOWNERS files without headers. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent c3cace6 commit e682520

File tree

13 files changed

+85
-89
lines changed

13 files changed

+85
-89
lines changed

src/ownership/codeowners_file_parser.rs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ fn codeowner_sections(codeowners_file: &str) -> Result<Vec<Section>, Box<dyn Err
161161

162162
if let Some(section_name) = current_section {
163163
sections.push(Section::new(section_name, current_lines));
164+
} else if !current_lines.is_empty() {
165+
// If no section headers were found, treat all lines as a single section
166+
sections.push(Section::new("# Generated entries".to_string(), current_lines));
164167
}
165168

166169
Ok(sections)
@@ -196,8 +199,12 @@ pub fn parse_for_team(team_name: String, codeowners_file: &str) -> Result<Vec<Te
196199
}
197200
}
198201
team_line if team_line.ends_with(&team_name) => {
199-
let section = current_section.as_mut().ok_or(error_message)?;
202+
// If no section header exists, create a default one
203+
if current_section.is_none() {
204+
current_section = Some(TeamOwnership::new("# Owned Files".to_string()));
205+
}
200206

207+
let section = current_section.as_mut().ok_or(error_message)?;
201208
let glob = line.split_once(' ').ok_or(error_message)?.0.to_string();
202209
section.globs.push(glob);
203210
}
@@ -342,10 +349,20 @@ mod tests {
342349
/another/owned/path @Foo
343350
"};
344351

345-
let team_ownership = parse_for_team("@Foo".to_string(), codeownership_file);
346-
assert!(
347-
team_ownership
348-
.is_err_and(|e| e.to_string() == "CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file")
352+
let team_ownership = parse_for_team("@Foo".to_string(), codeownership_file)?;
353+
// Now handles files without section headers by creating a default "# Owned Files" section
354+
vecs_match(
355+
&team_ownership,
356+
&vec![
357+
TeamOwnership {
358+
heading: "# First Section".to_string(),
359+
globs: vec!["/path/to/owned".to_string()],
360+
},
361+
TeamOwnership {
362+
heading: "# Owned Files".to_string(),
363+
globs: vec!["/another/owned/path".to_string()],
364+
},
365+
],
349366
);
350367
Ok(())
351368
}

src/ownership/file_generator.rs

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,16 @@ impl FileGenerator {
1111
let mut lines: Vec<String> = Vec::new();
1212
lines.append(&mut Self::disclaimer());
1313

14+
// Collect all entries from all mappers
15+
let mut all_entries: Vec<Entry> = Vec::new();
1416
for mapper in &self.mappers {
15-
if mapper.entries().is_empty() {
16-
continue;
17-
}
18-
19-
lines.push(format!("# {}", mapper.name()));
20-
lines.append(&mut Self::to_sorted_lines(&mapper.entries()));
21-
lines.push("".to_owned());
17+
all_entries.extend(mapper.entries());
2218
}
2319

24-
lines.join("\n")
20+
// Sort all entries together to ensure correct specificity ordering
21+
lines.append(&mut Self::to_sorted_lines(&all_entries));
22+
23+
format!("{}\n", lines.join("\n"))
2524
}
2625

2726
pub fn disclaimer() -> Vec<String> {
@@ -200,4 +199,34 @@ mod tests {
200199
let sorted = FileGenerator::to_sorted_lines(&entries);
201200
assert_eq!(sorted, vec!["/directory/owner-1/** @foo", "/directory/owner_2/** @bar"]);
202201
}
202+
203+
#[test]
204+
fn test_specific_file_overrides_glob_pattern() {
205+
// This tests the case where a specific file should override a general glob pattern.
206+
// In CODEOWNERS, the last matching rule wins, so general patterns must come first.
207+
let entries = vec![
208+
Entry {
209+
path: "js/packages/zp-constants/src/__generated__/global.ts".to_string(),
210+
github_team: "@Gusto/dev-productivity-web".to_string(),
211+
team_name: "dev-productivity-web".to_string(),
212+
disabled: false,
213+
},
214+
Entry {
215+
path: "js/packages/zp-constants/**/**".to_string(),
216+
github_team: "@Gusto/dev-productivity-modularity-unowned".to_string(),
217+
team_name: "dev-productivity-modularity-unowned".to_string(),
218+
disabled: false,
219+
},
220+
];
221+
let sorted = FileGenerator::to_sorted_lines(&entries);
222+
// The glob pattern should come FIRST, then the specific file
223+
// This way the specific file rule overrides the general glob
224+
assert_eq!(
225+
sorted,
226+
vec![
227+
"/js/packages/zp-constants/**/** @Gusto/dev-productivity-modularity-unowned",
228+
"/js/packages/zp-constants/src/__generated__/global.ts @Gusto/dev-productivity-web"
229+
]
230+
);
231+
}
203232
}

src/ownership/mapper.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ pub use team_yml_mapper::TeamYmlMapper;
2424
use super::Entry;
2525

2626
pub trait Mapper {
27-
fn name(&self) -> String;
2827
fn entries(&self) -> Vec<Entry>;
2928
fn owner_matchers(&self) -> Vec<OwnerMatcher>;
3029
}

src/ownership/mapper/annotated_file_mapper.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,6 @@ impl Mapper for TeamFileMapper {
6161

6262
vec![OwnerMatcher::ExactMatches(path_to_team, Source::AnnotatedFile)]
6363
}
64-
65-
fn name(&self) -> String {
66-
"Annotations at the top of file".to_owned()
67-
}
6864
}
6965

7066
#[cfg(test)]

src/ownership/mapper/directory_mapper.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,6 @@ impl Mapper for DirectoryMapper {
5555

5656
owner_matchers
5757
}
58-
59-
fn name(&self) -> String {
60-
"Owner in .codeowner".to_owned()
61-
}
6258
}
6359

6460
#[cfg(test)]

src/ownership/mapper/package_mapper.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,6 @@ impl Mapper for RubyPackageMapper {
3131
fn owner_matchers(&self) -> Vec<OwnerMatcher> {
3232
PackageMapper::build(self.project.clone()).owner_matchers(&PackageType::Ruby)
3333
}
34-
35-
fn name(&self) -> String {
36-
"Owner metadata key in package.yml".to_owned()
37-
}
3834
}
3935

4036
impl JavascriptPackageMapper {
@@ -51,10 +47,6 @@ impl Mapper for JavascriptPackageMapper {
5147
fn owner_matchers(&self) -> Vec<OwnerMatcher> {
5248
PackageMapper::build(self.project.clone()).owner_matchers(&PackageType::Javascript)
5349
}
54-
55-
fn name(&self) -> String {
56-
"Owner metadata key in package.json".to_owned()
57-
}
5850
}
5951

6052
impl PackageMapper {

src/ownership/mapper/team_gem_mapper.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,6 @@ impl Mapper for TeamGemMapper {
5757

5858
owner_matchers
5959
}
60-
61-
fn name(&self) -> String {
62-
"Team owned gems".to_owned()
63-
}
6460
}
6561

6662
#[cfg(test)]

src/ownership/mapper/team_glob_mapper.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,6 @@ impl Mapper for TeamGlobMapper {
4949

5050
owner_matchers
5151
}
52-
53-
fn name(&self) -> String {
54-
"Team-specific owned globs".to_owned()
55-
}
5652
}
5753

5854
#[cfg(test)]

src/ownership/mapper/team_yml_mapper.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,6 @@ impl Mapper for TeamYmlMapper {
4242

4343
vec![OwnerMatcher::ExactMatches(path_to_team, Source::TeamYml)]
4444
}
45-
46-
fn name(&self) -> String {
47-
"Team YML ownership".to_owned()
48-
}
4945
}
5046

5147
#[cfg(test)]

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,8 @@
77
# https://help.github.com/en/articles/about-code-owners
88

99

10-
# Owner in .codeowner
1110
/app/consumers/**/** @barteam
1211
/app/services/**/** @footeam
1312
/app/services/exciting/**/** @barteam
14-
15-
# Team YML ownership
1613
/config/teams/bar.yml @barteam
1714
/config/teams/foo.yml @footeam

0 commit comments

Comments
 (0)