diff --git a/crates/mq-lang/src/tarn/compiler.rs b/crates/mq-lang/src/tarn/compiler.rs index 127d8e5e1..fb81dad2e 100644 --- a/crates/mq-lang/src/tarn/compiler.rs +++ b/crates/mq-lang/src/tarn/compiler.rs @@ -26,7 +26,7 @@ pub(crate) enum CompileError { Unsupported(&'static str, TokenId), Module(ModuleError), AssignToImmutable(String, TokenId), - /// A module binds the same `let`/`var` name more than once. + /// A module binds the same `let`/`var`/`def` name more than once. DuplicateModuleBinding(String, TokenId), InvalidBytecode(String), /// `yield` outside any `def`/`fn` body. @@ -1166,6 +1166,7 @@ impl Compiler { } let mut deferred = Vec::with_capacity(program.len()); let mut defs: Program = Vec::new(); + Self::check_module_extensions(program)?; for node in program { self.current_token_id = node.token_id; match &node.expr { @@ -1353,6 +1354,7 @@ impl Compiler { } fn compile_body(&mut self, body: &Program) -> CompileResult<()> { + Self::check_module_extensions(body)?; for node in body { if let Expr::Def(ident, _, _) = &node.expr { self.scope_mut().declare(ident.name); @@ -2056,7 +2058,8 @@ impl Compiler { fn compile_module_directives(&mut self, modules: &Program, parent_module_path: &[Ident]) -> CompileResult<()> { let previously_pruning = std::mem::replace(&mut self.prune_module_functions, false); - let result = self.compile_discarding_in_module_path(modules, parent_module_path); + let result = Self::check_module_extensions(modules) + .and_then(|()| self.compile_discarding_in_module_path(modules, parent_module_path)); self.prune_module_functions = previously_pruning; result } @@ -2183,6 +2186,7 @@ impl Compiler { )); }; let module = self.load_module_or_reload(path)?; + Self::check_imported_module(&module)?; let module_alias = alias.map(|a| a.name).unwrap_or_else(|| Ident::new(&module.name)); let depth = self.scopes.len() - 1; self.push_module_barrier(depth, module_alias); @@ -2259,11 +2263,18 @@ impl Compiler { } self.push_module_barrier(depth, module_path[0]); + Self::check_module_extensions(program)?; let mut def_slots = FxHashMap::default(); let mut let_slots = FxHashMap::default(); for node in program { match &node.expr { Expr::Def(def_ident, _, _) => { + if def_slots.contains_key(&def_ident.name) || let_slots.contains_key(&def_ident.name) { + return Err(CompileError::DuplicateModuleBinding( + def_ident.name.to_string(), + node.token_id, + )); + } let slot = self.scope_mut().declare(def_ident.name); self.scope_mut().mark_immutable(slot); def_slots.insert(def_ident.name, slot); @@ -2274,7 +2285,7 @@ impl Compiler { names.sort(); names.dedup(); for name in names { - if let_slots.contains_key(&name) { + if let_slots.contains_key(&name) || def_slots.contains_key(&name) { return Err(CompileError::DuplicateModuleBinding(name.to_string(), node.token_id)); } let slot = self.scope_mut().declare(name); @@ -2352,6 +2363,62 @@ impl Compiler { Ok((rest, let_slots, barrier)) } + /// Rejects a member bound by more than one declaration of the same inline module in `program`, + /// descending into nested modules. + fn check_module_extensions(program: &Program) -> CompileResult<()> { + Self::check_module_members(program, &mut Vec::new(), &mut FxHashMap::default()) + } + + /// Rejects an imported file that binds a name more than once, or whose nested modules do. + fn check_imported_module(module: &Module) -> CompileResult<()> { + let mut bound = FxHashSet::default(); + for node in module.functions.iter().chain(&module.vars) { + for name in Self::bound_names(node) { + if !bound.insert(name) { + return Err(CompileError::DuplicateModuleBinding(name.to_string(), node.token_id)); + } + } + } + Self::check_module_extensions(&module.modules) + } + + fn check_module_members( + program: &Program, + path: &mut Vec, + seen: &mut FxHashMap, FxHashSet>, + ) -> CompileResult<()> { + for node in program { + let Expr::Module(ident, body) = &node.expr else { + continue; + }; + path.push(ident.name); + let bound = seen.entry(path.clone()).or_default(); + for name in body.iter().flat_map(|member| Self::bound_names(member)) { + if !bound.insert(name) { + return Err(CompileError::DuplicateModuleBinding(name.to_string(), node.token_id)); + } + } + Self::check_module_members(body, path, seen)?; + path.pop(); + } + Ok(()) + } + + /// Names a module member binds directly. + fn bound_names(member: &Node) -> Vec { + match &member.expr { + Expr::Def(ident, _, _) => vec![ident.name], + Expr::Let(pattern, _) | Expr::Var(pattern, _) => { + let mut names = Vec::new(); + collect_pattern_idents(pattern, &mut names); + names.sort(); + names.dedup(); + names + } + _ => Vec::new(), + } + } + /// Opens a module whose members are declared from here on. fn push_module_barrier(&mut self, depth: usize, root: Ident) { self.module_barriers.push(ModuleBarrier { diff --git a/crates/mq-lang/src/tarn/tests.rs b/crates/mq-lang/src/tarn/tests.rs index a59e13b66..11aeb90be 100644 --- a/crates/mq-lang/src/tarn/tests.rs +++ b/crates/mq-lang/src/tarn/tests.rs @@ -4774,6 +4774,70 @@ fn let_rebinding_does_not_change_existing_closures(#[case] code: &str, #[case] e assert_eq!(run(code), expected); } +#[rstest] +#[case::def_def("module m: def a(): 1; def a(): 2; end | m::a()", "a")] +#[case::def_then_let("module m: def a(): 1; let a = 2 end | m::a", "a")] +#[case::let_then_def("module m: let a = 2 | def a(): 1; end | m::a", "a")] +fn module_rejects_binding_the_same_name_twice_across_kinds(#[case] code: &str, #[case] name: &str) { + let token_arena = Shared::new(SharedCell::new(Arena::new(16))); + let program = crate::parse(code, Shared::clone(&token_arena)).unwrap(); + let result = compile_and_run(&program, token_arena); + assert!( + matches!(&result, Err(Error::Compile(compiler::CompileError::DuplicateModuleBinding(n, _))) if n == name), + "{result:?}" + ); +} + +#[rstest] +#[case::def_def("module m: def a(): 1; end | module m: def a(): 2; end | m::a()")] +#[case::let_def("module m: let a = 1 end | module m: def a(): 2; end | m::a")] +fn module_extension_rejects_existing_member(#[case] code: &str) { + let token_arena = Shared::new(SharedCell::new(Arena::new(16))); + let program = crate::parse(code, Shared::clone(&token_arena)).unwrap(); + let result = compile_and_run(&program, token_arena); + assert!( + matches!(&result, Err(Error::Compile(compiler::CompileError::DuplicateModuleBinding(name, _))) if name == "a"), + "{result:?}" + ); +} + +#[rstest] +#[case::nested( + "module outer: module inner: def a(): 1; end end | module outer: module inner: def a(): 2; end end | outer::inner::a()" +)] +#[case::in_function("def f(): module m: def a(): 1; end | module m: def a(): 2; end | m::a(); | f()")] +fn module_extension_rejects_duplicate_member_in_nested_scope(#[case] code: &str) { + let token_arena = Shared::new(SharedCell::new(Arena::new(16))); + let program = crate::parse(code, Shared::clone(&token_arena)).unwrap(); + let result = compile_and_run(&program, token_arena); + assert!( + matches!(&result, Err(Error::Compile(compiler::CompileError::DuplicateModuleBinding(name, _))) if name == "a"), + "{result:?}" + ); +} + +#[rstest] +#[case::def_def("def a(): 1; def a(): 2;", r#"import "lib" | lib::a()"#)] +#[case::def_let("def a(): 1; let a = 2", r#"import "lib" | lib::a"#)] +#[case::inline_module( + "module m: def a(): 1; end | module m: def a(): 2; end", + r#"import "lib" | lib::m::a()"# +)] +fn import_rejects_duplicate_binding(#[case] lib: &str, #[case] code: &str) { + let dir = tempfile::TempDir::new().unwrap(); + std::fs::write(dir.path().join("lib.mq"), lib).unwrap(); + let token_arena = Shared::new(SharedCell::new(Arena::new(100))); + let program = crate::parse(code, Shared::clone(&token_arena)).unwrap(); + let resolver = + crate::module::resolver::local_fs_resolver::LocalFsModuleResolver::new(Some(vec![dir.path().to_path_buf()])); + let result = compiler::compile_program(&program, token_arena, ModuleLoader::new(resolver)); + assert!( + matches!(&result, Err(compiler::CompileError::DuplicateModuleBinding(name, _)) if name == "a"), + "{:?}", + result.err() + ); +} + #[test] fn module_rejects_binding_the_same_name_twice() { let token_arena = Shared::new(SharedCell::new(Arena::new(16))); diff --git a/docs/books/src/reference/modules_and_imports.md b/docs/books/src/reference/modules_and_imports.md index 3ab5b21a7..471395c26 100644 --- a/docs/books/src/reference/modules_and_imports.md +++ b/docs/books/src/reference/modules_and_imports.md @@ -55,6 +55,14 @@ end Module-level `let` values are computed once per run. +A module may bind each name only once across its `let`, `var` and `def` declarations. Declaring `module m` again extends the existing module with the new members. A member name already bound by an earlier declaration is a compile error: + +```mq +module m: def a(): 1; def a(): 2; end # error: module binds "a" more than once +module m: def a(): 1; end | module m: def b(): 2; end # ok: m has a and b +module m: def a(): 1; end | module m: def a(): 2; end # error: module binds "a" more than once +``` + ## Import Loads a module from an external file using the syntax `import "module_path"`.