diff --git a/compiler-core/src/analyse.rs b/compiler-core/src/analyse.rs index d78229d84ab..340c49f1c2a 100644 --- a/compiler-core/src/analyse.rs +++ b/compiler-core/src/analyse.rs @@ -937,7 +937,7 @@ impl<'a, A> ModuleAnalyzer<'a, A> { ) .collect(); let typed_parameters = environment - .get_type_constructor(&None, &name) + .get_type_constructor(&None, &name, Some(parameters.len())) .expect("Could not find preregistered type constructor") .parameters .clone(); @@ -1601,7 +1601,7 @@ fn analyse_type_alias(t: UntypedTypeAlias, environment: &mut Environment<'_>) -> // analysis aims to be fault tolerant to get the best possible feedback for // the programmer in the language server, so the analyser gets here even // though there was previously errors. - let type_ = match environment.get_type_constructor(&None, &alias) { + let type_ = match environment.get_type_constructor(&None, &alias, Some(args.len())) { Ok(constructor) => constructor.type_.clone(), Err(_) => environment.new_generic_var(), }; diff --git a/compiler-core/src/error.rs b/compiler-core/src/error.rs index bbc7a87d46f..93c9e66ac59 100644 --- a/compiler-core/src/error.rs +++ b/compiler-core/src/error.rs @@ -1,4 +1,5 @@ #![allow(clippy::unwrap_used, clippy::expect_used)] +use crate::ast::Layer; use crate::build::{Origin, Outcome, Runtime, Target}; use crate::diagnostic::{Diagnostic, ExtraLabel, Label, Location}; use crate::strings::{to_snake_case, to_upper_camel_case}; @@ -2339,6 +2340,7 @@ Note: If the same type variable is used for multiple fields, all those fields ne location, name, hint, + suggestions } => { let label_text = match hint { UnknownTypeHint::AlternativeTypes(types) => did_you_mean(name, types), @@ -2363,7 +2365,10 @@ but no type in scope with that name." Diagnostic { title: "Unknown type".into(), text, - hint: None, + hint: match label_text { + None => suggestions.first().map(|suggestion| suggestion.suggest_unqualified_import(name, Layer::Type)), + Some(_) => None + }, level: Level::Error, location: Some(Location { label: Label { @@ -2382,6 +2387,7 @@ but no type in scope with that name." variables, name, type_with_name_in_scope, + suggestions } => { let text = if *type_with_name_in_scope { wrap_format!("`{name}` is a type, it cannot be used as a value.") @@ -2397,7 +2403,7 @@ but no type in scope with that name." Diagnostic { title: "Unknown variable".into(), text, - hint: None, + hint: suggestions.first().map(|suggestion| suggestion.suggest_unqualified_import(name, Layer::Value)), level: Level::Error, location: Some(Location { label: Label { @@ -2452,7 +2458,7 @@ Private types can only be used within the module that defines them.", } => Diagnostic { title: "Unknown module".into(), text: format!("No module has been found with the name `{name}`."), - hint: suggestions.first().map(|suggestion| suggestion.suggestion(name)), + hint: suggestions.first().map(|suggestion| suggestion.suggest_import(name)), level: Level::Error, location: Some(Location { label: Label { diff --git a/compiler-core/src/exhaustiveness.rs b/compiler-core/src/exhaustiveness.rs index 12454fe67c1..1cea92f2f6e 100644 --- a/compiler-core/src/exhaustiveness.rs +++ b/compiler-core/src/exhaustiveness.rs @@ -815,7 +815,7 @@ impl Variable { .. } => { let constructors = ConstructorSpecialiser::specialise_constructors( - env.get_constructors_for_type(module, name) + env.get_constructors_for_type(module, name, Some(args.len())) .expect("Custom type variants must exist"), args.as_slice(), ); diff --git a/compiler-core/src/exhaustiveness/missing_patterns.rs b/compiler-core/src/exhaustiveness/missing_patterns.rs index f26192db121..7a2dd6b18ea 100644 --- a/compiler-core/src/exhaustiveness/missing_patterns.rs +++ b/compiler-core/src/exhaustiveness/missing_patterns.rs @@ -181,7 +181,7 @@ impl<'a, 'env> MissingPatternsGenerator<'a, 'env> { let name = self .environment - .get_constructors_for_type(&module, &name) + .get_constructors_for_type(&module, &name, Some(fields.len())) .expect("Custom type constructor must have custom type kind") .variants .get(*index) diff --git a/compiler-core/src/language_server/code_action.rs b/compiler-core/src/language_server/code_action.rs index 5a9cadd37ce..dcec139f93e 100644 --- a/compiler-core/src/language_server/code_action.rs +++ b/compiler-core/src/language_server/code_action.rs @@ -32,7 +32,10 @@ use vec1::{Vec1, vec1}; use super::{ TextEdits, compiler::LspProjectCompiler, - edits::{add_newlines_after_import, get_import_edit, position_of_first_definition_if_import}, + edits::{ + add_newlines_after_import, add_unqualified_import, get_import_edit, + get_unqualified_import_edit, position_of_first_definition_if_import, + }, engine::{overlaps, within}, files::FileSystemProxy, reference::find_variable_references, @@ -1011,6 +1014,220 @@ fn suggest_imports( } } +pub fn code_action_import_qualified_module_for_type_or_value( + module: &Module, + line_numbers: &LineNumbers, + params: &CodeActionParams, + error: &Option, + actions: &mut Vec, +) { + let uri = ¶ms.text_document.uri; + let Some(Error::Type { errors, .. }) = error else { + return; + }; + + let missing_imports = errors + .into_iter() + .filter_map(|e| match e { + type_::Error::UnknownType { + location, + suggestions, + name, + .. + } => suggest_unqualified_imports(*location, suggestions) + .map(|(location, suggestions)| (location, suggestions, name)), + type_::Error::UnknownVariable { + location, + suggestions, + name, + .. + } => suggest_unqualified_imports(*location, suggestions) + .map(|(location, suggestions)| (location, suggestions, name)), + _ => None, + }) + .collect_vec(); + + if missing_imports.is_empty() { + return; + } + + let first_import_pos = position_of_first_definition_if_import(module, line_numbers); + let first_is_import = first_import_pos.is_some(); + let import_location = first_import_pos.unwrap_or_default(); + + let after_import_newlines = + add_newlines_after_import(import_location, first_is_import, line_numbers, &module.code); + + for (location, suggestions, name) in missing_imports { + let range = src_span_to_lsp_range(location, line_numbers); + if !overlaps(params.range, range) { + continue; + } + + for suggestion in suggestions { + let (edits, title) = match suggestion { + ModuleSuggestion::Importable(full_name) => ( + vec![ + get_import_edit(import_location, full_name, &after_import_newlines), + TextEdit { + range: Range { + start: range.start, + end: range.start, + }, + new_text: suggestion.last_name_component().to_string() + ".", + }, + ], + &format!("Import `{full_name}` and reference it"), + ), + ModuleSuggestion::Imported(full_name) => { + let mut matching_import = None; + + for def in &module.ast.definitions { + if let ast::Definition::Import(import) = def { + if &import.module == full_name { + matching_import = Some(import); + break; + } + } + } + + let import = matching_import.expect("Couldn't find matching import"); + + ( + vec![TextEdit { + range: Range { + start: range.start, + end: range.start, + }, + new_text: import.used_name().unwrap_or(name.clone()).to_string() + ".", + }], + &format!( + "Qualify and reference `{}`", + import + .used_name() + .unwrap_or(suggestion.last_name_component().into()) + ), + ) + } + }; + + CodeActionBuilder::new(title) + .kind(CodeActionKind::QUICKFIX) + .changes(uri.clone(), edits) + .preferred(true) + .push_to(actions); + } + } +} + +pub fn code_action_import_unqualified_module_for_type_or_value( + module: &Module, + line_numbers: &LineNumbers, + params: &CodeActionParams, + error: &Option, + actions: &mut Vec, +) { + let uri = ¶ms.text_document.uri; + let Some(Error::Type { errors, .. }) = error else { + return; + }; + + let missing_imports = errors + .into_iter() + .filter_map(|e| match e { + type_::Error::UnknownType { + location, + suggestions, + name, + .. + } => suggest_unqualified_imports(*location, suggestions) + .map(|(location, suggestions)| (location, suggestions, name, ast::Layer::Type)), + type_::Error::UnknownVariable { + location, + suggestions, + name, + .. + } => suggest_unqualified_imports(*location, suggestions) + .map(|(location, suggestions)| (location, suggestions, name, ast::Layer::Value)), + _ => None, + }) + .collect_vec(); + + if missing_imports.is_empty() { + return; + } + + let first_import_pos = position_of_first_definition_if_import(module, line_numbers); + let first_is_import = first_import_pos.is_some(); + let import_location = first_import_pos.unwrap_or_default(); + + let after_import_newlines = + add_newlines_after_import(import_location, first_is_import, line_numbers, &module.code); + + for (location, suggestions, name, layer) in missing_imports { + let range = src_span_to_lsp_range(location, line_numbers); + if !overlaps(params.range, range) { + continue; + } + + for suggestion in suggestions { + let (edit, title) = match suggestion { + ModuleSuggestion::Importable(full_name) => ( + get_unqualified_import_edit( + import_location, + full_name, + name, + layer, + &after_import_newlines, + ), + &format!("Import `{name}` from `{full_name}`"), + ), + ModuleSuggestion::Imported(full_name) => { + let mut matching_import = None; + + for def in &module.ast.definitions { + if let ast::Definition::Import(import) = def { + if &import.module == full_name { + matching_import = Some(import); + break; + } + } + } + + let import = matching_import.expect("Couldn't find matching import"); + + ( + add_unqualified_import(name, layer, module, import, line_numbers), + &format!( + "Update import of `{}`", + import + .used_name() + .unwrap_or(suggestion.last_name_component().into()) + ), + ) + } + }; + + CodeActionBuilder::new(title) + .kind(CodeActionKind::QUICKFIX) + .changes(uri.clone(), vec![edit]) + .preferred(true) + .push_to(actions); + } + } +} + +fn suggest_unqualified_imports( + location: SrcSpan, + suggestions: &[ModuleSuggestion], +) -> Option<(SrcSpan, &[ModuleSuggestion])> { + if suggestions.is_empty() { + None + } else { + Some((location, suggestions)) + } +} + pub fn code_action_add_missing_patterns( module: &Module, line_numbers: &LineNumbers, diff --git a/compiler-core/src/language_server/edits.rs b/compiler-core/src/language_server/edits.rs index 0f40cdd8692..7707eb0e379 100644 --- a/compiler-core/src/language_server/edits.rs +++ b/compiler-core/src/language_server/edits.rs @@ -2,7 +2,7 @@ use ecow::EcoString; use lsp_types::{Position, Range, TextEdit}; use crate::{ - ast::{Definition, Import, TypedDefinition}, + ast::{Definition, Import, Layer, SrcSpan, TypedDefinition}, build::Module, line_numbers::LineNumbers, }; @@ -75,3 +75,147 @@ pub fn get_import_edit( new_text: ["import ", module_full_name, new_lines].concat(), } } + +pub fn get_unqualified_import_edit( + import_location: Position, + module_full_name: &str, + name_to_import: &str, + layer: Layer, + insert_newlines: &Newlines, +) -> TextEdit { + let new_lines = match insert_newlines { + Newlines::Single => "\n", + Newlines::Double => "\n\n", + }; + let prefix = match layer { + Layer::Type => "type ", + Layer::Value => "", + }; + TextEdit { + range: Range { + start: import_location, + end: import_location, + }, + new_text: [ + "import ", + module_full_name, + ".{", + prefix, + name_to_import, + "}", + new_lines, + ] + .concat(), + } +} + +pub fn add_unqualified_import( + name: &EcoString, + layer: Layer, + module: &Module, + import: &Import, + line_numbers: &LineNumbers, +) -> TextEdit { + let import_code = get_import_code(module, import); + let has_brace = import_code.contains('}'); + + let name = match layer { + Layer::Type => &format!("type {}", name).into(), + Layer::Value => name, + }; + + let (pos, new_text) = if has_brace { + insert_into_braced_import(module, name, import) + } else { + insert_into_unbraced_import(module, name, import) + }; + + TextEdit { + range: src_span_to_lsp_range( + SrcSpan { + start: pos, + end: pos, + }, + line_numbers, + ), + new_text, + } +} + +fn get_import_code<'a>(module: &'a Module, import: &'a Import) -> &'a str { + module + .code + .get(import.location.start as usize..import.location.end as usize) + .expect("import not found") +} + +// Handle inserting into an unbraced import +fn insert_into_unbraced_import( + module: &Module, + name: &EcoString, + import: &Import, +) -> (u32, String) { + let location = import.location; + match import.alias_location() { + // Case: import module + None => (location.end, format!(".{{{}}}", name)), + Some(as_pos) => { + // Case: import module as alias + let import_code = &get_import_code(module, import); + let before_as_pos = import_code + .get(..(as_pos.start as usize)) + .and_then(|s| s.rfind(|c: char| !c.is_whitespace())) + .map(|pos| location.start as usize + pos + 1) + .expect("Expected non-whitespace character before ' as '"); + (before_as_pos as u32, format!(".{{{}}}", name)) + } + } +} + +// Handle inserting into a braced import +fn insert_into_braced_import( + module: &Module, + name: &EcoString, + import: &Import, +) -> (u32, String) { + if let Some((pos, c)) = find_last_char_before_closing_brace(module, import) { + // Case: import module.{Existing, } (as alias) + if c == ',' { + (pos as u32 + 1, format!(" {}", name)) + } else { + // Case: import module.{Existing} (as alias) + (pos as u32 + 1, format!(", {}", name)) + } + } else { + // Case: import module.{} (as alias) + let import_code = get_import_code(module, import); + let left_brace_pos = import_code + .find('{') + .map(|pos| import.location.start as usize + pos) + .expect("Expected '{' in import statement"); + (left_brace_pos as u32 + 1, name.to_string()) + } +} + +fn find_last_char_before_closing_brace( + module: &Module, + import: &Import, +) -> Option<(usize, char)> { + let import_code = get_import_code(module, import); + let closing_brace_pos = import_code.rfind('}')?; + + let bytes = import_code.as_bytes(); + let mut pos = closing_brace_pos; + while pos > 0 { + pos -= 1; + let c = (*bytes.get(pos)?) as char; + if c.is_whitespace() { + continue; + } + if c == '{' { + break; + } + return Some((import.location.start as usize + pos, c)); + } + None +} diff --git a/compiler-core/src/language_server/engine.rs b/compiler-core/src/language_server/engine.rs index 1cb826d8526..e7181f8952c 100644 --- a/compiler-core/src/language_server/engine.rs +++ b/compiler-core/src/language_server/engine.rs @@ -45,6 +45,8 @@ use super::{ WrapInBlock, code_action_add_missing_patterns, code_action_convert_qualified_constructor_to_unqualified, code_action_convert_unqualified_constructor_to_qualified, code_action_import_module, + code_action_import_qualified_module_for_type_or_value, + code_action_import_unqualified_module_for_type_or_value, code_action_inexhaustive_let_to_case, }, completer::Completer, @@ -398,6 +400,20 @@ where ); code_action_fix_names(&lines, ¶ms, &this.error, &mut actions); code_action_import_module(module, &lines, ¶ms, &this.error, &mut actions); + code_action_import_qualified_module_for_type_or_value( + module, + &lines, + ¶ms, + &this.error, + &mut actions, + ); + code_action_import_unqualified_module_for_type_or_value( + module, + &lines, + ¶ms, + &this.error, + &mut actions, + ); code_action_add_missing_patterns(module, &lines, ¶ms, &this.error, &mut actions); code_action_inexhaustive_let_to_case( module, diff --git a/compiler-core/src/type_/environment.rs b/compiler-core/src/type_/environment.rs index e3b604dd184..5d724d263c6 100644 --- a/compiler-core/src/type_/environment.rs +++ b/compiler-core/src/type_/environment.rs @@ -2,7 +2,7 @@ use pubgrub::range::Range; use crate::{ analyse::TargetSupport, - ast::{PIPE_VARIABLE, Publicity}, + ast::{Layer, PIPE_VARIABLE, Publicity}, build::Target, error::edit_distance, reference::{EntityKind, ReferenceTracker}, @@ -374,6 +374,7 @@ impl Environment<'_> { &mut self, module: &Option<(EcoString, SrcSpan)>, name: &EcoString, + arity: Option, ) -> Result<&TypeConstructor, UnknownTypeConstructorError> { match module { None => self @@ -382,6 +383,7 @@ impl Environment<'_> { .ok_or_else(|| UnknownTypeConstructorError::Type { name: name.clone(), hint: self.unknown_type_hint(name), + suggestions: self.suggest_unqualified_modules(name, Layer::Type, arity), }), Some((module_name, _)) => { @@ -419,6 +421,7 @@ impl Environment<'_> { &self, module: &EcoString, name: &EcoString, + arity: Option, ) -> Result<&TypeVariantConstructors, UnknownTypeConstructorError> { let module = if module.is_empty() || *module == self.current_module { None @@ -430,6 +433,7 @@ impl Environment<'_> { UnknownTypeConstructorError::Type { name: name.clone(), hint: self.unknown_type_hint(name), + suggestions: self.suggest_unqualified_modules(name, Layer::Type, arity), } }), @@ -458,6 +462,7 @@ impl Environment<'_> { &mut self, module: Option<&EcoString>, name: &EcoString, + arity: Option, ) -> Result<&ValueConstructor, UnknownValueConstructorError> { match module { None => self.scope.get(name).ok_or_else(|| { @@ -466,6 +471,7 @@ impl Environment<'_> { name: name.clone(), variables: self.local_value_names(), type_with_name_in_scope, + suggestions: self.suggest_unqualified_modules(name, Layer::Value, arity), } }), @@ -495,8 +501,9 @@ impl Environment<'_> { &self, module: &EcoString, name: &EcoString, + arity: Option, ) -> Vec<&EcoString> { - self.get_constructors_for_type(module, name) + self.get_constructors_for_type(module, name, arity) .iter() .flat_map(|c| &c.variants) .filter_map(|variant| { @@ -750,6 +757,137 @@ impl Environment<'_> { .collect() } + /// Suggest modules to import or use unqualified + pub fn suggest_unqualified_modules( + &self, + name: &EcoString, + layer: Layer, + arity: Option, + ) -> Vec { + // Don't suggest unqualified imports for values which aren't record constructors. + if layer.is_value() && name.chars().next().is_none_or(|c| c.is_lowercase()) { + return vec![]; + } + + let suggestions = match layer { + Layer::Type => self.suggest_modules_for_type(name, arity), + Layer::Value => self.suggest_modules_for_value(name, arity), + }; + + // Sort options based on if its already imported and on lexicographical order. + suggestions.into_iter().sorted().collect() + } + + fn suggest_modules_for_type( + &self, + name: &EcoString, + arity: Option, + ) -> Vec { + let mut imported_modules = HashSet::new(); + + let mut suggestions = self + .imported_modules + .iter() + .filter_map(|(_, (_, module_info))| { + let Some(type_) = module_info.get_public_type(name) else { + return None; + }; + // If we couldn't find the arity of the type, consider all modules. + let Some(arity) = arity else { + // Should be impossible to exist already + let _ = imported_modules.insert(module_info.name.clone()); + return Some(ModuleSuggestion::Imported(module_info.name.clone())); + }; + // Make sure the arities match. + if type_.parameters.len() == arity { + // Should be impossible to exist already + let _ = imported_modules.insert(module_info.name.clone()); + Some(ModuleSuggestion::Imported(module_info.name.clone())) + } else { + None + } + }) + .collect_vec(); + + suggestions.extend(self.importable_modules.iter().filter_map( + |(importable, module_info)| { + if imported_modules.contains(importable) { + return None; + } + let Some(type_) = module_info.get_public_type(name) else { + return None; + }; + // If we couldn't find the arity of the type, consider all modules. + let Some(arity) = arity else { + return Some(ModuleSuggestion::Importable(importable.clone())); + }; + // Make sure the arities match. + if type_.parameters.len() == arity { + Some(ModuleSuggestion::Importable(importable.clone())) + } else { + None + } + }, + )); + + suggestions + } + + fn suggest_modules_for_value( + &self, + name: &EcoString, + arity: Option, + ) -> Vec { + let mut imported_modules = HashSet::new(); + + let mut suggestions = self + .imported_modules + .iter() + .filter_map(|(_, (_, module_info))| { + let Some(value) = module_info.get_public_value(name) else { + return None; + }; + // If we couldn't find the arity of the value, consider all modules. + if arity == None { + // Should be impossible to exist already + let _ = imported_modules.insert(module_info.name.clone()); + return Some(ModuleSuggestion::Imported(module_info.name.clone())); + } + // Make sure the arities match. + if value.type_.fn_arity() == arity { + // Should be impossible to exist already + let _ = imported_modules.insert(module_info.name.clone()); + Some(ModuleSuggestion::Imported(module_info.name.clone())) + } else { + None + } + }) + .collect_vec(); + + suggestions.extend(self.importable_modules.iter().filter_map( + |(importable, module_info)| { + if imported_modules.contains(importable) { + return None; + } + let Some(value) = module_info.get_public_value(name) else { + return None; + }; + // If we couldn't find the arity of the value, consider all modules. + if arity == None { + return Some(ModuleSuggestion::Importable(module_info.name.clone())); + } + // Make sure the arities match. + if value.type_.fn_arity() == arity { + Some(ModuleSuggestion::Importable(module_info.name.clone())) + } else { + None + } + }, + )); + + suggestions + } + /// Suggest modules to import or use, for an unknown module pub fn suggest_modules(&self, module: &str, imported: Imported) -> Vec { let mut suggestions = self diff --git a/compiler-core/src/type_/error.rs b/compiler-core/src/type_/error.rs index 4f57090aed2..b9d5d64b2b9 100644 --- a/compiler-core/src/type_/error.rs +++ b/compiler-core/src/type_/error.rs @@ -119,7 +119,7 @@ pub enum ModuleSuggestion { } impl ModuleSuggestion { - pub fn suggestion(&self, module: &str) -> String { + pub fn suggest_import(&self, module: &str) -> String { match self { ModuleSuggestion::Importable(name) => { // Add a little extra information if the names don't match @@ -134,6 +134,23 @@ impl ModuleSuggestion { } } + pub fn suggest_unqualified_import(&self, name: &str, layer: Layer) -> String { + match self { + ModuleSuggestion::Importable(module) => match layer { + Layer::Type => { + format!("Did you mean to import the `{name}` type from the `{module}` module?") + } + Layer::Value => { + format!("Did you mean to import the `{name}` value from the `{module}` module?") + } + }, + + ModuleSuggestion::Imported(module) => { + format!("Did you mean to update the import of `{module}`?") + } + } + } + pub fn last_name_component(&self) -> &str { match self { ModuleSuggestion::Imported(name) | ModuleSuggestion::Importable(name) => { @@ -168,12 +185,14 @@ pub enum Error { name: EcoString, variables: Vec, type_with_name_in_scope: bool, + suggestions: Vec, }, UnknownType { location: SrcSpan, name: EcoString, hint: UnknownTypeHint, + suggestions: Vec, }, UnknownModule { @@ -1225,6 +1244,7 @@ pub enum UnknownValueConstructorError { name: EcoString, variables: Vec, type_with_name_in_scope: bool, + suggestions: Vec, }, Module { @@ -1250,11 +1270,13 @@ pub fn convert_get_value_constructor_error( name, variables, type_with_name_in_scope, + suggestions, } => Error::UnknownVariable { location, name, variables, type_with_name_in_scope, + suggestions, }, UnknownValueConstructorError::Module { name, suggestions } => Error::UnknownModule { @@ -1308,6 +1330,7 @@ pub enum UnknownTypeConstructorError { Type { name: EcoString, hint: UnknownTypeHint, + suggestions: Vec, }, Module { @@ -1329,10 +1352,15 @@ pub fn convert_get_type_constructor_error( module_location: Option, ) -> Error { match e { - UnknownTypeConstructorError::Type { name, hint } => Error::UnknownType { + UnknownTypeConstructorError::Type { + name, + hint, + suggestions, + } => Error::UnknownType { location: *location, name, hint, + suggestions, }, UnknownTypeConstructorError::Module { name, suggestions } => Error::UnknownModule { diff --git a/compiler-core/src/type_/expression.rs b/compiler-core/src/type_/expression.rs index f2d45a8d570..995b13442e0 100644 --- a/compiler-core/src/type_/expression.rs +++ b/compiler-core/src/type_/expression.rs @@ -1133,7 +1133,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { register_reference: ReferenceRegistration, ) -> Result { let constructor = - self.do_infer_value_constructor(&None, &name, &location, register_reference)?; + self.do_infer_value_constructor(&None, &name, &location, register_reference, None)?; self.narrow_implementations(location, &constructor.variant)?; Ok(TypedExpr::Var { constructor, @@ -2153,7 +2153,7 @@ 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, None)?; // We cannot support all values in guard expressions as the BEAM does not let definition_location = match &constructor.variant { @@ -2950,7 +2950,13 @@ impl<'a, 'b> ExprTyper<'a, 'b> { let value_constructor = self .environment - .get_value_constructor(module.map(|(module, _)| module), name) + .get_value_constructor( + module.map(|(module, _)| module), + name, + typed_constructor + .record_constructor_arity() + .map(|v| v as usize), + ) .map_err(|e| { convert_get_value_constructor_error( e, @@ -3267,13 +3273,16 @@ impl<'a, 'b> ExprTyper<'a, 'b> { module, name, inferred_variant, + args, .. } = record_type.deref() else { return error(UnknownField::NoFields); }; - let all_fields = self.environment.get_type_variants_fields(module, name); + let all_fields = self + .environment + .get_type_variants_fields(module, name, Some(args.len())); if all_fields.is_empty() { return error(UnknownField::NoFields); @@ -3297,12 +3306,14 @@ impl<'a, 'b> ExprTyper<'a, 'b> { module: &Option<(EcoString, SrcSpan)>, name: &EcoString, location: &SrcSpan, + arity: Option, ) -> Result { self.do_infer_value_constructor( module, name, location, ReferenceRegistration::RegisterReferences, + arity, ) } @@ -3312,6 +3323,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { name: &EcoString, location: &SrcSpan, register_reference: ReferenceRegistration, + arity: Option, ) -> Result { let constructor = match module { // Look in the current scope for a binding with this name @@ -3319,7 +3331,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, arity))?, // Look in an imported module for a binding with this name Some((module_name, module_location)) => { @@ -3447,7 +3459,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, + arity: Option, + ) -> Error { // First try to see if this is a module alias: // `import gleam/io` // `io.debug(io)` @@ -3467,6 +3484,11 @@ impl<'a, 'b> ExprTyper<'a, 'b> { .module_types .keys() .any(|typ| typ == name), + suggestions: self.environment.suggest_unqualified_modules( + name, + Layer::Value, + arity, + ), }, } } @@ -3525,7 +3547,8 @@ impl<'a, 'b> ExprTyper<'a, 'b> { .. } if args.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, Some(args.len()))?; let (tag, field_map) = match &constructor.variant { ValueConstructorVariant::Record { @@ -3564,7 +3587,8 @@ 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, Some(args.len()))?; let (tag, field_map) = match &constructor.variant { ValueConstructorVariant::Record { @@ -3694,7 +3718,7 @@ 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, None)?; match constructor.variant { ValueConstructorVariant::ModuleConstant { .. } | ValueConstructorVariant::LocalConstant { .. } @@ -3859,7 +3883,11 @@ impl<'a, 'b> ExprTyper<'a, 'b> { Ok(self .environment - .get_value_constructor(module, name)? + .get_value_constructor( + module, + name, + constructor.record_constructor_arity().map(|v| v as usize), + )? .field_map()) } diff --git a/compiler-core/src/type_/hydrator.rs b/compiler-core/src/type_/hydrator.rs index 64230e27774..3bae91c09b6 100644 --- a/compiler-core/src/type_/hydrator.rs +++ b/compiler-core/src/type_/hydrator.rs @@ -133,7 +133,7 @@ impl Hydrator { deprecation, .. } = environment - .get_type_constructor(module, name) + .get_type_constructor(module, name, Some(args.len())) .map_err(|e| { convert_get_type_constructor_error( e, @@ -273,6 +273,11 @@ impl Hydrator { name: name.clone(), location: *location, hint, + suggestions: environment.suggest_unqualified_modules( + name, + Layer::Type, + None, + ), }) } } diff --git a/compiler-core/src/type_/pattern.rs b/compiler-core/src/type_/pattern.rs index 9f3101e8f2c..01e4a5bafac 100644 --- a/compiler-core/src/type_/pattern.rs +++ b/compiler-core/src/type_/pattern.rs @@ -621,6 +621,11 @@ impl<'a, 'b> PatternTyper<'a, 'b> { .module_types .keys() .any(|type_| type_ == &name), + suggestions: self.environment.suggest_unqualified_modules( + &name, + Layer::Value, + None, + ), }); return Pattern::Invalid { location, type_ }; } @@ -861,9 +866,11 @@ impl<'a, 'b> PatternTyper<'a, 'b> { // Register the value as seen for detection of unused values self.environment.increment_usage(&name); - let constructor = self - .environment - .get_value_constructor(module.as_ref().map(|(module, _)| module), &name); + let constructor = self.environment.get_value_constructor( + module.as_ref().map(|(module, _)| module), + &name, + Some(pattern_args.len()), + ); let constructor = match constructor { Ok(constructor) => constructor, diff --git a/compiler-core/src/type_/tests/errors.rs b/compiler-core/src/type_/tests/errors.rs index 6c5509406af..77cc2413ae6 100644 --- a/compiler-core/src/type_/tests/errors.rs +++ b/compiler-core/src/type_/tests/errors.rs @@ -3125,3 +3125,80 @@ fn bit_array_using_pattern_variables_from_other_bit_array() { fn non_utf8_string_assignment() { assert_error!(r#"let assert <<"Hello" as message:utf16>> = <<>>"#); } + +#[test] +fn suggest_unqualified_import_for_type_without_existing_import() { + assert_module_error!( + ( + "gleam/wibble", + " + pub type Wobble { Wobble } + " + ), + " + pub fn go(wob: Wobble) { 1 } + " + ); +} + +#[test] +fn suggest_unqualified_import_for_type_with_existing_import() { + assert_module_error!( + ( + "gleam/wibble", + " + pub type Wobble { Wobble } + " + ), + " + import gleam/wibble + pub fn go(wob: Wobble) { 1 } + " + ); +} + +#[test] +fn suggest_unqualified_import_for_constructor_without_existing_import() { + assert_module_error!( + ( + "gleam/wibble", + " + pub type Wobble { Wobble } + " + ), + " + pub fn go() { Wobble } + " + ); +} + +#[test] +fn suggest_unqualified_import_for_constructor_with_existing_import() { + assert_module_error!( + ( + "gleam/wibble", + " + pub type Wobble { Wobble } + " + ), + " + import gleam/wibble + pub fn go() { Wobble } + " + ); +} + +#[test] +fn dont_suggest_unqualified_import_for_value() { + assert_module_error!( + ( + "gleam/wibble", + " + pub fn wobble() { 1 } + " + ), + " + pub fn go() { wobble() } + " + ); +} diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__dont_suggest_unqualified_import_for_value.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__dont_suggest_unqualified_import_for_value.snap new file mode 100644 index 00000000000..7eba6ed1fb5 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__dont_suggest_unqualified_import_for_value.snap @@ -0,0 +1,23 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\n pub fn go() { wobble() }\n " +--- +----- SOURCE CODE +-- gleam/wibble.gleam + + pub fn wobble() { 1 } + + +-- main.gleam + + pub fn go() { wobble() } + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:2:23 + │ +2 │ pub fn go() { wobble() } + │ ^^^^^^ + +The name `wobble` is not in scope here. diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__suggest_unqualified_import_for_constructor_with_existing_import.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__suggest_unqualified_import_for_constructor_with_existing_import.snap new file mode 100644 index 00000000000..8ed24405223 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__suggest_unqualified_import_for_constructor_with_existing_import.snap @@ -0,0 +1,25 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\n import gleam/wibble\n pub fn go() { Wobble }\n " +--- +----- SOURCE CODE +-- gleam/wibble.gleam + + pub type Wobble { Wobble } + + +-- main.gleam + + import gleam/wibble + pub fn go() { Wobble } + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:3:23 + │ +3 │ pub fn go() { Wobble } + │ ^^^^^^ + +The custom type variant constructor `Wobble` is not in scope here. +Hint: Did you mean to update the import of `gleam/wibble`? diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__suggest_unqualified_import_for_constructor_without_existing_import.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__suggest_unqualified_import_for_constructor_without_existing_import.snap new file mode 100644 index 00000000000..cd3aa2e351b --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__suggest_unqualified_import_for_constructor_without_existing_import.snap @@ -0,0 +1,24 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\n pub fn go() { Wobble }\n " +--- +----- SOURCE CODE +-- gleam/wibble.gleam + + pub type Wobble { Wobble } + + +-- main.gleam + + pub fn go() { Wobble } + + +----- ERROR +error: Unknown variable + ┌─ /src/one/two.gleam:2:23 + │ +2 │ pub fn go() { Wobble } + │ ^^^^^^ + +The custom type variant constructor `Wobble` is not in scope here. +Hint: Did you mean to import the `Wobble` value from the `gleam/wibble` module? diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__suggest_unqualified_import_for_type_with_existing_import.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__suggest_unqualified_import_for_type_with_existing_import.snap new file mode 100644 index 00000000000..2b215e007db --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__suggest_unqualified_import_for_type_with_existing_import.snap @@ -0,0 +1,25 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\n import gleam/wibble\n pub fn go(wob: Wobble) { 1 }\n " +--- +----- SOURCE CODE +-- gleam/wibble.gleam + + pub type Wobble { Wobble } + + +-- main.gleam + + import gleam/wibble + pub fn go(wob: Wobble) { 1 } + + +----- ERROR +error: Unknown type + ┌─ /src/one/two.gleam:3:24 + │ +3 │ pub fn go(wob: Wobble) { 1 } + │ ^^^^^^ + +The type `Wobble` is not defined or imported in this module. +Hint: Did you mean to update the import of `gleam/wibble`? diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__suggest_unqualified_import_for_type_without_existing_import.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__suggest_unqualified_import_for_type_without_existing_import.snap new file mode 100644 index 00000000000..e0b79147997 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__suggest_unqualified_import_for_type_without_existing_import.snap @@ -0,0 +1,24 @@ +--- +source: compiler-core/src/type_/tests/errors.rs +expression: "\n pub fn go(wob: Wobble) { 1 }\n " +--- +----- SOURCE CODE +-- gleam/wibble.gleam + + pub type Wobble { Wobble } + + +-- main.gleam + + pub fn go(wob: Wobble) { 1 } + + +----- ERROR +error: Unknown type + ┌─ /src/one/two.gleam:2:24 + │ +2 │ pub fn go(wob: Wobble) { 1 } + │ ^^^^^^ + +The type `Wobble` is not defined or imported in this module. +Hint: Did you mean to import the `Wobble` type from the `gleam/wibble` module? diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__imported_constructor_instead_of_type.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__imported_constructor_instead_of_type.snap index 287aefa1d7b..a3f2d8fd833 100644 --- a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__imported_constructor_instead_of_type.snap +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__imported_constructor_instead_of_type.snap @@ -23,3 +23,4 @@ error: Unknown type The type `Wibble` is not defined or imported in this module. There is a value in scope with the name `Wibble`, but no type in scope with that name. +Hint: Did you mean to update the import of `module`?