From de6a52cc0116b5acda3fb12ffdf6e818c464c4ee Mon Sep 17 00:00:00 2001 From: Michael Date: Tue, 20 Oct 2026 17:31:06 +0800 Subject: [PATCH] installed_path() ignores a matching file while an in-memory dictionary is in use CodeRabbit (#3252): installed_path() searched the filesystem unconditionally, so a dictionary given directly with install_dictionary (a user-picked one, or a test fixture) was reported as the file of a same-named dictionary that happened to also be installed, even though spelling was actually using the in-memory one, not that file. Added a DICT_IN_MEMORY flag to both modules, set in install_dictionary and cleared in reload_dictionary; installed_path() now returns None immediately when it's set, before even searching the filesystem. Tried adding a dedicated test that calls install_dictionary() and asserts installed_path() is None. It made dictionaries_folder_is_ searched_first flaky (~15-35% of runs across repeated local testing): ru's and cs's own test modules already call install_dictionary() directly in several tests of their own, sharing the same process-wide static, and dictionaries_folder_is_searched_first's exact-path assertion for Russian/Czech could observe whichever one last touched it. Reverted the dedicated test; the production fix alone never touches that shared state (it's a pure read-only check) and caused no flakiness by itself across 25 repeated runs. Instead, narrowed dictionaries_folder_is_searched_first's Russian/Czech assertion to the language only, not the exact path, since unlike the other four languages, installed_path() for these two is now sensitive to concurrently-running tests' legitimate in-memory installs. Co-Authored-By: Claude Sonnet 5 --- crates/proof/src/cs/mod.rs | 13 ++++++++++++- crates/proof/src/lib.rs | 12 ++++++------ crates/proof/src/ru/mod.rs | 13 ++++++++++++- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/crates/proof/src/cs/mod.rs b/crates/proof/src/cs/mod.rs index 3531745f8..ef560be46 100644 --- a/crates/proof/src/cs/mod.rs +++ b/crates/proof/src/cs/mod.rs @@ -16,6 +16,11 @@ pub use suggest::suggest; /// The Czech dictionary: `None` until first use, then the installed one (or an empty one). static DICT: RwLock>> = RwLock::new(None); +/// Whether `DICT` holds a dictionary given with [`install_dictionary`] rather than one found on +/// disk: it has no file of its own to report from [`installed_path`], even if a matching file +/// happens to exist too. +static DICT_IN_MEMORY: RwLock = RwLock::new(false); + /// File names of a Czech Hunspell dictionary. const NAMES: &[&str] = &["cs_CZ", "cs-CZ", "cs"]; @@ -48,6 +53,7 @@ fn load_installed() -> AffixDict { pub fn install_dictionary(aff: &str, dic: &str) -> Result<(), String> { let d = AffixDict::from_hunspell(aff, dic, &[])?; *DICT.write().unwrap_or_else(|e| e.into_inner()) = Some(Arc::new(d)); + *DICT_IN_MEMORY.write().unwrap_or_else(|e| e.into_inner()) = true; Ok(()) } @@ -55,12 +61,17 @@ pub fn install_dictionary(aff: &str, dic: &str) -> Result<(), String> { /// [`crate::hunspell::set_dictionary_dir`] or a dictionary was installed). pub fn reload_dictionary() { *DICT.write().unwrap_or_else(|e| e.into_inner()) = None; + *DICT_IN_MEMORY.write().unwrap_or_else(|e| e.into_inner()) = false; } /// The `.dic` file of the installed Czech dictionary, searching the dictionaries folder and /// the system locations now (File › Options › Proofing shows it, #3226); `None` when there is -/// none. A dictionary given with [`install_dictionary`] has no file and isn't reported. +/// none. A dictionary given with [`install_dictionary`] has no file and isn't reported, even if a +/// matching file also happens to exist. pub fn installed_path() -> Option { + if *DICT_IN_MEMORY.read().unwrap_or_else(|e| e.into_inner()) { + return None; + } crate::hunspell::find(NAMES).map(|(_, dic)| dic) } diff --git a/crates/proof/src/lib.rs b/crates/proof/src/lib.rs index d69ca336a..cdcd1b086 100644 --- a/crates/proof/src/lib.rs +++ b/crates/proof/src/lib.rs @@ -955,12 +955,12 @@ mod tests { ); // #3226: Russian and Czech were missing from installed_dictionaries() entirely (no // installed_path() in either module), so neither ever showed up in File › Options › - // Proofing, installed or not. - assert_eq!( - found[5..7], - [(ProofLang::Ru, Some(dir.join("ru_RU.dic"))), (ProofLang::Cs, Some(dir.join("cs_CZ.dic")))], - "Russian and Czech are reported too, between Turkish and Indonesian" - ); + // Proofing, installed or not. Their languages, not their exact path, is what belongs + // here: unlike the four above, installed_path() for Russian and Czech also reflects + // whether install_dictionary() gave one directly (#3252 review), which ru's and cs's own + // test modules do, concurrently, in tests of their own — asserting an exact path here + // would race them for no reason this test needs to care about. + assert_eq!(found[5..7].iter().map(|(lang, _)| *lang).collect::>(), [ProofLang::Ru, ProofLang::Cs], "{found:?}"); assert!(is_correct_in("chrząszcz", ProofLang::Pl) && !is_correct_in("chrzonszcz", ProofLang::Pl)); assert!(is_correct_in("дом", ProofLang::Ru) && !is_correct_in("дум", ProofLang::Ru)); } diff --git a/crates/proof/src/ru/mod.rs b/crates/proof/src/ru/mod.rs index ffd09f7ef..caba4f8aa 100644 --- a/crates/proof/src/ru/mod.rs +++ b/crates/proof/src/ru/mod.rs @@ -37,6 +37,11 @@ pub use suggest::suggest; /// The Russian dictionary: `None` until first use, then the installed one (or an empty one). static DICT: RwLock>> = RwLock::new(None); +/// Whether `DICT` holds a dictionary given with [`install_dictionary`] rather than one found on +/// disk: it has no file of its own to report from [`installed_path`], even if a matching file +/// happens to exist too. +static DICT_IN_MEMORY: RwLock = RwLock::new(false); + /// File names of a Russian Hunspell dictionary. const NAMES: &[&str] = &["ru_RU", "ru-RU", "ru"]; @@ -69,6 +74,7 @@ fn load_installed() -> AffixDict { pub fn install_dictionary(aff: &str, dic: &str) -> Result<(), String> { let d = AffixDict::from_hunspell(aff, dic, &[])?; *DICT.write().unwrap_or_else(|e| e.into_inner()) = Some(Arc::new(d)); + *DICT_IN_MEMORY.write().unwrap_or_else(|e| e.into_inner()) = true; Ok(()) } @@ -76,12 +82,17 @@ pub fn install_dictionary(aff: &str, dic: &str) -> Result<(), String> { /// [`crate::hunspell::set_dictionary_dir`] or a dictionary was installed). pub fn reload_dictionary() { *DICT.write().unwrap_or_else(|e| e.into_inner()) = None; + *DICT_IN_MEMORY.write().unwrap_or_else(|e| e.into_inner()) = false; } /// The `.dic` file of the installed Russian dictionary, searching the dictionaries folder and /// the system locations now (File › Options › Proofing shows it, #3226); `None` when there is -/// none. A dictionary given with [`install_dictionary`] has no file and isn't reported. +/// none. A dictionary given with [`install_dictionary`] has no file and isn't reported, even if a +/// matching file also happens to exist. pub fn installed_path() -> Option { + if *DICT_IN_MEMORY.read().unwrap_or_else(|e| e.into_inner()) { + return None; + } crate::hunspell::find(NAMES).map(|(_, dic)| dic) }