From 564e8f221131004a91bdefb1a37d294223ce1ce6 Mon Sep 17 00:00:00 2001 From: Paul Kim Date: Sat, 18 Apr 2026 14:25:25 +0200 Subject: [PATCH 1/3] 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. Signed-off-by: Paul Kim --- rewatch/tests/suite.sh | 1 + .../02-watch-warnings-persist-atomic-save.sh | 129 ++++++++++++++++++ 2 files changed, 130 insertions(+) create mode 100755 rewatch/tests/watch/02-watch-warnings-persist-atomic-save.sh diff --git a/rewatch/tests/suite.sh b/rewatch/tests/suite.sh index 14f9330b885..3f1313b6c63 100755 --- a/rewatch/tests/suite.sh +++ b/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 && diff --git a/rewatch/tests/watch/02-watch-warnings-persist-atomic-save.sh b/rewatch/tests/watch/02-watch-warnings-persist-atomic-save.sh new file mode 100755 index 00000000000..63adb0fd2bf --- /dev/null +++ b/rewatch/tests/watch/02-watch-warnings-persist-atomic-save.sh @@ -0,0 +1,129 @@ +#!/bin/bash +cd $(dirname $0) +source "../utils.sh" +cd ../../testrepo + +bold "Test: Warnings persist after atomic-save rename of unrelated module" + +error_output=$(rewatch clean 2>&1) +if [ $? -eq 0 ]; +then + success "Repo Cleaned" +else + error "Error Cleaning Repo" + printf "%s\n" "$error_output" >&2 + exit 1 +fi + +wait_for_pattern() { + local file="$1"; local pattern="$2"; local timeout="${3:-30}" + while [ "$timeout" -gt 0 ]; do + grep -q "$pattern" "$file" 2>/dev/null && return 0 + sleep 1 + timeout=$((timeout - 1)) + done + return 1 +} + +wait_for_changed_completed_log() { + local file="$1"; local baseline="$2"; local timeout="${3:-30}" + while [ "$timeout" -gt 0 ]; do + if [ -f "$file" ] && ! cmp -s "$file" "$baseline" && grep -q "#Done(" "$file" 2>/dev/null; then + return 0 + fi + sleep 1 + timeout=$((timeout - 1)) + done + return 1 +} + +COMPILER_LOG="./packages/watch-warnings/lib/bs/.compiler.log" +B_FILE="./packages/watch-warnings/src/B.res" +WATCH_STDOUT="rewatch-stdout.log" +WATCH_STDERR="rewatch-stderr.log" +TMP_DIR="$(mktemp -d "${TMPDIR:-/tmp}/rewatch-watch-warning-atomic-save.XXXXXX")" +INITIAL_LOG="$TMP_DIR/initial.compiler.log" +BACKUP_B="$TMP_DIR/B.res.bak" + +cp "$B_FILE" "$BACKUP_B" + +cleanup() { + set +e + if [ -f "$BACKUP_B" ]; then + cp "$BACKUP_B" "$B_FILE" + fi + exit_watcher + sleep 1 + rm -rf "$TMP_DIR" + rm -f "$WATCH_STDOUT" "$WATCH_STDERR" +} +trap cleanup EXIT + +rewatch_bg watch > "$WATCH_STDOUT" 2> "$WATCH_STDERR" & + +if ! wait_for_pattern "$COMPILER_LOG" "unused value unusedValue" 30 || ! wait_for_pattern "$COMPILER_LOG" "#Done(" 30; then + error "Initial build did not finish with warning from ModuleA.res in $COMPILER_LOG" + echo + echo "=== compiler log ===" + cat "$COMPILER_LOG" 2>/dev/null || true + echo + echo "=== watch stderr ===" + cat "$WATCH_STDERR" 2>/dev/null || true + echo + echo "=== watch stdout ===" + cat "$WATCH_STDOUT" 2>/dev/null || true + exit 1 +fi +success "Initial build shows warning from ModuleA.res in $COMPILER_LOG" + +cp "$COMPILER_LOG" "$INITIAL_LOG" + +# Simulate an editor atomic save by writing a temp file and renaming it into place. +tmp_b_save="$(mktemp "${TMPDIR:-/tmp}/B.res.XXXXXX")" +printf 'let world = () => Console.log("world")\n// trigger atomic save\n' > "$tmp_b_save" +mv "$tmp_b_save" "$B_FILE" + +if ! wait_for_changed_completed_log "$COMPILER_LOG" "$INITIAL_LOG" 30; then + error "Compiler log did not complete a new cycle after atomic save of B.res" + echo + echo "=== compiler log ===" + cat "$COMPILER_LOG" 2>/dev/null || true + echo + echo "=== watch stderr ===" + cat "$WATCH_STDERR" 2>/dev/null || true + echo + echo "=== watch stdout ===" + cat "$WATCH_STDOUT" 2>/dev/null || true + exit 1 +fi + +if grep -q "unused value unusedValue" "$COMPILER_LOG"; then + success "Warning from ModuleA.res persists after atomic save of B.res" +else + error "Warning from ModuleA.res was lost after atomic save of B.res" + echo + echo "=== compiler log ===" + cat "$COMPILER_LOG" + echo + echo "=== watch stderr ===" + cat "$WATCH_STDERR" 2>/dev/null || true + echo + echo "=== watch stdout ===" + cat "$WATCH_STDOUT" 2>/dev/null || true + exit 1 +fi + +cp "$BACKUP_B" "$B_FILE" +sleep 1 +exit_watcher +sleep 2 +rm -f "$WATCH_STDOUT" "$WATCH_STDERR" + +if git diff --exit-code ./packages/watch-warnings > /dev/null 2>&1; +then + success "No leftover changes in watch-warnings package" +else + error "Leftover changes detected in watch-warnings package" + git diff ./packages/watch-warnings + exit 1 +fi From 1604e3b57a0ccde324e2836e42bba3aa09bb4077 Mon Sep 17 00:00:00 2001 From: Paul Kim Date: Sat, 18 Apr 2026 14:46:37 +0200 Subject: [PATCH 2/3] 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. Signed-off-by: Paul Kim --- rewatch/src/watcher.rs | 240 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 239 insertions(+), 1 deletion(-) diff --git a/rewatch/src/watcher.rs b/rewatch/src/watcher.rs index 8a37cdaf618..3eba1f1d0b0 100644 --- a/rewatch/src/watcher.rs +++ b/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, ¤t_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); + } +} From a5f664c8d62080b22073431c2b52fc5a80c17a1d Mon Sep 17 00:00:00 2001 From: Paul Kim Date: Sat, 18 Apr 2026 16:44:19 +0200 Subject: [PATCH 3/3] docs: add changelog entry for watch warning persistence fix --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7615554e0b..c31fbe9d27c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,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 #### :memo: Documentation