Merge pull request #293 from reddevilmidzy/rename-dir

feat: Improve rename detection and directory tracking

Author: reddevilmidzy

src/git/file_tracker.rs

@@ -1,92 +1,104 @@
-use git2::{Commit, Delta, DiffFindOptions, DiffOptions, ErrorCode, Repository};
+use git2::{Commit, Delta, DiffFindOptions, ErrorCode, Repository};
 use std::path;
 
-/// Finds the last commit ID that contains the target file
+/// Represents the result of searching for the last commit that touched a target path.
+///
+/// This struct contains both the commit that last modified the target path and,
+/// if the path was renamed in that commit, the new path it was renamed to.
+///
+/// # Fields
+/// * `commit` - The commit that last touched the target path
+/// * `renamed_path` - If the target path was renamed in this commit, contains the new path.
+///   For files, this is the full new file path. For directories, this is the new directory
+///   path with a trailing slash. If the path was not renamed, this is `None`.
+pub struct CommitSearchResult<'a> {
+    /// The commit that last touched the target path
+    pub commit: Commit<'a>,
+    /// The new path if the target was renamed in this commit, `None` otherwise
+    pub renamed_path: Option<String>,
+}
+
+/// This function searches through the commit history from HEAD backwards to find
+/// the most recent commit that modified the target path. It also detects if the
+/// path was renamed in that commit and returns the new path.
+///
+/// The function ignores merge commits (commits with 2+ parents) and initial commits
+/// (commits with 0 parents), following the same behavior as `git whatchanged`.
 ///
 /// # Arguments
-/// * `target_file` - The path to the target file
+/// * `target_file` - The path to the target file or directory to search for
 /// * `repo` - The repository to search in
 ///
 /// # Returns
-/// * `Ok(git2::Commit)` - The last commit ID that contains the target file
-/// * `Err(git2::Error)` - If there was an error accessing the repository
+/// * `Ok(CommitSearchResult)` - Contains the commit that last touched the target path
+///   and optionally the new path if it was renamed in that commit
+/// * `Err(git2::Error)` - If there was an error accessing the repository or if the
+///   target path was not found in the repository history
 pub fn find_last_commit_id<'a>(
     target_file: &str,
     repo: &'a Repository,
-) -> Result<Commit<'a>, git2::Error> {
+) -> Result<CommitSearchResult<'a>, git2::Error> {
+    let target_path = path::Path::new(target_file);
     let mut revwalk = repo.revwalk()?;
     revwalk.push_head()?;
 
     for commit_id in revwalk {
         let commit_id = commit_id?;
         let commit = repo.find_commit(commit_id)?;
 
-        // Ignore merge commits(2+ Parents) because that's what 'git whatchenged' does.
-        // Ignore commit with 0 parent (initial commit) because there's nothing to diff against.
-
         if commit.parent_count() == 1 {
             let prev_commit = commit.parent(0)?;
             let tree = commit.tree()?;
             let prev_tree = prev_commit.tree()?;
-            let diff = repo.diff_tree_to_tree(Some(&prev_tree), Some(&tree), None)?;
+            let mut diff = repo.diff_tree_to_tree(Some(&prev_tree), Some(&tree), None)?;
+
+            let mut find_opts = DiffFindOptions::new();
+            find_opts.rename_threshold(50); // Git default threshold 50%
+            diff.find_similar(Some(&mut find_opts))?;
             for delta in diff.deltas() {
+                let mut renamed_path = None;
+
+                // file check
                 if let Some(file_path) = delta.new_file().path()
-                    && let Some(file_path_str) = file_path.to_str()
-                    && file_path_str == target_file
+                    && file_path == target_path
+                {
+                    return Ok(CommitSearchResult {
+                        commit,
+                        renamed_path: None,
+                    });
+                }
+                // directory check
+                if let Some(old_path) = delta.old_file().path()
+                    && old_path.starts_with(target_path)
                 {
-                    return Ok(commit);
+                    if old_path == target_path && delta.status() == Delta::Renamed {
+                        renamed_path = delta
+                            .new_file()
+                            .path()
+                            .and_then(|p| p.to_str())
+                            .map(|s| s.to_string());
+                    } else if delta.status() == Delta::Renamed
+                        && let Some(path) = delta.new_file().path()
+                        && let Some(parent) = path.parent()
+                    {
+                        let mut dir = parent.to_string_lossy().to_string();
+                        if !dir.ends_with('/') && !dir.ends_with('\\') {
+                            dir.push('/');
+                        }
+                        renamed_path = Some(dir);
+                    }
+
+                    return Ok(CommitSearchResult {
+                        commit,
+                        renamed_path,
+                    });
                 }
             }
         }
     }
-
     Err(git2::Error::from_str("File not found"))
 }
 
-/// Finds the new path of a file that has been moved in a commit
-///
-/// # Arguments
-/// * `repo` - The repository to search in
-/// * `commit` - The commit to search in
-/// * `target_file` - The path to the target file
-///
-pub fn track_file_rename_in_commit(
-    repo: &Repository,
-    commit: &Commit,
-    target_file: &str,
-) -> Result<Option<String>, git2::Error> {
-    if commit.parent_count() != 1 {
-        return Ok(None);
-    }
-
-    let mut diff_opts = DiffOptions::new();
-
-    let mut find_opts = DiffFindOptions::new();
-    // TODO 적절한 값 찾기
-    find_opts.rename_threshold(28);
-
-    let parent = commit.parent(0)?;
-    let tree = commit.tree()?;
-    let parent_tree = parent.tree()?;
-
-    let mut diff = repo.diff_tree_to_tree(Some(&parent_tree), Some(&tree), Some(&mut diff_opts))?;
-    diff.find_similar(Some(&mut find_opts))?;
-
-    for delta in diff.deltas() {
-        if delta.status() == Delta::Renamed
-            && (delta.old_file().path().and_then(|p| p.to_str()) == Some(target_file))
-        {
-            return Ok(delta
-                .new_file()
-                .path()
-                .and_then(|p| p.to_str())
-                .map(|s| s.to_string()));
-        }
-    }
-
-    Ok(None)
-}
-
 /// Checks if a file exists in the repository at the given path
 ///
 /// # Arguments
