From 7483f20eb31132df194f5d3df9d8f64d41200911 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= <kubouch@gmail.com>
Date: Sun, 23 Jun 2024 14:13:18 +0300
Subject: [PATCH] Recursively reload modules only when necessary

---
 crates/nu-parser/src/parse_keywords.rs | 60 +++++++++++++++++++-------
 crates/nu-protocol/src/module.rs       |  8 ++--
 tests/modules/mod.rs                   | 14 ++++++
 3 files changed, 63 insertions(+), 19 deletions(-)

diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs
index f951a8abe9..d1b1ca0102 100644
--- a/crates/nu-parser/src/parse_keywords.rs
+++ b/crates/nu-parser/src/parse_keywords.rs
@@ -1886,6 +1886,48 @@ pub fn parse_module_block(
     (block, module, module_comments)
 }
 
+fn module_needs_reloading(working_set: &StateWorkingSet, module_id: ModuleId) -> bool {
+    let module = working_set.get_module(module_id);
+
+    fn submodule_need_reloading(working_set: &StateWorkingSet, submodule_id: ModuleId) -> bool {
+        let submodule = working_set.get_module(submodule_id);
+        let submodule_changed = if let Some((file_path, file_id)) = &submodule.file {
+            let existing_contents = working_set.get_contents_of_file(*file_id);
+            let file_contents = file_path.read(working_set);
+
+            if let (Some(existing), Some(new)) = (existing_contents, file_contents) {
+                existing != new
+            } else {
+                false
+            }
+        } else {
+            false
+        };
+
+        if submodule_changed {
+            true
+        } else {
+            module_needs_reloading(working_set, submodule_id)
+        }
+    }
+
+    let export_submodule_changed = module
+        .submodules
+        .iter()
+        .any(|(_, submodule_id)| submodule_need_reloading(working_set, *submodule_id));
+
+    if export_submodule_changed {
+        return true;
+    }
+
+    let private_submodule_changed = module
+        .imported_modules
+        .iter()
+        .any(|submodule_id| submodule_need_reloading(working_set, *submodule_id));
+
+    private_submodule_changed
+}
+
 /// Parse a module from a file.
 ///
 /// The module name is inferred from the stem of the file, unless specified in `name_override`.
@@ -1924,25 +1966,13 @@ fn parse_module_file(
 
     // Check if we've parsed the module before.
     if let Some(module_id) = working_set.find_module_by_span(new_span) {
-        let module = working_set.get_module(module_id);
-        // check if it contains submodules with a file name.
-        let has_export_submodules = module
-            .submodules
-            .iter()
-            .any(|(_, m)| working_set.get_module(*m).file.is_some());
-        // check if it's using other modules with a file name.
-        let has_use_submodules = module
-            .imported_modules
-            .iter()
-            .any(|m| working_set.get_module(*m).file.is_some());
-        if !(has_export_submodules || has_use_submodules) {
+        if !module_needs_reloading(working_set, module_id) {
             return Some(module_id);
         }
     }
 
     // Add the file to the stack of files being processed.
-    let path_buf = path.path_buf();
-    if let Err(e) = working_set.files.push(path_buf.clone(), path_span) {
+    if let Err(e) = working_set.files.push(path.clone().path_buf(), path_span) {
         working_set.error(e);
         return None;
     }
@@ -1955,7 +1985,7 @@ fn parse_module_file(
     working_set.files.pop();
 
     let _ = working_set.add_block(Arc::new(block));
-    module.file = Some(path_buf);
+    module.file = Some((path, file_id));
     let module_id = working_set.add_module(&module_name, module, module_comments);
 
     Some(module_id)
diff --git a/crates/nu-protocol/src/module.rs b/crates/nu-protocol/src/module.rs
index 2a2d6663f6..6b4aec0c60 100644
--- a/crates/nu-protocol/src/module.rs
+++ b/crates/nu-protocol/src/module.rs
@@ -1,9 +1,9 @@
 use crate::{
-    ast::ImportPatternMember, engine::StateWorkingSet, BlockId, DeclId, ModuleId, ParseError, Span,
-    Value, VarId,
+    ast::ImportPatternMember, engine::StateWorkingSet, BlockId, DeclId, FileId, ModuleId,
+    ParseError, Span, Value, VarId,
 };
-use std::path::PathBuf;
 
+use crate::parser_path::ParserPath;
 use indexmap::IndexMap;
 
 pub struct ResolvedImportPattern {
@@ -37,7 +37,7 @@ pub struct Module {
     pub main: Option<DeclId>,       // `export def main`
     pub span: Option<Span>,
     pub imported_modules: Vec<ModuleId>, // use other_module.nu
-    pub file: Option<PathBuf>,
+    pub file: Option<(ParserPath, FileId)>,
 }
 
 impl Module {
diff --git a/tests/modules/mod.rs b/tests/modules/mod.rs
index 228c5aaa79..9aa1282f21 100644
--- a/tests/modules/mod.rs
+++ b/tests/modules/mod.rs
@@ -903,5 +903,19 @@ fn use_nested_submodules() {
         ];
         let actual = nu!(cwd: dirs.test(), nu_repl_code(&inp));
         assert_eq!(actual.out, "true");
+
+        sandbox.with_files(&[
+            FileWithContent("animals.nu", r#"export use nested_animals.nu cat"#),
+            FileWithContent("nested_animals.nu", "export def cat [] { 'meow' }"),
+        ]);
+        let inp = [
+            "module voice { export module animals.nu }",
+            "use voice",
+            r#""export def cat [] {'woem'}" | save -f nested_animals.nu"#,
+            "use voice.nu",
+            "(voice animals cat) == 'woem'",
+        ];
+        let actual = nu!(cwd: dirs.test(), nu_repl_code(&inp));
+        assert_eq!(actual.out, "true");
     })
 }