From edffd3493d89236af0e8e5e713b9e0b2d5035a22 Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Thu, 27 Aug 2026 10:55:04 +0200 Subject: [PATCH 1/2] fix: disallow inheriting tuples and funcs --- src/check/context/field/python.rs | 62 +++++++++++++++++++ src/check/context/parent/generic.rs | 20 ++++-- src/check/mod.rs | 6 +- tests/README.md | 15 +++++ tests/check/invalid.rs | 20 ++++-- tests/parse/invalid.rs | 12 +++- .../syntax/unrecognized_character.mamba | 1 + .../type/class/parent_function_type.mamba | 1 + .../type/class/parent_tuple_type.mamba | 1 + 9 files changed, 123 insertions(+), 15 deletions(-) create mode 100644 tests/resource/invalid/syntax/unrecognized_character.mamba create mode 100644 tests/resource/invalid/type/class/parent_function_type.mamba create mode 100644 tests/resource/invalid/type/class/parent_tuple_type.mamba diff --git a/src/check/context/field/python.rs b/src/check/context/field/python.rs index d500feb54..27d619018 100644 --- a/src/check/context/field/python.rs +++ b/src/check/context/field/python.rs @@ -81,3 +81,65 @@ impl From<&Expression> for GenericFields { } } } + +#[cfg(test)] +mod test { + use python_parser::ast::Statement; + + use crate::check::context::field::generic::GenericFields; + + fn assignment_targets(source: &str) -> (Vec, bool) { + let (_, statements) = + python_parser::file_input(python_parser::make_strspan(source)).expect("parse source"); + match statements.first().expect("non empty statements") { + Statement::Assignment(left, _) => (left.clone(), false), + Statement::TypedAssignment(left, _, _) => (left.clone(), true), + other => panic!("Not an assignment but {other:?}"), + } + } + + #[test] + fn single_name() { + let (left, _) = assignment_targets("x = 0"); + let fields = GenericFields::from((&left, &None)).fields; + + assert_eq!(fields.len(), 1); + let field = fields.iter().next().expect("field"); + assert_eq!(field.name, String::from("x")); + assert!(field.mutable); + assert!(field.ty.is_none()); + } + + #[test] + fn typed_single_name() { + let (left, _) = assignment_targets("x: int = 0"); + let fields = GenericFields::from(( + &left, + &Some(python_parser::ast::Expression::Name(String::from("int"))), + )) + .fields; + + assert_eq!(fields.len(), 1); + let field = fields.iter().next().expect("field"); + assert_eq!(field.name, String::from("x")); + assert!(field.ty.is_some()); + } + + #[test] + fn tuple_destructuring() { + // A single *parenthesized* tuple target (`(a, b) = 0, 0`) parses as one + // `Expression::TupleLiteral`, unlike the unparenthesized `a, b = 0, 0` (which the parser + // treats as two separate assignment targets, each going through the `Expression::Name` + // branch instead). A tuple target cannot be annotated in Python either way, so this + // always goes through the untyped `From<(&Vec, &Option)>` path. + let (left, _) = assignment_targets("(a, b) = 0, 0"); + let fields = GenericFields::from((&left, &None)).fields; + + assert_eq!(fields.len(), 2); + let mut names: Vec<&String> = fields.iter().map(|f| &f.name).collect(); + names.sort(); + assert_eq!(names, vec!["a", "b"]); + assert!(fields.iter().all(|f| !f.mutable)); + assert!(fields.iter().all(|f| f.ty.is_none())); + } +} diff --git a/src/check/context/parent/generic.rs b/src/check/context/parent/generic.rs index c9606d801..4fea22629 100644 --- a/src/check/context/parent/generic.rs +++ b/src/check/context/parent/generic.rs @@ -18,11 +18,21 @@ impl TryFrom<&AST> for GenericParent { fn try_from(ast: &AST) -> TypeResult { match &ast.node { - Node::Parent { ty, .. } => Ok(GenericParent { - is_py_type: false, - name: TrueName::try_from(ty)?, - pos: ast.pos, - }), + Node::Parent { ty, .. } => match &ty.node { + Node::TypeFun { .. } => { + let msg = "A class or trait cannot inherit from a function type"; + Err(vec![TypeErr::new(ty.pos, msg)]) + } + Node::TypeTup { .. } => { + let msg = "A class or trait cannot inherit from a tuple type"; + Err(vec![TypeErr::new(ty.pos, msg)]) + } + _ => Ok(GenericParent { + is_py_type: false, + name: TrueName::try_from(ty)?, + pos: ast.pos, + }), + }, _ => { let msg = format!("Expected parent, was {}", ast.node); Err(vec![TypeErr::new(ast.pos, &msg)]) diff --git a/src/check/mod.rs b/src/check/mod.rs index d1db85e80..0ebfe0f6b 100644 --- a/src/check/mod.rs +++ b/src/check/mod.rs @@ -97,7 +97,6 @@ mod tests { } #[test] - #[ignore] // not sure if the check stage should pass as of yet fn it_stmt_as_expression_int_and_str() { let src = "def a := if True then 10 else \"asdf\""; let ast = src.parse::().unwrap(); @@ -114,9 +113,6 @@ mod tests { panic!("Expected variabledef: {:?}", statements[0].node) }; - assert_eq!( - expr.ty, - Some(Name::from("Int").union(&Name::from("String"))) - ); + assert_eq!(expr.ty, Some(Name::from("Int").union(&Name::from("Str")))); } } diff --git a/tests/README.md b/tests/README.md index 5543b1e6f..d819a8251 100644 --- a/tests/README.md +++ b/tests/README.md @@ -272,6 +272,21 @@ gets disallowed in future (detecting the cycle and rejecting it at check time), should move to `tests/resource/invalid/type/class/` and get a `matches Err(_)` test_case instead of being deleted outright, so the "was silently accepted" behavior isn't lost from history. +## Fixed: a class could inherit from a function or tuple type + +`docs/spec/grammar.md`'s `class-def` rule restricts a parent to `type-not-fun`, but +`parse_parent` (`src/parse/class.rs`) parses it with the unrestricted `parse_type`, so the parser +alone never rejected `class Foo(a: Int): (Int) -> Int` or `class Foo(a: Int): (Int, Str)`. Before +this fix, `StringName`/`TrueName`'s `TryFrom<&AST>` impls (`check/name/string_name/generic.rs`, +`check/name/true_name/generic.rs`) both happily resolved a `Node::TypeFun`/`Node::TypeTup` parent +to a callable/tuple `StringName`, so both examples type-checked and generated `class +Foo(Callable[[int], int]): ... Callable.__init__(self)` / `class Foo(Tuple[int, str]): ... +Tuple.__init__(self)` — Python that would raise at runtime, since neither `typing.Callable` nor +`typing.Tuple` has a real `__init__` to call this way. `GenericParent::try_from` +(`check/context/parent/generic.rs`) now rejects a `Node::TypeFun`/`Node::TypeTup` parent directly +(shared by both class and trait parent resolution), before ever reaching `TrueName::try_from`; see +`tests/resource/invalid/type/class/parent_function_type.mamba` and `parent_tuple_type.mamba`. + ## Known checker gap: multi-variable builder/comprehension syntax `[(x, y) | x in a, y in b]`-style builders (list, set, and dict alike) only resolve the *first* diff --git a/tests/check/invalid.rs b/tests/check/invalid.rs index 2536096e8..aeea2d7ab 100644 --- a/tests/check/invalid.rs +++ b/tests/check/invalid.rs @@ -5,6 +5,7 @@ use tests_util::resource_content; use mamba::check::check_all; use mamba::check::result::TypeResult; +use mamba::common::result::WithSource; use mamba::parse::ast::AST; #[test_case("access", "access_list_with_string" => matches Err(_))] @@ -36,6 +37,8 @@ use mamba::parse::ast::AST; #[test_case("class", "one_tuple_not_assigned_to" => matches Err(_))] #[test_case("class", "reassign_to_unassigned_class_var" => matches Err(_))] #[test_case("class", "access_unassigned_class_var" => matches Err(_))] +#[test_case("class", "parent_function_type" => matches Err(_))] +#[test_case("class", "parent_tuple_type" => matches Err(_))] #[test_case("class", "same_parent_twice" => matches Err(_))] #[test_case("class", "top_level_class_not_assigned_to" => matches Err(_))] #[test_case("class", "wrong_generic_type" => matches Err(_))] @@ -139,19 +142,28 @@ use mamba::parse::ast::AST; fn fail_check(input_dir: &str, file_name: &str) -> TypeResult<()> { let file_name = format!("{file_name}.mamba"); let source = resource_content(false, &["type", input_dir], &file_name).unwrap(); + let path = PathBuf::new().join("type").join(input_dir).join(file_name); // except no parse error, but if we got one, print it. let ast = source .parse::() .map_err(|mut e| { - e.source = Some(source); - e.path = Some(PathBuf::new().join("type").join(input_dir).join(file_name)); + e.source = Some(source.clone()); + e.path = Some(path.clone()); println!("{e}"); e }) .unwrap(); - // expect error when type checking - check_all(&[ast]).map(|_| ()) + // expect error when type checking; print it, exercising Display/with_source + check_all(&[ast]).map(|_| ()).map_err(|errs| { + errs.into_iter() + .map(|e| { + let e = e.with_source(&Some(source.clone()), &Some(path.clone())); + println!("{e}"); + e + }) + .collect() + }) } diff --git a/tests/parse/invalid.rs b/tests/parse/invalid.rs index 723f333fd..e590a0b4a 100644 --- a/tests/parse/invalid.rs +++ b/tests/parse/invalid.rs @@ -1,7 +1,10 @@ +use std::path::PathBuf; + use mamba::parse::ast::AST; use test_case::test_case; use tests_util::resource_content; +use mamba::common::result::WithSource; use mamba::parse::result::ParseResult; #[test_case("assign_and_while"=> matches Err(_))] @@ -11,9 +14,16 @@ use mamba::parse::result::ParseResult; #[test_case("fin_without_def"=> matches Err(_))] #[test_case("class_parent_bad_token"=> matches Err(_))] #[test_case("class_parent_arg_bad_token"=> matches Err(_))] +#[test_case("unrecognized_character"=> matches Err(_))] fn syntax(file_name: &str) -> ParseResult<()> { let file_name = format!("{file_name}.mamba"); let source = resource_content(false, &["syntax"], &file_name).unwrap(); - source.parse::().map(|_| ()) + // expect a parse error; print it, exercising Display/with_source + source.parse::().map(|_| ()).map_err(|e| { + let path = PathBuf::new().join("syntax").join(&file_name); + let e = e.with_source(&Some(source), &Some(path)); + println!("{e}"); + Box::from(e) + }) } diff --git a/tests/resource/invalid/syntax/unrecognized_character.mamba b/tests/resource/invalid/syntax/unrecognized_character.mamba new file mode 100644 index 000000000..d50073e53 --- /dev/null +++ b/tests/resource/invalid/syntax/unrecognized_character.mamba @@ -0,0 +1 @@ +def a := 1 @ 2 diff --git a/tests/resource/invalid/type/class/parent_function_type.mamba b/tests/resource/invalid/type/class/parent_function_type.mamba new file mode 100644 index 000000000..a59923a59 --- /dev/null +++ b/tests/resource/invalid/type/class/parent_function_type.mamba @@ -0,0 +1 @@ +class Foo(a: Int): (Int) -> Int diff --git a/tests/resource/invalid/type/class/parent_tuple_type.mamba b/tests/resource/invalid/type/class/parent_tuple_type.mamba new file mode 100644 index 000000000..db9ae9160 --- /dev/null +++ b/tests/resource/invalid/type/class/parent_tuple_type.mamba @@ -0,0 +1 @@ +class Foo(a: Int): (Int, Str) From 3317b88f61e9661b6a23bea1989ebbbaf4560200 Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Thu, 27 Aug 2026 11:31:37 +0200 Subject: [PATCH 2/2] test: some tests for ordering in Py output Also get rid of ugly debug err msg return in main. --- src/check/name/mod.rs | 56 +++++++++++++++++++ src/check/name/string_name/mod.rs | 44 ++++++++++++++- src/lib.rs | 17 ++++++ src/main.rs | 23 +++----- tests/check/valid.rs | 2 + .../valid/class/field_deps_collections.mamba | 13 +++++ .../valid/class/field_deps_collections.py | 28 ++++++++++ .../valid/class/field_deps_operators.mamba | 20 +++++++ .../valid/class/field_deps_operators.py | 38 +++++++++++++ 9 files changed, 226 insertions(+), 15 deletions(-) create mode 100644 tests/resource/valid/class/field_deps_collections.mamba create mode 100644 tests/resource/valid/class/field_deps_collections.py create mode 100644 tests/resource/valid/class/field_deps_operators.mamba create mode 100644 tests/resource/valid/class/field_deps_operators.py diff --git a/src/check/name/mod.rs b/src/check/name/mod.rs index 80d661d95..6ae0a9607 100644 --- a/src/check/name/mod.rs +++ b/src/check/name/mod.rs @@ -511,8 +511,11 @@ mod tests { use crate::check::name::{ match_name, Any, ColType, Empty, IsSuperSet, Name, Nullable, TupleCallable, Union, }; + use std::convert::TryFrom; + use crate::check::result::TypeResult; use crate::common::position::Position; + use crate::parse::ast::AST; #[test] fn trim_super_nullable() { @@ -963,6 +966,59 @@ mod tests { assert!(collection_ty.is_err()); } + #[test] + fn col_type_of_undefined_class() { + let ctx = Context::try_from(Vec::::new().as_slice()).unwrap(); + let name = Name::from("TotallyUndefinedType"); + + let collection_ty = name.col_type(&ctx, Position::invisible()); + assert!(collection_ty.is_err()); + } + + #[test] + fn name_tuple_is_tuple_not_callable() { + let name = Name::tuple(&[Name::from(INT), Name::from(BOOL)]); + + assert_eq!(name.is_tuple(), HashSet::from([true])); + assert_eq!(name.is_callable(), HashSet::from([false])); + assert_eq!( + name.elements(Position::invisible()).unwrap(), + HashSet::from([vec![Name::from(INT), Name::from(BOOL)]]) + ); + assert!(name.args(Position::invisible()).is_err()); + assert!(name.ret_ty(Position::invisible()).is_err()); + } + + #[test] + fn name_callable_is_callable_not_tuple() { + let name = Name::callable(&[Name::from(INT)], &Name::from(BOOL)); + + assert_eq!(name.is_callable(), HashSet::from([true])); + assert_eq!(name.is_tuple(), HashSet::from([false])); + assert_eq!( + name.args(Position::invisible()).unwrap(), + HashSet::from([vec![Name::from(INT)]]) + ); + assert_eq!( + name.ret_ty(Position::invisible()).unwrap(), + HashSet::from([Name::from(BOOL)]) + ); + assert!(name.elements(Position::invisible()).is_err()); + } + + #[test] + fn col_type_iter_return_type_not_a_class() { + // `__iter__` is defined, but its return type is never itself defined as a class, so + // there is no `__next__` to look up on it. + let source = "class HasIter(x: Int) where\n def __iter__(self) -> Bogus := self\nend"; + let file = source.parse::().unwrap(); + let ctx = Context::try_from(vec![file].as_slice()).unwrap(); + let name = Name::from("HasIter"); + + let collection_ty = name.col_type(&ctx, Position::invisible()); + assert!(collection_ty.is_err()); + } + #[test] fn name_fold() { let int_name = Name::from(INT); diff --git a/src/check/name/string_name/mod.rs b/src/check/name/string_name/mod.rs index 0f57edb00..c82514119 100644 --- a/src/check/name/string_name/mod.rs +++ b/src/check/name/string_name/mod.rs @@ -295,7 +295,7 @@ mod test { use crate::check::context::clss::{HasParent, ANY, BOOL, INT, STRING}; use crate::check::context::LookupClass; use crate::check::name::string_name::StringName; - use crate::check::name::IsSuperSet; + use crate::check::name::{IsSuperSet, Name, TupleCallable}; use crate::common::position::Position; use crate::Context; @@ -364,4 +364,46 @@ mod test { .is_superset_of(&name_2, &ctx, Position::invisible()) .unwrap()) } + + #[test] + fn tuple_is_tuple_not_callable() { + let name = StringName::tuple(&[Name::from(INT), Name::from(BOOL)]); + + assert!(name.is_tuple()); + assert!(!name.is_callable()); + assert_eq!( + name.elements(Position::invisible()).unwrap(), + vec![Name::from(INT), Name::from(BOOL)] + ); + assert!(name.args(Position::invisible()).is_err()); + assert!(name.ret_ty(Position::invisible()).is_err()); + } + + #[test] + fn callable_is_callable_not_tuple() { + let name = StringName::callable(&[Name::from(INT)], &Name::from(BOOL)); + + assert!(name.is_callable()); + assert!(!name.is_tuple()); + assert_eq!( + name.args(Position::invisible()).unwrap(), + vec![Name::from(INT)] + ); + assert_eq!( + name.ret_ty(Position::invisible()).unwrap(), + Name::from(BOOL) + ); + assert!(name.elements(Position::invisible()).is_err()); + } + + #[test] + fn plain_name_is_neither_tuple_nor_callable() { + let name = StringName::from(INT); + + assert!(!name.is_tuple()); + assert!(!name.is_callable()); + assert!(name.elements(Position::invisible()).is_err()); + assert!(name.args(Position::invisible()).is_err()); + assert!(name.ret_ty(Position::invisible()).is_err()); + } } diff --git a/src/lib.rs b/src/lib.rs index e7975fd2f..af56664bd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -196,3 +196,20 @@ pub(crate) fn check_sources( trace!("Checked {} files", typed_ast.len()); Ok((ctx, typed_ast)) } + +#[cfg(test)] +mod test { + use std::path::Path; + + use crate::{transpile_dir, Arguments}; + + #[test] + fn transpile_dir_reports_missing_source_directory() { + let dir = Path::new("/does/not/exist/anywhere"); + let result = transpile_dir(dir, None, None, &Arguments::default()); + + let errs = result.unwrap_err(); + assert_eq!(errs.len(), 1); + assert!(errs[0].contains("Source directory does not exist")); + } +} diff --git a/src/main.rs b/src/main.rs index 45e5825fc..a94935b5b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,13 +8,13 @@ use mamba::{transpile_dir, Arguments}; const VERSION: &str = env!("CARGO_PKG_VERSION"); -pub fn main() -> Result<(), String> { +pub fn main() { #[cfg(windows)] ansi_term::enable_ansi_support().unwrap(); // if error, then defer printing error to clap let Ok(cli_input) = Cli::try_parse().map_err(|e| e.print()) else { - return Err(String::new()); + std::process::exit(1); }; loggerv::Logger::new() @@ -44,23 +44,18 @@ pub fn main() -> Result<(), String> { }; info!("Mamba 🐍 {VERSION}"); - let current_dir = std::env::current_dir().map_err(|err| { + let current_dir = std::env::current_dir().unwrap_or_else(|err| { error!("Error while finding current directory: {err}"); - format!("Error while finding current directory: {err}") - })?; + std::process::exit(1); + }); - transpile_dir( + if let Err(errors) = transpile_dir( ¤t_dir, cli_input.input.as_deref(), cli_input.output.as_deref(), &arguments, - ) - .map_err(|errors| { + ) { errors.iter().unique().for_each(|msg| eprintln!("{msg}")); - match errors.first() { - Some(msg) => msg.clone(), - None => String::new(), - } - }) - .map(|_| ()) + std::process::exit(1); + } } diff --git a/tests/check/valid.rs b/tests/check/valid.rs index a6fdc459d..faec4df84 100644 --- a/tests/check/valid.rs +++ b/tests/check/valid.rs @@ -14,6 +14,8 @@ use test_case::test_case; #[test_case("class", "cyclic_field_dependency")] #[test_case("class", "generics")] #[test_case("class", "field_depends_on_later_field")] +#[test_case("class", "field_deps_operators")] +#[test_case("class", "field_deps_collections")] #[test_case("class", "assign_types_nested")] #[test_case("class", "class_super_one_line_init")] #[test_case("class", "assign_types_double_nested")] diff --git a/tests/resource/valid/class/field_deps_collections.mamba b/tests/resource/valid/class/field_deps_collections.mamba new file mode 100644 index 000000000..fadc2c505 --- /dev/null +++ b/tests/resource/valid/class/field_deps_collections.mamba @@ -0,0 +1,13 @@ +class Deps2(x: Int) where + def gt: Bool := self.x > self.later + def leq: Bool := self.x <= self.later + def sq: Float := sqrt self.later + def tup: (Int, Int) := (self.x, self.later) + def st: Set[Int] := { self.x, self.later } + def dc: Dict[Int, Int] := { self.x => self.later } + def ifv: Int := if self.gt then self.x else self.later + def later: Int := self.x * 2 +end + +def d := Deps2(3) +print(d.gt) diff --git a/tests/resource/valid/class/field_deps_collections.py b/tests/resource/valid/class/field_deps_collections.py new file mode 100644 index 000000000..a29564af1 --- /dev/null +++ b/tests/resource/valid/class/field_deps_collections.py @@ -0,0 +1,28 @@ +import math +from typing import Tuple + + +class Deps2: + gt: bool = None + leq: bool = None + sq: float = None + tup: Tuple[int, int] = None + st: set[int] = None + dc: dict[int, int] = None + ifv: int = None + later: int = None + + def __init__(self, x: int): + self.x = x + self.later = self.x * 2 + self.gt = self.x > self.later + self.leq = self.x <= self.later + self.sq = math.sqrt(self.later) + self.tup = (self.x, self.later) + self.st = {self.x, self.later} + self.dc = {self.x: self.later} + self.ifv = self.x if self.gt else self.later + + +d: Deps2 = Deps2(3) +print(d.gt) diff --git a/tests/resource/valid/class/field_deps_operators.mamba b/tests/resource/valid/class/field_deps_operators.mamba new file mode 100644 index 000000000..6b47e1fa7 --- /dev/null +++ b/tests/resource/valid/class/field_deps_operators.mamba @@ -0,0 +1,20 @@ +class Deps(x: Int) where + def cmp: Bool := self.x < self.later + def eqcmp: Bool := self.x = self.later + def geqcmp: Bool := self.x >= self.later + def logic: Bool := self.cmp or self.eqcmp + def logic2: Bool := self.cmp and self.eqcmp + def notv: Bool := not self.cmp + def negv: Int := -self.later + def posv: Int := +self.later + def subv: Int := self.x - self.later + def modv: Int := self.x mod self.later + def powv: Int := self.x ^ self.later + def divv: Float := self.x / self.later + def fdivv: Int := self.x // self.later + def inv: Bool := self.x in [self.later] + def later: Int := self.x * 2 +end + +def d := Deps(3) +print(d.cmp) diff --git a/tests/resource/valid/class/field_deps_operators.py b/tests/resource/valid/class/field_deps_operators.py new file mode 100644 index 000000000..3b9195632 --- /dev/null +++ b/tests/resource/valid/class/field_deps_operators.py @@ -0,0 +1,38 @@ +class Deps: + cmp: bool = None + eqcmp: bool = None + geqcmp: bool = None + logic: bool = None + logic2: bool = None + notv: bool = None + negv: int = None + posv: int = None + subv: int = None + modv: int = None + powv: int = None + divv: float = None + fdivv: int = None + inv: bool = None + later: int = None + + def __init__(self, x: int): + self.x = x + self.later = self.x * 2 + self.cmp = self.x < self.later + self.eqcmp = self.x == self.later + self.geqcmp = self.x >= self.later + self.logic = self.cmp or self.eqcmp + self.logic2 = self.cmp and self.eqcmp + self.notv = not self.cmp + self.negv = -self.later + self.posv = +self.later + self.subv = self.x - self.later + self.modv = self.x % self.later + self.powv = self.x ** self.later + self.divv = self.x / self.later + self.fdivv = self.x // self.later + self.inv = self.x in [self.later] + + +d: Deps = Deps(3) +print(d.cmp)