Skip to content

Commit 0c73f82

Browse files
committed
Add suggestions for UntypedExpr::Call that check the arity.
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 ```
1 parent 490e07a commit 0c73f82

File tree

2 files changed

+126
-35
lines changed

2 files changed

+126
-35
lines changed

compiler-core/src/type_.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1680,6 +1680,16 @@ pub enum FieldAccessUsage {
16801680
Other,
16811681
}
16821682

1683+
/// This is used to know when a variable is used as a value or as a call:
1684+
/// function call, a pipeline or a custom type variant constructor
1685+
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
1686+
pub enum VarUsage {
1687+
/// Used as `variable()`
1688+
Call { arity: usize },
1689+
/// Used as `variable`
1690+
Other,
1691+
}
1692+
16831693
/// Verify that a value is suitable to be used as a main function.
16841694
fn assert_suitable_main_function(
16851695
value: &ValueConstructor,

compiler-core/src/type_/expression.rs

Lines changed: 116 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -432,9 +432,12 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
432432
message,
433433
} => Ok(self.infer_echo(location, keyword_end, expression, message)),
434434

435-
UntypedExpr::Var { location, name, .. } => {
436-
self.infer_var(name, location, ReferenceRegistration::RegisterReferences)
437-
}
435+
UntypedExpr::Var { location, name, .. } => self.infer_var(
436+
name,
437+
location,
438+
VarUsage::Other,
439+
ReferenceRegistration::RegisterReferences,
440+
),
438441

439442
UntypedExpr::Int {
440443
location,
@@ -1247,10 +1250,16 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
12471250
&mut self,
12481251
name: EcoString,
12491252
location: SrcSpan,
1253+
var_usage: VarUsage,
12501254
register_reference: ReferenceRegistration,
12511255
) -> Result<TypedExpr, Error> {
1252-
let constructor =
1253-
self.do_infer_value_constructor(&None, &name, &location, register_reference)?;
1256+
let constructor = self.do_infer_value_constructor(
1257+
&None,
1258+
&name,
1259+
&location,
1260+
var_usage,
1261+
register_reference,
1262+
)?;
12541263
self.narrow_implementations(location, &constructor.variant)?;
12551264
Ok(TypedExpr::Var {
12561265
constructor,
@@ -1345,6 +1354,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
13451354
UntypedExpr::Var { location, name } => self.infer_var(
13461355
name,
13471356
location,
1357+
VarUsage::Other,
13481358
ReferenceRegistration::DoNotRegisterReferences,
13491359
),
13501360
_ => self.infer_or_error(container),
@@ -2309,7 +2319,8 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
23092319
fn infer_clause_guard(&mut self, guard: UntypedClauseGuard) -> Result<TypedClauseGuard, Error> {
23102320
match guard {
23112321
ClauseGuard::Var { location, name, .. } => {
2312-
let constructor = self.infer_value_constructor(&None, &name, &location)?;
2322+
let constructor =
2323+
self.infer_value_constructor(&None, &name, &location, VarUsage::Other)?;
23132324

23142325
// We cannot support all values in guard expressions as the BEAM does not
23152326
let definition_location = match &constructor.variant {
@@ -3472,11 +3483,13 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
34723483
module: &Option<(EcoString, SrcSpan)>,
34733484
name: &EcoString,
34743485
location: &SrcSpan,
3486+
var_usage: VarUsage,
34753487
) -> Result<ValueConstructor, Error> {
34763488
self.do_infer_value_constructor(
34773489
module,
34783490
name,
34793491
location,
3492+
var_usage,
34803493
ReferenceRegistration::RegisterReferences,
34813494
)
34823495
}
@@ -3486,6 +3499,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
34863499
module: &Option<(EcoString, SrcSpan)>,
34873500
name: &EcoString,
34883501
location: &SrcSpan,
3502+
var_usage: VarUsage,
34893503
register_reference: ReferenceRegistration,
34903504
) -> Result<ValueConstructor, Error> {
34913505
let constructor = match module {
@@ -3494,7 +3508,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
34943508
.environment
34953509
.get_variable(name)
34963510
.cloned()
3497-
.ok_or_else(|| self.report_name_error(name, location))?,
3511+
.ok_or_else(|| self.report_name_error(name, location, var_usage))?,
34983512

34993513
// Look in an imported module for a binding with this name
35003514
Some((module_name, module_location)) => {
@@ -3622,7 +3636,12 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
36223636
}
36233637
}
36243638

3625-
fn report_name_error(&mut self, name: &EcoString, location: &SrcSpan) -> Error {
3639+
fn report_name_error(
3640+
&mut self,
3641+
name: &EcoString,
3642+
location: &SrcSpan,
3643+
var_usage: VarUsage,
3644+
) -> Error {
36263645
// First try to see if this is a module alias:
36273646
// `import gleam/io`
36283647
// `io.debug(io)`
@@ -3633,30 +3652,59 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
36333652
location: *location,
36343653
name: name.clone(),
36353654
},
3636-
None => Error::UnknownVariable {
3637-
location: *location,
3638-
name: name.clone(),
3639-
variables: self.environment.local_value_names(),
3640-
discarded_location: self
3641-
.environment
3642-
.discarded_names
3643-
.get(&eco_format!("_{name}"))
3644-
.cloned(),
3645-
type_with_name_in_scope: self
3646-
.environment
3647-
.module_types
3648-
.keys()
3649-
.any(|typ| typ == name),
3650-
imported_modules_with_same_public_variable_name: self
3651-
.environment
3652-
.imported_modules
3653-
.iter()
3654-
.filter_map(|(module_name, (_, module))| {
3655-
module.get_public_value(name).map(|_| module_name)
3656-
})
3657-
.cloned()
3658-
.collect_vec(),
3659-
},
3655+
None => {
3656+
let imported_modules_with_same_public_variable_name = match var_usage {
3657+
// This is a function call, we need to suggest a public
3658+
// variable which is a function with the correct arity
3659+
VarUsage::Call { arity } => self
3660+
.environment
3661+
.imported_modules
3662+
.iter()
3663+
.filter_map(|(module_name, (_, module))| {
3664+
module
3665+
.get_public_value(name)
3666+
.filter(|value_constructor| {
3667+
match value_constructor.type_.fn_types() {
3668+
Some((fn_arguments_type, _)) => {
3669+
fn_arguments_type.len() == arity
3670+
}
3671+
None => false,
3672+
}
3673+
})
3674+
.map(|_| module_name)
3675+
})
3676+
.cloned()
3677+
.collect_vec(),
3678+
// This is a reference to a variable, we need to suggest
3679+
// public variables of any type
3680+
VarUsage::Other => self
3681+
.environment
3682+
.imported_modules
3683+
.iter()
3684+
.filter_map(|(module_name, (_, module))| {
3685+
module.get_public_value(name).map(|_| module_name)
3686+
})
3687+
.cloned()
3688+
.collect_vec(),
3689+
};
3690+
3691+
Error::UnknownVariable {
3692+
location: *location,
3693+
name: name.clone(),
3694+
variables: self.environment.local_value_names(),
3695+
discarded_location: self
3696+
.environment
3697+
.discarded_names
3698+
.get(&eco_format!("_{name}"))
3699+
.cloned(),
3700+
type_with_name_in_scope: self
3701+
.environment
3702+
.module_types
3703+
.keys()
3704+
.any(|typ| typ == name),
3705+
imported_modules_with_same_public_variable_name,
3706+
}
3707+
}
36603708
}
36613709
}
36623710

@@ -3714,7 +3762,8 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
37143762
..
37153763
} if arguments.is_empty() => {
37163764
// Type check the record constructor
3717-
let constructor = self.infer_value_constructor(&module, &name, &location)?;
3765+
let constructor =
3766+
self.infer_value_constructor(&module, &name, &location, VarUsage::Other)?;
37183767

37193768
let (tag, field_map) = match &constructor.variant {
37203769
ValueConstructorVariant::Record {
@@ -3753,7 +3802,14 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
37533802
// field_map, is always None here because untyped not yet unified
37543803
..
37553804
} => {
3756-
let constructor = self.infer_value_constructor(&module, &name, &location)?;
3805+
let constructor = self.infer_value_constructor(
3806+
&module,
3807+
&name,
3808+
&location,
3809+
VarUsage::Call {
3810+
arity: arguments.len(),
3811+
},
3812+
)?;
37573813

37583814
let (tag, field_map, variant_index) = match &constructor.variant {
37593815
ValueConstructorVariant::Record {
@@ -3895,7 +3951,8 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
38953951
..
38963952
} => {
38973953
// Infer the type of this constant
3898-
let constructor = self.infer_value_constructor(&module, &name, &location)?;
3954+
let constructor =
3955+
self.infer_value_constructor(&module, &name, &location, VarUsage::Other)?;
38993956
match constructor.variant {
39003957
ValueConstructorVariant::ModuleConstant { .. }
39013958
| ValueConstructorVariant::LocalConstant { .. }
@@ -4072,6 +4129,30 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
40724129
kind: CallKind,
40734130
) -> (TypedExpr, Vec<TypedCallArg>, Arc<Type>) {
40744131
let fun = match fun {
4132+
UntypedExpr::Var { location, name } => {
4133+
// Because we are not calling infer / infer_or_error directly,
4134+
// we do not warn if there was a previous panic. Check for that
4135+
// here. Not perfect, but works.
4136+
if self.previous_panics {
4137+
self.warn_for_unreachable_code(location, PanicPosition::PreviousExpression);
4138+
}
4139+
4140+
match self.infer_var(
4141+
name,
4142+
location,
4143+
VarUsage::Call {
4144+
arity: arguments.len(),
4145+
},
4146+
ReferenceRegistration::RegisterReferences,
4147+
) {
4148+
Ok(typed_expr) => typed_expr,
4149+
Err(error) => {
4150+
self.problems.error(error);
4151+
self.error_expr(location)
4152+
}
4153+
}
4154+
}
4155+
40754156
UntypedExpr::FieldAccess {
40764157
label,
40774158
container,

0 commit comments

Comments
 (0)