chore: update TODOs and clarify panics

simongoricar · Aug 8, 2024 · f61816d · f61816d
1 parent 344aaa4
commit f61816d
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 3 deletions.
diff --git a/src/directory/prepared.rs b/src/directory/prepared.rs
@@ -716,12 +716,23 @@ fn scan_and_plan_directory_copy(
                                 SymlinkType::File
                             } else if resolved_symlink_file_type.is_dir() {
                                 SymlinkType::Directory
-                            } else {
+                            } else if resolved_symlink_file_type.is_symlink() {
                                 // FIXME Can this branch ever be reached? Is panicking okay?
+                                //       Context for future readers: this branch seems impossible to reach,
+                                //       since we used fs::metadata, which follows symbolic links.
                                 panic!(
                                     "unexpected filesystem state: followed symbolic link(s), \
                                     but arrived at another symbolic link"
                                 )
+                            } else {
+                                // FIXME Can this branch ever be reached? Is panicking okay?
+                                //       Context for future readers: this branch seems impossible to reach,
+                                //       since we used fs::metadata. For this to happen, is_file, is_dir and is_symlink
+                                //       all need to return `false`. If you encounter this panic, report it to the issue tracker.
+                                panic!(
+                                    "unexpected filesystem state: followed symbolic link(s), \
+                                    but arrived at something that is none of: file, directory, symlink"
+                                );
                             };
 
 
@@ -791,6 +802,8 @@ fn scan_and_plan_directory_copy(
                             };
                         } else if resolved_symlink_file_type.is_symlink() {
                             // FIXME Can this branch ever be reached? Is panicking okay?
+                            //       Context for future readers: this branch seems impossible to reach,
+                            //       since we used fs::metadata, which follows symbolic links.
                             panic!(
                                 "unexpected filesystem state: followed symbolic link(s), \
                                 but arrived at another symbolic link"

diff --git a/src/directory/scan/mod.rs b/src/directory/scan/mod.rs
@@ -294,7 +294,7 @@ pub fn is_directory_empty<P>(directory_path: P) -> Result<bool, DirectoryEmptine
 where
     P: AsRef<Path>,
 {
-    // TODO needs more tests!
+    // TODO Needs a more comprehensive set of tests for edge cases (though I'm not sure what they are yet).
 
     let directory_path: &Path = directory_path.as_ref();
 

diff --git a/src/file/size.rs b/src/file/size.rs
@@ -40,7 +40,7 @@ where
 {
     let file_path = file_path.as_ref();
 
-    // TODO Make symbolic link behaviour configurable here (and write tests for that).
+    // TODO Perhaps we should make symbolic link behaviour configurable?
 
     // Ensure the file exists. We use `try_exists`
     // instead of `exists` to catch permission and other IO errors