Skip directories that hold no tracked files in the project walk

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "codeowners"
-version = "0.7.0"
+version = "0.7.1"
 edition = "2024"
 license = "MIT"
 

src/path_utils.rs

@@ -64,10 +64,31 @@ pub(crate) fn matches_globs(path: &Path, globs: &[String]) -> bool {
     }
 }
 
+/// The leading components of `glob` with no glob syntax in them. Every path the glob can match is under this one.
+pub(crate) fn literal_glob_prefix(glob: &str) -> PathBuf {
+    Path::new(glob)
+        .components()
+        .take_while(|component| !component.as_os_str().to_string_lossy().contains(['*', '?', '[', '{', '\\', '!']))
+        .collect()
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
 
+    #[test]
+    fn test_literal_glob_prefix() {
+        for (glob, prefix) in [
+            ("config/teams/**/*.yml", "config/teams"),
+            ("config/teams/*.yml", "config/teams"),
+            ("config/{teams,groups}/*.yml", "config"),
+            ("**/teams/*.yml", ""),
+            ("config/teams/payroll.yml", "config/teams/payroll.yml"),
+        ] {
+            assert_eq!(literal_glob_prefix(glob), PathBuf::from(prefix), "{glob}");
+        }
+    }
+
     const OWNED_GLOB: &str = "{app,components,config,frontend,lib,packs,spec,danger,script}/**/*.{rb,arb,erb,rake,js,jsx,ts,tsx}";
 
     #[test]

src/project_builder.rs

@@ -1,4 +1,5 @@
 use std::{
+    collections::HashSet,
     fs::File,
     path::{Path, PathBuf},
     sync::{Arc, Mutex},
@@ -12,7 +13,7 @@ use tracing::instrument;
 use crate::{
     cache::Cache,
     config::Config,
-    path_utils::matches_globs,
+    path_utils::{literal_glob_prefix, matches_globs},
     project::{DirectoryCodeownersFile, Error, Package, PackageType, Project, ProjectFile, Team, VendoredGem, deserializers},
     project_file_builder::ProjectFileBuilder,
     tracked_files,
@@ -77,6 +78,11 @@ impl<'a> ProjectBuilder<'a> {
         let team_file_glob = self.config.team_file_glob.clone();
         let base_path = self.base_path.clone();
         let tracked_files = tracked_files::find_tracked_files(&self.base_path);
+        // git's ignore rules are off, so without this the walk reads every ignored directory, like build output or worktrees.
+        let tracked_directories = tracked_files
+            .as_ref()
+            .map(|files| tracked_files::tracked_directories(&base_path, files));
+        let team_file_roots: Vec<PathBuf> = team_file_glob.iter().map(|glob| literal_glob_prefix(glob)).collect();
 
         builder.filter_entry(move |entry: &DirEntry| {
             let path = entry.path();
@@ -101,6 +107,14 @@ impl<'a> ProjectBuilder<'a> {
             {
                 return false;
             }
+            if let Some(tracked_directories) = &tracked_directories
+                && let Some(ft) = entry.file_type()
+                && ft.is_dir()
+                && let Ok(rel) = path.strip_prefix(&base_path)
+                && !walk_enters(rel, tracked_directories, &team_file_roots)
+            {
+                return false;
+            }
             if let Some(ft) = entry.file_type()
                 && ft.is_dir()
                 && let Ok(rel) = path.strip_prefix(&base_path)
@@ -332,6 +346,15 @@ impl<'a> ProjectBuilder<'a> {
     }
 }
 
+/// Whether the walk has to enter `relative_directory`. Only a directory that holds a tracked file can hold a file the project
+/// includes, apart from team files, which are included untracked.
+fn walk_enters(relative_directory: &Path, tracked_directories: &HashSet<PathBuf>, team_file_roots: &[PathBuf]) -> bool {
+    tracked_directories.contains(relative_directory)
+        || team_file_roots
+            .iter()
+            .any(|root| relative_directory.starts_with(root) || root.starts_with(relative_directory))
+}
+
 pub(crate) fn ruby_package_owner(path: &Path, node_limit: &NodeLimit) -> Result<Option<String>, Report<Error>> {
     let bytes = std::fs::read(path).change_context(Error::Io)?;
     // An empty, comment-only or `---`-only file is a null document, which serde-saphyr 2.1 no longer reads as a struct.
@@ -369,6 +392,35 @@ mod tests {
 
     use super::*;
 
+    #[test]
+    fn test_walk_enters() {
+        let tracked_directories: HashSet<PathBuf> = ["", "app", "app/models"].iter().map(PathBuf::from).collect();
+        let team_file_roots = vec![PathBuf::from("config/teams")];
+        for (directory, entered) in [
+            ("", true),
+            ("app", true),
+            ("app/models", true),
+            ("app/assets", false),
+            ("node_modules", false),
+            ("config", true),
+            ("config/teams", true),
+            ("config/teams/new_area", true),
+            ("config/initializers", false),
+        ] {
+            assert_eq!(
+                walk_enters(Path::new(directory), &tracked_directories, &team_file_roots),
+                entered,
+                "{directory}"
+            );
+        }
+    }
+
+    #[test]
+    fn test_walk_enters_every_directory_when_a_team_file_glob_starts_with_a_wildcard() {
+        let tracked_directories = HashSet::from([PathBuf::new()]);
+        assert!(walk_enters(Path::new("node_modules"), &tracked_directories, &[PathBuf::new()]));
+    }
+
     #[test]
     fn test_ruby_package_owner_top_level() {
         let yaml = "owner: TeamA\n";

src/tracked_files.rs

@@ -1,5 +1,5 @@
 use std::{
-    collections::HashMap,
+    collections::{HashMap, HashSet},
     path::{Path, PathBuf},
     process::Command,
 };
@@ -25,6 +25,21 @@ pub(crate) fn find_tracked_files(base_path: &Path) -> Option<HashMap<PathBuf, bo
     Some(results)
 }
 
+/// Every tracked path and every directory above one, relative to `base_path`. A tracked path is itself a directory when it's a
+/// submodule, which `git ls-files` lists without its contents. The root is the empty path, and is always included.
+pub(crate) fn tracked_directories(base_path: &Path, tracked_files: &HashMap<PathBuf, bool>) -> HashSet<PathBuf> {
+    let mut directories = HashSet::from([PathBuf::new()]);
+    for relative_file in tracked_files.keys().filter_map(|file| file.strip_prefix(base_path).ok()) {
+        for directory in relative_file.ancestors() {
+            // Its ancestors were added along with it.
+            if !directories.insert(directory.to_path_buf()) {
+                break;
+            }
+        }
+    }
+    directories
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -87,4 +102,25 @@ mod tests {
         assert!(tracked.get(&tracked_file).unwrap());
         assert!(!tracked.contains_key(&backend_dir.join("backend/app/models/foo.rb")));
     }
+
+    #[test]
+    fn test_tracked_directories() {
+        let base_path = Path::new("/project");
+        let tracked: HashMap<PathBuf, bool> = ["app/models/user.rb", "app/models/concerns/named.rb", "vendored/submodule_gem"]
+            .iter()
+            .map(|file| (base_path.join(file), true))
+            .collect();
+
+        let directories = tracked_directories(base_path, &tracked);
+        for directory in ["", "app", "app/models", "app/models/concerns", "vendored", "vendored/submodule_gem"] {
+            assert!(directories.contains(Path::new(directory)), "{directory}");
+        }
+        assert!(!directories.contains(Path::new("app/assets")));
+    }
+
+    #[test]
+    fn test_tracked_directories_always_include_the_root() {
+        let expected: HashSet<PathBuf> = HashSet::from([PathBuf::new()]);
+        assert_eq!(tracked_directories(Path::new("/project"), &HashMap::new()), expected);
+    }
 }

tests/untracked_directory_test.rs

@@ -0,0 +1,85 @@
+use assert_cmd::prelude::*;
+use std::{error::Error, fs, path::Path, process::Command};
+
+mod common;
+use common::{git_add_all_files, setup_fixture_repo};
+
+const FIXTURE: &str = "tests/fixtures/valid_project";
+
+fn generate(project_root: &Path) -> Result<String, Box<dyn Error>> {
+    let codeowners_path = project_root.join("tmp/CODEOWNERS");
+    fs::create_dir_all(codeowners_path.parent().unwrap())?;
+    Command::cargo_bin("codeowners")?
+        .arg("--project-root")
+        .arg(project_root)
+        .arg("--codeowners-file-path")
+        .arg(&codeowners_path)
+        .arg("--no-cache")
+        .arg("generate")
+        .arg("--skip-stage")
+        .assert()
+        .success();
+    Ok(fs::read_to_string(&codeowners_path)?)
+}
+
+/// The walk doesn't enter a directory that holds no tracked file, so a vendored gem that exists only in one checkout doesn't
+/// change the CODEOWNERS file that checkout generates.
+#[test]
+fn test_generate_leaves_out_a_vendored_gem_with_no_tracked_files() -> Result<(), Box<dyn Error>> {
+    let temp_dir = setup_fixture_repo(Path::new(FIXTURE));
+    let project_root = temp_dir.path();
+    git_add_all_files(project_root);
+    let untrack = Command::new("git")
+        .args(["rm", "-r", "-q", "--cached", "gems/pets"])
+        .current_dir(project_root)
+        .output()?;
+    assert!(untrack.status.success(), "{}", String::from_utf8_lossy(&untrack.stderr));
+
+    let codeowners = generate(project_root)?;
+    assert!(codeowners.contains("/gems/payroll_calculator/**/** @PayrollTeam"), "{codeowners}");
+    assert!(!codeowners.contains("/gems/pets/"), "{codeowners}");
+    Ok(())
+}
+
+/// `git ls-files` lists a submodule as one path, without its contents, so a vendored gem that's a submodule is tracked too.
+#[test]
+fn test_generate_keeps_a_vendored_gem_that_is_a_submodule() -> Result<(), Box<dyn Error>> {
+    let temp_dir = setup_fixture_repo(Path::new(FIXTURE));
+    let project_root = temp_dir.path();
+    git_add_all_files(project_root);
+    for args in [
+        vec!["rm", "-r", "-q", "--cached", "gems/pets"],
+        vec![
+            "update-index",
+            "--add",
+            "--cacheinfo",
+            "160000,0123456789abcdef0123456789abcdef01234567,gems/pets",
+        ],
+    ] {
+        let output = Command::new("git").args(&args).current_dir(project_root).output()?;
+        assert!(output.status.success(), "{}", String::from_utf8_lossy(&output.stderr));
+    }
+
+    let codeowners = generate(project_root)?;
+    assert!(codeowners.contains("/gems/pets/**/** @UX"), "{codeowners}");
+    Ok(())
+}
+
+/// Team files count before they're tracked (rubyatscale/code_ownership#149), so the walk still enters an untracked directory
+/// that the team file glob can match in.
+#[test]
+fn test_generate_reads_an_untracked_team_file_in_a_new_directory() -> Result<(), Box<dyn Error>> {
+    let temp_dir = setup_fixture_repo(Path::new(FIXTURE));
+    let project_root = temp_dir.path();
+    git_add_all_files(project_root);
+    fs::create_dir_all(project_root.join("config/teams/docs"))?;
+    fs::write(
+        project_root.join("config/teams/docs/docs.yml"),
+        "name: Docs\ngithub:\n  team: '@DocsTeam'\nowned_globs:\n  - ruby/app/models/bank_account.rb\n",
+    )?;
+
+    let codeowners = generate(project_root)?;
+    assert!(codeowners.contains("/config/teams/docs/docs.yml @DocsTeam"), "{codeowners}");
+    assert!(codeowners.contains("/ruby/app/models/bank_account.rb @DocsTeam"), "{codeowners}");
+    Ok(())
+}