Revise desugaring following feedback

- We now consistently desugar an expect in the last position as
    `Void`. Regardless of the pattern. Desugaring to a boolean value is
    deemed too confusing.

  - This commit also removes the desugaring for let-binding. It's only
    ever allowed for _expect_ which then behaves like a side effect.

  - We also now allow tests to return either `Bool` or `Void`. A test
    that returns `Void` is treated the same as a test returning `True`.

crates/aiken-lang/src/ast.rs

@@ -1423,26 +1423,6 @@ impl UntypedPattern {
             is_record: false,
         }
     }
-
-    /// Returns Some(<bool>) if the pattern is a [`Boolean`] literal,
-    /// holding the target value. None if it isn't a bool pattern.
-    pub fn get_bool(&self) -> Option<bool> {
-        match self {
-            Self::Constructor {
-                module: None,
-                name,
-                constructor: (),
-                ..
-            } if name == "True" => Some(true),
-            Self::Constructor {
-                module: None,
-                name,
-                constructor: (),
-                ..
-            } if name == "False" => Some(false),
-            _ => None,
-        }
-    }
 }
 
 impl TypedPattern {

crates/aiken-lang/src/expr.rs

@@ -8,7 +8,7 @@ pub(crate) use crate::{
         TypedDataType, TypedIfBranch, TypedRecordUpdateArg, UnOp, UntypedArg,
         UntypedAssignmentKind, UntypedClause, UntypedIfBranch, UntypedRecordUpdateArg,
     },
-    builtins::{bool, void},
+    builtins::void,
     parser::token::Base,
     tipo::{
         check_replaceable_opaque_type, convert_opaque_type, lookup_data_type_by_tipo,
@@ -492,25 +492,6 @@ impl TypedExpr {
             location,
         }
     }
-
-    pub fn bool(value: bool, location: Span) -> Self {
-        TypedExpr::Var {
-            name: "Bool".to_string(),
-            constructor: ValueConstructor {
-                public: true,
-                variant: ValueConstructorVariant::Record {
-                    name: if value { "True" } else { "False" }.to_string(),
-                    arity: 0,
-                    field_map: None,
-                    location: Span::empty(),
-                    module: String::new(),
-                    constructors_count: 2,
-                },
-                tipo: bool(),
-            },
-            location,
-        }
-    }
 }
 
 // Represent how a function was written so that we can format it back.

crates/aiken-lang/src/parser/definition/snapshots/def_invalid_property_test.snap

