Skip to content

Commit ac4e471

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 732ade2 commit ac4e471

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
@@ -1662,6 +1662,16 @@ pub enum FieldAccessUsage {
16621662
Other,
16631663
}
16641664

1665+
/// This is used to know when a variable is used as a value or as a call:
1666+
/// function call, a pipeline or a custom type variant constructor
1667+
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
1668+
pub enum VarUsage {
1669+
/// Used as `variable()`
1670+
Call { arity: usize },
1671+
/// Used as `variable`
1672+
Other,
1673+
}
1674+
16651675
/// Verify that a value is suitable to be used as a main function.
16661676
fn assert_suitable_main_function(
16671677
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,
@@ -1139,10 +1142,16 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
11391142
&mut self,
11401143
name: EcoString,
11411144
location: SrcSpan,
1145+
var_usage: VarUsage,
11421146
register_reference: ReferenceRegistration,
11431147
) -> Result<TypedExpr, Error> {
1144-
let constructor =
1145-
self.do_infer_value_constructor(&None, &name, &location, register_reference)?;
1148+
let constructor = self.do_infer_value_constructor(
1149+
&None,
1150+
&name,
1151+
&location,
1152+
var_usage,
1153+
register_reference,
1154+
)?;
11461155
self.narrow_implementations(location, &constructor.variant)?;
11471156
Ok(TypedExpr::Var {
11481157
constructor,
@@ -1237,6 +1246,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
12371246
UntypedExpr::Var { location, name } => self.infer_var(
12381247
name,
12391248
location,
1249+
VarUsage::Other,
12401250
ReferenceRegistration::DoNotRegisterReferences,
12411251
),
12421252
_ => self.infer_or_error(container),
@@ -2202,7 +2212,8 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
22022212
fn infer_clause_guard(&mut self, guard: UntypedClauseGuard) -> Result<TypedClauseGuard, Error> {
22032213
match guard {
22042214
ClauseGuard::Var { location, name, .. } => {
2205-
let constructor = self.infer_value_constructor(&None, &name, &location)?;
2215+
let constructor =
2216+
self.infer_value_constructor(&None, &name, &location, VarUsage::Other)?;
22062217

22072218
// We cannot support all values in guard expressions as the BEAM does not
22082219
let definition_location = match &constructor.variant {
@@ -3352,11 +3363,13 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
33523363
module: &Option<(EcoString, SrcSpan)>,
33533364
name: &EcoString,
33543365
location: &SrcSpan,
3366+
var_usage: VarUsage,
33553367
) -> Result<ValueConstructor, Error> {
33563368
self.do_infer_value_constructor(
33573369
module,
33583370
name,
33593371
location,
3372+
var_usage,
33603373
ReferenceRegistration::RegisterReferences,
33613374
)
33623375
}
@@ -3366,6 +3379,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
33663379
module: &Option<(EcoString, SrcSpan)>,
33673380
name: &EcoString,
33683381
location: &SrcSpan,
3382+
var_usage: VarUsage,
33693383
register_reference: ReferenceRegistration,
33703384
) -> Result<ValueConstructor, Error> {
33713385
let constructor = match module {
@@ -3374,7 +3388,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
33743388
.environment
33753389
.get_variable(name)
33763390
.cloned()
3377-
.ok_or_else(|| self.report_name_error(name, location))?,
3391+
.ok_or_else(|| self.report_name_error(name, location, var_usage))?,
33783392

33793393
// Look in an imported module for a binding with this name
33803394
Some((module_name, module_location)) => {
@@ -3502,7 +3516,12 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
35023516
}
35033517
}
35043518

3505-
fn report_name_error(&mut self, name: &EcoString, location: &SrcSpan) -> Error {
3519+
fn report_name_error(
3520+
&mut self,
3521+
name: &EcoString,
3522+
location: &SrcSpan,
3523+
var_usage: VarUsage,
3524+
) -> Error {
35063525
// First try to see if this is a module alias:
35073526
// `import gleam/io`
35083527
// `io.debug(io)`
@@ -3513,30 +3532,59 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
35133532
location: *location,
35143533
name: name.clone(),
35153534
},
3516-
None => Error::UnknownVariable {
3517-
location: *location,
3518-
name: name.clone(),
3519-
variables: self.environment.local_value_names(),
3520-
discarded_location: self
3521-
.environment
3522-
.discarded_names
3523-
.get(&eco_format!("_{name}"))
3524-
.cloned(),
3525-
type_with_name_in_scope: self
3526-
.environment
3527-
.module_types
3528-
.keys()
3529-
.any(|typ| typ == name),
3530-
imported_modules_with_same_public_variable_name: self
3531-
.environment
3532-
.imported_modules
3533-
.iter()
3534-
.filter_map(|(module_name, (_, module))| {
3535-
module.get_public_value(name).map(|_| module_name)
3536-
})
3537-
.cloned()
3538-
.collect_vec(),
3539-
},
3535+
None => {
3536+
let imported_modules_with_same_public_variable_name = match var_usage {
3537+
// This is a function call, we need to suggest a public
3538+
// variable which is a function with the correct arity
3539+
VarUsage::Call { arity } => self
3540+
.environment
3541+
.imported_modules
3542+
.iter()
3543+
.filter_map(|(module_name, (_, module))| {
3544+
module
3545+
.get_public_value(name)
3546+
.filter(|value_constructor| {
3547+
match value_constructor.type_.fn_types() {
3548+
Some((fn_arguments_type, _)) => {
3549+
fn_arguments_type.len() == arity
3550+
}
3551+
None => false,
3552+
}
3553+
})
3554+
.map(|_| module_name)
3555+
})
3556+
.cloned()
3557+
.collect_vec(),
3558+
// This is a reference to a variable, we need to suggest
3559+
// public variables of any type
3560+
VarUsage::Other => self
3561+
.environment
3562+
.imported_modules
3563+
.iter()
3564+
.filter_map(|(module_name, (_, module))| {
3565+
module.get_public_value(name).map(|_| module_name)
3566+
})
3567+
.cloned()
3568+
.collect_vec(),
3569+
};
3570+
3571+
Error::UnknownVariable {
3572+
location: *location,
3573+
name: name.clone(),
3574+
variables: self.environment.local_value_names(),
3575+
discarded_location: self
3576+
.environment
3577+
.discarded_names
3578+
.get(&eco_format!("_{name}"))
3579+
.cloned(),
3580+
type_with_name_in_scope: self
3581+
.environment
3582+
.module_types
3583+
.keys()
3584+
.any(|typ| typ == name),
3585+
imported_modules_with_same_public_variable_name,
3586+
}
3587+
}
35403588
}
35413589
}
35423590

@@ -3594,7 +3642,8 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
35943642
..
35953643
} if arguments.is_empty() => {
35963644
// Type check the record constructor
3597-
let constructor = self.infer_value_constructor(&module, &name, &location)?;
3645+
let constructor =
3646+
self.infer_value_constructor(&module, &name, &location, VarUsage::Other)?;
35983647

35993648
let (tag, field_map) = match &constructor.variant {
36003649
ValueConstructorVariant::Record {
@@ -3633,7 +3682,14 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
36333682
// field_map, is always None here because untyped not yet unified
36343683
..
36353684
} => {
3636-
let constructor = self.infer_value_constructor(&module, &name, &location)?;
3685+
let constructor = self.infer_value_constructor(
3686+
&module,
3687+
&name,
3688+
&location,
3689+
VarUsage::Call {
3690+
arity: arguments.len(),
3691+
},
3692+
)?;
36373693

36383694
let (tag, field_map, variant_index) = match &constructor.variant {
36393695
ValueConstructorVariant::Record {
@@ -3775,7 +3831,8 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
37753831
..
37763832
} => {
37773833
// Infer the type of this constant
3778-
let constructor = self.infer_value_constructor(&module, &name, &location)?;
3834+
let constructor =
3835+
self.infer_value_constructor(&module, &name, &location, VarUsage::Other)?;
37793836
match constructor.variant {
37803837
ValueConstructorVariant::ModuleConstant { .. }
37813838
| ValueConstructorVariant::LocalConstant { .. }
@@ -3952,6 +4009,30 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
39524009
kind: CallKind,
39534010
) -> (TypedExpr, Vec<TypedCallArg>, Arc<Type>) {
39544011
let fun = match fun {
4012+
UntypedExpr::Var { location, name } => {
4013+
// Because we are not calling infer / infer_or_error directly,
4014+
// we do not warn if there was a previous panic. Check for that
4015+
// here. Not perfect, but works.
4016+
if self.previous_panics {
4017+
self.warn_for_unreachable_code(location, PanicPosition::PreviousExpression);
4018+
}
4019+
4020+
match self.infer_var(
4021+
name,
4022+
location,
4023+
VarUsage::Call {
4024+
arity: arguments.len(),
4025+
},
4026+
ReferenceRegistration::RegisterReferences,
4027+
) {
4028+
Ok(typed_expr) => typed_expr,
4029+
Err(error) => {
4030+
self.problems.error(error);
4031+
self.error_expr(location)
4032+
}
4033+
}
4034+
}
4035+
39554036
UntypedExpr::FieldAccess {
39564037
label,
39574038
container,

0 commit comments

Comments
 (0)