@@ -115,30 +127,6 @@ mod tests {
     use super::*;
     use serial_test::serial;
 
-    #[test]
-    #[serial]
-    fn test_track_file_rename_in_commit() -> Result<(), git2::Error> {
-        let github_url = GitHubUrl::new(
-            "reddevilmidzy".to_string(),
-            "kingsac".to_string(),
-            Some("main".to_string()),
-            None,
-        );
-        let repo_manager = RepoManager::from(&github_url)?;
-        let commit = find_last_commit_id("main.rs", repo_manager.get_repo())?;
-        // see https://github.com/reddevilmidzy/kingsac/commit/2f3e99cbea53c55c8428d5bc11bfe7f1ff5cccd7
-        assert_eq!(
-            commit.id().to_string(),
-            "2f3e99cbea53c55c8428d5bc11bfe7f1ff5cccd7"
-        );
-        assert_eq!(
-            track_file_rename_in_commit(repo_manager.get_repo(), &commit, "main.rs")?,
-            Some("src/main.rs".to_string())
-        );
-
-        Ok(())
-    }
-
     #[test]
     #[serial]
     fn test_file_exists_in_repo() -> Result<(), git2::Error> {
@@ -180,23 +168,19 @@ mod tests {
         let repo_manager = RepoManager::from(&github_url)?;
 
         // 1. Find the commit where test_for_multiple_moves.rs was moved to foo/test_for_multiple_moves.rs
-        let commit = find_last_commit_id("test_for_multiple_moves.rs", repo_manager.get_repo())?;
-        let new_path = track_file_rename_in_commit(
-            repo_manager.get_repo(),
-            &commit,
-            "test_for_multiple_moves.rs",
-        )?;
-        assert_eq!(new_path, Some("foo/test_for_multiple_moves.rs".to_string()));
+        let result = find_last_commit_id("test_for_multiple_moves.rs", repo_manager.get_repo())?;
+        assert_eq!(
+            result.renamed_path,
+            Some("foo/test_for_multiple_moves.rs".to_string())
+        );
 
         // 2. Find the commit where foo/test_for_multiple_moves.rs was moved to bar/test_for_multiple_moves.rs
-        let commit =
+        let result =
             find_last_commit_id("foo/test_for_multiple_moves.rs", repo_manager.get_repo())?;
-        let new_path = track_file_rename_in_commit(
-            repo_manager.get_repo(),
-            &commit,
-            "foo/test_for_multiple_moves.rs",
-        )?;
-        assert_eq!(new_path, Some("bar/test_for_multiple_moves.rs".to_string()));
+        assert_eq!(
+            result.renamed_path,
+            Some("bar/test_for_multiple_moves.rs".to_string())
+        );
 
         // 3. Verify that the file exists at the final location
         assert!(file_exists_in_repo(

src/git/repo.rs

@@ -1,4 +1,4 @@
-use crate::{GitHubUrl, file_exists_in_repo, find_last_commit_id, track_file_rename_in_commit};
+use crate::{GitHubUrl, file_exists_in_repo, find_last_commit_id};
 use git2::{
     BranchType, Cred, Oid, PushOptions, RemoteCallbacks, Repository, Signature,
     build::CheckoutBuilder,
@@ -110,23 +110,23 @@ impl RepoManager {
                 return Ok(Some(current_path));
             }
 
-            let commit = match find_last_commit_id(&current_path, repo) {
-                Ok(commit) => commit,
+            let result = match find_last_commit_id(&current_path, repo) {
+                Ok(result) => result,
                 Err(e) => {
                     error!("Error finding last commit for {}: {}", current_path, e);
                     return Ok(None);
                 }
             };
 
-            match track_file_rename_in_commit(repo, &commit, &current_path)? {
+            match result.renamed_path {
                 Some(new_path) => {
                     current_path = new_path;
                 }
                 None => {
                     error!(
                         "Could not find new path for {} in commit {}",
                         current_path,
-                        commit.id()
+                        result.commit.id()
                     );
                     return Ok(None);
                 }
@@ -399,7 +399,7 @@ mod tests {
     /// 1. Initially located at: tmp.txt (root directory)
     /// 2. First moved to: dockerfile_history/tmp.txt
     /// 3. Finally moved to: img/tmp.txt
-    fn test_find_current_location() {
+    fn test_find_current_location_file() {
         let url = GitHubUrl::parse(
             "https://github.com/reddevilmidzy/zero2prod/blob/test_for_queensac/tmp.txt",
         )
@@ -413,6 +413,19 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_find_current_location_directory() {
+        let url =
+            GitHubUrl::parse("https://github.com/reddevilmidzy/kingsac/tree/main/foo/intrinsics")
+                .unwrap();
+        let repo_manager = RepoManager::from(&url).unwrap();
+
+        assert_eq!(
+            repo_manager.find_current_location(&url).unwrap(),
+            Some("foo/".to_string())
+        )
+    }
+
     #[test]
     /// This test verifies the behavior when a file cannot be found in the repository.
     /// It tests two scenarios: