Fix: Disallow inheriting from tuples and functions

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<python_parser::ast::Expression>, 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<Expression>, &Option<Expression>)>` 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()));
+    }
+}

src/check/context/parent/generic.rs

@@ -18,11 +18,21 @@ impl TryFrom<&AST> for GenericParent {
 
     fn try_from(ast: &AST) -> TypeResult<GenericParent> {
         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)])

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::<AST>().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"))));
     }
 }

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::<AST>::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::<AST>().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);

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());
+    }
 }

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"));
+    }
+}

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(
         &current_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);
+    }
 }

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*

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::<AST>()
         .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()
+    })
 }

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")]