diff --git a/src/ast.rs b/src/ast.rs index ee31f60a..87915d32 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1022,16 +1022,24 @@ impl Scope { collected }; - // Phase 2: insert into current scope - let current = self.current_module_mut(); + // Phase 2: validate against existing names and stage the complete import. + // Failed declarations discard these maps without changing the module tree. + let current = self.current_module(); + let mut pending = ModuleScope::default(); for (alias_res, func_res, mod_res) in collected { Self::resolve_processing_use_items_error(&[ - Self::insert_collected(alias_res, &mut current.aliases), - Self::insert_collected(func_res, &mut current.functions), - Self::insert_collected(mod_res, &mut current.submodules), + Self::stage_collected(alias_res, ¤t.aliases, &mut pending.aliases), + Self::stage_collected(func_res, ¤t.functions, &mut pending.functions), + Self::stage_collected(mod_res, ¤t.submodules, &mut pending.submodules), ])?; } + // Phase 3: commit because every item was resolved successfully + let current = self.current_module_mut(); + current.aliases.extend(pending.aliases); + current.functions.extend(pending.functions); + current.submodules.extend(pending.submodules); + Ok(()) } @@ -1070,26 +1078,34 @@ impl Scope { )) } - /// Inserts a successfully collected item into the current scope's map. + /// Stages a collected item, checking existing bindings and earlier staged names. /// /// ## Errors /// /// * [`Error::RedefinedItem`] An item with the same name is already defined in the target scope. /// * Propagates any upstream resolution error passed into the `res` argument. - fn insert_collected( + fn stage_collected( res: Result<(K, (V, Visibility)), Error>, - map: &mut HashMap, + existing: &HashMap, + pending: &mut HashMap, ) -> Result<(), Error> where K: Eq + std::hash::Hash + std::fmt::Display, { - res.and_then(|(k, v)| match map.entry(k) { - Entry::Occupied(entry) => Err(Error::RedefinedItem { - name: entry.key().to_string(), - }), - Entry::Vacant(entry) => { - entry.insert(v); - Ok(()) + res.and_then(|(k, v)| { + if existing.contains_key(&k) { + return Err(Error::RedefinedItem { + name: k.to_string(), + }); + } + match pending.entry(k) { + Entry::Occupied(entry) => Err(Error::RedefinedItem { + name: entry.key().to_string(), + }), + Entry::Vacant(entry) => { + entry.insert(v); + Ok(()) + } } }) } @@ -3850,3 +3866,121 @@ mod literal_tests { } } } + +#[cfg(test)] +mod transactional_use_tests { + use super::*; + use crate::parse::ParseFromStr; + + fn resolve(scope: &mut Scope, source: &str) -> Result<(), Error> { + let program = parse::Program::parse_from_str(source).unwrap(); + let parse::Item::Use(decl) = &program.items()[0] else { + panic!("expected use declaration"); + }; + scope.resolve_use(decl) + } + + fn scope() -> Scope { + let mut scope = Scope::default(); + scope + .enter_module(ModuleName::from_str_unchecked("source"), Visibility::Public) + .unwrap(); + scope.current_module_mut().aliases.insert( + AliasName::from_str_unchecked("Good"), + (ResolvedType::u32(), Visibility::Public), + ); + scope.current_module_mut().aliases.insert( + AliasName::from_str_unchecked("Secret"), + (ResolvedType::u16(), Visibility::Private), + ); + scope.exit_module(); + scope + } + + #[test] + fn failed_imports_preserve_the_complete_module_tree() { + for (source, expected) in [ + ( + "use crate::source::{Good, Missing};", + Error::UnresolvedItem { + name: "Missing".into(), + }, + ), + ( + "use crate::source::{Good, Secret};", + Error::PrivateItem { + name: "Secret".into(), + }, + ), + ( + "use crate::source::{Good as Same, Good as Same};", + Error::RedefinedItem { + name: "Same".into(), + }, + ), + ( + "use crate::source::{Good, Good as main};", + Error::MainCannotBeAlias, + ), + ( + "use crate::source::{Good, Good as Taken};", + Error::RedefinedItem { + name: "Taken".into(), + }, + ), + ] { + for nested in [false, true] { + let mut scope = scope(); + if nested { + scope + .enter_module( + ModuleName::from_str_unchecked("consumer"), + Visibility::Private, + ) + .unwrap(); + } + scope.current_module_mut().aliases.insert( + AliasName::from_str_unchecked("Taken"), + (ResolvedType::u8(), Visibility::Private), + ); + let before = scope.root.clone(); + let path = scope.module_path.clone(); + let error = resolve(&mut scope, source).expect_err(source); + assert_eq!(error.to_string(), expected.to_string(), "{source}"); + assert_eq!(scope.root, before, "failed import mutated scope: {source}"); + assert_eq!(scope.module_path, path, "{source}"); + resolve(&mut scope, "use crate::source::Good;").unwrap(); + } + } + } + + #[test] + fn successful_import_commits_all_namespaces_and_visibility() { + let mut scope = scope(); + let source = &mut scope + .root + .submodules + .get_mut(&ModuleName::from_str_unchecked("source")) + .unwrap() + .0; + source.submodules.insert( + ModuleName::from_str_unchecked("Good"), + (ModuleScope::default(), Visibility::Public), + ); + resolve(&mut scope, "pub use crate::source::Good as Renamed;").unwrap(); + assert_eq!( + scope + .root + .aliases + .get(&AliasName::from_str_unchecked("Renamed")), + Some(&(ResolvedType::u32(), Visibility::Public)) + ); + assert!(matches!( + scope + .root + .submodules + .get(&ModuleName::from_str_unchecked("Renamed")), + Some((_, Visibility::Public)) + )); + } +}