Fix file watcher to handle rename events

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+* improve file watching to handle rename events and remove empty folders after processing if they weren't present before the initial run ([#351](https://github.com/seaofvoices/darklua/pull/351))
 * add a new parameter for the `rename_variables` rule so that globals can be detected automatically and then avoided in the renaming pass ([#348](https://github.com/seaofvoices/darklua/pull/348))
 * add support for `const` declaration of variables and functions (e.g. `const var = true` or `const function test() end`) and add rule (`make_assignment_local`) to convert those assignments to `local` assignments ([#346](https://github.com/seaofvoices/darklua/pull/346))
 * add support for type instantiation prefixes and methods (e.g. `func<<string>>()`) ([#345](https://github.com/seaofvoices/darklua/pull/345))

src/cli/utils/file_watcher.rs

@@ -2,14 +2,14 @@ use std::{
     collections::HashSet,
     env,
     hash::Hash,
-    iter,
+    io, iter,
     path::{Path, PathBuf},
     sync::mpsc::{self, Receiver, Sender},
     time::{Duration, Instant},
 };
 
 use darklua_core::{Options, Resources, WorkerTree};
-use notify::{EventKind, RecursiveMode};
+use notify::{event::ModifyKind, EventKind, RecursiveMode};
 use notify_debouncer_full::{new_debouncer, DebounceEventResult, DebouncedEvent};
 
 use crate::cli::{error::CliError, process::Options as ProcessOptions, CommandResult};
@@ -297,9 +297,16 @@ impl FileWatcher {
                 EventKind::Create(_create_kind) => {
                     has_created = paths_iterator.next().is_some();
                 }
-                EventKind::Modify(_modify_kind) => {
-                    for path in paths_iterator {
-                        worker_tree.source_changed(path);
+                EventKind::Modify(modify_kind) => {
+                    if let ModifyKind::Name(_rename_mode) = modify_kind {
+                        for path in paths_iterator {
+                            worker_tree.remove_source(path);
+                        }
+                        has_created = true;
+                    } else {
+                        for path in paths_iterator {
+                            worker_tree.source_changed(path);
+                        }
                     }
                 }
                 EventKind::Remove(_remove_kind) => {
@@ -401,6 +408,7 @@ fn iter_all_links(location: PathBuf) -> impl Iterator<Item = PathBuf> {
                         dir_entries.push(location.to_path_buf());
                     };
                 }
+                Err(err) if err.kind() == io::ErrorKind::NotFound => {}
                 Err(err) => {
                     log::warn!(
                         "unable to read metadata from file `{}`: {}",

src/frontend/mod.rs

@@ -68,6 +68,11 @@ pub fn convert_data(value: impl Serialize) -> Result<String, DarkluaError> {
 pub fn process(resources: &Resources, options: Options) -> DarkluaResult<WorkerTree> {
     let mut worker_tree = WorkerTree::default();
 
+    // take a snapshot of the output structure in order to know what folders to preserve when cleaning up
+    if let Some(output) = options.output().filter(|output| *output != options.input()) {
+        worker_tree.snapshot_output_structure(resources, output)?;
+    }
+
     worker_tree.collect_work(resources, &options)?;
     worker_tree.process(resources, options)?;
 

src/frontend/resources.rs

@@ -106,6 +106,55 @@ impl Source {
         }
     }
 
+    pub(crate) fn walk_all(&self, location: &Path) -> impl Iterator<Item = ResourceContent> {
+        match self {
+            Self::FileSystem => Box::new(walk_all_file_system(location.to_path_buf()))
+                as Box<dyn Iterator<Item = ResourceContent>>,
+            Self::Memory(data) => {
+                let data = data.lock().unwrap();
+                let location = normalize_path(location);
+                let mut paths: Vec<_> = data.keys().map(normalize_path).collect();
+                paths.retain(|path| path.starts_with(&location));
+
+                Box::new(paths.into_iter().map(ResourceContent::File))
+            }
+        }
+    }
+
+    pub(crate) fn is_empty_directory(&self, location: &Path) -> ResourceResult<bool> {
+        if !self.is_directory(location)? {
+            return Ok(false);
+        }
+
+        match self {
+            Self::FileSystem => match location.read_dir() {
+                Ok(read_dir) => {
+                    for entry in read_dir {
+                        match entry {
+                            Ok(_) => return Ok(false),
+                            Err(err) if err.kind() == io::ErrorKind::NotFound => {}
+                            Err(err) => {
+                                log::warn!(
+                                    "unable to read directory entry `{}`: {}",
+                                    location.display(),
+                                    err
+                                );
+                                return Ok(false);
+                            }
+                        }
+                    }
+
+                    Ok(true)
+                }
+                Err(err) => {
+                    log::warn!("unable to read directory `{}`: {}", location.display(), err);
+                    Ok(false)
+                }
+            },
+            Self::Memory(_data) => Ok(false),
+        }
+    }
+
     fn remove(&self, location: &Path) -> Result<(), ResourceError> {
         match self {
             Self::FileSystem => {
@@ -136,18 +185,19 @@ impl Source {
     }
 }
 
-fn walk_file_system(location: PathBuf) -> impl Iterator<Item = PathBuf> {
+fn walk_all_file_system(location: PathBuf) -> impl Iterator<Item = ResourceContent> {
     let mut unknown_paths = vec![location];
-    let mut file_paths = Vec::new();
+    let mut entries = Vec::new();
     let mut dir_entries = Vec::new();
 
     iter::from_fn(move || loop {
         if let Some(location) = unknown_paths.pop() {
             match location.metadata() {
                 Ok(metadata) => {
                     if metadata.is_file() {
-                        file_paths.push(location.to_path_buf());
+                        entries.push(ResourceContent::File(location.to_path_buf()));
                     } else if metadata.is_dir() {
+                        entries.push(ResourceContent::Directory(location.to_path_buf()));
                         dir_entries.push(location.to_path_buf());
                     } else if metadata.is_symlink() {
                         log::warn!("unexpected symlink `{}` not followed", location.display());
@@ -195,14 +245,21 @@ fn walk_file_system(location: PathBuf) -> impl Iterator<Item = PathBuf> {
                     );
                 }
             }
-        } else if let Some(path) = file_paths.pop() {
+        } else if let Some(path) = entries.pop() {
             break Some(path);
         } else {
             break None;
         }
     })
 }
 
+fn walk_file_system(location: PathBuf) -> impl Iterator<Item = PathBuf> {
+    walk_all_file_system(location).filter_map(|content| match content {
+        ResourceContent::File(path) => Some(path),
+        ResourceContent::Directory(_) => None,
+    })
+}
+
 /// A resource manager for handling file operations.
 ///
 /// This struct provides an abstraction over file system operations, allowing
@@ -275,6 +332,23 @@ impl Resources {
     pub fn walk(&self, location: impl AsRef<Path>) -> impl Iterator<Item = PathBuf> {
         self.source.walk(location.as_ref())
     }
+
+    /// Walks through all files and directories in a directory.
+    pub(crate) fn walk_all(
+        &self,
+        location: impl AsRef<Path>,
+    ) -> impl Iterator<Item = ResourceContent> {
+        self.source.walk_all(location.as_ref())
+    }
+
+    pub(crate) fn is_empty_directory(&self, location: impl AsRef<Path>) -> ResourceResult<bool> {
+        self.source.is_empty_directory(location.as_ref())
+    }
+}
+
+pub(crate) enum ResourceContent {
+    File(PathBuf),
+    Directory(PathBuf),
 }
 
 /// An error that can occur during operations on [`Resource`].

src/frontend/worker_tree.rs

@@ -13,8 +13,8 @@ use crate::{
 };
 
 use super::{
-    normalize_path, work_item::WorkStatus, Configuration, DarkluaResult, Options, Resources,
-    WorkItem, Worker,
+    normalize_path, resources::ResourceContent, work_item::WorkStatus, Configuration,
+    DarkluaResult, Options, Resources, WorkItem, Worker,
 };
 
 /// A structure that manages the processing of Lua/Luau files and their dependencies.
@@ -30,6 +30,7 @@ pub struct WorkerTree {
     external_dependencies: HashMap<PathBuf, HashSet<NodeIndex>>,
     remove_files: Vec<PathBuf>,
     last_configuration_hash: Option<u64>,
+    output_structure: Option<HashMap<PathBuf, bool>>,
 }
 
 impl WorkerTree {
@@ -104,21 +105,6 @@ impl WorkerTree {
     pub fn process(&mut self, resources: &Resources, mut options: Options) -> DarkluaResult<()> {
         clear_luau_configuration_cache();
 
-        if !self.remove_files.is_empty() {
-            let remove_count = self.remove_files.len();
-            log::debug!(
-                "clean {} file{} before beginning process",
-                remove_count,
-                maybe_plural(remove_count)
-            );
-            for path in self.remove_files.drain(..) {
-                log::trace!("remove file {}", path.display());
-                if let Err(err) = resources.remove(path).map_err(DarkluaError::from) {
-                    log::warn!("failed to remove resource: {}", err);
-                }
-            }
-        }
-
         let mut worker = Worker::new(resources);
         worker.setup_worker(&mut options)?;
 
@@ -237,9 +223,42 @@ impl WorkerTree {
 
         log::info!("executed work in {}", work_timer.duration_label());
 
+        self.clean_files(resources);
+
         Ok(())
     }
 
+    fn clean_files(&mut self, resources: &Resources) {
+        if self.remove_files.is_empty() {
+            return;
+        }
+
+        let remove_count = self.remove_files.len();
+        log::debug!("clean {} file{}", remove_count, maybe_plural(remove_count));
+
+        for path in self.remove_files.drain(..) {
+            log::trace!("remove file {}", path.display());
+            if let Err(err) = resources.remove(&path).map_err(DarkluaError::from) {
+                log::warn!("failed to remove resource: {}", err);
+            } else if let Some(structure) = self.output_structure.as_ref() {
+                for ancestor in path.ancestors().skip(1) {
+                    if ancestor.components().count() == 0 {
+                        break;
+                    }
+                    if !structure.contains_key(ancestor)
+                        && resources.is_empty_directory(ancestor).unwrap_or_default()
+                    {
+                        if let Err(err) = resources.remove(ancestor).map_err(DarkluaError::from) {
+                            log::warn!("failed to remove resource: {}", err);
+                        }
+                    } else {
+                        break;
+                    }
+                }
+            }
+        }
+    }
+
     /// Returns the final result of processing all work items.
     ///
     /// This method consumes the `WorkerTree` and returns either Ok(()) if all work items
@@ -438,4 +457,29 @@ impl WorkerTree {
             .map(|last_hash| new_hash != last_hash)
             .unwrap_or_default()
     }
+
+    pub(crate) fn snapshot_output_structure(
+        &mut self,
+        resources: &Resources,
+        location: &Path,
+    ) -> DarkluaResult<()> {
+        if self.output_structure.is_some()
+            || !resources.exists(location)?
+            || !resources.is_directory(location)?
+        {
+            return Ok(());
+        }
+
+        let structure = resources
+            .walk_all(location)
+            .map(|content| match content {
+                ResourceContent::File(path) => (path, true),
+                ResourceContent::Directory(path) => (path, false),
+            })
+            .collect();
+
+        self.output_structure = Some(structure);
+
+        Ok(())
+    }
 }