Skip to content

Commit 76adbd9

Browse files
committed
fix: github pattern matching
1 parent baf9d88 commit 76adbd9

1 file changed

Lines changed: 222 additions & 2 deletions

File tree

src/core/types.rs

Lines changed: 222 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,22 @@ use std::path::PathBuf;
33
use ignore::overrides::Override;
44
use serde::{Deserialize, Serialize};
55

6+
/// Normalizes a CODEOWNERS pattern to match GitHub's behavior
7+
///
8+
/// GitHub CODEOWNERS directory matching rules:
9+
/// - `/path/to/dir/` matches all files and subdirectories under that path (converted to `/path/to/dir/**`)
10+
/// - `/path/to/dir/*` matches direct files only (kept as-is)
11+
/// - `/path/to/dir/**` matches everything recursively (kept as-is)
12+
/// - Other patterns are kept as-is
13+
fn normalize_codeowners_pattern(pattern: &str) -> String {
14+
// If pattern ends with `/` but not `*/` or `**/`, convert to `/**`
15+
if pattern.ends_with('/') && !pattern.ends_with("*/") && !pattern.ends_with("**/") {
16+
format!("{}**", pattern)
17+
} else {
18+
pattern.to_string()
19+
}
20+
}
21+
622
/// CODEOWNERS entry with source tracking
723
#[derive(Debug, Serialize, Deserialize)]
824
pub struct CodeownersEntry {
@@ -38,9 +54,13 @@ pub fn codeowners_entry_to_matcher(entry: &CodeownersEntry) -> CodeownersEntryMa
3854

3955
let mut builder = ignore::overrides::OverrideBuilder::new(codeowners_dir);
4056

41-
if let Err(e) = builder.add(&entry.pattern) {
57+
// Transform directory patterns to match GitHub CODEOWNERS behavior
58+
let pattern = normalize_codeowners_pattern(&entry.pattern);
59+
60+
if let Err(e) = builder.add(&pattern) {
4261
eprintln!(
43-
"Invalid pattern '{}' in {}: {}",
62+
"Invalid pattern '{}' (normalized from '{}') in {}: {}",
63+
pattern,
4464
entry.pattern,
4565
entry.source_file.display(),
4666
e
@@ -205,3 +225,203 @@ pub enum CacheEncoding {
205225
Bincode,
206226
Json,
207227
}
228+
229+
#[cfg(test)]
230+
mod tests {
231+
use super::*;
232+
233+
#[test]
234+
fn test_normalize_codeowners_pattern_directory_patterns() {
235+
// Directory patterns ending with / should be converted to /**
236+
assert_eq!(
237+
normalize_codeowners_pattern("/builtin/logical/aws/"),
238+
"/builtin/logical/aws/**"
239+
);
240+
assert_eq!(
241+
normalize_codeowners_pattern("src/components/"),
242+
"src/components/**"
243+
);
244+
assert_eq!(
245+
normalize_codeowners_pattern("docs/"),
246+
"docs/**"
247+
);
248+
assert_eq!(
249+
normalize_codeowners_pattern("/"),
250+
"/**"
251+
);
252+
}
253+
254+
#[test]
255+
fn test_normalize_codeowners_pattern_already_globbed() {
256+
// Patterns already ending with */ or **/ should be left as-is
257+
assert_eq!(
258+
normalize_codeowners_pattern("/builtin/logical/aws/*"),
259+
"/builtin/logical/aws/*"
260+
);
261+
assert_eq!(
262+
normalize_codeowners_pattern("/builtin/logical/aws/**"),
263+
"/builtin/logical/aws/**"
264+
);
265+
assert_eq!(
266+
normalize_codeowners_pattern("src/*/"),
267+
"src/*/"
268+
);
269+
assert_eq!(
270+
normalize_codeowners_pattern("docs/**/"),
271+
"docs/**/"
272+
);
273+
}
274+
275+
#[test]
276+
fn test_normalize_codeowners_pattern_file_patterns() {
277+
// File patterns should be left as-is
278+
assert_eq!(
279+
normalize_codeowners_pattern("*.rs"),
280+
"*.rs"
281+
);
282+
assert_eq!(
283+
normalize_codeowners_pattern("/src/main.rs"),
284+
"/src/main.rs"
285+
);
286+
assert_eq!(
287+
normalize_codeowners_pattern("package.json"),
288+
"package.json"
289+
);
290+
assert_eq!(
291+
normalize_codeowners_pattern("**/*.ts"),
292+
"**/*.ts"
293+
);
294+
}
295+
296+
#[test]
297+
fn test_normalize_codeowners_pattern_edge_cases() {
298+
// Edge cases
299+
assert_eq!(
300+
normalize_codeowners_pattern(""),
301+
""
302+
);
303+
assert_eq!(
304+
normalize_codeowners_pattern("file"),
305+
"file"
306+
);
307+
assert_eq!(
308+
normalize_codeowners_pattern("./relative/"),
309+
"./relative/**"
310+
);
311+
assert_eq!(
312+
normalize_codeowners_pattern("../parent/"),
313+
"../parent/**"
314+
);
315+
}
316+
317+
#[test]
318+
fn test_normalize_codeowners_pattern_complex_patterns() {
319+
// Complex patterns that should and shouldn't be transformed
320+
assert_eq!(
321+
normalize_codeowners_pattern("/path/with spaces/"),
322+
"/path/with spaces/**"
323+
);
324+
assert_eq!(
325+
normalize_codeowners_pattern("/path-with-dashes/"),
326+
"/path-with-dashes/**"
327+
);
328+
assert_eq!(
329+
normalize_codeowners_pattern("/path.with.dots/"),
330+
"/path.with.dots/**"
331+
);
332+
333+
// These should not be transformed
334+
assert_eq!(
335+
normalize_codeowners_pattern("/path/*/something"),
336+
"/path/*/something"
337+
);
338+
assert_eq!(
339+
normalize_codeowners_pattern("/path/**/something"),
340+
"/path/**/something"
341+
);
342+
}
343+
344+
#[test]
345+
fn test_codeowners_entry_to_matcher_directory_pattern_github_behavior() {
346+
use std::fs;
347+
use tempfile::TempDir;
348+
349+
// Create a temporary directory structure for testing
350+
let temp_dir = TempDir::new().unwrap();
351+
let base_path = temp_dir.path();
352+
353+
// Create directory structure: builtin/logical/aws/
354+
let aws_dir = base_path.join("builtin").join("logical").join("aws");
355+
fs::create_dir_all(&aws_dir).unwrap();
356+
357+
// Create some test files
358+
fs::write(aws_dir.join("config.go"), "package aws").unwrap();
359+
fs::write(aws_dir.join("client.go"), "package aws").unwrap();
360+
361+
// Create subdirectory with files
362+
let sub_dir = aws_dir.join("auth");
363+
fs::create_dir_all(&sub_dir).unwrap();
364+
fs::write(sub_dir.join("token.go"), "package auth").unwrap();
365+
366+
// Create CODEOWNERS file
367+
let codeowners_path = base_path.join("CODEOWNERS");
368+
369+
// Test directory pattern: /builtin/logical/aws/ should match all files under it
370+
let entry = CodeownersEntry {
371+
source_file: codeowners_path.clone(),
372+
line_number: 1,
373+
pattern: "/builtin/logical/aws/".to_string(),
374+
owners: vec![Owner {
375+
identifier: "@hashicorp/vault-ecosystem".to_string(),
376+
owner_type: OwnerType::Team,
377+
}],
378+
tags: vec![],
379+
};
380+
381+
let matcher = codeowners_entry_to_matcher(&entry);
382+
383+
// Test files that should match
384+
let test_files = vec![
385+
"builtin/logical/aws/config.go",
386+
"builtin/logical/aws/client.go",
387+
"builtin/logical/aws/auth/token.go",
388+
];
389+
390+
for file_path in test_files {
391+
let path = base_path.join(file_path);
392+
let relative_path = path.strip_prefix(base_path).unwrap();
393+
394+
// The override matcher should match these files
395+
let is_match = matcher.override_matcher.matched(relative_path, false).is_whitelist();
396+
assert!(
397+
is_match,
398+
"Pattern '/builtin/logical/aws/' should match file '{}' (like GitHub)",
399+
relative_path.display()
400+
);
401+
}
402+
403+
// Test files that should NOT match
404+
let non_matching_files = vec![
405+
"builtin/logical/database/config.go",
406+
"builtin/auth/aws/config.go",
407+
"other/aws/config.go",
408+
];
409+
410+
for file_path in non_matching_files {
411+
let path = base_path.join(file_path);
412+
// Create parent dirs for these test files
413+
if let Some(parent) = path.parent() {
414+
fs::create_dir_all(parent).unwrap();
415+
}
416+
fs::write(&path, "test content").unwrap();
417+
418+
let relative_path = path.strip_prefix(base_path).unwrap();
419+
let is_match = matcher.override_matcher.matched(relative_path, false).is_whitelist();
420+
assert!(
421+
!is_match,
422+
"Pattern '/builtin/logical/aws/' should NOT match file '{}'",
423+
relative_path.display()
424+
);
425+
}
426+
}
427+
}

0 commit comments

Comments
 (0)