From d46fbbc2318429915c057fc596cd13c348013c1b Mon Sep 17 00:00:00 2001 From: abs0luty Date: Mon, 1 Dec 2025 18:47:04 +0500 Subject: [PATCH 1/2] fix(type-checker): instantiate generics in mutually recursive functions before unification ```gleam pub type Test(a) { Test(a) } pub fn it(value: Test(a)) { it2(value) } pub fn it2(value: Test(a)) -> Test(a) { it(value) } pub fn main() { it(Test(1)) } ``` Before this change the compiler compared generic "sketches" directly to inferred types in mutually recursive calls, causing false mismatches when generics behaved rigidly. After the patch the compiler instantiates generics (based on annotations or inferred concrete types) and updates the environment so mutually recursive functions unify correctly. This resolves spurious type-errors in recursive scenarios. --- CHANGELOG.md | 32 ++++++++++ compiler-core/src/analyse.rs | 23 ++++++-- ...re_considered_when_adding_annotations.snap | 2 +- compiler-core/src/type_/expression.rs | 58 +++++++++++-------- compiler-core/src/type_/tests/functions.rs | 50 ++++++++++++++++ 5 files changed, 135 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88d31b18820..706e22e9bfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -148,6 +148,38 @@ ([Adi Salimgereyev](https://github.com/abs0luty)) +- Type inference now preserves generic type parameters when constructors or functions are used without explicit annotations, eliminating false errors in mutually recursive code: + ```gleam + type Test(a) { + Test(a) + } + + fn it(value: Test(a)) { + it2(value) + } + + fn it2(value: Test(a)) -> Test(a) { + it(value) + } + ``` + Previously this could fail with an incorrect "Type mismatch" error: + ``` + Type mismatch + + The type of this returned value doesn't match the return type + annotation of this function. + + Expected type: + + Test(a) + + Found type: + + Test(a) + ``` + + ([Adi Salimgereyev](https://github.com/abs0luty)) + ### Build tool - The help text displayed by `gleam dev --help`, `gleam test --help`, and diff --git a/compiler-core/src/analyse.rs b/compiler-core/src/analyse.rs index 5b0ef38718c..f1c8e2ad0b4 100644 --- a/compiler-core/src/analyse.rs +++ b/compiler-core/src/analyse.rs @@ -695,8 +695,18 @@ impl<'a, A> ModuleAnalyzer<'a, A> { } // Assert that the inferred type matches the type of any recursive call - if let Err(error) = unify(preregistered_type.clone(), type_) { - self.problems.error(convert_unify_error(error, location)); + if let Err(error) = unify(preregistered_type.clone(), type_.clone()) { + let mut instantiated_ids = im::HashMap::new(); + let flexible_hydrator = Hydrator::new(); + let instantiated_annotation = environment.instantiate( + preregistered_type.clone(), + &mut instantiated_ids, + &flexible_hydrator, + ); + + if unify(instantiated_annotation, type_.clone()).is_err() { + self.problems.error(convert_unify_error(error, location)); + } } // Ensure that the current target has an implementation for the function. @@ -737,10 +747,13 @@ impl<'a, A> ModuleAnalyzer<'a, A> { purity, }; + // Store the inferred type (not the preregistered type) in the environment. + // This ensures concrete type information flows through recursive calls - e.g., if we infer + // `fn() -> Test(Int)`, callers see that instead of the generic `fn() -> Test(a)`. environment.insert_variable( name.clone(), variant, - preregistered_type.clone(), + type_.clone(), publicity, deprecation.clone(), ); @@ -753,6 +766,8 @@ impl<'a, A> ModuleAnalyzer<'a, A> { ReferenceKind::Definition, ); + // Use the inferred return type for the typed AST node. + // This matches the type stored in the environment above. let function = Function { documentation: doc, location, @@ -763,7 +778,7 @@ impl<'a, A> ModuleAnalyzer<'a, A> { body_start, end_position: end_location, return_annotation, - return_type: preregistered_type + return_type: type_ .return_type() .expect("Could not find return type for fn"), body, diff --git a/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__type_variables_in_let_bindings_are_considered_when_adding_annotations.snap b/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__type_variables_in_let_bindings_are_considered_when_adding_annotations.snap index 9ede82bdd94..55fe8e69179 100644 --- a/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__type_variables_in_let_bindings_are_considered_when_adding_annotations.snap +++ b/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__type_variables_in_let_bindings_are_considered_when_adding_annotations.snap @@ -15,7 +15,7 @@ fn wibble(a, b, c) { ----- AFTER ACTION -fn wibble(a: e, b: f, c: g) -> fn(b, c) -> d { +fn wibble(a: e, b: f, c: g) -> fn(b, c) -> h { let x: a = todo fn(a: b, b: c) -> d { todo diff --git a/compiler-core/src/type_/expression.rs b/compiler-core/src/type_/expression.rs index b80fe0725aa..cd9dcbebb0c 100644 --- a/compiler-core/src/type_/expression.rs +++ b/compiler-core/src/type_/expression.rs @@ -4697,31 +4697,39 @@ impl<'a, 'b> ExprTyper<'a, 'b> { if let Ok(body) = Vec1::try_from_vec(body) { let mut body = body_typer.infer_statements(body); - // Check that any return type is accurate. - if let Some(return_type) = return_type - && let Err(error) = unify(return_type, body.last().type_()) - { - let error = error - .return_annotation_mismatch() - .into_error(body.last().type_defining_location()); - body_typer.problems.error(error); - - // If the return type doesn't match with the annotation we - // add a new expression to the end of the function to match - // the annotated type and allow type inference to keep - // going. - body.push(Statement::Expression(TypedExpr::Invalid { - // This is deliberately an empty span since this - // placeholder expression is implicitly inserted by the - // compiler and doesn't actually appear in the source - // code. - location: SrcSpan { - start: body.last().location().end, - end: body.last().location().end, - }, - type_: body_typer.new_unbound_var(), - extra_information: None, - })) + // Check that any return type is compatible with the annotation. + if let Some(return_type) = return_type { + let mut instantiated_ids = hashmap![]; + let flexible_hydrator = Hydrator::new(); + let instantiated_annotation = body_typer.environment.instantiate( + return_type.clone(), + &mut instantiated_ids, + &flexible_hydrator, + ); + + if let Err(error) = unify(instantiated_annotation, body.last().type_()) { + let error = error + .return_annotation_mismatch() + .into_error(body.last().type_defining_location()); + body_typer.problems.error(error); + + // If the return type doesn't match with the annotation we + // add a new expression to the end of the function to match + // the annotated type and allow type inference to keep + // going. + body.push(Statement::Expression(TypedExpr::Invalid { + // This is deliberately an empty span since this + // placeholder expression is implicitly inserted by the + // compiler and doesn't actually appear in the source + // code. + location: SrcSpan { + start: body.last().location().end, + end: body.last().location().end, + }, + type_: body_typer.new_unbound_var(), + extra_information: None, + })) + } }; Ok((arguments, body.to_vec())) diff --git a/compiler-core/src/type_/tests/functions.rs b/compiler-core/src/type_/tests/functions.rs index 7bac5c0813e..8e2b380dc68 100644 --- a/compiler-core/src/type_/tests/functions.rs +++ b/compiler-core/src/type_/tests/functions.rs @@ -169,6 +169,56 @@ pub fn two(x) { ); } +// https://github.com/gleam-lang/gleam/issues/2550 +#[test] +fn mutual_recursion_keeps_generic_return_annotation() { + assert_module_infer!( + r#" +pub type Test(a) { + Test(a) +} + +pub fn it(value: Test(a)) { + it2(value) +} + +pub fn it2(value: Test(a)) -> Test(a) { + it(value) +} + +pub fn main() { + it(Test(1)) +} +"#, + vec![ + (r#"Test"#, r#"fn(a) -> Test(a)"#), + (r#"it"#, r#"fn(Test(a)) -> Test(a)"#), + (r#"it2"#, r#"fn(Test(a)) -> Test(a)"#), + (r#"main"#, r#"fn() -> Test(Int)"#) + ] + ); +} + +// https://github.com/gleam-lang/gleam/issues/2533 +#[test] +fn unbound_type_variable_in_top_level_definition() { + assert_module_infer!( + r#" +pub type Foo(a) { + Foo(value: Int) +} + +pub fn main() { + Foo(1) +} +"#, + vec![ + (r#"Foo"#, r#"fn(Int) -> Foo(a)"#), + (r#"main"#, r#"fn() -> Foo(a)"#), + ] + ); +} + #[test] fn no_impl_function_fault_tolerance() { // A function not having an implementation does not stop analysis. From c05b9655b0abddc45751a17221edf92f03520092 Mon Sep 17 00:00:00 2001 From: abs0luty Date: Sat, 25 Oct 2025 10:58:04 +0400 Subject: [PATCH 2/2] fix(lsp): preserve annotated type-variable names when adding annotations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``` fn wibble(a, b, c) { ↑ let x: a = todo fn(a: b, b: c) -> d { todo } } ``` After action: ```diff - fn wibble(a: e, b: f, c: g) -> fn(b, c) -> h + fn wibble(a: e, b: f, c: g) -> fn(b, c) -> d ^ original name is preserved ``` When the "add annotations" code-action is used, the compiler now retains the original type-variable names from the user's annotation instead of replacing them with fresh or default names. This ensures that added annotations reflect the user's naming exactly and prevents confusing or misleading variable renaming. --- .../src/language_server/code_action.rs | 174 ++++++++++++++++++ ...re_considered_when_adding_annotations.snap | 2 +- compiler-core/src/type_/printer.rs | 6 + 3 files changed, 181 insertions(+), 1 deletion(-) diff --git a/compiler-core/src/language_server/code_action.rs b/compiler-core/src/language_server/code_action.rs index 126c37e6acc..00e541ec141 100644 --- a/compiler-core/src/language_server/code_action.rs +++ b/compiler-core/src/language_server/code_action.rs @@ -1372,6 +1372,73 @@ fn collect_type_variables(printer: &mut Printer<'_>, function: &ast::TypedFuncti } impl<'ast, 'a, 'b> ast::visit::Visit<'ast> for TypeVariableCollector<'a, 'b> { + fn visit_typed_function(&mut self, fun: &'ast ast::TypedFunction) { + for argument in fun.arguments.iter() { + if let Some(annotation) = &argument.annotation { + register_type_variables_from_annotation( + self.printer, + annotation, + argument.type_.as_ref(), + ); + } + } + + if let Some(annotation) = &fun.return_annotation { + register_type_variables_from_annotation( + self.printer, + annotation, + fun.return_type.as_ref(), + ); + } + + ast::visit::visit_typed_function(self, fun); + } + + fn visit_typed_expr_fn( + &mut self, + location: &'ast SrcSpan, + type_: &'ast Arc, + kind: &'ast FunctionLiteralKind, + arguments: &'ast [TypedArg], + body: &'ast Vec1, + return_annotation: &'ast Option, + ) { + if let Type::Fn { + arguments: argument_types, + return_: return_type, + .. + } = type_.as_ref() + { + for (argument, argument_type) in arguments.iter().zip(argument_types) { + if let Some(annotation) = &argument.annotation { + register_type_variables_from_annotation( + self.printer, + annotation, + argument_type.as_ref(), + ); + } + } + + if let Some(annotation) = return_annotation { + register_type_variables_from_annotation( + self.printer, + annotation, + return_type.as_ref(), + ); + } + } + + ast::visit::visit_typed_expr_fn( + self, + location, + type_, + kind, + arguments, + body, + return_annotation, + ); + } + fn visit_type_ast_var(&mut self, _location: &'ast SrcSpan, name: &'ast EcoString) { // Register this type variable so that we don't duplicate names when // adding annotations. @@ -1379,6 +1446,113 @@ impl<'ast, 'a, 'b> ast::visit::Visit<'ast> for TypeVariableCollector<'a, 'b> { } } +fn register_type_variables_from_annotation( + printer: &mut Printer<'_>, + annotation: &ast::TypeAst, + type_: &Type, +) { + // fn wibble(a, b, c) { + // fn(a: b, b: c) -> d { ... } + // ^ + // Without this tracking the printer could rename `d` to a fresh `h`. + match (annotation, type_) { + (ast::TypeAst::Var(ast::TypeAstVar { name, .. }), Type::Var { type_ }) => { + match &*type_.borrow() { + TypeVar::Generic { id } | TypeVar::Unbound { id } => { + let id = *id; + printer.register_type_variable(name.clone()); + printer.register_type_variable_with_id(id, name.clone()); + } + TypeVar::Link { type_ } => { + register_type_variables_from_annotation(printer, annotation, type_.as_ref()); + } + } + } + + ( + ast::TypeAst::Fn(ast::TypeAstFn { + arguments: annotation_arguments, + return_: annotation_return, + .. + }), + Type::Fn { + arguments: type_arguments, + return_: type_return, + .. + }, + ) => { + for (argument_annotation, argument_type) in + annotation_arguments.iter().zip(type_arguments) + { + // Maintain the names from each `fn(arg: name, ...)` position. + register_type_variables_from_annotation( + printer, + argument_annotation, + argument_type.as_ref(), + ); + } + + // And likewise propagate the annotated return variable. + register_type_variables_from_annotation( + printer, + annotation_return.as_ref(), + type_return.as_ref(), + ); + } + + ( + ast::TypeAst::Constructor(ast::TypeAstConstructor { + arguments: annotation_arguments, + .. + }), + Type::Named { + arguments: type_arguments, + .. + }, + ) => { + for (argument_annotation, argument_type) in + annotation_arguments.iter().zip(type_arguments) + { + // Track aliases introduced inside named type arguments. + register_type_variables_from_annotation( + printer, + argument_annotation, + argument_type.as_ref(), + ); + } + } + + ( + ast::TypeAst::Tuple(ast::TypeAstTuple { + elements: annotation_elements, + .. + }), + Type::Tuple { + elements: type_elements, + .. + }, + ) => { + for (element_annotation, element_type) in annotation_elements.iter().zip(type_elements) + { + // Tuples can hide extra annotations; ensure each slot retains its label. + register_type_variables_from_annotation( + printer, + element_annotation, + element_type.as_ref(), + ); + } + } + + (_, Type::Var { type_ }) => { + if let TypeVar::Link { type_ } = &*type_.borrow() { + register_type_variables_from_annotation(printer, annotation, type_.as_ref()); + } + } + + _ => {} + } +} + pub struct QualifiedConstructor<'a> { import: &'a Import, used_name: EcoString, diff --git a/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__type_variables_in_let_bindings_are_considered_when_adding_annotations.snap b/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__type_variables_in_let_bindings_are_considered_when_adding_annotations.snap index 55fe8e69179..9ede82bdd94 100644 --- a/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__type_variables_in_let_bindings_are_considered_when_adding_annotations.snap +++ b/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__type_variables_in_let_bindings_are_considered_when_adding_annotations.snap @@ -15,7 +15,7 @@ fn wibble(a, b, c) { ----- AFTER ACTION -fn wibble(a: e, b: f, c: g) -> fn(b, c) -> h { +fn wibble(a: e, b: f, c: g) -> fn(b, c) -> d { let x: a = todo fn(a: b, b: c) -> d { todo diff --git a/compiler-core/src/type_/printer.rs b/compiler-core/src/type_/printer.rs index 34dc3829e5c..8b11ff8cf0f 100644 --- a/compiler-core/src/type_/printer.rs +++ b/compiler-core/src/type_/printer.rs @@ -454,6 +454,12 @@ impl<'a> Printer<'a> { _ = self.printed_type_variable_names.insert(name); } + /// Record that the given type-variable id is already using the supplied name. + pub fn register_type_variable_with_id(&mut self, id: u64, name: EcoString) { + _ = self.printed_type_variable_names.insert(name.clone()); + _ = self.printed_type_variables.insert(id, name); + } + pub fn print_type(&mut self, type_: &Type) -> EcoString { let mut buffer = EcoString::new(); self.print(type_, &mut buffer, PrintMode::Normal);