From ab1f5133286d4158effa557d371d891b0bb80b4e Mon Sep 17 00:00:00 2001 From: Perry Hertler Date: Sun, 2 Mar 2025 09:55:47 -0600 Subject: [PATCH 1/4] team from file --- src/cli.rs | 24 ++++++------ src/main.rs | 2 +- src/ownership.rs | 9 ++++- src/ownership/parser.rs | 56 +++++++++++++++++++++++++- src/runner.rs | 74 ++++++++++++++++++++++++----------- tests/invalid_project_test.rs | 2 + tests/valid_project_test.rs | 5 ++- 7 files changed, 135 insertions(+), 37 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 0023cc0..e0c854b 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,6 +1,6 @@ use clap::{Parser, Subcommand}; -use codeowners::runner::{Error as RunnerError, RunResult}; -use codeowners::runner::{RunConfig, Runner}; +use codeowners::runner::RunConfig; +use codeowners::runner::{self, Error as RunnerError, RunResult}; use error_stack::{Result, ResultExt}; use path_clean::PathClean; use std::path::{Path, PathBuf}; @@ -8,7 +8,11 @@ use std::path::{Path, PathBuf}; #[derive(Subcommand, Debug)] enum Command { #[clap(about = "Finds the owner of a given file.", visible_alias = "f")] - ForFile { name: String }, + ForFile { + #[arg(short, long, default_value = "false")] + verbose: bool, + name: String, + }, #[clap(about = "Finds code ownership information for a given team ", visible_alias = "t")] ForTeam { name: String }, @@ -90,15 +94,13 @@ pub fn cli() -> Result { no_cache: args.no_cache, }; - let runner = Runner::new(&run_config)?; - let runner_result = match args.command { - Command::Validate => runner.validate(), - Command::Generate => runner.generate(), - Command::GenerateAndValidate => runner.generate_and_validate(), - Command::ForFile { name } => runner.for_file(&name), - Command::ForTeam { name } => runner.for_team(&name), - Command::DeleteCache => runner.delete_cache(), + Command::Validate => runner::validate(&run_config, vec![]), + Command::Generate => runner::generate(&run_config), + Command::GenerateAndValidate => runner::generate_and_validate(&run_config, vec![]), + Command::ForFile { name, verbose } => runner::for_file(&run_config, &name, verbose), + Command::ForTeam { name } => runner::for_team(&run_config, &name), + Command::DeleteCache => runner::delete_cache(&run_config), }; Ok(runner_result) diff --git a/src/main.rs b/src/main.rs index 8b3dcf8..d34ab1e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -21,7 +21,7 @@ fn maybe_print_errors(result: RunResult) -> Result<(), RunnerError> { } if !result.io_errors.is_empty() || !result.validation_errors.is_empty() { for msg in result.io_errors { - println!("{}", msg); + eprintln!("{}", msg); } for msg in result.validation_errors { println!("{}", msg); diff --git a/src/ownership.rs b/src/ownership.rs index 2017ad5..12fc2d8 100644 --- a/src/ownership.rs +++ b/src/ownership.rs @@ -4,7 +4,8 @@ use mapper::{OwnerMatcher, Source, TeamName}; use std::{ error::Error, fmt::{self, Display}, - path::Path, + fs, + path::{Path, PathBuf}, sync::Arc, }; use tracing::{info, instrument}; @@ -178,6 +179,12 @@ impl Ownership { } } +pub fn fast_team_name_from_file_path(file_path: &str, code_owners_file_path: &PathBuf) -> Result, Box> { + let code_owners = fs::read_to_string(code_owners_file_path)?; + let team_name = parser::team_name_from_file_path(Path::new(file_path), &code_owners); + Ok(team_name) +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/ownership/parser.rs b/src/ownership/parser.rs index eb07163..dde0324 100644 --- a/src/ownership/parser.rs +++ b/src/ownership/parser.rs @@ -1,5 +1,32 @@ use crate::ownership::{FileGenerator, TeamOwnership}; -use std::error::Error; +use fast_glob::glob_match; +use std::{error::Error, path::Path}; + +pub fn team_name_from_file_path(file_path: &Path, codeowners_file: &str) -> Option { + let stripped_lines = stripped_lines_by_priority(codeowners_file); + let slash_prefixed = if file_path.starts_with("/") { + file_path.to_str().unwrap().to_string() + } else { + format!("/{}", file_path.to_str()?) + }; + for line in stripped_lines { + let (glob, team_name) = line.split_once(' ')?; + if glob_match(glob, &slash_prefixed) { + return Some(team_name.to_string()); + } + } + + None +} + +fn stripped_lines_by_priority(codeowners_file: &str) -> Vec { + codeowners_file + .lines() + .filter(|line| !line.trim().is_empty() && !line.trim().starts_with("#")) + .map(|line| line.trim().to_string()) + .rev() + .collect() +} pub fn parse_for_team(team_name: String, codeowners_file: &str) -> Result, Box> { let mut output = vec![]; @@ -212,4 +239,31 @@ mod tests { ); Ok(()) } + + #[test] + fn test_stripped_lines_by_priority() -> Result<(), Box> { + let codeownership_file = indoc! {" + # First Section + /path/to/owned @Foo + "}; + + let stripped_lines = stripped_lines_by_priority(codeownership_file); + assert_eq!(stripped_lines, vec!["/path/to/owned @Foo"]); + Ok(()) + } + + #[test] + fn test_stripped_lines_by_priority_with_multiple_sections() -> Result<(), Box> { + let codeownership_file = indoc! {" + # First Section + /path/to/owned @Foo + + # Second Section + /another/path/to/owned @Bar + "}; + + let stripped_lines = stripped_lines_by_priority(codeownership_file); + assert_eq!(stripped_lines, vec!["/another/path/to/owned @Bar", "/path/to/owned @Foo"]); + Ok(()) + } } diff --git a/src/runner.rs b/src/runner.rs index e1b975c..0f9e0ff 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -7,10 +7,57 @@ use serde::{Deserialize, Serialize}; use crate::{ cache::{Cache, Caching, file::GlobalCache, noop::NoopCache}, config::Config, - ownership::{FileOwner, Ownership}, + ownership::{FileOwner, Ownership, fast_team_name_from_file_path}, project_builder::ProjectBuilder, }; +#[derive(Debug, Default, Serialize, Deserialize)] +pub struct RunResult { + pub validation_errors: Vec, + pub io_errors: Vec, + pub info_messages: Vec, +} +#[derive(Debug, Clone)] +pub struct RunConfig { + pub project_root: PathBuf, + pub codeowners_file_path: PathBuf, + pub config_path: PathBuf, + pub no_cache: bool, +} + +pub struct Runner { + run_config: RunConfig, + ownership: Ownership, + cache: Cache, +} + +pub fn for_file(run_config: &RunConfig, file_path: &str, verbose: bool) -> RunResult { + dbg!(verbose); + if verbose { + run_with_runner(run_config, |runner| runner.for_file(file_path)) + } else { + let result = fast_team_name_from_file_path(file_path, &run_config.codeowners_file_path); + match result { + Ok(Some(team_name)) => RunResult { + info_messages: vec![format!("{}", team_name)], + ..Default::default() + }, + Ok(None) => RunResult { + info_messages: vec!["No team found".to_string()], + ..Default::default() + }, + Err(err) => RunResult { + io_errors: vec![err.to_string()], + ..Default::default() + }, + } + } +} + +pub fn for_team(run_config: &RunConfig, team_name: &str) -> RunResult { + run_with_runner(run_config, |runner| runner.for_team(team_name)) +} + pub fn validate(run_config: &RunConfig, _file_paths: Vec) -> RunResult { run_with_runner(run_config, |runner| runner.validate()) } @@ -29,7 +76,10 @@ pub fn delete_cache(run_config: &RunConfig) -> RunResult { pub type Runnable = fn(Runner) -> RunResult; -pub fn run_with_runner(run_config: &RunConfig, runnable: Runnable) -> RunResult { +pub fn run_with_runner(run_config: &RunConfig, runnable: F) -> RunResult +where + F: FnOnce(Runner) -> RunResult, +{ let runner = match Runner::new(run_config) { Ok(runner) => runner, Err(err) => { @@ -42,26 +92,6 @@ pub fn run_with_runner(run_config: &RunConfig, runnable: Runnable) -> RunResult runnable(runner) } -#[derive(Debug, Default, Serialize, Deserialize)] -pub struct RunResult { - pub validation_errors: Vec, - pub io_errors: Vec, - pub info_messages: Vec, -} -#[derive(Debug, Clone)] -pub struct RunConfig { - pub project_root: PathBuf, - pub codeowners_file_path: PathBuf, - pub config_path: PathBuf, - pub no_cache: bool, -} - -pub struct Runner { - run_config: RunConfig, - ownership: Ownership, - cache: Cache, -} - impl RunResult { pub fn has_errors(&self) -> bool { !self.validation_errors.is_empty() || !self.io_errors.is_empty() diff --git a/tests/invalid_project_test.rs b/tests/invalid_project_test.rs index 824fa63..15c5092 100644 --- a/tests/invalid_project_test.rs +++ b/tests/invalid_project_test.rs @@ -48,6 +48,7 @@ fn test_for_file() -> Result<(), Box> { .arg("tests/fixtures/invalid_project") .arg("--no-cache") .arg("for-file") + .arg("--verbose") .arg("ruby/app/models/blockchain.rb") .assert() .success() @@ -68,6 +69,7 @@ fn test_for_file_multiple_owners() -> Result<(), Box> { .arg("tests/fixtures/invalid_project") .arg("--no-cache") .arg("for-file") + .arg("--verbose") .arg("ruby/app/services/multi_owned.rb") .assert() .failure() diff --git a/tests/valid_project_test.rs b/tests/valid_project_test.rs index f4ba603..db1c33d 100644 --- a/tests/valid_project_test.rs +++ b/tests/valid_project_test.rs @@ -43,6 +43,7 @@ fn test_for_file() -> Result<(), Box> { .arg("tests/fixtures/valid_project") .arg("--no-cache") .arg("for-file") + .arg("--verbose") .arg("ruby/app/models/payroll.rb") .assert() .success() @@ -63,6 +64,7 @@ fn test_for_file_same_team_multiple_ownerships() -> Result<(), Box> { .arg("tests/fixtures/valid_project") .arg("--no-cache") .arg("for-file") + .arg("--verbose") .arg("javascript/packages/PayrollFlow/index.tsx") .assert() .success() @@ -84,6 +86,7 @@ fn test_for_file_with_2_ownerships() -> Result<(), Box> { .arg("tests/fixtures/valid_project") .arg("--no-cache") .arg("for-file") + .arg("--verbose") .arg("javascript/packages/PayrollFlow/index.tsx") .assert() .success() @@ -148,7 +151,7 @@ fn test_for_missing_team() -> Result<(), Box> { .arg("Nope") .assert() .failure() - .stdout(predicate::eq(indoc! {" + .stderr(predicate::eq(indoc! {" Team not found "})); From 9d68536be598e5db3f10fde272c000d246281b36 Mon Sep 17 00:00:00 2001 From: Perry Hertler Date: Mon, 3 Mar 2025 15:57:15 -0600 Subject: [PATCH 2/4] only build teams by name once --- src/ownership/mapper/directory_mapper.rs | 2 +- src/ownership/mapper/package_mapper.rs | 4 ++-- src/ownership/mapper/team_file_mapper.rs | 4 ++-- src/project.rs | 13 ++----------- src/project_builder.rs | 3 +++ 5 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/ownership/mapper/directory_mapper.rs b/src/ownership/mapper/directory_mapper.rs index 9b7ba0a..7254b4b 100644 --- a/src/ownership/mapper/directory_mapper.rs +++ b/src/ownership/mapper/directory_mapper.rs @@ -18,7 +18,7 @@ impl DirectoryMapper { impl Mapper for DirectoryMapper { fn entries(&self) -> Vec { let mut entries: Vec = Vec::new(); - let team_by_name = self.project.team_by_name(); + let team_by_name = self.project.teams_by_name.clone(); for directory_codeowner_file in &self.project.directory_codeowner_files { let dir_root = directory_codeowner_file.directory_root().to_string_lossy(); diff --git a/src/ownership/mapper/package_mapper.rs b/src/ownership/mapper/package_mapper.rs index 58d0db8..fcb67d3 100644 --- a/src/ownership/mapper/package_mapper.rs +++ b/src/ownership/mapper/package_mapper.rs @@ -66,7 +66,7 @@ impl PackageMapper { impl PackageMapper { fn entries(&self, package_type: &PackageType) -> Vec { let mut entries: Vec = Vec::new(); - let team_by_name = self.project.team_by_name(); + let team_by_name = self.project.teams_by_name.clone(); for package in self.project.packages.iter().filter(|package| &package.package_type == package_type) { let package_root = package.package_root().to_string_lossy(); @@ -87,7 +87,7 @@ impl PackageMapper { fn owner_matchers(&self, package_type: &PackageType) -> Vec { let mut owner_matchers: Vec = Vec::new(); - let team_by_name = self.project.team_by_name(); + let team_by_name = self.project.teams_by_name.clone(); let packages = &self.project.packages; let packages: Vec<&Package> = packages.iter().filter(|package| &package.package_type == package_type).collect(); diff --git a/src/ownership/mapper/team_file_mapper.rs b/src/ownership/mapper/team_file_mapper.rs index 96cf8d2..3bd8e77 100644 --- a/src/ownership/mapper/team_file_mapper.rs +++ b/src/ownership/mapper/team_file_mapper.rs @@ -21,7 +21,7 @@ impl TeamFileMapper { impl Mapper for TeamFileMapper { fn entries(&self) -> Vec { let mut entries: Vec = Vec::new(); - let team_by_name = self.project.team_by_name(); + let team_by_name = self.project.teams_by_name.clone(); for owned_file in &self.project.files { if let Some(ref owner) = owned_file.owner { @@ -44,7 +44,7 @@ impl Mapper for TeamFileMapper { } fn owner_matchers(&self) -> Vec { - let team_by_name = self.project.team_by_name(); + let team_by_name = self.project.teams_by_name.clone(); let mut path_to_team: HashMap = HashMap::new(); diff --git a/src/project.rs b/src/project.rs index 0ef1728..61e1b89 100644 --- a/src/project.rs +++ b/src/project.rs @@ -15,6 +15,7 @@ pub struct Project { pub teams: Vec, pub codeowners_file_path: PathBuf, pub directory_codeowner_files: Vec, + pub teams_by_name: HashMap, } #[derive(Clone, Debug)] @@ -162,17 +163,7 @@ impl Project { } pub fn get_team(&self, name: &str) -> Option { - self.team_by_name().get(name).cloned() - } - - pub fn team_by_name(&self) -> HashMap { - let mut result: HashMap = HashMap::new(); - - for team in &self.teams { - result.insert(team.name.clone(), team.clone()); - } - - result + self.teams_by_name.get(name).cloned() } pub fn vendored_gem_by_name(&self) -> HashMap { diff --git a/src/project_builder.rs b/src/project_builder.rs index 9d92ab8..e9139f6 100644 --- a/src/project_builder.rs +++ b/src/project_builder.rs @@ -177,6 +177,8 @@ impl<'a> ProjectBuilder<'a> { acc }, ); + let teams_by_name = teams.iter() + .map(|team| (team.name.clone(), team.clone())).collect(); Ok(Project { base_path: self.base_path.to_owned(), @@ -186,6 +188,7 @@ impl<'a> ProjectBuilder<'a> { packages, codeowners_file_path: self.codeowners_file_path.to_path_buf(), directory_codeowner_files: directory_codeowners, + teams_by_name, }) } } From f708aa9966aa594cff312944388dd627d91b7e57 Mon Sep 17 00:00:00 2001 From: Perry Hertler Date: Mon, 3 Mar 2025 16:36:10 -0600 Subject: [PATCH 3/4] from_team_file_path for reuse --- src/config.rs | 2 +- src/ownership.rs | 5 +---- src/ownership/parser.rs | 40 +++++++++++++++++++++++++++++----------- src/project.rs | 16 ++++++++++++++++ src/project_builder.rs | 15 +++------------ src/runner.rs | 19 ++++++++----------- 6 files changed, 58 insertions(+), 39 deletions(-) diff --git a/src/config.rs b/src/config.rs index 918a152..ae500a5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,6 +1,6 @@ use serde::Deserialize; -#[derive(Deserialize, Debug)] +#[derive(Deserialize, Debug, Clone)] pub struct Config { pub owned_globs: Vec, diff --git a/src/ownership.rs b/src/ownership.rs index 12fc2d8..4c1bcbc 100644 --- a/src/ownership.rs +++ b/src/ownership.rs @@ -4,7 +4,6 @@ use mapper::{OwnerMatcher, Source, TeamName}; use std::{ error::Error, fmt::{self, Display}, - fs, path::{Path, PathBuf}, sync::Arc, }; @@ -180,9 +179,7 @@ impl Ownership { } pub fn fast_team_name_from_file_path(file_path: &str, code_owners_file_path: &PathBuf) -> Result, Box> { - let code_owners = fs::read_to_string(code_owners_file_path)?; - let team_name = parser::team_name_from_file_path(Path::new(file_path), &code_owners); - Ok(team_name) + parser::team_name_from_file_path(Path::new(file_path), code_owners_file_path) } #[cfg(test)] diff --git a/src/ownership/parser.rs b/src/ownership/parser.rs index dde0324..e2f920d 100644 --- a/src/ownership/parser.rs +++ b/src/ownership/parser.rs @@ -1,22 +1,40 @@ use crate::ownership::{FileGenerator, TeamOwnership}; use fast_glob::glob_match; -use std::{error::Error, path::Path}; - -pub fn team_name_from_file_path(file_path: &Path, codeowners_file: &str) -> Option { - let stripped_lines = stripped_lines_by_priority(codeowners_file); - let slash_prefixed = if file_path.starts_with("/") { - file_path.to_str().unwrap().to_string() +use std::{ + error::Error, + fs, + io::Error as IoError, + path::{Path, PathBuf}, +}; + +pub fn team_name_from_file_path(file_path: &Path, codeowners_file_path: &PathBuf) -> Result, Box> { + let file_path_str = file_path + .to_str() + .ok_or(IoError::new(std::io::ErrorKind::InvalidInput, "Invalid file path"))?; + let slash_prefixed = if file_path_str.starts_with("/") { + file_path_str.to_string() } else { - format!("/{}", file_path.to_str()?) + format!("/{}", file_path_str) }; - for line in stripped_lines { - let (glob, team_name) = line.split_once(' ')?; + + let codeowners_lines_in_priorty = build_codeowners_lines_in_priority(codeowners_file_path)?; + + for line in codeowners_lines_in_priorty { + let (glob, team_name) = line + .split_once(' ') + .ok_or(IoError::new(std::io::ErrorKind::InvalidInput, "Invalid line"))?; if glob_match(glob, &slash_prefixed) { - return Some(team_name.to_string()); + return Ok(Some(team_name.to_string())); } } - None + Ok(None) +} + +fn build_codeowners_lines_in_priority(codeowners_file_path: &PathBuf) -> Result, Box> { + let codeowners_file = fs::read_to_string(codeowners_file_path)?; + let stripped_lines = stripped_lines_by_priority(&codeowners_file); + Ok(stripped_lines) } fn stripped_lines_by_priority(codeowners_file: &str) -> Vec { diff --git a/src/project.rs b/src/project.rs index 61e1b89..f7259f7 100644 --- a/src/project.rs +++ b/src/project.rs @@ -2,6 +2,7 @@ use core::fmt; use std::{ collections::HashMap, fmt::Display, + fs::File, path::{Path, PathBuf}, }; @@ -40,6 +41,21 @@ pub struct Team { pub avoid_ownership: bool, } +impl Team { + pub fn from_team_file_path(absolute_path: PathBuf) -> Result { + let file = File::open(&absolute_path).change_context(Error::Io)?; + let deserializer: deserializers::Team = serde_yaml::from_reader(file).change_context(Error::SerdeYaml)?; + Ok(Self { + path: absolute_path.to_owned(), + name: deserializer.name, + github_team: deserializer.github.team, + owned_globs: deserializer.owned_globs, + owned_gems: deserializer.ruby.map(|ruby| ruby.owned_gems).unwrap_or_default(), + avoid_ownership: deserializer.github.do_not_add_to_codeowners_file, + }) + } +} + #[derive(Clone, Debug)] pub struct Package { pub path: PathBuf, diff --git a/src/project_builder.rs b/src/project_builder.rs index e9139f6..396ab29 100644 --- a/src/project_builder.rs +++ b/src/project_builder.rs @@ -150,16 +150,8 @@ impl<'a> ProjectBuilder<'a> { }); } EntryType::TeamFile(absolute_path, _relative_path) => { - let file = File::open(&absolute_path).unwrap(); - let deserializer: deserializers::Team = serde_yaml::from_reader(file).unwrap(); - team_files.push(Team { - path: absolute_path.to_owned(), - name: deserializer.name, - github_team: deserializer.github.team, - owned_globs: deserializer.owned_globs, - owned_gems: deserializer.ruby.map(|ruby| ruby.owned_gems).unwrap_or_default(), - avoid_ownership: deserializer.github.do_not_add_to_codeowners_file, - }); + let team = Team::from_team_file_path(absolute_path).unwrap(); + team_files.push(team); } EntryType::NullEntry() => {} } @@ -177,8 +169,7 @@ impl<'a> ProjectBuilder<'a> { acc }, ); - let teams_by_name = teams.iter() - .map(|team| (team.name.clone(), team.clone())).collect(); + let teams_by_name = teams.iter().map(|team| (team.name.clone(), team.clone())).collect(); Ok(Project { base_path: self.base_path.to_owned(), diff --git a/src/runner.rs b/src/runner.rs index 0f9e0ff..00bb91d 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -32,7 +32,6 @@ pub struct Runner { } pub fn for_file(run_config: &RunConfig, file_path: &str, verbose: bool) -> RunResult { - dbg!(verbose); if verbose { run_with_runner(run_config, |runner| runner.for_file(file_path)) } else { @@ -114,19 +113,17 @@ impl fmt::Display for Error { } } +fn config_from_path(path: &PathBuf) -> Result { + let config_file = File::open(path) + .change_context(Error::Io(format!("Can't open config file: {}", &path.to_string_lossy()))) + .attach_printable(format!("Can't open config file: {}", &path.to_string_lossy()))?; + + serde_yaml::from_reader(config_file).change_context(Error::Io(format!("Can't parse config file: {}", &path.to_string_lossy()))) +} impl Runner { pub fn new(run_config: &RunConfig) -> Result { - let config_file = File::open(&run_config.config_path) - .change_context(Error::Io(format!( - "Can't open config file: {}", - &run_config.config_path.to_string_lossy() - ))) - .attach_printable(format!("Can't open config file: {}", &run_config.config_path.to_string_lossy()))?; + let config = config_from_path(&run_config.config_path)?; - let config: Config = serde_yaml::from_reader(config_file).change_context(Error::Io(format!( - "Can't parse config file: {}", - &run_config.config_path.to_string_lossy() - )))?; let cache: Cache = if run_config.no_cache { NoopCache::default().into() } else { From bee9414ac71bb3ceb91be341e43fd5493463ba02 Mon Sep 17 00:00:00 2001 From: Perry Hertler Date: Mon, 3 Mar 2025 17:23:26 -0600 Subject: [PATCH 4/4] memoize codeowners --- Cargo.lock | 93 ++++++++++++++++++++++++++++++--- Cargo.toml | 3 +- src/cli.rs | 11 ++-- src/config.rs | 61 +++++++++++++++++++++- src/ownership.rs | 8 +-- src/ownership/parser.rs | 96 +++++++++++++++++++++++++++-------- src/runner.rs | 63 ++++++++++++++++------- tests/invalid_project_test.rs | 20 +++++++- tests/valid_project_test.rs | 39 ++++++++++++-- 9 files changed, 331 insertions(+), 63 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0255ccc..928d46d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,17 @@ # It is not intended for manual editing. version = 4 +[[package]] +name = "ahash" +version = "0.7.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "891477e0c6a8957309ee5c45a6368af3ae14bb510732d2684ffa19af310920f9" +dependencies = [ + "getrandom", + "once_cell", + "version_check", +] + [[package]] name = "aho-corasick" version = "1.1.3" @@ -151,7 +162,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn", + "syn 2.0.87", ] [[package]] @@ -175,6 +186,7 @@ dependencies = [ "indoc", "itertools", "lazy_static", + "memoize", "path-clean", "predicates", "pretty_assertions", @@ -261,7 +273,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn", + "syn 2.0.87", ] [[package]] @@ -336,11 +348,22 @@ dependencies = [ "unicode-width", ] +[[package]] +name = "getrandom" +version = "0.2.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c4567c8db10ae91089c99af84c68c38da3ec2f087c3f82960bcdbf3656b6f4d7" +dependencies = [ + "cfg-if", + "libc", + "wasi", +] + [[package]] name = "glob" -version = "0.3.1" +version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d2fabcfbdc87f4758337ca535fb41a6d701b65693ce38287d856d1674551ec9b" +checksum = "a8d1add55171497b4705a648c6b583acafb01d58050a51727785f0b2c8e0a2b2" [[package]] name = "globset" @@ -360,6 +383,9 @@ name = "hashbrown" version = "0.12.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" +dependencies = [ + "ahash", +] [[package]] name = "hashbrown" @@ -460,6 +486,15 @@ version = "0.4.22" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a7a70ba024b9dc04c27ea2f0c0548feb474ec5c54bba33a7f72f873a39d07b24" +[[package]] +name = "lru" +version = "0.7.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e999beba7b6e8345721bd280141ed958096a2e4abdf74f67ff4ce49b4b54e47a" +dependencies = [ + "hashbrown 0.12.3", +] + [[package]] name = "matchers" version = "0.1.0" @@ -484,6 +519,29 @@ dependencies = [ "autocfg", ] +[[package]] +name = "memoize" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8d1d5792299bab3f8b5d88d1b7a7cb50ad7ef039a8c4d45a6b84880a6526276" +dependencies = [ + "lazy_static", + "lru", + "memoize-inner", +] + +[[package]] +name = "memoize-inner" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6dd8f89255d8ff313afabed9a3c83ef0993cc056679dfd001f5111a026f876f7" +dependencies = [ + "lazy_static", + "proc-macro2", + "quote", + "syn 1.0.109", +] + [[package]] name = "nias" version = "0.5.0" @@ -739,7 +797,7 @@ checksum = "de523f781f095e28fa605cdce0f8307e451cc0fd14e2eb4cd2e98a355b147766" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.87", ] [[package]] @@ -788,6 +846,17 @@ version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" +[[package]] +name = "syn" +version = "1.0.109" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72b64191b275b66ffe2469e8af2c1cfe3bafa67b529ead792a6d0160888b4237" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "syn" version = "2.0.87" @@ -856,7 +925,7 @@ checksum = "34704c8d6ebcbc939824180af020566b01a7c01f80641264eba0999f6c2b6be7" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.87", ] [[package]] @@ -928,6 +997,12 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "830b7e5d4d90034032940e4ace0d9a9a057e7a45cd94e6c007832e39edb82f6d" +[[package]] +name = "version_check" +version = "0.9.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" + [[package]] name = "wait-timeout" version = "0.2.0" @@ -947,6 +1022,12 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "wasi" +version = "0.11.0+wasi-snapshot-preview1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" + [[package]] name = "winapi" version = "0.3.9" diff --git a/Cargo.toml b/Cargo.toml index 68f8bfd..3f77a40 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,9 +15,11 @@ clap_derive = "4.5.18" error-stack = "0.5.0" enum_dispatch = "0.3.13" fast-glob = "0.4.0" +glob = "0.3.2" ignore = "0.4.23" itertools = "0.14.0" lazy_static = "1.5.0" +memoize = "0.5.1" path-clean = "1.0.1" rayon = "1.10.0" regex = "1.11.1" @@ -30,7 +32,6 @@ tracing-subscriber = { version = "0.3.18", features = ["env-filter"] } [dev-dependencies] assert_cmd = "2.0.16" -glob = "0.3.1" rusty-hook = "^0.11.2" predicates = "3.1.2" pretty_assertions = "1.4.1" # Shows a more readable diff when comparing objects diff --git a/src/cli.rs b/src/cli.rs index e0c854b..1f94124 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -9,8 +9,13 @@ use std::path::{Path, PathBuf}; enum Command { #[clap(about = "Finds the owner of a given file.", visible_alias = "f")] ForFile { - #[arg(short, long, default_value = "false")] - verbose: bool, + #[arg( + short, + long, + default_value = "false", + help = "Find the owner from the CODEOWNERS file and just return the team name and yml path" + )] + fast: bool, name: String, }, @@ -98,7 +103,7 @@ pub fn cli() -> Result { Command::Validate => runner::validate(&run_config, vec![]), Command::Generate => runner::generate(&run_config), Command::GenerateAndValidate => runner::generate_and_validate(&run_config, vec![]), - Command::ForFile { name, verbose } => runner::for_file(&run_config, &name, verbose), + Command::ForFile { name, fast } => runner::for_file(&run_config, &name, fast), Command::ForTeam { name } => runner::for_team(&run_config, &name), Command::DeleteCache => runner::delete_cache(&run_config), }; diff --git a/src/config.rs b/src/config.rs index ae500a5..abb54c6 100644 --- a/src/config.rs +++ b/src/config.rs @@ -7,14 +7,16 @@ pub struct Config { #[serde(default = "ruby_package_paths")] pub ruby_package_paths: Vec, - #[serde(alias = "js_package_paths")] + #[serde(alias = "js_package_paths", default = "javascript_package_paths")] pub javascript_package_paths: Vec, #[serde(default = "team_file_glob")] pub team_file_glob: Vec, + + #[serde(default = "unowned_globs")] pub unowned_globs: Vec, - #[serde(alias = "unbuilt_gems_path")] + #[serde(alias = "unbuilt_gems_path", default = "vendored_gems_path")] pub vendored_gems_path: String, #[serde(default = "default_cache_directory")] @@ -39,3 +41,58 @@ fn team_file_glob() -> Vec { fn default_cache_directory() -> String { String::from("tmp/cache/codeowners") } + +fn javascript_package_paths() -> Vec { + vec!["frontend/**/*".to_owned()] +} + +fn unowned_globs() -> Vec { + vec![ + "frontend/**/node_modules/**/*".to_owned(), + "frontend/**/__generated__/**/*".to_owned(), + ] +} + +fn vendored_gems_path() -> String { + "vendored/".to_string() +} + +#[cfg(test)] +mod tests { + use std::{ + error::Error, + fs::{self, File}, + }; + + use indoc::indoc; + use tempfile::tempdir; + + use super::*; + + #[test] + fn test_parse_config() -> Result<(), Box> { + let temp_dir = tempdir()?; + let config_path = temp_dir.path().join("config.yml"); + let config_str = indoc! {" + --- + owned_globs: + - \"{app,components,config,frontend,lib,packs,spec}/**/*.{rb,rake,js,jsx,ts,tsx,json,yml}\" + "}; + fs::write(&config_path, config_str)?; + let config_file = File::open(&config_path)?; + let config: Config = serde_yaml::from_reader(config_file)?; + assert_eq!( + config.owned_globs, + vec!["{app,components,config,frontend,lib,packs,spec}/**/*.{rb,rake,js,jsx,ts,tsx,json,yml}"] + ); + assert_eq!(config.ruby_package_paths, vec!["packs/**/*", "components/**"]); + assert_eq!(config.javascript_package_paths, vec!["frontend/**/*"]); + assert_eq!(config.team_file_glob, vec!["config/teams/**/*.yml"]); + assert_eq!( + config.unowned_globs, + vec!["frontend/**/node_modules/**/*", "frontend/**/__generated__/**/*"] + ); + assert_eq!(config.vendored_gems_path, "vendored/"); + Ok(()) + } +} diff --git a/src/ownership.rs b/src/ownership.rs index 4c1bcbc..aede707 100644 --- a/src/ownership.rs +++ b/src/ownership.rs @@ -4,7 +4,7 @@ use mapper::{OwnerMatcher, Source, TeamName}; use std::{ error::Error, fmt::{self, Display}, - path::{Path, PathBuf}, + path::Path, sync::Arc, }; use tracing::{info, instrument}; @@ -12,7 +12,7 @@ use tracing::{info, instrument}; mod file_generator; mod file_owner_finder; pub(crate) mod mapper; -mod parser; +pub(crate) mod parser; mod validator; use crate::{ @@ -178,10 +178,6 @@ impl Ownership { } } -pub fn fast_team_name_from_file_path(file_path: &str, code_owners_file_path: &PathBuf) -> Result, Box> { - parser::team_name_from_file_path(Path::new(file_path), code_owners_file_path) -} - #[cfg(test)] mod tests { use super::*; diff --git a/src/ownership/parser.rs b/src/ownership/parser.rs index e2f920d..57a3ae9 100644 --- a/src/ownership/parser.rs +++ b/src/ownership/parser.rs @@ -1,40 +1,92 @@ -use crate::ownership::{FileGenerator, TeamOwnership}; +use crate::{ + ownership::{FileGenerator, TeamOwnership}, + project::Team, +}; use fast_glob::glob_match; +use memoize::memoize; use std::{ + collections::HashMap, error::Error, fs, io::Error as IoError, path::{Path, PathBuf}, }; -pub fn team_name_from_file_path(file_path: &Path, codeowners_file_path: &PathBuf) -> Result, Box> { - let file_path_str = file_path - .to_str() - .ok_or(IoError::new(std::io::ErrorKind::InvalidInput, "Invalid file path"))?; - let slash_prefixed = if file_path_str.starts_with("/") { - file_path_str.to_string() - } else { - format!("/{}", file_path_str) - }; +pub struct Parser { + pub project_root: PathBuf, + pub codeowners_file_path: PathBuf, + pub team_file_globs: Vec, +} - let codeowners_lines_in_priorty = build_codeowners_lines_in_priority(codeowners_file_path)?; +impl Parser { + pub fn team_from_file_path(&self, file_path: &Path) -> Result, Box> { + let file_path_str = file_path + .to_str() + .ok_or(IoError::new(std::io::ErrorKind::InvalidInput, "Invalid file path"))?; + let slash_prefixed = if file_path_str.starts_with("/") { + file_path_str.to_string() + } else { + format!("/{}", file_path_str) + }; + + let codeowners_lines_in_priorty = build_codeowners_lines_in_priority(self.codeowners_file_path.to_string_lossy().into_owned()); + for line in codeowners_lines_in_priorty { + let (glob, team_name) = line + .split_once(' ') + .ok_or(IoError::new(std::io::ErrorKind::InvalidInput, "Invalid line"))?; + if glob_match(glob, &slash_prefixed) { + let tbn = teams_by_github_team_name(self.absolute_team_files_globs()); + let team: Option = tbn.get(team_name.to_string().as_str()).cloned(); + return Ok(team); + } + } + + Ok(None) + } - for line in codeowners_lines_in_priorty { - let (glob, team_name) = line - .split_once(' ') - .ok_or(IoError::new(std::io::ErrorKind::InvalidInput, "Invalid line"))?; - if glob_match(glob, &slash_prefixed) { - return Ok(Some(team_name.to_string())); + fn absolute_team_files_globs(&self) -> Vec { + self.team_file_globs + .iter() + .map(|glob| format!("{}/{}", self.project_root.display(), glob)) + .collect() + } +} + +#[memoize] +fn teams_by_github_team_name(team_file_glob: Vec) -> HashMap { + dbg!("in teams_by_github_team_name"); + let mut teams = HashMap::new(); + for glob in team_file_glob { + let paths = glob::glob(&glob).expect("Failed to read glob pattern").filter_map(Result::ok); + + for path in paths { + let team = match Team::from_team_file_path(path) { + Ok(team) => team, + Err(e) => { + eprintln!("Error parsing team file: {}", e); + continue; + } + }; + teams.insert(team.github_team.clone(), team); } } - Ok(None) + teams } -fn build_codeowners_lines_in_priority(codeowners_file_path: &PathBuf) -> Result, Box> { - let codeowners_file = fs::read_to_string(codeowners_file_path)?; - let stripped_lines = stripped_lines_by_priority(&codeowners_file); - Ok(stripped_lines) +#[memoize] +fn build_codeowners_lines_in_priority(codeowners_file_path: String) -> Vec { + dbg!("in build_codeowners_lines_in_priority"); + dbg!(&codeowners_file_path); + let codeowners_file = match fs::read_to_string(codeowners_file_path) { + Ok(codeowners_file) => codeowners_file, + Err(e) => { + // we can't return the error because it's not clonable + eprintln!("Error reading codeowners file: {}", e); + return vec![]; + } + }; + stripped_lines_by_priority(&codeowners_file) } fn stripped_lines_by_priority(codeowners_file: &str) -> Vec { diff --git a/src/runner.rs b/src/runner.rs index 00bb91d..25a3518 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -1,5 +1,8 @@ use core::fmt; -use std::{fs::File, path::PathBuf}; +use std::{ + fs::File, + path::{Path, PathBuf}, +}; use error_stack::{Context, Result, ResultExt}; use serde::{Deserialize, Serialize}; @@ -7,7 +10,8 @@ use serde::{Deserialize, Serialize}; use crate::{ cache::{Cache, Caching, file::GlobalCache, noop::NoopCache}, config::Config, - ownership::{FileOwner, Ownership, fast_team_name_from_file_path}, + ownership::{FileOwner, Ownership}, + project::Team, project_builder::ProjectBuilder, }; @@ -31,28 +35,51 @@ pub struct Runner { cache: Cache, } -pub fn for_file(run_config: &RunConfig, file_path: &str, verbose: bool) -> RunResult { - if verbose { - run_with_runner(run_config, |runner| runner.for_file(file_path)) +pub fn for_file(run_config: &RunConfig, file_path: &str, fast: bool) -> RunResult { + if fast { + for_file_from_codeowners(run_config, file_path) } else { - let result = fast_team_name_from_file_path(file_path, &run_config.codeowners_file_path); - match result { - Ok(Some(team_name)) => RunResult { - info_messages: vec![format!("{}", team_name)], - ..Default::default() - }, - Ok(None) => RunResult { - info_messages: vec!["No team found".to_string()], - ..Default::default() - }, - Err(err) => RunResult { - io_errors: vec![err.to_string()], + run_with_runner(run_config, |runner| runner.for_file(file_path)) + } +} + +fn for_file_from_codeowners(run_config: &RunConfig, file_path: &str) -> RunResult { + match team_for_file_from_codeowners(run_config, file_path) { + Ok(Some(team)) => { + let relative_team_yml_path = team.path.strip_prefix(&run_config.project_root).unwrap_or(&team.path); + + RunResult { + info_messages: vec![ + format!("Team: {}", team.name), + format!("Team YML: {}", relative_team_yml_path.display()), + ], ..Default::default() - }, + } } + Ok(None) => RunResult { + info_messages: vec!["Team: Unowned".to_string(), "Team YML:".to_string()], + ..Default::default() + }, + Err(err) => RunResult { + io_errors: vec![err.to_string()], + ..Default::default() + }, } } +pub fn team_for_file_from_codeowners(run_config: &RunConfig, file_path: &str) -> Result, Error> { + let config = config_from_path(&run_config.config_path)?; + + let parser = crate::ownership::parser::Parser { + project_root: run_config.project_root.clone(), + codeowners_file_path: run_config.codeowners_file_path.clone(), + team_file_globs: config.team_file_glob.clone(), + }; + Ok(parser + .team_from_file_path(Path::new(file_path)) + .map_err(|e| Error::Io(e.to_string()))?) +} + pub fn for_team(run_config: &RunConfig, team_name: &str) -> RunResult { run_with_runner(run_config, |runner| runner.for_team(team_name)) } diff --git a/tests/invalid_project_test.rs b/tests/invalid_project_test.rs index 15c5092..9ed2192 100644 --- a/tests/invalid_project_test.rs +++ b/tests/invalid_project_test.rs @@ -48,7 +48,6 @@ fn test_for_file() -> Result<(), Box> { .arg("tests/fixtures/invalid_project") .arg("--no-cache") .arg("for-file") - .arg("--verbose") .arg("ruby/app/models/blockchain.rb") .assert() .success() @@ -62,6 +61,24 @@ fn test_for_file() -> Result<(), Box> { Ok(()) } +#[test] +fn test_fast_for_file() -> Result<(), Box> { + Command::cargo_bin("codeowners")? + .arg("--project-root") + .arg("tests/fixtures/invalid_project") + .arg("--no-cache") + .arg("for-file") + .arg("--fast") + .arg("ruby/app/models/blockchain.rb") + .assert() + .success() + .stdout(predicate::eq(indoc! {" + Team: Unowned + Team YML: + "})); + Ok(()) +} + #[test] fn test_for_file_multiple_owners() -> Result<(), Box> { Command::cargo_bin("codeowners")? @@ -69,7 +86,6 @@ fn test_for_file_multiple_owners() -> Result<(), Box> { .arg("tests/fixtures/invalid_project") .arg("--no-cache") .arg("for-file") - .arg("--verbose") .arg("ruby/app/services/multi_owned.rb") .assert() .failure() diff --git a/tests/valid_project_test.rs b/tests/valid_project_test.rs index db1c33d..d72277f 100644 --- a/tests/valid_project_test.rs +++ b/tests/valid_project_test.rs @@ -43,7 +43,6 @@ fn test_for_file() -> Result<(), Box> { .arg("tests/fixtures/valid_project") .arg("--no-cache") .arg("for-file") - .arg("--verbose") .arg("ruby/app/models/payroll.rb") .assert() .success() @@ -57,6 +56,24 @@ fn test_for_file() -> Result<(), Box> { Ok(()) } +#[test] +fn test_fast_for_file() -> Result<(), Box> { + Command::cargo_bin("codeowners")? + .arg("--project-root") + .arg("tests/fixtures/valid_project") + .arg("--no-cache") + .arg("for-file") + .arg("--fast") + .arg("ruby/app/models/payroll.rb") + .assert() + .success() + .stdout(predicate::eq(indoc! {" + Team: Payroll + Team YML: config/teams/payroll.yml + "})); + Ok(()) +} + #[test] fn test_for_file_same_team_multiple_ownerships() -> Result<(), Box> { Command::cargo_bin("codeowners")? @@ -64,7 +81,6 @@ fn test_for_file_same_team_multiple_ownerships() -> Result<(), Box> { .arg("tests/fixtures/valid_project") .arg("--no-cache") .arg("for-file") - .arg("--verbose") .arg("javascript/packages/PayrollFlow/index.tsx") .assert() .success() @@ -79,6 +95,24 @@ fn test_for_file_same_team_multiple_ownerships() -> Result<(), Box> { Ok(()) } +#[test] +fn test_fast_for_file_same_team_multiple_ownerships() -> Result<(), Box> { + Command::cargo_bin("codeowners")? + .arg("--project-root") + .arg("tests/fixtures/valid_project") + .arg("--no-cache") + .arg("for-file") + .arg("--fast") + .arg("javascript/packages/PayrollFlow/index.tsx") + .assert() + .success() + .stdout(predicate::eq(indoc! {" + Team: Payroll + Team YML: config/teams/payroll.yml + "})); + Ok(()) +} + #[test] fn test_for_file_with_2_ownerships() -> Result<(), Box> { Command::cargo_bin("codeowners")? @@ -86,7 +120,6 @@ fn test_for_file_with_2_ownerships() -> Result<(), Box> { .arg("tests/fixtures/valid_project") .arg("--no-cache") .arg("for-file") - .arg("--verbose") .arg("javascript/packages/PayrollFlow/index.tsx") .assert() .success()