@@ -52,14 +52,7 @@ Test(
         location: 0..26,
         name: "foo",
         public: false,
-        return_annotation: Some(
-            Constructor {
-                location: 0..39,
-                module: None,
-                name: "Bool",
-                arguments: [],
-            },
-        ),
+        return_annotation: None,
         return_type: (),
         end_position: 38,
         on_test_failure: FailImmediately,

crates/aiken-lang/src/parser/definition/snapshots/def_property_test.snap

@@ -37,14 +37,7 @@ Test(
         location: 0..28,
         name: "foo",
         public: false,
-        return_annotation: Some(
-            Constructor {
-                location: 0..41,
-                module: None,
-                name: "Bool",
-                arguments: [],
-            },
-        ),
+        return_annotation: None,
         return_type: (),
         end_position: 40,
         on_test_failure: FailImmediately,

crates/aiken-lang/src/parser/definition/snapshots/def_property_test_annotated_fuzzer.snap

@@ -44,14 +44,7 @@ Test(
         location: 0..26,
         name: "foo",
         public: false,
-        return_annotation: Some(
-            Constructor {
-                location: 0..39,
-                module: None,
-                name: "Bool",
-                arguments: [],
-            },
-        ),
+        return_annotation: None,
         return_type: (),
         end_position: 38,
         on_test_failure: FailImmediately,

crates/aiken-lang/src/parser/definition/snapshots/def_test.snap

@@ -13,14 +13,7 @@ Test(
         location: 0..10,
         name: "foo",
         public: false,
-        return_annotation: Some(
-            Constructor {
-                location: 0..23,
-                module: None,
-                name: "Bool",
-                arguments: [],
-            },
-        ),
+        return_annotation: None,
         return_type: (),
         end_position: 22,
         on_test_failure: FailImmediately,

crates/aiken-lang/src/parser/definition/snapshots/def_test_fail.snap

@@ -44,14 +44,7 @@ Test(
         location: 0..26,
         name: "invalid_inputs",
         public: false,
-        return_annotation: Some(
-            Constructor {
-                location: 0..61,
-                module: None,
-                name: "Bool",
-                arguments: [],
-            },
-        ),
+        return_annotation: None,
         return_type: (),
         end_position: 60,
         on_test_failure: SucceedEventually,

crates/aiken-lang/src/parser/definition/test.rs

@@ -45,7 +45,7 @@ pub fn parser() -> impl Parser<Token, ast::UntypedDefinition, Error = ParseError
                 end_position: span.end - 1,
                 name,
                 public: false,
-                return_annotation: Some(ast::Annotation::boolean(span)),
+                return_annotation: None,
                 return_type: (),
                 on_test_failure: fail.unwrap_or(OnTestFailure::FailImmediately),
             })

crates/aiken-lang/src/tests/check.rs

@@ -1009,6 +1009,107 @@ fn anonymous_function_dupicate_args() {
     ))
 }
 
+#[test]
+fn assignement_last_expr_when() {
+    let source_code = r#"
+        pub fn foo() {
+          let bar = None
+
+          when bar is {
+            Some(_) -> {
+              let wow = 1
+            }
+            None -> {
+              2
+            }
+          }
+        }
+    "#;
+
+    assert!(matches!(
+        check(parse(source_code)),
+        Err((_, Error::LastExpressionIsAssignment { .. }))
+    ))
+}
+
+#[test]
+fn assignement_last_expr_if_first_branch() {
+    let source_code = r#"
+        pub fn foo() {
+          if True {
+            let thing = 1
+          } else {
+            1
+          }
+        }
+    "#;
+
+    assert!(matches!(
+        check(parse(source_code)),
+        Err((_, Error::LastExpressionIsAssignment { .. }))
+    ))
+}
+
+#[test]
+fn assignement_last_expr_if_branches() {
+    let source_code = r#"
+        pub fn foo() {
+          if True {
+            2
+          } else if False {
+            let thing = 1
+          } else {
+            1
+          }
+        }
+    "#;
+
+    assert!(matches!(
+        check(parse(source_code)),
+        Err((_, Error::LastExpressionIsAssignment { .. }))
+    ))
+}
+
+#[test]
+fn assignement_last_expr_if_final_else() {
+    let source_code = r#"
+        pub fn foo() {
+          if True {
+            1
+          } else {
+            let thing = 1
+          }
+        }
+    "#;
+
+    assert!(matches!(
+        check(parse(source_code)),
+        Err((_, Error::LastExpressionIsAssignment { .. }))
+    ))
+}
+
+#[test]
+fn assignment_last_expr_logical_chain() {
+    let source_code = r#"
+        pub fn foo() -> Bool {
+            and {
+                expect 1 + 1 == 2,
+                True,
+                2 > 0,
+                or {
+                    expect True,
+                    False,
+                }
+            }
+        }
+    "#;
+
+    assert!(matches!(
+        check(parse(source_code)),
+        Err((_, Error::LastExpressionIsAssignment { .. }))
+    ))
+}
+
 #[test]
 fn if_scoping() {
     let source_code = r#"
@@ -1668,8 +1769,7 @@ fn pipe_wrong_arity_fully_saturated_return_fn() {
 fn fuzzer_ok_basic() {
     let source_code = r#"
         fn int() -> Fuzzer<Int> { todo }
-
-        test prop(n via int()) { todo }
+        test prop(n via int()) { True }
     "#;
 
     assert!(check(parse(source_code)).is_ok());
@@ -1679,8 +1779,7 @@ fn fuzzer_ok_basic() {
 fn fuzzer_ok_explicit() {
     let source_code = r#"
         fn int(prng: PRNG) -> Option<(PRNG, Int)> { todo }
-
-        test prop(n via int) { todo }
+        test prop(n via int) { Void }
     "#;
 
     assert!(check(parse(source_code)).is_ok());
@@ -1692,7 +1791,7 @@ fn fuzzer_ok_list() {
         fn int() -> Fuzzer<Int> { todo }
         fn list(a: Fuzzer<a>) -> Fuzzer<List<a>> { todo }
 
-        test prop(xs via list(int())) { todo }
+        test prop(xs via list(int())) { True }
     "#;
 
     assert!(check(parse(source_code)).is_ok());
@@ -2906,24 +3005,19 @@ fn recover_no_assignment_when_clause() {
     let source_code = r#"
         pub fn main(foo) {
             when foo is {
-                [] -> let bar = foo
+                [] -> Void
                 [x, ..] -> expect _: Int = x
             }
         }
     "#;
 
-    let (warnings, _) = check(parse(source_code)).unwrap();
-
-    assert!(matches!(
-        &warnings[..],
-        [Warning::UnusedVariable { name, .. }] if name == "bar",
-    ))
+    assert!(check(parse(source_code)).is_ok());
 }
 
 #[test]
 fn recover_no_assignment_fn_if_then_else() {
     let source_code = r#"
-        pub fn foo(weird_maths) -> Bool {
+        pub fn foo(weird_maths) -> Void {
             if weird_maths {
                 expect 1 == 2
             } else {
@@ -2936,20 +3030,38 @@ fn recover_no_assignment_fn_if_then_else() {
 }
 
 #[test]
-fn recover_no_assignment_logical_chain_op() {
+fn test_return_explicit_void() {
     let source_code = r#"
-        pub fn foo() -> Bool {
-            and {
-                expect 1 + 1 == 2,
-                True,
-                2 > 0,
-                or {
-                    expect True,
-                    False,
-                }
-            }
+        test foo() {
+            Void
         }
     "#;
 
     assert!(check(parse(source_code)).is_ok());
 }
+
+#[test]
+fn test_return_implicit_void() {
+    let source_code = r#"
+        test foo() {
+            let data: Data = 42
+            expect _: Int = data
+        }
+    "#;
+
+    assert!(check(parse(source_code)).is_ok());
+}
+
+#[test]
+fn test_return_illegal() {
+    let source_code = r#"
+        test foo() {
+            42
+        }
+    "#;
+
+    assert!(matches!(
+        check(parse(source_code)),
+        Err((_, Error::IllegalTestType { .. }))
+    ))
+}

crates/aiken-lang/src/tipo/error.rs

@@ -2,7 +2,7 @@ use super::Type;
 use crate::{
     ast::{Annotation, BinOp, CallArg, LogicalOpChainKind, Span, UntypedFunction, UntypedPattern},
     error::ExtraData,
-    expr::{self, AssignmentPattern, UntypedExpr},
+    expr::{self, AssignmentPattern, UntypedAssignmentKind, UntypedExpr},
     format::Formatter,
     levenshtein,
     pretty::Documentable,
@@ -472,6 +472,7 @@ If you really meant to return that last expression, try to replace it with the f
         location: Span,
         expr: expr::UntypedExpr,
         patterns: Vec1<AssignmentPattern>,
+        kind: UntypedAssignmentKind,
     },
 
     #[error(
@@ -1025,7 +1026,7 @@ The best thing to do from here is to remove it."#))]
     },
 
     #[error("I caught a test with too many arguments.\n")]
-    #[diagnostic(code("illegal::test_arity"))]
+    #[diagnostic(code("illegal::test::arity"))]
     #[diagnostic(help(
         "Tests are allowed to have 0 or 1 argument, but no more. Here I've found a test definition with {count} arguments. If you need to provide multiple values to a test, use a Record or a Tuple.",
     ))]
@@ -1035,6 +1036,18 @@ The best thing to do from here is to remove it."#))]
         location: Span,
     },
 
+    #[error("I caught a test with an illegal return type.\n")]
+    #[diagnostic(code("illegal::test::return"))]
+    #[diagnostic(help(
+        "Tests must return either {Bool} or {Void}. Note that `expect` assignment are implicitly typed {Void} (and thus, may be the last expression of a test).",
+        Bool = "Bool".if_supports_color(Stderr, |s| s.cyan()),
+        Void = "Void".if_supports_color(Stderr, |s| s.cyan()),
+    ))]
+    IllegalTestType {
+        #[label("expected Bool or Void")]
+        location: Span,
+    },
+
     #[error("I choked on a generic type left in an outward-facing interface.\n")]
     #[diagnostic(code("illegal::generic_in_abi"))]
     #[diagnostic(help(
@@ -1104,6 +1117,7 @@ impl ExtraData for Error {
             | Error::UpdateMultiConstructorType { .. }
             | Error::ValidatorImported { .. }
             | Error::IncorrectTestArity { .. }
+            | Error::IllegalTestType { .. }
             | Error::GenericLeftAtBoundary { .. }
             | Error::UnexpectedMultiPatternAssignment { .. }
             | Error::ExpectOnOpaqueType { .. }

crates/aiken-lang/src/tipo/expr.rs

@@ -1803,17 +1803,8 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
         let mut typed_expressions = vec![];
 
         for expression in expressions {
-            let typed_expression = if let Some(filler) = recover_from_no_assignment(
-                assert_no_assignment(&expression),
-                expression.location(),
-            )? {
-                TypedExpr::Sequence {
-                    location: expression.location(),
-                    expressions: vec![self.infer(expression)?, filler],
-                }
-            } else {
-                self.infer(expression)?
-            };
+            assert_no_assignment(&expression)?;
+            let typed_expression = self.infer(expression)?;
 
             self.unify(
                 bool(),
@@ -2545,24 +2536,32 @@ fn recover_from_no_assignment(
     result: Result<(), Error>,
     span: Span,
 ) -> Result<Option<TypedExpr>, Error> {
-    if let Err(Error::LastExpressionIsAssignment { patterns, .. }) = result {
-        match patterns.first().pattern.get_bool() {
-            Some(expected) if patterns.len() == 1 => Ok(Some(TypedExpr::bool(expected, span))),
-            _ => Ok(Some(TypedExpr::void(span))),
+    if let Err(Error::LastExpressionIsAssignment {
+        ref patterns,
+        ref kind,
+        ..
+    }) = result
+    {
+        if matches!(kind, AssignmentKind::Expect { ..} if patterns.len() == 1) {
+            return Ok(Some(TypedExpr::void(span)));
         }
-    } else {
+    }
+
     result.map(|()| None)
-    }
 }
 
 fn assert_no_assignment(expr: &UntypedExpr) -> Result<(), Error> {
     match expr {
         UntypedExpr::Assignment {
-            value, patterns, ..
+            value,
+            patterns,
+            kind,
+            ..
         } => Err(Error::LastExpressionIsAssignment {
             location: expr.location(),
             expr: *value.clone(),
             patterns: patterns.clone(),
+            kind: *kind,
         }),
         UntypedExpr::Trace { then, .. } => assert_no_assignment(then),
         UntypedExpr::Fn { .. }

crates/aiken-lang/src/tipo/infer.rs

@@ -392,12 +392,25 @@ fn infer_definition(
 
             let typed_f = infer_function(&f.into(), module_name, hydrators, environment, tracing)?;
 
-            environment.unify(
+            let is_bool = environment.unify(
                 typed_f.return_type.clone(),
                 builtins::bool(),
                 typed_f.location,
                 false,
-            )?;
+            );
+
+            let is_void = environment.unify(
+                typed_f.return_type.clone(),
+                builtins::void(),
+                typed_f.location,
+                false,
+            );
+
+            if is_bool.or(is_void).is_err() {
+                return Err(Error::IllegalTestType {
+                    location: typed_f.location,
+                });
+            }
 
             Ok(Definition::Test(Function {
                 doc: typed_f.doc,

crates/uplc/src/machine/eval_result.rs

@@ -39,7 +39,11 @@ impl EvalResult {
         } else {
             self.result.is_err()
                 || matches!(self.result, Ok(Term::Error))
-                || !matches!(self.result, Ok(Term::Constant(ref con)) if matches!(con.as_ref(), Constant::Bool(true)))
+                || !matches!(
+                  self.result,
+                  Ok(Term::Constant(ref con))
+                  if matches!(con.as_ref(), Constant::Bool(true)) || matches!(con.as_ref(), Constant::Unit)
+                )
         }
     }