From 490e07a2f9107643d3f6d2c37e7c484fab3e9ed8 Mon Sep 17 00:00:00 2001 From: raphrous Date: Sun, 27 Jul 2025 18:05:37 +0200 Subject: [PATCH 1/9] Add imported module variables suggestions for unknown variables. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a variable is not in the current scope, an `Error::UnknownVariable` is triggered. However the only suggestions were for variables in the scope with a similar name computed by the "did you mean" algorithm. With this commit, we also suggest public variables / identifiers (such as constants, functions or type variant constructor) in imported modules with the same name. For example with the following code: ```gleam import gleam/io pub fn main() -> Nil { println("Hello, World!") } ``` The suggestions are: ``` ┌─ /path/to/project/src/project.gleam:4:3 │ 4 │ println("Hello, World!") │ ^^^^^^^ The name `println` is not in scope here. Consider using one of these variables: io.println ``` However because we are only checking the name of the variable, we could have wrong suggestions, as in this case: ```gleam import gleam/float import gleam/int pub fn main() -> Nil { to_string(3) } ``` Here, it is clear that we want suggestions on a function named `to_string` and accepting one parameter of type `Int`, but this is not the case: ``` ┌─ /path/to/project/src/project.gleam:5:3 │ 5 │ to_string(3) │ ^^^^^^^^^ The name `to_string` is not in scope here. Consider using one of these implementations: float.to_string int.to_string ``` --- compiler-core/src/error.rs | 31 +++++++++++++++++++------- compiler-core/src/type_/environment.rs | 20 ++++++++++++----- compiler-core/src/type_/error.rs | 6 +++++ compiler-core/src/type_/expression.rs | 9 ++++++++ compiler-core/src/type_/pattern.rs | 9 ++++++++ 5 files changed, 61 insertions(+), 14 deletions(-) diff --git a/compiler-core/src/error.rs b/compiler-core/src/error.rs index d2e3e7f308b..0b96e4eae97 100644 --- a/compiler-core/src/error.rs +++ b/compiler-core/src/error.rs @@ -2402,9 +2402,9 @@ but no type in scope with that name." discarded_location, name, type_with_name_in_scope, + imported_modules_with_same_public_variable_name, } => { let title = String::from("Unknown variable"); - if let Some(ignored_location) = discarded_location { let location = Location { label: Label { @@ -2434,17 +2434,32 @@ but no type in scope with that name." let text = if *type_with_name_in_scope { wrap_format!("`{name}` is a type, it cannot be used as a value.") } else { - let is_first_char_uppercase = - name.chars().next().is_some_and(char::is_uppercase); - - if is_first_char_uppercase { + let mut text = if name.starts_with(char::is_uppercase) { wrap_format!( - "The custom type variant constructor \ -`{name}` is not in scope here." + "The custom type variant constructor `{name}` is not in scope here." ) - } else { + } + else { wrap_format!("The name `{name}` is not in scope here.") + }; + + // If there are some suggestions about variables in imported modules + // put a "consider" text after the main message + if !imported_modules_with_same_public_variable_name.is_empty() { + let consider_text = imported_modules_with_same_public_variable_name + .iter() + .fold( + String::from("Consider using one of these variables:\n\n"), + |mut acc, module_name| { + acc.push_str(&wrap_format!(" {module_name}.{name}\n")); + acc + } + ); + text.push('\n'); + text.push_str(&consider_text); } + + text }; Diagnostic { diff --git a/compiler-core/src/type_/environment.rs b/compiler-core/src/type_/environment.rs index b2e394bd3be..f883a5e6546 100644 --- a/compiler-core/src/type_/environment.rs +++ b/compiler-core/src/type_/environment.rs @@ -481,14 +481,22 @@ impl Environment<'_> { name: &EcoString, ) -> Result<&ValueConstructor, UnknownValueConstructorError> { match module { - None => self.scope.get(name).ok_or_else(|| { - let type_with_name_in_scope = self.module_types.keys().any(|type_| type_ == name); - UnknownValueConstructorError::Variable { + None => self + .scope + .get(name) + .ok_or_else(|| UnknownValueConstructorError::Variable { name: name.clone(), variables: self.local_value_names(), - type_with_name_in_scope, - } - }), + type_with_name_in_scope: self.module_types.keys().any(|typ| typ == name), + imported_modules_with_same_public_variable_name: self + .imported_modules + .iter() + .filter_map(|(module_name, (_, module))| { + module.get_public_value(name).map(|_| module_name) + }) + .cloned() + .collect_vec(), + }), Some(module_name) => { let (_, module) = self.imported_modules.get(module_name).ok_or_else(|| { diff --git a/compiler-core/src/type_/error.rs b/compiler-core/src/type_/error.rs index 4e15ea45a8e..214da2e1d26 100644 --- a/compiler-core/src/type_/error.rs +++ b/compiler-core/src/type_/error.rs @@ -165,6 +165,9 @@ pub enum Error { /// this will contain its location. discarded_location: Option, type_with_name_in_scope: bool, + /// Filled with the name of imported modules when the module has public value + /// with the same name as this variable + imported_modules_with_same_public_variable_name: Vec, }, UnknownType { @@ -1314,6 +1317,7 @@ pub enum UnknownValueConstructorError { name: EcoString, variables: Vec, type_with_name_in_scope: bool, + imported_modules_with_same_public_variable_name: Vec, }, Module { @@ -1339,12 +1343,14 @@ pub fn convert_get_value_constructor_error( name, variables, type_with_name_in_scope, + imported_modules_with_same_public_variable_name, } => Error::UnknownVariable { location, name, variables, discarded_location: None, type_with_name_in_scope, + imported_modules_with_same_public_variable_name, }, UnknownValueConstructorError::Module { name, suggestions } => Error::UnknownModule { diff --git a/compiler-core/src/type_/expression.rs b/compiler-core/src/type_/expression.rs index c85a1624e92..5669bd98c56 100644 --- a/compiler-core/src/type_/expression.rs +++ b/compiler-core/src/type_/expression.rs @@ -3647,6 +3647,15 @@ impl<'a, 'b> ExprTyper<'a, 'b> { .module_types .keys() .any(|typ| typ == name), + imported_modules_with_same_public_variable_name: self + .environment + .imported_modules + .iter() + .filter_map(|(module_name, (_, module))| { + module.get_public_value(name).map(|_| module_name) + }) + .cloned() + .collect_vec(), }, } } diff --git a/compiler-core/src/type_/pattern.rs b/compiler-core/src/type_/pattern.rs index 915296ed804..21759d6dae7 100644 --- a/compiler-core/src/type_/pattern.rs +++ b/compiler-core/src/type_/pattern.rs @@ -1259,6 +1259,15 @@ impl<'a, 'b> PatternTyper<'a, 'b> { .module_types .keys() .any(|type_| type_ == &name), + imported_modules_with_same_public_variable_name: self + .environment + .imported_modules + .iter() + .filter_map(|(module_name, (_, module))| { + module.get_public_value(&name).map(|_| module_name) + }) + .cloned() + .collect_vec(), }); } }, From 0c73f8273dc6110e1daadd0df86e5753ed3aff4d Mon Sep 17 00:00:00 2001 From: raphrous Date: Sun, 27 Jul 2025 18:05:37 +0200 Subject: [PATCH 2/9] Add suggestions for `UntypedExpr::Call` that check the arity. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this commit, all functions / constructors were suggested even if they do not have the correct arity. With this change, only functions / constructors with the correct arity are suggested. However, even if the arity is correct the type of arguments can mismatch, resulting in incorrect suggestions. ```gleam // fn to_string(Int) -> String import gleam/int // fn to_string(Float) -> String import gleam/float pub fn main() -> Nil { to_string(1) } error: Unknown variable ┌─ /path/to/project/src/project.gleam:8:3 │ 8 │ to_string(1) │ ^^^^^^^^^ The name `to_string` is not in scope here. Consider using one of these variables: int.to_string float.to_string ``` --- compiler-core/src/type_.rs | 10 ++ compiler-core/src/type_/expression.rs | 151 ++++++++++++++++++++------ 2 files changed, 126 insertions(+), 35 deletions(-) diff --git a/compiler-core/src/type_.rs b/compiler-core/src/type_.rs index 653fb76e94e..9467fb3bf52 100644 --- a/compiler-core/src/type_.rs +++ b/compiler-core/src/type_.rs @@ -1680,6 +1680,16 @@ pub enum FieldAccessUsage { Other, } +/// This is used to know when a variable is used as a value or as a call: +/// function call, a pipeline or a custom type variant constructor +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum VarUsage { + /// Used as `variable()` + Call { arity: usize }, + /// Used as `variable` + Other, +} + /// Verify that a value is suitable to be used as a main function. fn assert_suitable_main_function( value: &ValueConstructor, diff --git a/compiler-core/src/type_/expression.rs b/compiler-core/src/type_/expression.rs index 5669bd98c56..14c3c0452c5 100644 --- a/compiler-core/src/type_/expression.rs +++ b/compiler-core/src/type_/expression.rs @@ -432,9 +432,12 @@ impl<'a, 'b> ExprTyper<'a, 'b> { message, } => Ok(self.infer_echo(location, keyword_end, expression, message)), - UntypedExpr::Var { location, name, .. } => { - self.infer_var(name, location, ReferenceRegistration::RegisterReferences) - } + UntypedExpr::Var { location, name, .. } => self.infer_var( + name, + location, + VarUsage::Other, + ReferenceRegistration::RegisterReferences, + ), UntypedExpr::Int { location, @@ -1247,10 +1250,16 @@ impl<'a, 'b> ExprTyper<'a, 'b> { &mut self, name: EcoString, location: SrcSpan, + var_usage: VarUsage, register_reference: ReferenceRegistration, ) -> Result { - let constructor = - self.do_infer_value_constructor(&None, &name, &location, register_reference)?; + let constructor = self.do_infer_value_constructor( + &None, + &name, + &location, + var_usage, + register_reference, + )?; self.narrow_implementations(location, &constructor.variant)?; Ok(TypedExpr::Var { constructor, @@ -1345,6 +1354,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { UntypedExpr::Var { location, name } => self.infer_var( name, location, + VarUsage::Other, ReferenceRegistration::DoNotRegisterReferences, ), _ => self.infer_or_error(container), @@ -2309,7 +2319,8 @@ impl<'a, 'b> ExprTyper<'a, 'b> { fn infer_clause_guard(&mut self, guard: UntypedClauseGuard) -> Result { match guard { ClauseGuard::Var { location, name, .. } => { - let constructor = self.infer_value_constructor(&None, &name, &location)?; + let constructor = + self.infer_value_constructor(&None, &name, &location, VarUsage::Other)?; // We cannot support all values in guard expressions as the BEAM does not let definition_location = match &constructor.variant { @@ -3472,11 +3483,13 @@ impl<'a, 'b> ExprTyper<'a, 'b> { module: &Option<(EcoString, SrcSpan)>, name: &EcoString, location: &SrcSpan, + var_usage: VarUsage, ) -> Result { self.do_infer_value_constructor( module, name, location, + var_usage, ReferenceRegistration::RegisterReferences, ) } @@ -3486,6 +3499,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { module: &Option<(EcoString, SrcSpan)>, name: &EcoString, location: &SrcSpan, + var_usage: VarUsage, register_reference: ReferenceRegistration, ) -> Result { let constructor = match module { @@ -3494,7 +3508,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { .environment .get_variable(name) .cloned() - .ok_or_else(|| self.report_name_error(name, location))?, + .ok_or_else(|| self.report_name_error(name, location, var_usage))?, // Look in an imported module for a binding with this name Some((module_name, module_location)) => { @@ -3622,7 +3636,12 @@ impl<'a, 'b> ExprTyper<'a, 'b> { } } - fn report_name_error(&mut self, name: &EcoString, location: &SrcSpan) -> Error { + fn report_name_error( + &mut self, + name: &EcoString, + location: &SrcSpan, + var_usage: VarUsage, + ) -> Error { // First try to see if this is a module alias: // `import gleam/io` // `io.debug(io)` @@ -3633,30 +3652,59 @@ impl<'a, 'b> ExprTyper<'a, 'b> { location: *location, name: name.clone(), }, - None => Error::UnknownVariable { - location: *location, - name: name.clone(), - variables: self.environment.local_value_names(), - discarded_location: self - .environment - .discarded_names - .get(&eco_format!("_{name}")) - .cloned(), - type_with_name_in_scope: self - .environment - .module_types - .keys() - .any(|typ| typ == name), - imported_modules_with_same_public_variable_name: self - .environment - .imported_modules - .iter() - .filter_map(|(module_name, (_, module))| { - module.get_public_value(name).map(|_| module_name) - }) - .cloned() - .collect_vec(), - }, + None => { + let imported_modules_with_same_public_variable_name = match var_usage { + // This is a function call, we need to suggest a public + // variable which is a function with the correct arity + VarUsage::Call { arity } => self + .environment + .imported_modules + .iter() + .filter_map(|(module_name, (_, module))| { + module + .get_public_value(name) + .filter(|value_constructor| { + match value_constructor.type_.fn_types() { + Some((fn_arguments_type, _)) => { + fn_arguments_type.len() == arity + } + None => false, + } + }) + .map(|_| module_name) + }) + .cloned() + .collect_vec(), + // This is a reference to a variable, we need to suggest + // public variables of any type + VarUsage::Other => self + .environment + .imported_modules + .iter() + .filter_map(|(module_name, (_, module))| { + module.get_public_value(name).map(|_| module_name) + }) + .cloned() + .collect_vec(), + }; + + Error::UnknownVariable { + location: *location, + name: name.clone(), + variables: self.environment.local_value_names(), + discarded_location: self + .environment + .discarded_names + .get(&eco_format!("_{name}")) + .cloned(), + type_with_name_in_scope: self + .environment + .module_types + .keys() + .any(|typ| typ == name), + imported_modules_with_same_public_variable_name, + } + } } } @@ -3714,7 +3762,8 @@ impl<'a, 'b> ExprTyper<'a, 'b> { .. } if arguments.is_empty() => { // Type check the record constructor - let constructor = self.infer_value_constructor(&module, &name, &location)?; + let constructor = + self.infer_value_constructor(&module, &name, &location, VarUsage::Other)?; let (tag, field_map) = match &constructor.variant { ValueConstructorVariant::Record { @@ -3753,7 +3802,14 @@ impl<'a, 'b> ExprTyper<'a, 'b> { // field_map, is always None here because untyped not yet unified .. } => { - let constructor = self.infer_value_constructor(&module, &name, &location)?; + let constructor = self.infer_value_constructor( + &module, + &name, + &location, + VarUsage::Call { + arity: arguments.len(), + }, + )?; let (tag, field_map, variant_index) = match &constructor.variant { ValueConstructorVariant::Record { @@ -3895,7 +3951,8 @@ impl<'a, 'b> ExprTyper<'a, 'b> { .. } => { // Infer the type of this constant - let constructor = self.infer_value_constructor(&module, &name, &location)?; + let constructor = + self.infer_value_constructor(&module, &name, &location, VarUsage::Other)?; match constructor.variant { ValueConstructorVariant::ModuleConstant { .. } | ValueConstructorVariant::LocalConstant { .. } @@ -4072,6 +4129,30 @@ impl<'a, 'b> ExprTyper<'a, 'b> { kind: CallKind, ) -> (TypedExpr, Vec, Arc) { let fun = match fun { + UntypedExpr::Var { location, name } => { + // Because we are not calling infer / infer_or_error directly, + // we do not warn if there was a previous panic. Check for that + // here. Not perfect, but works. + if self.previous_panics { + self.warn_for_unreachable_code(location, PanicPosition::PreviousExpression); + } + + match self.infer_var( + name, + location, + VarUsage::Call { + arity: arguments.len(), + }, + ReferenceRegistration::RegisterReferences, + ) { + Ok(typed_expr) => typed_expr, + Err(error) => { + self.problems.error(error); + self.error_expr(location) + } + } + } + UntypedExpr::FieldAccess { label, container, From 9f9ae93573df343577b67e5f158d6dc47b80e162 Mon Sep 17 00:00:00 2001 From: raphrous Date: Sun, 27 Jul 2025 18:05:37 +0200 Subject: [PATCH 3/9] Add suggestions for pipeline similar to function but with arity of 1. At this stage, pipeline are not considered as `Call` so previous work on `UntypedExpr::Call` does not apply directly to pipeline. With this change, we handle the pipeline case when the function / constructor is not a `Call`. ```gleam 1 |> to_string ``` When there is a call in the pipeline, it is more delicate to address because something like that: ```gleam 1 |> add(2) ``` Can be desugared into two possibilities and so suggestions are not the same. The function `report_name_error` does not have enough context to make good suggestions: ```gleam add(1, 2) add(2)(1) ``` In this case, we just suggest every functions. --- compiler-core/src/type_.rs | 4 ++- compiler-core/src/type_/expression.rs | 22 ++++++++++++--- compiler-core/src/type_/pipe.rs | 39 +++++++++++++++++++-------- 3 files changed, 50 insertions(+), 15 deletions(-) diff --git a/compiler-core/src/type_.rs b/compiler-core/src/type_.rs index 9467fb3bf52..b4d0b36a92c 100644 --- a/compiler-core/src/type_.rs +++ b/compiler-core/src/type_.rs @@ -1684,8 +1684,10 @@ pub enum FieldAccessUsage { /// function call, a pipeline or a custom type variant constructor #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum VarUsage { - /// Used as `variable()` + /// Used as `call()` or `left |> right` Call { arity: usize }, + /// Used as `left |> right(..)` + PipelineCall, /// Used as `variable` Other, } diff --git a/compiler-core/src/type_/expression.rs b/compiler-core/src/type_/expression.rs index 14c3c0452c5..d849d8ceeb5 100644 --- a/compiler-core/src/type_/expression.rs +++ b/compiler-core/src/type_/expression.rs @@ -727,7 +727,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { } // Helper to create a new error expr. - fn error_expr(&mut self, location: SrcSpan) -> TypedExpr { + pub(crate) fn error_expr(&mut self, location: SrcSpan) -> TypedExpr { TypedExpr::Invalid { location, type_: self.new_unbound_var(), @@ -1246,7 +1246,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { } } - fn infer_var( + pub(crate) fn infer_var( &mut self, name: EcoString, location: SrcSpan, @@ -3675,6 +3675,22 @@ impl<'a, 'b> ExprTyper<'a, 'b> { }) .cloned() .collect_vec(), + // This is a `Call` into a `Pipeline`. This is hard to make + // good suggestions because of the way it can be desugared. + // In this case, return every functions with the same name + // even if they have wrong arity. + VarUsage::PipelineCall => self + .environment + .imported_modules + .iter() + .filter_map(|(module_name, (_, module))| { + module + .get_public_value(name) + .filter(|value_constructor| value_constructor.type_.is_fun()) + .map(|_| module_name) + }) + .cloned() + .collect_vec(), // This is a reference to a variable, we need to suggest // public variables of any type VarUsage::Other => self @@ -4752,7 +4768,7 @@ fn is_trusted_pure_module(environment: &Environment<'_>) -> bool { } #[derive(Debug, Clone, Copy)] -enum ReferenceRegistration { +pub(crate) enum ReferenceRegistration { RegisterReferences, DoNotRegisterReferences, } diff --git a/compiler-core/src/type_/pipe.rs b/compiler-core/src/type_/pipe.rs index a36a8bb976c..f3249bb8405 100644 --- a/compiler-core/src/type_/pipe.rs +++ b/compiler-core/src/type_/pipe.rs @@ -1,6 +1,6 @@ use self::expression::CallKind; -use super::*; +use super::{expression::ReferenceRegistration, *}; use crate::ast::{ FunctionLiteralKind, ImplicitCallArgOrigin, PIPE_VARIABLE, PipelineAssignmentKind, Statement, TypedPipelineAssignment, UntypedExpr, @@ -139,17 +139,24 @@ impl<'a, 'b, 'c> PipeTyper<'a, 'b, 'c> { location, .. } => { - let fun = match self.expr_typer.infer_or_error(*fun) { + let fun_result = match *fun { + UntypedExpr::Var { location, name } => self.expr_typer.infer_var( + name, + location, + VarUsage::PipelineCall, + ReferenceRegistration::RegisterReferences, + ), + fun => self.expr_typer.infer_or_error(fun), + }; + + let fun = match fun_result { Ok(fun) => fun, Err(e) => { // In case we cannot infer the function we'll // replace it with an invalid expression with an // unbound type to keep going! self.expr_typer.problems.error(e); - TypedExpr::Invalid { - location, - type_: self.expr_typer.new_unbound_var(), - } + self.expr_typer.error_expr(location) } }; @@ -350,16 +357,26 @@ impl<'a, 'b, 'c> PipeTyper<'a, 'b, 'c> { /// b is the `function` argument. fn infer_apply_pipe(&mut self, function: UntypedExpr) -> TypedExpr { let function_location = function.location(); - let function = Box::new(match self.expr_typer.infer_or_error(function) { + // Need one more step for better error suggestion. If the function is + // a variable / identifier, then we can suggest variables / identifier + // from imported modules with the same name and same arity. + let function_result = match function { + UntypedExpr::Var { location, name } => self.expr_typer.infer_var( + name, + location, + VarUsage::Call { arity: 1 }, + ReferenceRegistration::RegisterReferences, + ), + _ => self.expr_typer.infer_or_error(function), + }; + + let function = Box::new(match function_result { Ok(function) => function, Err(error) => { // If we cannot infer the function we put an invalid expression // in its place so we can still keep going with the other steps. self.expr_typer.problems.error(error); - TypedExpr::Invalid { - location: function_location, - type_: self.expr_typer.new_unbound_var(), - } + self.expr_typer.error_expr(function_location) } }); From 0e9d8a853be5c554916d3c0af14db8b24274befa Mon Sep 17 00:00:00 2001 From: raphrous Date: Tue, 29 Jul 2025 13:07:21 +0200 Subject: [PATCH 4/9] Update tests to reflect the previous changes. In this example, the name `zoo` exist in the module `wibble` which is imported. The compiler correctly suggest to use `wibble.zoo`. --- ...ore__type___tests__errors__same_imports_multiple_times.snap | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__same_imports_multiple_times.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__same_imports_multiple_times.snap index 283a8505862..a2000d106c0 100644 --- a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__same_imports_multiple_times.snap +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__same_imports_multiple_times.snap @@ -35,3 +35,6 @@ error: Unknown variable │ ^^^ The name `zoo` is not in scope here. +Consider using one of these variables: + + wibble.zoo From 5f928071693a92f58018065a98f9e9f91168f989 Mon Sep 17 00:00:00 2001 From: raphrous Date: Mon, 28 Jul 2025 16:26:08 +0200 Subject: [PATCH 5/9] Update of the CHANGELOG.md file. --- CHANGELOG.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c65e43a6716..9b9d6846a15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,34 @@ ### Compiler +- The compiler now suggest public values from imported modules when the variable + in unknown. These values are suggested based on name and arity. + + Considering this program: + ```gleam + import gleam/io + + pub fn main() -> Nil { + println("Hello, World!") + } + ``` + + The compiler will display this error message: + ```text + error: Unknown variable + ┌─ /path/to/project/src/project.gleam:4:3 + │ + 4 │ println("Hello, World!") + │ ^^^^^^^ + + The name `println` is not in scope here. + Consider using one of these variables: + + io.println + ``` + + ([raphrous](https://github.com/realraphrous)) + - The compiler now performs function inlining optimisations for a specific set of standard library functions, which can allow functions which were previously not tail-recursive on the JavaScript target to become tail-recursive. For From 8879a5e53bc1e638d677e26e9556b3ff0db28b20 Mon Sep 17 00:00:00 2001 From: raphrous Date: Thu, 7 Aug 2025 16:11:19 +0200 Subject: [PATCH 6/9] Implement lpil suggestions after review. Rename all terms related to variables: variable / functions, Var, identifiers to value(s) in code, comment and CHANGELOG.md Rename the new field in `Error::UnknownVariable` from `imported_modules_with_same_public_variable_name` to `possible_modules`. The field name was too long. Change the fold pattern with immutable accumulator to a for loop mutating a variable. Rewrite the compiler message to a "Did you mean" sentence. --- compiler-core/src/error.rs | 23 ++-- compiler-core/src/type_.rs | 18 ++- compiler-core/src/type_/environment.rs | 2 +- compiler-core/src/type_/error.rs | 8 +- compiler-core/src/type_/expression.rs | 108 +++++++----------- compiler-core/src/type_/pattern.rs | 2 +- compiler-core/src/type_/pipe.rs | 61 ++++------ ...__errors__same_imports_multiple_times.snap | 4 +- 8 files changed, 93 insertions(+), 133 deletions(-) diff --git a/compiler-core/src/error.rs b/compiler-core/src/error.rs index 0b96e4eae97..f3ef7712bab 100644 --- a/compiler-core/src/error.rs +++ b/compiler-core/src/error.rs @@ -2402,7 +2402,7 @@ but no type in scope with that name." discarded_location, name, type_with_name_in_scope, - imported_modules_with_same_public_variable_name, + possible_modules, } => { let title = String::from("Unknown variable"); if let Some(ignored_location) = discarded_location { @@ -2443,20 +2443,15 @@ but no type in scope with that name." wrap_format!("The name `{name}` is not in scope here.") }; - // If there are some suggestions about variables in imported modules - // put a "consider" text after the main message - if !imported_modules_with_same_public_variable_name.is_empty() { - let consider_text = imported_modules_with_same_public_variable_name - .iter() - .fold( - String::from("Consider using one of these variables:\n\n"), - |mut acc, module_name| { - acc.push_str(&wrap_format!(" {module_name}.{name}\n")); - acc - } - ); + // If there are some suggestions about public values in imported + // modules put a "did you mean" text after the main message + if !possible_modules.is_empty() { + let mut did_you_mean_text = String::from("Did you mean one of these:\n\n"); + for module_name in possible_modules { + did_you_mean_text.push_str(&format!(" - {module_name}.{name}\n")); + } text.push('\n'); - text.push_str(&consider_text); + text.push_str(&did_you_mean_text); } text diff --git a/compiler-core/src/type_.rs b/compiler-core/src/type_.rs index b4d0b36a92c..6e4f5370996 100644 --- a/compiler-core/src/type_.rs +++ b/compiler-core/src/type_.rs @@ -1103,6 +1103,15 @@ impl ModuleInterface { } } + pub fn get_public_function(&self, name: &str, arity: usize) -> Option<&ValueConstructor> { + self.get_public_value(name).filter(|value_constructor| { + match value_constructor.type_.fn_types() { + Some((fn_arguments_type, _)) => fn_arguments_type.len() == arity, + None => false, + } + }) + } + pub fn get_public_type(&self, name: &str) -> Option<&TypeConstructor> { let type_ = self.types.get(name)?; if type_.publicity.is_importable() { @@ -1680,14 +1689,11 @@ pub enum FieldAccessUsage { Other, } -/// This is used to know when a variable is used as a value or as a call: -/// function call, a pipeline or a custom type variant constructor +/// This is used to know when a value is used as a call or not. #[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub enum VarUsage { - /// Used as `call()` or `left |> right` +pub enum ValueUsage { + /// Used as `call(..)`, `Type(..)`, `left |> right` or `left |> right(..)` Call { arity: usize }, - /// Used as `left |> right(..)` - PipelineCall, /// Used as `variable` Other, } diff --git a/compiler-core/src/type_/environment.rs b/compiler-core/src/type_/environment.rs index f883a5e6546..6b3dd194bb4 100644 --- a/compiler-core/src/type_/environment.rs +++ b/compiler-core/src/type_/environment.rs @@ -488,7 +488,7 @@ impl Environment<'_> { name: name.clone(), variables: self.local_value_names(), type_with_name_in_scope: self.module_types.keys().any(|typ| typ == name), - imported_modules_with_same_public_variable_name: self + possible_modules: self .imported_modules .iter() .filter_map(|(module_name, (_, module))| { diff --git a/compiler-core/src/type_/error.rs b/compiler-core/src/type_/error.rs index 214da2e1d26..5c1f0712716 100644 --- a/compiler-core/src/type_/error.rs +++ b/compiler-core/src/type_/error.rs @@ -167,7 +167,7 @@ pub enum Error { type_with_name_in_scope: bool, /// Filled with the name of imported modules when the module has public value /// with the same name as this variable - imported_modules_with_same_public_variable_name: Vec, + possible_modules: Vec, }, UnknownType { @@ -1317,7 +1317,7 @@ pub enum UnknownValueConstructorError { name: EcoString, variables: Vec, type_with_name_in_scope: bool, - imported_modules_with_same_public_variable_name: Vec, + possible_modules: Vec, }, Module { @@ -1343,14 +1343,14 @@ pub fn convert_get_value_constructor_error( name, variables, type_with_name_in_scope, - imported_modules_with_same_public_variable_name, + possible_modules, } => Error::UnknownVariable { location, name, variables, discarded_location: None, type_with_name_in_scope, - imported_modules_with_same_public_variable_name, + possible_modules, }, UnknownValueConstructorError::Module { name, suggestions } => Error::UnknownModule { diff --git a/compiler-core/src/type_/expression.rs b/compiler-core/src/type_/expression.rs index d849d8ceeb5..b4421706579 100644 --- a/compiler-core/src/type_/expression.rs +++ b/compiler-core/src/type_/expression.rs @@ -435,7 +435,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { UntypedExpr::Var { location, name, .. } => self.infer_var( name, location, - VarUsage::Other, + ValueUsage::Other, ReferenceRegistration::RegisterReferences, ), @@ -727,7 +727,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { } // Helper to create a new error expr. - pub(crate) fn error_expr(&mut self, location: SrcSpan) -> TypedExpr { + fn error_expr(&mut self, location: SrcSpan) -> TypedExpr { TypedExpr::Invalid { location, type_: self.new_unbound_var(), @@ -1246,11 +1246,11 @@ impl<'a, 'b> ExprTyper<'a, 'b> { } } - pub(crate) fn infer_var( + fn infer_var( &mut self, name: EcoString, location: SrcSpan, - var_usage: VarUsage, + var_usage: ValueUsage, register_reference: ReferenceRegistration, ) -> Result { let constructor = self.do_infer_value_constructor( @@ -1354,7 +1354,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { UntypedExpr::Var { location, name } => self.infer_var( name, location, - VarUsage::Other, + ValueUsage::Other, ReferenceRegistration::DoNotRegisterReferences, ), _ => self.infer_or_error(container), @@ -2320,7 +2320,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { match guard { ClauseGuard::Var { location, name, .. } => { let constructor = - self.infer_value_constructor(&None, &name, &location, VarUsage::Other)?; + self.infer_value_constructor(&None, &name, &location, ValueUsage::Other)?; // We cannot support all values in guard expressions as the BEAM does not let definition_location = match &constructor.variant { @@ -3483,7 +3483,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { module: &Option<(EcoString, SrcSpan)>, name: &EcoString, location: &SrcSpan, - var_usage: VarUsage, + var_usage: ValueUsage, ) -> Result { self.do_infer_value_constructor( module, @@ -3499,7 +3499,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { module: &Option<(EcoString, SrcSpan)>, name: &EcoString, location: &SrcSpan, - var_usage: VarUsage, + var_usage: ValueUsage, register_reference: ReferenceRegistration, ) -> Result { let constructor = match module { @@ -3640,7 +3640,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { &mut self, name: &EcoString, location: &SrcSpan, - var_usage: VarUsage, + var_usage: ValueUsage, ) -> Error { // First try to see if this is a module alias: // `import gleam/io` @@ -3653,47 +3653,21 @@ impl<'a, 'b> ExprTyper<'a, 'b> { name: name.clone(), }, None => { - let imported_modules_with_same_public_variable_name = match var_usage { + let possible_modules = match var_usage { // This is a function call, we need to suggest a public - // variable which is a function with the correct arity - VarUsage::Call { arity } => self + // value which is a function with the correct arity + ValueUsage::Call { arity } => self .environment .imported_modules .iter() .filter_map(|(module_name, (_, module))| { - module - .get_public_value(name) - .filter(|value_constructor| { - match value_constructor.type_.fn_types() { - Some((fn_arguments_type, _)) => { - fn_arguments_type.len() == arity - } - None => false, - } - }) - .map(|_| module_name) - }) - .cloned() - .collect_vec(), - // This is a `Call` into a `Pipeline`. This is hard to make - // good suggestions because of the way it can be desugared. - // In this case, return every functions with the same name - // even if they have wrong arity. - VarUsage::PipelineCall => self - .environment - .imported_modules - .iter() - .filter_map(|(module_name, (_, module))| { - module - .get_public_value(name) - .filter(|value_constructor| value_constructor.type_.is_fun()) - .map(|_| module_name) + module.get_public_function(name, arity).map(|_| module_name) }) .cloned() .collect_vec(), // This is a reference to a variable, we need to suggest - // public variables of any type - VarUsage::Other => self + // a public value of any type + ValueUsage::Other => self .environment .imported_modules .iter() @@ -3718,7 +3692,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { .module_types .keys() .any(|typ| typ == name), - imported_modules_with_same_public_variable_name, + possible_modules, } } } @@ -3779,7 +3753,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { } if arguments.is_empty() => { // Type check the record constructor let constructor = - self.infer_value_constructor(&module, &name, &location, VarUsage::Other)?; + self.infer_value_constructor(&module, &name, &location, ValueUsage::Other)?; let (tag, field_map) = match &constructor.variant { ValueConstructorVariant::Record { @@ -3822,7 +3796,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { &module, &name, &location, - VarUsage::Call { + ValueUsage::Call { arity: arguments.len(), }, )?; @@ -3968,7 +3942,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { } => { // Infer the type of this constant let constructor = - self.infer_value_constructor(&module, &name, &location, VarUsage::Other)?; + self.infer_value_constructor(&module, &name, &location, ValueUsage::Other)?; match constructor.variant { ValueConstructorVariant::ModuleConstant { .. } | ValueConstructorVariant::LocalConstant { .. } @@ -4146,27 +4120,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { ) -> (TypedExpr, Vec, Arc) { let fun = match fun { UntypedExpr::Var { location, name } => { - // Because we are not calling infer / infer_or_error directly, - // we do not warn if there was a previous panic. Check for that - // here. Not perfect, but works. - if self.previous_panics { - self.warn_for_unreachable_code(location, PanicPosition::PreviousExpression); - } - - match self.infer_var( - name, - location, - VarUsage::Call { - arity: arguments.len(), - }, - ReferenceRegistration::RegisterReferences, - ) { - Ok(typed_expr) => typed_expr, - Err(error) => { - self.problems.error(error); - self.error_expr(location) - } - } + self.infer_called_var(name, location, arguments.len()) } UntypedExpr::FieldAccess { @@ -4206,6 +4160,26 @@ impl<'a, 'b> ExprTyper<'a, 'b> { (fun, arguments, type_) } + pub fn infer_called_var( + &mut self, + name: EcoString, + location: SrcSpan, + arity: usize, + ) -> TypedExpr { + match self.infer_var( + name, + location, + ValueUsage::Call { arity }, + ReferenceRegistration::RegisterReferences, + ) { + Ok(typed_expr) => typed_expr, + Err(error) => { + self.problems.error(error); + self.error_expr(location) + } + } + } + fn infer_fn_with_call_context( &mut self, arguments: Vec, @@ -4768,7 +4742,7 @@ fn is_trusted_pure_module(environment: &Environment<'_>) -> bool { } #[derive(Debug, Clone, Copy)] -pub(crate) enum ReferenceRegistration { +enum ReferenceRegistration { RegisterReferences, DoNotRegisterReferences, } diff --git a/compiler-core/src/type_/pattern.rs b/compiler-core/src/type_/pattern.rs index 21759d6dae7..49dbebdf964 100644 --- a/compiler-core/src/type_/pattern.rs +++ b/compiler-core/src/type_/pattern.rs @@ -1259,7 +1259,7 @@ impl<'a, 'b> PatternTyper<'a, 'b> { .module_types .keys() .any(|type_| type_ == &name), - imported_modules_with_same_public_variable_name: self + possible_modules: self .environment .imported_modules .iter() diff --git a/compiler-core/src/type_/pipe.rs b/compiler-core/src/type_/pipe.rs index f3249bb8405..244665c5ef4 100644 --- a/compiler-core/src/type_/pipe.rs +++ b/compiler-core/src/type_/pipe.rs @@ -1,6 +1,6 @@ use self::expression::CallKind; -use super::{expression::ReferenceRegistration, *}; +use super::*; use crate::ast::{ FunctionLiteralKind, ImplicitCallArgOrigin, PIPE_VARIABLE, PipelineAssignmentKind, Statement, TypedPipelineAssignment, UntypedExpr, @@ -139,25 +139,24 @@ impl<'a, 'b, 'c> PipeTyper<'a, 'b, 'c> { location, .. } => { - let fun_result = match *fun { - UntypedExpr::Var { location, name } => self.expr_typer.infer_var( - name, - location, - VarUsage::PipelineCall, - ReferenceRegistration::RegisterReferences, - ), - fun => self.expr_typer.infer_or_error(fun), - }; - - let fun = match fun_result { - Ok(fun) => fun, - Err(e) => { - // In case we cannot infer the function we'll - // replace it with an invalid expression with an - // unbound type to keep going! - self.expr_typer.problems.error(e); - self.expr_typer.error_expr(location) + let fun = match *fun { + UntypedExpr::Var { location, name } => { + self.expr_typer + .infer_called_var(name, location, arguments.len() + 1) } + untyped_fun => match self.expr_typer.infer_or_error(untyped_fun) { + Ok(typed_fun) => typed_fun, + Err(e) => { + // In case we cannot infer the function we'll + // replace it with an invalid expression with an + // unbound type to keep going! + self.expr_typer.problems.error(e); + TypedExpr::Invalid { + location, + type_: self.expr_typer.new_unbound_var(), + } + } + }, }; match fun.type_().fn_types() { @@ -357,27 +356,13 @@ impl<'a, 'b, 'c> PipeTyper<'a, 'b, 'c> { /// b is the `function` argument. fn infer_apply_pipe(&mut self, function: UntypedExpr) -> TypedExpr { let function_location = function.location(); - // Need one more step for better error suggestion. If the function is - // a variable / identifier, then we can suggest variables / identifier + // If the function is used as a variable, then we can suggest values // from imported modules with the same name and same arity. - let function_result = match function { - UntypedExpr::Var { location, name } => self.expr_typer.infer_var( - name, - location, - VarUsage::Call { arity: 1 }, - ReferenceRegistration::RegisterReferences, - ), - _ => self.expr_typer.infer_or_error(function), - }; - - let function = Box::new(match function_result { - Ok(function) => function, - Err(error) => { - // If we cannot infer the function we put an invalid expression - // in its place so we can still keep going with the other steps. - self.expr_typer.problems.error(error); - self.expr_typer.error_expr(function_location) + let function = Box::new(match function { + UntypedExpr::Var { location, name } => { + self.expr_typer.infer_called_var(name, location, 1) } + _ => self.expr_typer.infer(function), }); self.expr_typer.purity = self diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__same_imports_multiple_times.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__same_imports_multiple_times.snap index a2000d106c0..13472063940 100644 --- a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__same_imports_multiple_times.snap +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__same_imports_multiple_times.snap @@ -35,6 +35,6 @@ error: Unknown variable │ ^^^ The name `zoo` is not in scope here. -Consider using one of these variables: +Did you mean one of these: - wibble.zoo + - wibble.zoo From 0c30c19b568696884278ce46f9fd0652f01e44a9 Mon Sep 17 00:00:00 2001 From: raphrous Date: Fri, 15 Aug 2025 11:48:40 +0200 Subject: [PATCH 7/9] Add better unreachable code warnings when a panic is before a use expression. With previous changes, a test was broken when a panic was right before a use expression. The test was like that: ```gleam pub fn wibble(_) { 1 } pub fn main() { panic use <- wibble 1 } ``` This was the case because, before these changes, `UntypedExpr::Var` has not special call path in `do_infer_call` function. `infer` and `infer_or_error` was called has expected and a warning was emitted by `warn_for_unreachable_code` in case of a previous panic expression. Actually, `UntypedExpr::FieldAccess` has a special call path in `do_infer_call` and compiler does not warned about a program like that: ```gleam // Define pub fn wibble(_) { 1 } import mylib/mymod pub fn main() { panic use <- mymod.wibble 1 } ``` With this change, use expression are always warned after a panic expression. --- compiler-core/src/type_/expression.rs | 8 ++++++ ...nings__unreachable_use_after_panic_1.snap} | 9 ++++--- ...rnings__unreachable_use_after_panic_2.snap | 27 +++++++++++++++++++ compiler-core/src/type_/tests/warnings.rs | 17 +++++++++++- 4 files changed, 56 insertions(+), 5 deletions(-) rename compiler-core/src/type_/tests/snapshots/{gleam_core__type___tests__warnings__unreachable_use_after_panic.snap => gleam_core__type___tests__warnings__unreachable_use_after_panic_1.snap} (74%) create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unreachable_use_after_panic_2.snap diff --git a/compiler-core/src/type_/expression.rs b/compiler-core/src/type_/expression.rs index b4421706579..f04858b7054 100644 --- a/compiler-core/src/type_/expression.rs +++ b/compiler-core/src/type_/expression.rs @@ -852,6 +852,14 @@ impl<'a, 'b> ExprTyper<'a, 'b> { // We use `stacker` to prevent overflowing the stack when many `use` // expressions are chained. See https://github.com/gleam-lang/gleam/issues/4287 let infer_call = || { + // We need this in the case where `call.function` has a special call path depending + // on the type such as `UntypedExpr::Var`. In these cases, `infer_call` does not call + // `infer_or_error`. `infer_or_error` is responsible for registering warnings about + // unreachable code and thus, warnings about unreachable code are not registered. + if self.previous_panics { + self.warn_for_unreachable_code(call_location, PanicPosition::PreviousExpression); + } + self.infer_call( *call.function, call.arguments, diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unreachable_use_after_panic.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unreachable_use_after_panic_1.snap similarity index 74% rename from compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unreachable_use_after_panic.snap rename to compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unreachable_use_after_panic_1.snap index 5d01624f481..d5402292651 100644 --- a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unreachable_use_after_panic.snap +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unreachable_use_after_panic_1.snap @@ -14,9 +14,10 @@ expression: "\n pub fn wibble(_) { 1 }\n pub fn main() {\n ----- WARNING warning: Unreachable code - ┌─ /src/warning/wrn.gleam:5:20 - │ -5 │ use <- wibble - │ ^^^^^^ + ┌─ /src/warning/wrn.gleam:5:13 + │ +5 │ ╭ use <- wibble +6 │ │ 1 + │ ╰─────────────^ This code is unreachable because it comes after a `panic`. diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unreachable_use_after_panic_2.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unreachable_use_after_panic_2.snap new file mode 100644 index 00000000000..fc4487191a8 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unreachable_use_after_panic_2.snap @@ -0,0 +1,27 @@ +--- +source: compiler-core/src/type_/tests/warnings.rs +expression: "\n import module\n pub fn a() {\n panic\n use <- module.wibble\n 1\n }\n " +--- +----- SOURCE CODE +-- module.gleam +pub fn wibble(_) { 1 } + +-- main.gleam + + import module + pub fn a() { + panic + use <- module.wibble + 1 + } + + +----- WARNING +warning: Unreachable code + ┌─ /src/warning/wrn.gleam:5:13 + │ +5 │ ╭ use <- module.wibble +6 │ │ 1 + │ ╰─────────────^ + +This code is unreachable because it comes after a `panic`. diff --git a/compiler-core/src/type_/tests/warnings.rs b/compiler-core/src/type_/tests/warnings.rs index 455d616e6c2..f346f01803e 100644 --- a/compiler-core/src/type_/tests/warnings.rs +++ b/compiler-core/src/type_/tests/warnings.rs @@ -1921,7 +1921,7 @@ fn doesnt_warn_twice_for_unreachable_code_if_has_already_warned_in_a_block_2() { } #[test] -fn unreachable_use_after_panic() { +fn unreachable_use_after_panic_1() { assert_warning!( r#" pub fn wibble(_) { 1 } @@ -1934,6 +1934,21 @@ fn unreachable_use_after_panic() { ); } +#[test] +fn unreachable_use_after_panic_2() { + assert_warning!( + ("package", "module", r#"pub fn wibble(_) { 1 }"#), + r#" + import module + pub fn a() { + panic + use <- module.wibble + 1 + } + "# + ); +} + #[test] fn unreachable_code_after_case_subject_panics_1() { assert_warning!( From 0ea266ce8b4c38c3c2e0e701eb7e358e8493618a Mon Sep 17 00:00:00 2001 From: raphrous Date: Tue, 26 Aug 2025 13:13:51 +0200 Subject: [PATCH 8/9] Add tests for all possible cases for this feature. --- compiler-core/src/type_/tests/errors.rs | 220 ++++++++++++++++++ ...__unknown_variable_possible_modules_1.snap | 27 +++ ..._unknown_variable_possible_modules_10.snap | 31 +++ ..._unknown_variable_possible_modules_11.snap | 28 +++ ..._unknown_variable_possible_modules_12.snap | 32 +++ ...__unknown_variable_possible_modules_2.snap | 27 +++ ...__unknown_variable_possible_modules_3.snap | 24 ++ ...__unknown_variable_possible_modules_4.snap | 27 +++ ...__unknown_variable_possible_modules_5.snap | 26 +++ ...__unknown_variable_possible_modules_6.snap | 30 +++ ...__unknown_variable_possible_modules_7.snap | 32 +++ ...__unknown_variable_possible_modules_8.snap | 39 ++++ ...__unknown_variable_possible_modules_9.snap | 31 +++ 13 files changed, 574 insertions(+) create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_1.snap create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_10.snap create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_11.snap create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_12.snap create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_2.snap create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_3.snap create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_4.snap create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_5.snap create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_6.snap create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_7.snap create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_8.snap create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_9.snap diff --git a/compiler-core/src/type_/tests/errors.rs b/compiler-core/src/type_/tests/errors.rs index 2ba6d7a5534..77c9d0b65b4 100644 --- a/compiler-core/src/type_/tests/errors.rs +++ b/compiler-core/src/type_/tests/errors.rs @@ -3308,3 +3308,223 @@ pub fn main() { " ); } + +#[test] +fn unknown_variable_possible_modules_1() { + assert_module_error!( + ("module", "pub const one = 1"), + " +import module +pub fn main() { + one +} +" + ); +} + +#[test] +fn unknown_variable_possible_modules_2() { + assert_module_error!( + ("module", "pub fn add(x: Int, y: Int) { x + y }"), + " +import module +pub fn main() { + add(1, 1) +} +" + ); +} + +#[test] +fn unknown_variable_possible_modules_3() { + assert_module_error!( + ("module", "pub fn add(x: Int) { x + 1 }"), + " +import module +pub fn main() { + add(1, 1) +} +" + ); +} + +#[test] +fn unknown_variable_possible_modules_4() { + assert_module_error!( + ("module", "pub fn add(x: Float, y: Float) { x +. y }"), + " +import module +pub fn main() { + add(1, 1) +} +" + ); +} + +#[test] +fn unknown_variable_possible_modules_5() { + assert_module_error!( + ( + "module", + " +fn add(x: Int, y: Int) { x + y } +" + ), + " +import module +pub fn main() { + add(1, 1) +} +" + ); +} + +#[test] +fn unknown_variable_possible_modules_6() { + assert_module_error!( + ( + "module", + " +pub const add = internal_add +fn internal_add(x: Int, y: Int) { x + y } +" + ), + " +import module +pub fn main() { + add(1, 1) +} +" + ); +} + +#[test] +fn unknown_variable_possible_modules_7() { + assert_module_error!( + ( + "module", + " +pub type OneOrTwo { + One + Two +} +" + ), + " +import module +pub fn main() { + One +} +" + ); +} + +#[test] +fn unknown_variable_possible_modules_8() { + assert_module_error!( + ( + "moduleone", + " +pub type MyType { + MyRecord(x: Int, y: Int) +} +" + ), + ( + "moduletwo", + " +pub type AnotherType { + MyRecord(x: Int) +} +" + ), + " +import moduleone +import moduletwo +pub fn main() { + MyRecord(1) +} +" + ); +} + +#[test] +fn unknown_variable_possible_modules_9() { + assert_module_error!( + ( + "module", + " +pub fn add(x: Int) { + x + 1 +} +" + ), + " +import module +pub fn main() { + 1 |> add +} +" + ); +} + +#[test] +fn unknown_variable_possible_modules_10() { + assert_module_error!( + ( + "module", + " +pub fn add(x: Int, y: Int) { + x + y +} +" + ), + " +import module +pub fn main() { + 1 |> add(2) +} +" + ); +} + +#[test] +fn unknown_variable_possible_modules_11() { + assert_module_error!( + ( + "module", + " +pub fn add(x: Int) { + fn(y: Int) { x + y } +} +" + ), + " +import module +pub fn main() { + 1 |> add(2) +} +" + ); +} + +#[test] +fn unknown_variable_possible_modules_12() { + assert_module_error!( + ( + "module", + " +pub fn wibble(_) { + 1 +} +" + ), + " +import module +pub fn main() { + use <- wibble + 1 +} +" + ); +} diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_1.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_1.snap new file mode 100644 index 00000000000..96c9f15f014 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_1.snap @@ -0,0 +1,27 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\nimport module\npub fn main() {\n one\n}\n" +--- +----- SOURCE CODE +-- module.gleam +pub const one = 1 + +-- main.gleam + +import module +pub fn main() { + one +} + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:4:5 + │ +4 │ one + │ ^^^ + +The name `one` is not in scope here. +Did you mean one of these: + + - module.one diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_10.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_10.snap new file mode 100644 index 00000000000..37c98f3137d --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_10.snap @@ -0,0 +1,31 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\nimport module\npub fn main() {\n 1 |> add(2)\n}\n" +--- +----- SOURCE CODE +-- module.gleam + +pub fn add(x: Int, y: Int) { + x + y +} + + +-- main.gleam + +import module +pub fn main() { + 1 |> add(2) +} + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:4:10 + │ +4 │ 1 |> add(2) + │ ^^^ + +The name `add` is not in scope here. +Did you mean one of these: + + - module.add diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_11.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_11.snap new file mode 100644 index 00000000000..d1dad471a7f --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_11.snap @@ -0,0 +1,28 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\nimport module\npub fn main() {\n 1 |> add(2)\n}\n" +--- +----- SOURCE CODE +-- module.gleam + +pub fn add(x: Int) { + fn(y: Int) { x + y } +} + + +-- main.gleam + +import module +pub fn main() { + 1 |> add(2) +} + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:4:10 + │ +4 │ 1 |> add(2) + │ ^^^ + +The name `add` is not in scope here. diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_12.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_12.snap new file mode 100644 index 00000000000..ec21e5b6e99 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_12.snap @@ -0,0 +1,32 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\nimport module\npub fn main() {\n use <- wibble\n 1\n}\n" +--- +----- SOURCE CODE +-- module.gleam + +pub fn wibble(_) { + 1 +} + + +-- main.gleam + +import module +pub fn main() { + use <- wibble + 1 +} + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:4:12 + │ +4 │ use <- wibble + │ ^^^^^^ + +The name `wibble` is not in scope here. +Did you mean one of these: + + - module.wibble diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_2.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_2.snap new file mode 100644 index 00000000000..b7cf032e862 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_2.snap @@ -0,0 +1,27 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\nimport module\npub fn main() {\n add(1, 1)\n}\n" +--- +----- SOURCE CODE +-- module.gleam +pub fn add(x: Int, y: Int) { x + y } + +-- main.gleam + +import module +pub fn main() { + add(1, 1) +} + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:4:5 + │ +4 │ add(1, 1) + │ ^^^ + +The name `add` is not in scope here. +Did you mean one of these: + + - module.add diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_3.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_3.snap new file mode 100644 index 00000000000..0273a066daa --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_3.snap @@ -0,0 +1,24 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\nimport module\npub fn main() {\n add(1, 1)\n}\n" +--- +----- SOURCE CODE +-- module.gleam +pub fn add(x: Int) { x + 1 } + +-- main.gleam + +import module +pub fn main() { + add(1, 1) +} + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:4:5 + │ +4 │ add(1, 1) + │ ^^^ + +The name `add` is not in scope here. diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_4.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_4.snap new file mode 100644 index 00000000000..be57a6be9b4 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_4.snap @@ -0,0 +1,27 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\nimport module\npub fn main() {\n add(1, 1)\n}\n" +--- +----- SOURCE CODE +-- module.gleam +pub fn add(x: Float, y: Float) { x +. y } + +-- main.gleam + +import module +pub fn main() { + add(1, 1) +} + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:4:5 + │ +4 │ add(1, 1) + │ ^^^ + +The name `add` is not in scope here. +Did you mean one of these: + + - module.add diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_5.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_5.snap new file mode 100644 index 00000000000..49d3b6121e3 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_5.snap @@ -0,0 +1,26 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\nimport module\npub fn main() {\n add(1, 1)\n}\n" +--- +----- SOURCE CODE +-- module.gleam + +fn add(x: Int, y: Int) { x + y } + + +-- main.gleam + +import module +pub fn main() { + add(1, 1) +} + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:4:5 + │ +4 │ add(1, 1) + │ ^^^ + +The name `add` is not in scope here. diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_6.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_6.snap new file mode 100644 index 00000000000..8ebb2ffae30 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_6.snap @@ -0,0 +1,30 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\nimport module\npub fn main() {\n add(1, 1)\n}\n" +--- +----- SOURCE CODE +-- module.gleam + +pub const add = internal_add +fn internal_add(x: Int, y: Int) { x + y } + + +-- main.gleam + +import module +pub fn main() { + add(1, 1) +} + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:4:5 + │ +4 │ add(1, 1) + │ ^^^ + +The name `add` is not in scope here. +Did you mean one of these: + + - module.add diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_7.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_7.snap new file mode 100644 index 00000000000..e33cde5b595 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_7.snap @@ -0,0 +1,32 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\nimport module\npub fn main() {\n One\n}\n" +--- +----- SOURCE CODE +-- module.gleam + +pub type OneOrTwo { + One + Two +} + + +-- main.gleam + +import module +pub fn main() { + One +} + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:4:5 + │ +4 │ One + │ ^^^ + +The custom type variant constructor `One` is not in scope here. +Did you mean one of these: + + - module.One diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_8.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_8.snap new file mode 100644 index 00000000000..393acd338c7 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_8.snap @@ -0,0 +1,39 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\nimport moduleone\nimport moduletwo\npub fn main() {\n MyRecord(1)\n}\n" +--- +----- SOURCE CODE +-- moduleone.gleam + +pub type MyType { + MyRecord(x: Int, y: Int) +} + + +-- moduletwo.gleam + +pub type AnotherType { + MyRecord(x: Int) +} + + +-- main.gleam + +import moduleone +import moduletwo +pub fn main() { + MyRecord(1) +} + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:5:5 + │ +5 │ MyRecord(1) + │ ^^^^^^^^ + +The custom type variant constructor `MyRecord` is not in scope here. +Did you mean one of these: + + - moduletwo.MyRecord diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_9.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_9.snap new file mode 100644 index 00000000000..68cf7ea57b7 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_9.snap @@ -0,0 +1,31 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\nimport module\npub fn main() {\n 1 |> add\n}\n" +--- +----- SOURCE CODE +-- module.gleam + +pub fn add(x: Int) { + x + 1 +} + + +-- main.gleam + +import module +pub fn main() { + 1 |> add +} + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:4:10 + │ +4 │ 1 |> add + │ ^^^ + +The name `add` is not in scope here. +Did you mean one of these: + + - module.add From 8345a557e48b68e11939b17d4ad69a0b19a6a64b Mon Sep 17 00:00:00 2001 From: raphrous Date: Fri, 29 Aug 2025 07:47:00 +0200 Subject: [PATCH 9/9] Implements GearsDatapacks suggestions. --- CHANGELOG.md | 4 +-- compiler-core/src/error.rs | 6 ++--- compiler-core/src/type_/tests/errors.rs | 26 ++++++++++++++++++ ..._unknown_variable_possible_modules_13.snap | 27 +++++++++++++++++++ ..._unknown_variable_possible_modules_14.snap | 27 +++++++++++++++++++ 5 files changed, 84 insertions(+), 6 deletions(-) create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_13.snap create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_14.snap diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b9d6846a15..25a6a214a97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,9 +25,9 @@ │ ^^^^^^^ The name `println` is not in scope here. - Consider using one of these variables: + Did you mean one of these: - io.println + - io.println ``` ([raphrous](https://github.com/realraphrous)) diff --git a/compiler-core/src/error.rs b/compiler-core/src/error.rs index f3ef7712bab..784498d794b 100644 --- a/compiler-core/src/error.rs +++ b/compiler-core/src/error.rs @@ -2446,12 +2446,10 @@ but no type in scope with that name." // If there are some suggestions about public values in imported // modules put a "did you mean" text after the main message if !possible_modules.is_empty() { - let mut did_you_mean_text = String::from("Did you mean one of these:\n\n"); + text.push_str("\nDid you mean one of these:\n\n"); for module_name in possible_modules { - did_you_mean_text.push_str(&format!(" - {module_name}.{name}\n")); + text.push_str(&format!(" - {module_name}.{name}\n")) } - text.push('\n'); - text.push_str(&did_you_mean_text); } text diff --git a/compiler-core/src/type_/tests/errors.rs b/compiler-core/src/type_/tests/errors.rs index 77c9d0b65b4..3a5bc619a64 100644 --- a/compiler-core/src/type_/tests/errors.rs +++ b/compiler-core/src/type_/tests/errors.rs @@ -3528,3 +3528,29 @@ pub fn main() { " ); } + +#[test] +fn unknown_variable_possible_modules_13() { + assert_module_error!( + ("module", "pub fn add(x: Int, y: Int) { x + y }"), + " +import module as wibble +pub fn main() { + add(1, 1) +} +" + ); +} + +#[test] +fn unknown_variable_possible_modules_14() { + assert_module_error!( + ("gleam/module", "pub fn add(x: Int, y: Int) { x + y }"), + " +import gleam/module +pub fn main() { + add(1, 1) +} +" + ); +} diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_13.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_13.snap new file mode 100644 index 00000000000..f85f37d7f9a --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_13.snap @@ -0,0 +1,27 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\nimport module as wibble\npub fn main() {\n add(1, 1)\n}\n" +--- +----- SOURCE CODE +-- module.gleam +pub fn add(x: Int, y: Int) { x + y } + +-- main.gleam + +import module as wibble +pub fn main() { + add(1, 1) +} + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:4:5 + │ +4 │ add(1, 1) + │ ^^^ + +The name `add` is not in scope here. +Did you mean one of these: + + - wibble.add diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_14.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_14.snap new file mode 100644 index 00000000000..7b94d72e39b --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_variable_possible_modules_14.snap @@ -0,0 +1,27 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\nimport gleam/module\npub fn main() {\n add(1, 1)\n}\n" +--- +----- SOURCE CODE +-- gleam/module.gleam +pub fn add(x: Int, y: Int) { x + y } + +-- main.gleam + +import gleam/module +pub fn main() { + add(1, 1) +} + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:4:5 + │ +4 │ add(1, 1) + │ ^^^ + +The name `add` is not in scope here. +Did you mean one of these: + + - module.add