From 71ff3123068e991bb49e6e923ac86d1f14926dd7 Mon Sep 17 00:00:00 2001 From: Owais Jamil Date: Wed, 3 Jun 2026 20:23:09 -0500 Subject: [PATCH] feat: resolve constructors and pattern scopes --- crates/core/src/ast.rs | 4 +- crates/core/src/resolve.rs | 300 +++++++++++++++--- .../src/internal/tasks/06_pattern_matching.md | 14 +- fixtures/resolve/pattern_bindings.gleam | 19 ++ 4 files changed, 292 insertions(+), 45 deletions(-) create mode 100644 fixtures/resolve/pattern_bindings.gleam diff --git a/crates/core/src/ast.rs b/crates/core/src/ast.rs index 5011579..21098c7 100644 --- a/crates/core/src/ast.rs +++ b/crates/core/src/ast.rs @@ -732,7 +732,9 @@ impl AstBuilder<'_> { fn name_field(&self, node: Node<'_>, field: &str) -> Result, Diagnostics> { node.child_by_field_name(field) .map(|child| match child.kind() { - "identifier" | "type_identifier" | "label" | "module" | "discard" => Ok(self.name(child)), + "identifier" | "type_identifier" | "constructor_name" | "label" | "module" | "discard" => { + Ok(self.name(child)) + } _ => Err(vec![self.unsupported(child)]), }) .transpose() diff --git a/crates/core/src/resolve.rs b/crates/core/src/resolve.rs index 00471b1..a226a0c 100644 --- a/crates/core/src/resolve.rs +++ b/crates/core/src/resolve.rs @@ -1,12 +1,8 @@ use std::collections::HashMap; -use crate::{ - ast::{self, Declaration, Expression, Pattern, Statement, UnqualifiedImportKind}, - diagnostic::{Diagnostic, DiagnosticCode, Diagnostics, Label}, - parse, - project::Project, - source::Span, -}; +use crate::ast::{self, Declaration, Expression, Pattern, Statement, UnqualifiedImportKind}; +use crate::diagnostic::{Diagnostic, DiagnosticCode, Diagnostics, Label}; +use crate::{parse, project::Project, source::Span}; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct SymbolId(pub u32); @@ -96,7 +92,7 @@ pub struct ResolvedProject { #[derive(Debug, Clone, PartialEq, Eq, Default)] struct ModuleInterface { - values: HashMap, + members: HashMap<(Namespace, String), ModuleMember>, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -384,7 +380,7 @@ impl Resolver { && let Some(module) = self.lookup(scope, Namespace::Module, &record.text) && let SymbolKind::Import { module: module_name } = &self.symbols[module.0 as usize].kind.clone() { - let symbol = self.resolve_project_member(module_name, &field_access.field); + let symbol = self.resolve_project_member(module_name, Namespace::Value, &field_access.field); self.references.push(ResolvedReference { name: record.clone(), target: ReferenceTarget::QualifiedMember { module, member: field_access.field.clone(), symbol }, @@ -395,11 +391,13 @@ impl Resolver { self.resolve_expression(scope, &field_access.record); } - fn resolve_project_member(&mut self, module_name: &str, member: &ast::Name) -> Option { + fn resolve_project_member( + &mut self, module_name: &str, namespace: Namespace, member: &ast::Name, + ) -> Option { let Some(interface) = self.project_modules.get(module_name) else { return None; }; - let Some(found) = interface.values.get(&member.text).cloned() else { + let Some(found) = interface.members.get(&(namespace, member.text.clone())).cloned() else { self.diagnostics.push( Diagnostic::new( DiagnosticCode::ResolveError, @@ -423,33 +421,42 @@ impl Resolver { } fn bind_pattern(&mut self, scope: ScopeId, pattern: &Pattern, kind: SymbolKind) { + let mut names = HashMap::new(); + self.bind_pattern_inner(scope, pattern, kind, &mut names); + } + + fn bind_pattern_inner( + &mut self, scope: ScopeId, pattern: &Pattern, kind: SymbolKind, names: &mut HashMap, + ) { match pattern { - Pattern::Name(name) => { - self.define(scope, name, Namespace::Value, kind); - } + Pattern::Name(name) => self.define_pattern_name(scope, name, kind, names), Pattern::Tuple(tuple) => { for element in &tuple.elements { - self.bind_pattern(scope, element, kind.clone()); + self.bind_pattern_inner(scope, element, kind.clone(), names); } } Pattern::List(list) => { for element in &list.elements { - self.bind_pattern(scope, element, kind.clone()); + self.bind_pattern_inner(scope, element, kind.clone(), names); } if let Some(ast::ListPatternTail::Name(name)) = &list.tail { - self.define(scope, name, Namespace::Value, kind.clone()); + self.define_pattern_name(scope, name, kind.clone(), names); } } Pattern::Constructor(constructor) => { + self.resolve_constructor_pattern(scope, constructor); for argument in &constructor.arguments { + self.resolve_record_pattern_field(scope, argument); if let Some(pattern) = &argument.pattern { - self.bind_pattern(scope, pattern, kind.clone()); + self.bind_pattern_inner(scope, pattern, kind.clone(), names); + } else if let Some(label) = &argument.label { + self.define_pattern_name(scope, label, kind.clone(), names); } } } Pattern::Alias(alias) => { - self.bind_pattern(scope, &alias.pattern, kind.clone()); - self.define(scope, &alias.alias, Namespace::Value, kind); + self.bind_pattern_inner(scope, &alias.pattern, kind.clone(), names); + self.define_pattern_name(scope, &alias.alias, kind, names); } Pattern::Discard(_) | Pattern::Integer(_) @@ -462,6 +469,83 @@ impl Resolver { } } + fn define_pattern_name( + &mut self, scope: ScopeId, name: &ast::Name, kind: SymbolKind, names: &mut HashMap, + ) { + if let Some(previous) = names.insert(name.text.clone(), name.span) { + self.diagnostics.push( + Diagnostic::new( + DiagnosticCode::ResolveError, + format!("duplicate pattern binding `{}`", name.text), + ) + .with_label(Label::primary(name.span, "bound again here")) + .with_label(Label::primary(previous, "previously bound here")), + ); + return; + } + self.define(scope, name, Namespace::Value, kind); + } + + fn resolve_constructor_pattern(&mut self, scope: ScopeId, constructor: &ast::ConstructorPattern) { + match &constructor.constructor { + ast::ConstructorName::Local(name) => { + self.resolve_pattern_symbol(scope, Namespace::Constructor, name, "constructor"); + } + ast::ConstructorName::Remote { module, name, .. } => { + let Some(module_symbol) = self.lookup(scope, Namespace::Module, &module.text) else { + self.diagnostics.push( + Diagnostic::new( + DiagnosticCode::ResolveError, + format!("unknown module `{}`", module.text), + ) + .with_label(Label::primary(module.span, "module not found")), + ); + return; + }; + if let SymbolKind::Import { module: module_name } = &self.symbols[module_symbol.0 as usize].kind.clone() + { + let symbol = self.resolve_project_member(module_name, Namespace::Constructor, name); + self.references.push(ResolvedReference { + name: module.clone(), + target: ReferenceTarget::QualifiedMember { + module: module_symbol, + member: name.clone(), + symbol, + }, + }); + } + } + } + } + + fn resolve_record_pattern_field(&mut self, scope: ScopeId, argument: &ast::RecordPatternArgument) { + if let Some(label) = &argument.label { + self.resolve_pattern_symbol(scope, Namespace::Field, label, "field"); + } + } + + fn resolve_pattern_symbol( + &mut self, scope: ScopeId, namespace: Namespace, name: &ast::Name, label: &str, + ) -> Option { + match self.lookup(scope, namespace, &name.text) { + Some(symbol) => { + if let SymbolKind::Imported { module, .. } = &self.symbols[symbol.0 as usize].kind.clone() { + self.resolve_project_member(module, namespace, name); + } + self.references + .push(ResolvedReference { name: name.clone(), target: ReferenceTarget::Symbol(symbol) }); + Some(symbol) + } + None => { + self.diagnostics.push( + Diagnostic::new(DiagnosticCode::ResolveError, format!("unknown {label} `{}`", name.text)) + .with_label(Label::primary(name.span, format!("{label} not found"))), + ); + None + } + } + } + fn resolve_name(&mut self, scope: ScopeId, name: &ast::Name) { match self .lookup(scope, Namespace::Value, &name.text) @@ -519,17 +603,46 @@ impl Resolver { } fn module_interface(module: &ast::Module) -> ModuleInterface { - let values = module - .functions - .iter() - .map(|function| { - ( - function.name.text.clone(), - ModuleMember { public: function.public, span: function.name.span }, - ) - }) - .collect(); - ModuleInterface { values } + let mut members = HashMap::new(); + + for function in &module.functions { + members.insert( + (Namespace::Value, function.name.text.clone()), + ModuleMember { public: function.public, span: function.name.span }, + ); + } + + for declaration in &module.declarations { + match declaration { + Declaration::TypeDefinition(raw) => { + let public = raw.source.trim_start().starts_with("pub "); + if let Some(name) = type_name(&raw.source) { + members.insert((Namespace::Type, name.into()), ModuleMember { public, span: raw.span }); + } + for constructor in constructors(&raw.source) { + members.insert( + (Namespace::Constructor, constructor.into()), + ModuleMember { public, span: raw.span }, + ); + } + for field in fields(&raw.source) { + members.insert( + (Namespace::Field, field.into()), + ModuleMember { public, span: raw.span }, + ); + } + } + Declaration::TypeAlias(raw) => { + let public = raw.source.trim_start().starts_with("pub "); + if let Some(name) = type_name(&raw.source) { + members.insert((Namespace::Type, name.into()), ModuleMember { public, span: raw.span }); + } + } + _ => {} + } + } + + ModuleInterface { members } } fn raw_name(raw: &ast::RawSyntax, name: &str) -> ast::Name { @@ -571,16 +684,13 @@ fn fields(source: &str) -> Vec<&str> { #[cfg(test)] mod tests { - use std::{fs, path::Path}; - - use tempfile::tempdir; - + use super::*; use crate::{ ast, parse, project, source::{SourceFile, SourceFileId}, }; - - use super::*; + use std::{fs, path::Path}; + use tempfile::tempdir; fn resolve_source(source: &str) -> Result { let source = SourceFile::new(SourceFileId(0), source); @@ -705,6 +815,61 @@ fn user(value) { value } assert!(resolved.references.iter().any(|reference| reference.name.text == "id")); } + #[test] + fn resolves_nested_pattern_bindings_constructors_fields_and_guards() { + let resolved = resolve_source(include_str!("../../../fixtures/resolve/pattern_bindings.gleam")) + .expect("resolve pattern names"); + + assert!(resolved.references.iter().any(|reference| reference.name.text == "Ok")); + assert!( + resolved + .references + .iter() + .any(|reference| reference.name.text == "Person") + ); + assert!(resolved.references.iter().any(|reference| reference.name.text == "age")); + + let value_references = resolved + .references + .iter() + .filter(|reference| reference.name.text == "value") + .count(); + assert_eq!(value_references, 2); + } + + #[test] + fn reports_duplicate_bindings_in_one_pattern() { + let diagnostics = resolve_source("fn main(pair) { case pair { #(x, x) -> x } }") + .expect_err("duplicate pattern binding should fail"); + + assert!( + diagnostics + .iter() + .any(|diagnostic| diagnostic.message.contains("duplicate pattern binding `x`")) + ); + } + + #[test] + fn reports_unknown_constructors_and_fields_in_patterns() { + let diagnostics = resolve_source( + r#"pub type Person { Person(name: String) } +fn main(person) { case person { Missing(age: value) -> value } } +"#, + ) + .expect_err("unknown pattern names should fail"); + + assert!( + diagnostics + .iter() + .any(|diagnostic| diagnostic.message.contains("unknown constructor `Missing`")) + ); + assert!( + diagnostics + .iter() + .any(|diagnostic| diagnostic.message.contains("unknown field `age`")) + ); + } + #[test] fn reports_ambiguous_unqualified_imports() { let diagnostics = resolve_source("import one.{id}\nimport two.{id}\nfn main() { id(1) }") @@ -761,4 +926,65 @@ fn user(value) { value } assert_eq!(resolved.modules.len(), 2); } + + #[test] + fn resolves_qualified_constructor_patterns_across_project_modules() { + let dir = tempdir().expect("tempdir"); + write( + &dir.path().join("gleam.toml"), + "name = \"sample\"\nversion = \"1.0.0\"\n", + ); + write(&dir.path().join("src/app.gleam"), "pub type Boxed { Boxed(Int) }\n"); + write( + &dir.path().join("src/main.gleam"), + "import app\nfn main(value) { case value { app.Boxed(inner) -> inner } }\n", + ); + let project = project::load_project(dir.path()).expect("load project"); + + let resolved = resolve_project(&project).expect("resolve project"); + + assert_eq!(resolved.modules.len(), 2); + } + + #[test] + fn resolves_unqualified_imported_constructor_patterns() { + let dir = tempdir().expect("tempdir"); + write( + &dir.path().join("gleam.toml"), + "name = \"sample\"\nversion = \"1.0.0\"\n", + ); + write(&dir.path().join("src/app.gleam"), "pub type Boxed { Boxed(Int) }\n"); + write( + &dir.path().join("src/main.gleam"), + "import app.{type Boxed}\nfn main(value) { case value { Boxed(inner) -> inner } }\n", + ); + let project = project::load_project(dir.path()).expect("load project"); + + let resolved = resolve_project(&project).expect("resolve project"); + + assert_eq!(resolved.modules.len(), 2); + } + + #[test] + fn rejects_private_qualified_constructor_patterns_across_project_modules() { + let dir = tempdir().expect("tempdir"); + write( + &dir.path().join("gleam.toml"), + "name = \"sample\"\nversion = \"1.0.0\"\n", + ); + write(&dir.path().join("src/app.gleam"), "type Boxed { Boxed(Int) }\n"); + write( + &dir.path().join("src/main.gleam"), + "import app\nfn main(value) { case value { app.Boxed(inner) -> inner } }\n", + ); + let project = project::load_project(dir.path()).expect("load project"); + + let diagnostics = resolve_project(&project).expect_err("private constructor should fail"); + + assert!( + diagnostics + .iter() + .any(|diagnostic| diagnostic.message.contains("member `Boxed` is private")) + ); + } } diff --git a/docs/src/internal/tasks/06_pattern_matching.md b/docs/src/internal/tasks/06_pattern_matching.md index 5851d13..71a0855 100644 --- a/docs/src/internal/tasks/06_pattern_matching.md +++ b/docs/src/internal/tasks/06_pattern_matching.md @@ -37,17 +37,17 @@ generation. ### Name resolution -- [ ] Bind names introduced by tuple, list, record, constructor, nested, and +- [x] Bind names introduced by tuple, list, record, constructor, nested, and `let assert` patterns. -- [ ] Resolve constructor names in patterns separately from variable bindings. -- [ ] Resolve record fields used in record patterns. -- [ ] Resolve qualified and imported constructors in patterns, including public +- [x] Resolve constructor names in patterns separately from variable bindings. +- [x] Resolve record fields used in record patterns. +- [x] Resolve qualified and imported constructors in patterns, including public and private visibility checks. -- [ ] Reject duplicate variable bindings in one pattern where Gleam disallows +- [x] Reject duplicate variable bindings in one pattern where Gleam disallows them. -- [ ] Ensure branch-local bindings are visible to the guard and branch body, but +- [x] Ensure branch-local bindings are visible to the guard and branch body, but not outside the branch. -- [ ] Add resolver fixtures for shadowing, imported constructors, branch-local +- [x] Add resolver fixtures for shadowing, imported constructors, branch-local names, and invalid duplicate bindings. ### Type checking and diagnostics diff --git a/fixtures/resolve/pattern_bindings.gleam b/fixtures/resolve/pattern_bindings.gleam new file mode 100644 index 0000000..6646204 --- /dev/null +++ b/fixtures/resolve/pattern_bindings.gleam @@ -0,0 +1,19 @@ +pub type Outcome { + Ok(Bool) + Error(String) +} + +pub type Person { + Person(name: String, age: Int) +} + +fn main(result, person) { + case result { + Ok(value) if value -> value + Error(reason) -> False + } + + case person { + Person(name:, age: _) -> name + } +} -- 2.51.2