Rewatch: preserve warnings after atomic-save full rebuilds (#8358)

* test: cover warning persistence after atomic saves

Add a rewatch integration test that simulates an editor-style atomic save by renaming B.res into place and verifies warnings from ModuleA.res remain in .compiler.log after the rebuild.

* Preserve warnings across watch full rebuilds

Atomic-save rename events force the watcher down the full rebuild path, which was recreating build state and dropping stored warnings for unchanged modules. Carry warning state forward into the new build state so unrelated module warnings continue to be re-emitted.\n\nAlso add unit tests for the warning carry-forward helper.

* docs: add changelog

Author: Bushuo

CHANGELOG.md

@@ -24,6 +24,7 @@
 #### :bug: Bug fix
 
 - Fix partial application generalization for `...`. https://github.com/rescript-lang/rescript/pull/8343
+- Rewatch: preserve warnings after atomic-save full rebuilds. https://github.com/rescript-lang/rescript/pull/8358
 
 - Preserve JSX prop locations across the AST0 translation layer, fixing `0:0` editor diagnostics in PPX-related flows. https://github.com/rescript-lang/rescript/pull/8350
 

rewatch/src/watcher.rs

@@ -140,6 +140,44 @@ fn unregister_watches(watcher: &mut RecommendedWatcher, watch_paths: &[(PathBuf,
     }
 }
 
+fn carry_forward_compile_warnings(previous: &BuildCommandState, next: &mut BuildCommandState) {
+    for (module_name, next_module) in next.build_state.modules.iter_mut() {
+        let Some(previous_module) = previous.build_state.modules.get(module_name) else {
+            continue;
+        };
+        if previous_module.package_name != next_module.package_name {
+            continue;
+        }
+
+        match (&previous_module.source_type, &mut next_module.source_type) {
+            (SourceType::SourceFile(previous_source), SourceType::SourceFile(next_source)) => {
+                if previous_source.implementation.path == next_source.implementation.path {
+                    next_source.implementation.compile_warnings =
+                        previous_source.implementation.compile_warnings.clone();
+
+                    if next_source.implementation.compile_warnings.is_some() {
+                        next_source.implementation.compile_state =
+                            previous_source.implementation.compile_state.clone();
+                    }
+                }
+
+                if let (Some(previous_interface), Some(next_interface)) =
+                    (&previous_source.interface, &mut next_source.interface)
+                    && previous_interface.path == next_interface.path
+                {
+                    next_interface.compile_warnings = previous_interface.compile_warnings.clone();
+
+                    if next_interface.compile_warnings.is_some() {
+                        next_interface.compile_state = previous_interface.compile_state.clone();
+                    }
+                }
+            }
+            (SourceType::MlMap(_), SourceType::MlMap(_)) => (),
+            _ => (),
+        }
+    }
+}
+
 struct AsyncWatchArgs<'a> {
     watcher: &'a mut RecommendedWatcher,
     current_watch_paths: Vec<(PathBuf, RecursiveMode)>,
@@ -374,7 +412,7 @@ async fn async_watch(
             }
             CompileType::Full => {
                 let timing_total = Instant::now();
-                build_state = build::initialize_build(
+                let mut next_build_state = build::initialize_build(
                     None,
                     filter,
                     show_progress,
@@ -384,6 +422,12 @@ async fn async_watch(
                 )
                 .expect("Could not initialize build");
 
+                // Full rebuilds can be triggered by editor atomic saves that surface as rename events.
+                // Preserve warning state for unchanged modules so their warnings are re-emitted after the
+                // fresh build state replaces the previous one.
+                carry_forward_compile_warnings(&build_state, &mut next_build_state);
+                build_state = next_build_state;
+
                 // Re-register watches based on the new build state
                 unregister_watches(watcher, &current_watch_paths);
                 current_watch_paths = compute_watch_paths(&build_state, path);
@@ -475,3 +519,197 @@ pub fn start(
         .await
     })
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::build::build_types::{
+        CompileState, CompilerInfo, Implementation, Interface, Module, ParseState, SourceFile, SourceType,
+    };
+    use crate::build::packages::{Namespace, Package};
+    use crate::config;
+    use crate::project_context::ProjectContext;
+    use ahash::{AHashMap, AHashSet};
+    use std::path::PathBuf;
+    use std::sync::RwLock;
+    use std::time::SystemTime;
+
+    fn test_project_context(root: &str) -> ProjectContext {
+        let root_path = PathBuf::from(root);
+        let config = config::tests::create_config(config::tests::CreateConfigArgs {
+            name: "test-root".to_string(),
+            bs_deps: vec![],
+            build_dev_deps: vec![],
+            allowed_dependents: None,
+            path: root_path.clone(),
+        });
+
+        ProjectContext {
+            current_config: config,
+            monorepo_context: None,
+            node_modules_exist_cache: RwLock::new(AHashMap::new()),
+            packages_cache: RwLock::new(AHashMap::new()),
+        }
+    }
+
+    fn test_package(name: &str, path: &str) -> Package {
+        let package_path = PathBuf::from(path);
+        Package {
+            name: name.to_string(),
+            config: config::tests::create_config(config::tests::CreateConfigArgs {
+                name: name.to_string(),
+                bs_deps: vec![],
+                build_dev_deps: vec![],
+                allowed_dependents: None,
+                path: package_path.clone(),
+            }),
+            source_folders: AHashSet::new(),
+            source_files: None,
+            namespace: Namespace::NoNamespace,
+            modules: None,
+            path: package_path,
+            dirs: None,
+            is_local_dep: true,
+            is_root: true,
+        }
+    }
+
+    fn test_build_state(module_name: &str, module: Module) -> BuildCommandState {
+        let root = "/tmp/rewatch-warning-carry-forward";
+        let package = test_package("test-package", root);
+        let mut packages = AHashMap::new();
+        packages.insert(package.name.clone(), package);
+
+        let compiler = CompilerInfo {
+            bsc_path: PathBuf::from("/tmp/bsc"),
+            bsc_hash: blake3::hash(b"test-bsc"),
+            runtime_path: PathBuf::from("/tmp/runtime"),
+        };
+
+        let mut build_state = BuildCommandState::new(
+            PathBuf::from(root),
+            test_project_context(root),
+            packages,
+            compiler,
+            None,
+        );
+        build_state.insert_module(module_name, module);
+        build_state
+    }
+
+    fn test_module(
+        implementation_path: &str,
+        implementation_warning: Option<&str>,
+        interface_path: Option<&str>,
+        interface_warning: Option<&str>,
+    ) -> Module {
+        let implementation_compile_state = if implementation_warning.is_some() {
+            CompileState::Warning
+        } else {
+            CompileState::Success
+        };
+        let interface_compile_state = if interface_warning.is_some() {
+            CompileState::Warning
+        } else {
+            CompileState::Success
+        };
+
+        Module {
+            source_type: SourceType::SourceFile(SourceFile {
+                implementation: Implementation {
+                    path: PathBuf::from(implementation_path),
+                    parse_state: ParseState::Success,
+                    compile_state: implementation_compile_state,
+                    last_modified: SystemTime::UNIX_EPOCH,
+                    parse_dirty: false,
+                    compile_warnings: implementation_warning.map(str::to_string),
+                },
+                interface: interface_path.map(|interface_path| Interface {
+                    path: PathBuf::from(interface_path),
+                    parse_state: ParseState::Success,
+                    compile_state: interface_compile_state,
+                    last_modified: SystemTime::UNIX_EPOCH,
+                    parse_dirty: false,
+                    compile_warnings: interface_warning.map(str::to_string),
+                }),
+            }),
+            deps: AHashSet::new(),
+            dependents: AHashSet::new(),
+            package_name: "test-package".to_string(),
+            compile_dirty: false,
+            last_compiled_cmi: None,
+            last_compiled_cmt: None,
+            deps_dirty: false,
+            is_type_dev: false,
+        }
+    }
+
+    #[test]
+    fn carries_forward_implementation_warnings_for_matching_module_paths() {
+        let previous = test_build_state(
+            "ModuleA",
+            test_module("src/ModuleA.res", Some("warning: impl"), None, None),
+        );
+        let mut next = test_build_state("ModuleA", test_module("src/ModuleA.res", None, None, None));
+
+        carry_forward_compile_warnings(&previous, &mut next);
+
+        let module = next.get_module("ModuleA").expect("module should exist");
+        let SourceType::SourceFile(source_file) = &module.source_type else {
+            panic!("expected source file module");
+        };
+
+        assert_eq!(
+            source_file.implementation.compile_warnings.as_deref(),
+            Some("warning: impl")
+        );
+        assert_eq!(source_file.implementation.compile_state, CompileState::Warning);
+    }
+
+    #[test]
+    fn does_not_carry_forward_warnings_when_module_paths_change() {
+        let previous = test_build_state(
+            "ModuleA",
+            test_module("src/ModuleA.res", Some("warning: impl"), None, None),
+        );
+        let mut next = test_build_state("ModuleA", test_module("src/ModuleARenamed.res", None, None, None));
+
+        carry_forward_compile_warnings(&previous, &mut next);
+
+        let module = next.get_module("ModuleA").expect("module should exist");
+        let SourceType::SourceFile(source_file) = &module.source_type else {
+            panic!("expected source file module");
+        };
+
+        assert_eq!(source_file.implementation.compile_warnings, None);
+        assert_eq!(source_file.implementation.compile_state, CompileState::Success);
+    }
+
+    #[test]
+    fn carries_forward_interface_warnings_for_matching_interface_paths() {
+        let previous = test_build_state(
+            "ModuleA",
+            test_module(
+                "src/ModuleA.res",
+                None,
+                Some("src/ModuleA.resi"),
+                Some("warning: interface"),
+            ),
+        );
+        let mut next = test_build_state(
+            "ModuleA",
+            test_module("src/ModuleA.res", None, Some("src/ModuleA.resi"), None),
+        );
+
+        carry_forward_compile_warnings(&previous, &mut next);
+
+        let module = next.get_module("ModuleA").expect("module should exist");
+        let SourceType::SourceFile(source_file) = &module.source_type else {
+            panic!("expected source file module");
+        };
+        let interface = source_file.interface.as_ref().expect("interface should exist");
+
+        assert_eq!(interface.compile_warnings.as_deref(), Some("warning: interface"));
+        assert_eq!(interface.compile_state, CompileState::Warning);
+    }
+}

rewatch/tests/suite.sh

@@ -86,6 +86,7 @@ fi
 # Watch tests
 ./watch/01-watch-recompile.sh &&
 ./watch/02-watch-warnings-persist.sh &&
+./watch/02-watch-warnings-persist-atomic-save.sh &&
 ./watch/03-watch-new-file.sh &&
 ./watch/04-watch-config-change.sh &&
 ./watch/05-watch-ignores-non-source.sh &&