Merge pull request #5287 from roc-lang/i5236

Catch non-recursive function arity mismatches during typechecking

crates/compiler/constrain/src/expr.rs

@@ -1782,7 +1782,7 @@ fn constrain_function_def(
 
             let signature_index = constraints.push_type(types, signature);
 
-            let (arg_types, signature_closure_type, ret_type) = match types[signature] {
+            let (arg_types, _signature_closure_type, ret_type) = match types[signature] {
                 TypeTag::Function(signature_closure_type, ret_type) => (
                     types.get_type_arguments(signature),
                     signature_closure_type,
@@ -1888,13 +1888,12 @@ fn constrain_function_def(
                 delayed_is_open_constraints: vec![],
             };
             let mut vars = Vec::with_capacity(argument_pattern_state.vars.capacity() + 1);
-            let ret_var = function_def.return_type;
             let closure_var = function_def.closure_type;
 
             let ret_type_index = constraints.push_type(types, ret_type);
 
-            vars.push(ret_var);
-            vars.push(closure_var);
+            vars.push(function_def.return_type);
+            vars.push(function_def.closure_type);
 
             let mut def_pattern_state = PatternState::default();
 
@@ -1913,21 +1912,22 @@ fn constrain_function_def(
 
             // TODO see if we can get away with not adding this constraint at all
             def_pattern_state.vars.push(expr_var);
-            let annotation_expected = FromAnnotation(
-                loc_pattern.clone(),
-                arity,
-                AnnotationSource::TypedBody {
-                    region: annotation.region,
-                },
-                signature_index,
-            );
+            let annotation_expected = {
+                constraints.push_expected_type(FromAnnotation(
+                    loc_pattern.clone(),
+                    arity,
+                    AnnotationSource::TypedBody {
+                        region: annotation.region,
+                    },
+                    signature_index,
+                ))
+            };
 
             {
                 let expr_type_index = constraints.push_variable(expr_var);
-                let expected_index = constraints.push_expected_type(annotation_expected);
                 def_pattern_state.constraints.push(constraints.equal_types(
                     expr_type_index,
-                    expected_index,
+                    annotation_expected,
                     Category::Storage(std::file!(), std::line!()),
                     Region::span_across(&annotation.region, &loc_body_expr.region),
                 ));
@@ -1955,8 +1955,8 @@ fn constrain_function_def(
                 &mut vars,
             );
 
-            let annotation_expected = constraints.push_expected_type(FromAnnotation(
-                loc_pattern.clone(),
+            let return_type_annotation_expected = constraints.push_expected_type(FromAnnotation(
+                loc_pattern,
                 arity,
                 AnnotationSource::TypedBody {
                     region: annotation.region,
@@ -1964,31 +1964,34 @@ fn constrain_function_def(
                 ret_type_index,
             ));
 
-            let ret_constraint = constrain_expr(
-                types,
-                constraints,
-                env,
-                loc_body_expr.region,
-                &loc_body_expr.value,
-                annotation_expected,
-            );
-            let ret_constraint = attach_resolution_constraints(constraints, env, ret_constraint);
+            let solved_fn_type = {
+                // TODO(types-soa) optimize for Variable
+                let pattern_types = types.from_old_type_slice(
+                    function_def.arguments.iter().map(|a| Type::Variable(a.0)),
+                );
+                let lambda_set = types.from_old_type(&Type::Variable(function_def.closure_type));
+                let ret_var = types.from_old_type(&Type::Variable(function_def.return_type));
+
+                let fn_type = types.function(pattern_types, lambda_set, ret_var);
+                constraints.push_type(types, fn_type)
+            };
+
+            let ret_constraint = {
+                let con = constrain_expr(
+                    types,
+                    constraints,
+                    env,
+                    loc_body_expr.region,
+                    &loc_body_expr.value,
+                    return_type_annotation_expected,
+                );
+                attach_resolution_constraints(constraints, env, con)
+            };
 
             vars.push(expr_var);
+
             let defs_constraint = constraints.and_constraint(argument_pattern_state.constraints);
 
-            let signature_closure_type = {
-                let signature_closure_type_index =
-                    constraints.push_type(types, signature_closure_type);
-                constraints.push_expected_type(Expected::FromAnnotation(
-                    loc_pattern,
-                    arity,
-                    AnnotationSource::TypedBody {
-                        region: annotation.region,
-                    },
-                    signature_closure_type_index,
-                ))
-            };
             let cons = [
                 constraints.let_constraint(
                     [],
@@ -1999,14 +2002,24 @@ fn constrain_function_def(
                     // This is a syntactic function, it can be generalized
                     Generalizable(true),
                 ),
-                constraints.equal_types_var(
-                    closure_var,
-                    signature_closure_type,
-                    Category::ClosureSize,
+                // Store the inferred ret var into the function type now, so that
+                // when we check that the solved function type matches the annotation, we can
+                // display the fully inferred return variable.
+                constraints.store(
+                    ret_type_index,
+                    function_def.return_type,
+                    std::file!(),
+                    std::line!(),
+                ),
+                // Now, check the solved function type matches the annotation.
+                constraints.equal_types(
+                    solved_fn_type,
+                    annotation_expected,
+                    Category::Lambda,
                     region,
                 ),
+                // Finally put the solved closure type into the dedicated def expr variable.
                 constraints.store(signature_index, expr_var, std::file!(), std::line!()),
-                constraints.store(ret_type_index, ret_var, std::file!(), std::line!()),
                 closure_constraint,
             ];
 
@@ -2719,7 +2732,7 @@ fn constrain_typed_def(
                 name,
                 ..
             }),
-            TypeTag::Function(signature_closure_type, ret_type),
+            TypeTag::Function(_signature_closure_type, ret_type),
         ) => {
             let arg_types = types.get_type_arguments(signature);
 
@@ -2765,6 +2778,17 @@ fn constrain_typed_def(
                 &mut vars,
             );
 
+            let solved_fn_type = {
+                // TODO(types-soa) optimize for Variable
+                let arg_types =
+                    types.from_old_type_slice(arguments.iter().map(|a| Type::Variable(a.0)));
+                let lambda_set = types.from_old_type(&Type::Variable(closure_var));
+                let ret_var = types.from_old_type(&Type::Variable(ret_var));
+
+                let fn_type = types.function(arg_types, lambda_set, ret_var);
+                constraints.push_type(types, fn_type)
+            };
+
             let body_type = constraints.push_expected_type(FromAnnotation(
                 def.loc_pattern.clone(),
                 arguments.len(),
@@ -2787,18 +2811,6 @@ fn constrain_typed_def(
             vars.push(*fn_var);
             let defs_constraint = constraints.and_constraint(argument_pattern_state.constraints);
 
-            let signature_closure_type = {
-                let signature_closure_type_index =
-                    constraints.push_type(types, signature_closure_type);
-                constraints.push_expected_type(Expected::FromAnnotation(
-                    def.loc_pattern.clone(),
-                    arity,
-                    AnnotationSource::TypedBody {
-                        region: annotation.region,
-                    },
-                    signature_closure_type_index,
-                ))
-            };
             let cons = [
                 constraints.let_constraint(
                     [],
@@ -2809,15 +2821,20 @@ fn constrain_typed_def(
                     // This is a syntactic function, it can be generalized
                     Generalizable(true),
                 ),
-                constraints.equal_types_var(
-                    closure_var,
-                    signature_closure_type,
-                    Category::ClosureSize,
+                // Store the inferred ret var into the function type now, so that
+                // when we check that the solved function type matches the annotation, we can
+                // display the fully inferred return variable.
+                constraints.store(ret_type_index, ret_var, std::file!(), std::line!()),
+                // Now, check the solved function type matches the annotation.
+                constraints.equal_types(
+                    solved_fn_type,
+                    annotation_expected,
+                    Category::Lambda,
                     region,
                 ),
+                // Finally put the solved closure type into the dedicated def expr variables.
                 constraints.store(signature_index, *fn_var, std::file!(), std::line!()),
                 constraints.store(signature_index, expr_var, std::file!(), std::line!()),
-                constraints.store(ret_type_index, ret_var, std::file!(), std::line!()),
                 closure_constraint,
             ];
 
@@ -3005,6 +3022,29 @@ fn constrain_typed_function_arguments(
             }
         }
     }
+
+    // There may be argument idents left over that don't line up with the function arity.
+    // Add their patterns' symbols in so that they are present in the env, even though this will
+    // wind up a type error.
+    if arguments.len() > arg_types.len() {
+        for (pattern_var, _annotated_mark, loc_pattern) in &arguments[arg_types.len()..] {
+            let pattern_var_index = constraints.push_variable(*pattern_var);
+
+            def_pattern_state.vars.push(*pattern_var);
+
+            let pattern_expected =
+                constraints.push_pat_expected_type(PExpected::NoExpectation(pattern_var_index));
+            constrain_pattern(
+                types,
+                constraints,
+                env,
+                &loc_pattern.value,
+                loc_pattern.region,
+                pattern_expected,
+                argument_pattern_state,
+            );
+        }
+    }
 }
 
 fn constrain_typed_function_arguments_simple(
@@ -3127,6 +3167,29 @@ fn constrain_typed_function_arguments_simple(
             }
         }
     }
+
+    // There may be argument idents left over that don't line up with the function arity.
+    // Add their patterns' symbols in so that they are present in the env, even though this will
+    // wind up a type error.
+    if arguments.len() > arg_types.len() {
+        for (pattern_var, _annotated_mark, loc_pattern) in &arguments[arg_types.len()..] {
+            let pattern_var_index = constraints.push_variable(*pattern_var);
+
+            def_pattern_state.vars.push(*pattern_var);
+
+            let pattern_expected =
+                constraints.push_pat_expected_type(PExpected::NoExpectation(pattern_var_index));
+            constrain_pattern(
+                types,
+                constraints,
+                env,
+                &loc_pattern.value,
+                loc_pattern.region,
+                pattern_expected,
+                argument_pattern_state,
+            );
+        }
+    }
 }
 
 #[inline(always)]

crates/reporting/tests/test_reporting.rs

@@ -2615,6 +2615,27 @@ mod test_reporting {
     I would have to crash if I saw one of those! So rather than pattern
     matching in function arguments, put a `when` in the function body to
     account for all possibilities.
+
+    ── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─
+
+    Something is off with the body of the `f` definition:
+
+     9│      f : Either -> {}
+    10│      f = \Left v -> v
+                 ^^^^^^^^^^^^
+
+    The body is an anonymous function of type:
+
+        […] -> {}
+
+    But the type annotation on `f` says it should be:
+
+        [Right Str, …] -> {}
+
+    Tip: Looks like a closed tag union does not have the `Right` tag.
+
+    Tip: Closed tag unions can't grow, because that might change the size
+    in memory. Can you use an open tag union?
     "###
     );
 
@@ -13410,4 +13431,134 @@ I recommend using camelCase. It's the standard style in Roc code!
     meant to unwrap it first?
     "###
     );
+
+    test_report!(
+        function_arity_mismatch_too_few,
+        indoc!(
+            r#"
+            app "test" provides [f] to "./platform"
+
+            f : U8, U8 -> U8
+            f = \x -> x
+            "#
+        ),
+        @r###"
+    ── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─
+
+    Something is off with the body of the `f` definition:
+
+    3│  f : U8, U8 -> U8
+    4│  f = \x -> x
+            ^^^^^^^
+
+    The body is an anonymous function of type:
+
+        (U8 -> U8)
+
+    But the type annotation on `f` says it should be:
+
+        (U8, U8 -> U8)
+
+    Tip: It looks like it takes too few arguments. I was expecting 1 more.
+    "###
+    );
+
+    test_report!(
+        function_arity_mismatch_too_many,
+        indoc!(
+            r#"
+            app "test" provides [f] to "./platform"
+
+            f : U8, U8 -> U8
+            f = \x, y, z -> x + y + z
+            "#
+        ),
+        @r###"
+    ── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─
+
+    Something is off with the body of the `f` definition:
+
+    3│  f : U8, U8 -> U8
+    4│  f = \x, y, z -> x + y + z
+            ^^^^^^^^^^^^^^^^^^^^^
+
+    The body is an anonymous function of type:
+
+        (U8, U8, Int Unsigned8 -> U8)
+
+    But the type annotation on `f` says it should be:
+
+        (U8, U8 -> U8)
+
+    Tip: It looks like it takes too many arguments. I'm seeing 1 extra.
+    "###
+    );
+
+    test_report!(
+        function_arity_mismatch_nested_too_few,
+        indoc!(
+            r#"
+            app "test" provides [main] to "./platform"
+
+            main =
+                f : U8, U8 -> U8
+                f = \x -> x
+
+                f
+            "#
+        ),
+        @r###"
+    ── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─
+
+    Something is off with the body of the `f` definition:
+
+    4│      f : U8, U8 -> U8
+    5│      f = \x -> x
+                ^^^^^^^
+
+    The body is an anonymous function of type:
+
+        (U8 -> U8)
+
+    But the type annotation on `f` says it should be:
+
+        (U8, U8 -> U8)
+
+    Tip: It looks like it takes too few arguments. I was expecting 1 more.
+    "###
+    );
+
+    test_report!(
+        function_arity_mismatch_nested_too_many,
+        indoc!(
+            r#"
+            app "test" provides [main] to "./platform"
+
+            main =
+                f : U8, U8 -> U8
+                f = \x, y, z -> x + y + z
+
+                f
+            "#
+        ),
+        @r###"
+    ── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─
+
+    Something is off with the body of the `f` definition:
+
+    4│      f : U8, U8 -> U8
+    5│      f = \x, y, z -> x + y + z
+                ^^^^^^^^^^^^^^^^^^^^^
+
+    The body is an anonymous function of type:
+
+        (U8, U8, Int Unsigned8 -> U8)
+
+    But the type annotation on `f` says it should be:
+
+        (U8, U8 -> U8)
+
+    Tip: It looks like it takes too many arguments. I'm seeing 1 extra.
+    "###
+    );
 }