DRY

Author: perryqh

src/crosscheck.rs

@@ -3,7 +3,7 @@ use std::path::Path;
 use crate::{
     cache::Cache,
     config::Config,
-    ownership::for_file_fast::find_file_owners,
+    ownership::file_owner_resolver::find_file_owners,
     project::Project,
     project_builder::ProjectBuilder,
     runner::{RunConfig, RunResult, config_from_path, team_for_file_from_codeowners},

src/lib.rs

@@ -3,6 +3,7 @@ pub(crate) mod common_test;
 pub mod config;
 pub mod crosscheck;
 pub mod ownership;
+pub mod path_utils;
 pub(crate) mod project;
 pub mod project_builder;
 pub mod project_file_builder;

src/ownership.rs

@@ -13,7 +13,7 @@ pub(crate) mod codeowners_file_parser;
 pub(crate) mod codeowners_query;
 mod file_generator;
 mod file_owner_finder;
-pub mod for_file_fast;
+pub mod file_owner_resolver;
 pub(crate) mod mapper;
 mod validator;
 

src/ownership/codeowners_query.rs

@@ -11,7 +11,7 @@ pub(crate) fn team_for_file_from_codeowners(
     file_path: &Path,
 ) -> Result<Option<Team>, String> {
     let relative_file_path = if file_path.is_absolute() {
-        file_path.strip_prefix(project_root).unwrap_or(file_path).to_path_buf()
+        crate::path_utils::relative_to_buf(project_root, file_path)
     } else {
         PathBuf::from(file_path)
     };
@@ -36,7 +36,7 @@ pub(crate) fn teams_for_files_from_codeowners(
         .map(Path::new)
         .map(|path| {
             if path.is_absolute() {
-                path.strip_prefix(project_root).unwrap_or(path).to_path_buf()
+                crate::path_utils::relative_to_buf(project_root, path)
             } else {
                 path.to_path_buf()
             }

src/ownership/for_file_fast.rs src/ownership/file_owner_resolver.rssrc/ownership/for_file_fast.rs renamed to src/ownership/file_owner_resolver.rs

@@ -17,10 +17,7 @@ pub fn find_file_owners(project_root: &Path, config: &Config, file_path: &Path)
     } else {
         project_root.join(file_path)
     };
-    let relative_file_path = absolute_file_path
-        .strip_prefix(project_root)
-        .unwrap_or(&absolute_file_path)
-        .to_path_buf();
+    let relative_file_path = crate::path_utils::relative_to_buf(project_root, &absolute_file_path);
 
     let teams = load_teams(project_root, &config.team_file_glob)?;
     let teams_by_name = build_teams_by_name_map(&teams);
@@ -68,7 +65,7 @@ pub fn find_file_owners(project_root: &Path, config: &Config, file_path: &Path)
     }
 
     for team in &teams {
-        let team_rel = team.path.strip_prefix(project_root).unwrap_or(&team.path).to_path_buf();
+        let team_rel = crate::path_utils::relative_to_buf(project_root, &team.path);
         if team_rel == relative_file_path {
             sources_by_team.entry(team.name.clone()).or_default().push(Source::TeamYml);
         }
@@ -77,10 +74,7 @@ pub fn find_file_owners(project_root: &Path, config: &Config, file_path: &Path)
     let mut file_owners: Vec<FileOwner> = Vec::new();
     for (team_name, sources) in sources_by_team.into_iter() {
         if let Some(team) = teams_by_name.get(&team_name) {
-            let relative_team_yml_path = team
-                .path
-                .strip_prefix(project_root)
-                .unwrap_or(&team.path)
+            let relative_team_yml_path = crate::path_utils::relative_to(project_root, &team.path)
                 .to_string_lossy()
                 .to_string();
             file_owners.push(FileOwner {
@@ -157,9 +151,7 @@ fn most_specific_directory_owner(
         if let Ok(owner_str) = fs::read_to_string(&codeowner_path) {
             let owner = owner_str.trim();
             if let Some(team) = teams_by_name.get(owner) {
-                let relative_dir = current
-                    .strip_prefix(project_root)
-                    .unwrap_or(current.as_path())
+                let relative_dir = crate::path_utils::relative_to(project_root, current.as_path())
                     .to_string_lossy()
                     .to_string();
                 let candidate = (team.name.clone(), Source::Directory(relative_dir));
@@ -191,7 +183,7 @@ fn nearest_package_owner(
         if !current.pop() {
             break;
         }
-        let parent_rel = current.strip_prefix(project_root).unwrap_or(current.as_path());
+        let parent_rel = crate::path_utils::relative_to(project_root, current.as_path());
         if let Some(rel_str) = parent_rel.to_str() {
             if glob_list_matches(rel_str, &config.ruby_package_paths) {
                 let pkg_yml = current.join("package.yml");

src/path_utils.rs

@@ -0,0 +1,49 @@
+use std::path::{Path, PathBuf};
+
+/// Return `path` relative to `root` if possible; otherwise return `path` unchanged.
+pub fn relative_to<'a>(root: &'a Path, path: &'a Path) -> &'a Path {
+    path.strip_prefix(root).unwrap_or(path)
+}
+
+/// Like `relative_to`, but returns an owned `PathBuf`.
+pub fn relative_to_buf(root: &Path, path: &Path) -> PathBuf {
+    relative_to(root, path).to_path_buf()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn relative_to_returns_relative_when_under_root() {
+        let root = Path::new("/a/b");
+        let path = Path::new("/a/b/c/d.txt");
+        let rel = relative_to(root, path);
+        assert_eq!(rel, Path::new("c/d.txt"));
+    }
+
+    #[test]
+    fn relative_to_returns_input_when_not_under_root() {
+        let root = Path::new("/a/b");
+        let path = Path::new("/x/y/z.txt");
+        let rel = relative_to(root, path);
+        assert_eq!(rel, path);
+    }
+
+    #[test]
+    fn relative_to_handles_equal_paths() {
+        let root = Path::new("/a/b");
+        let path = Path::new("/a/b");
+        let rel = relative_to(root, path);
+        assert_eq!(rel, Path::new(""));
+    }
+
+    #[test]
+    fn relative_to_buf_matches_relative_to() {
+        let root = Path::new("/proj");
+        let path = Path::new("/proj/src/lib.rs");
+        let rel_ref = relative_to(root, path);
+        let rel_buf = relative_to_buf(root, path);
+        assert_eq!(rel_ref, rel_buf.as_path());
+    }
+}

src/runner.rs

@@ -176,7 +176,7 @@ impl Runner {
     }
 
     pub fn owners_for_file(&self, file_path: &str) -> Result<Vec<FileOwner>, Error> {
-        use crate::ownership::for_file_fast::find_file_owners;
+        use crate::ownership::file_owner_resolver::find_file_owners;
         let owners = find_file_owners(&self.run_config.project_root, &self.config, std::path::Path::new(file_path)).map_err(Error::Io)?;
         Ok(owners)
     }
@@ -215,10 +215,7 @@ impl Runner {
     pub fn for_file_codeowners_only(&self, file_path: &str) -> RunResult {
         match team_for_file_from_codeowners(&self.run_config, file_path) {
             Ok(Some(team)) => {
-                let relative_team_path = team
-                    .path
-                    .strip_prefix(&self.run_config.project_root)
-                    .unwrap_or(team.path.as_path())
+                let relative_team_path = crate::path_utils::relative_to(&self.run_config.project_root, team.path.as_path())
                     .to_string_lossy()
                     .to_string();
                 RunResult {

src/runner/api.rs

@@ -68,9 +68,7 @@ pub fn teams_for_files_from_codeowners(
 }
 
 pub fn team_for_file_from_codeowners(run_config: &RunConfig, file_path: &str) -> error_stack::Result<Option<Team>, Error> {
-    let relative_file_path = Path::new(file_path)
-        .strip_prefix(&run_config.project_root)
-        .unwrap_or(Path::new(file_path));
+    let relative_file_path = crate::path_utils::relative_to(&run_config.project_root, Path::new(file_path));
 
     let config = config_from_path(&run_config.config_path)?;
     let res = crate::ownership::codeowners_query::team_for_file_from_codeowners(