diff --git a/CHANGELOG.md b/CHANGELOG.md index 53c89c1277b..c84dc515dc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -106,6 +106,19 @@ - Adds a hint to syntax error when defining a function inside a type. ([sobolevn](https://github.com/sobolevn)) +- Improve import error messages. Now this code produces: + + ```gleam + import gleam.io + ^^ + I was not expecting this. + + This syntax for import is not correct. Probably, you meant: + - `import gleam/io` to import module `io` from package `gleam` + - `import gleam.{io}` to import value `io` from module `gleam` + ``` + ([sobolevn](https://github.com/sobolevn)) + ### Formatter ### Language Server diff --git a/compiler-core/src/analyse/imports.rs b/compiler-core/src/analyse/imports.rs index d7f2de5676d..cda9ab4b5ad 100644 --- a/compiler-core/src/analyse/imports.rs +++ b/compiler-core/src/analyse/imports.rs @@ -48,11 +48,7 @@ impl<'context, 'problems> Importer<'context, 'problems> { // Find imported module let Some(module_info) = self.environment.importable_modules.get(&name) else { - self.problems.error(Error::UnknownModule { - location, - name: name.clone(), - imported_modules: self.environment.imported_modules.keys().cloned().collect(), - }); + self.module_not_found_error(name, location); return; }; @@ -75,6 +71,37 @@ impl<'context, 'problems> Importer<'context, 'problems> { } } + fn module_not_found_error(&mut self, name: EcoString, location: SrcSpan) { + let importable_modules = self + .environment + .importable_modules // many modules might not be loaded for now + .keys() + .cloned() + .collect(); + + if let Some((basename, last_part)) = name.rsplit_once("/") { + let basename = EcoString::from(basename); + if let Some(module) = self.environment.importable_modules.get(&basename) { + if module.get_public_value(last_part).is_some() { + self.problems + .error(Error::UnknownModuleWithRichSuggestions { + location, + name: name.clone(), + name_parts: (basename, EcoString::from(last_part)), + importable_modules, + }); + return; + } + }; + } + + self.problems.error(Error::UnknownModule { + location, + name: name.clone(), + imported_modules: importable_modules, + }); + } + fn register_unqualified_type(&mut self, import: &UnqualifiedImport, module: &ModuleInterface) { let imported_name = import.as_name.as_ref().unwrap_or(&import.name); diff --git a/compiler-core/src/error.rs b/compiler-core/src/error.rs index 8f9f1db0d3e..2e77ec9bdcd 100644 --- a/compiler-core/src/error.rs +++ b/compiler-core/src/error.rs @@ -1959,19 +1959,57 @@ Private types can only be used within the module that defines them.", name, imported_modules, } => Diagnostic { - title: "Unknown module".into(), - text: format!("No module has been found with the name `{name}`."), - hint: None, - level: Level::Error, - location: Some(Location { - label: Label { - text: did_you_mean(name, imported_modules), - span: *location, - }, - path: path.clone(), - src: src.clone(), - extra_labels: vec![], - }), + title: "Unknown module".into(), + text: format!("No module has been found with the name `{name}`."), + hint: None, + level: Level::Error, + location: Some(Location { + label: Label { + text: did_you_mean(name, imported_modules), + span: *location, + }, + path: path.clone(), + src: src.clone(), + extra_labels: vec![], + }), + }, + + TypeError::UnknownModuleWithRichSuggestions { + location, + name, + name_parts, + importable_modules, + } => { + // Improve error message when users confuse `import one/two` + // with `import one.{two}`. + let (basename, last_part) = name_parts; + let hint = Some( + wrap( + format!( + "Did you mean `import {basename}.{{{last_part}}}`? + +See: https://tour.gleam.run/basics/unqualified-imports + " + ) + .as_str(), + ) + .to_string(), + ); + Diagnostic { + title: "Unknown module".into(), + text: format!("No module has been found with the name `{name}`."), + hint, + level: Level::Error, + location: Some(Location { + label: Label { + text: did_you_mean(name, importable_modules), + span: *location, + }, + path: path.clone(), + src: src.clone(), + extra_labels: vec![], + }), + } }, TypeError::UnknownModuleType { diff --git a/compiler-core/src/parse.rs b/compiler-core/src/parse.rs index b484c8bffda..be8fe092016 100644 --- a/compiler-core/src/parse.rs +++ b/compiler-core/src/parse.rs @@ -2410,6 +2410,7 @@ where // import a // import a/b // import a/b.{c} + // import a/b.{type C} // import a/b.{c as d} as e fn parse_import(&mut self, import_start: u32) -> Result, ParseError> { let mut start = 0; @@ -2450,7 +2451,9 @@ where let mut unqualified_types = vec![]; if self.maybe_one(&Token::Dot).is_some() { - let _ = self.expect_one(&Token::LeftBrace)?; + let _ = self + .expect_one(&Token::LeftBrace) + .map_err(|err| self.add_import_syntax_hint(err, &module))?; let parsed = self.parse_unqualified_imports()?; unqualified_types = parsed.types; unqualified_values = parsed.values; @@ -2487,6 +2490,36 @@ where }))) } + fn add_import_syntax_hint(&self, err: ParseError, module: &String) -> ParseError { + match err.error { + ParseErrorType::UnexpectedToken { + token: Token::Name { ref name }, + .. + } => ParseError { + location: err.location, + error: ParseErrorType::ImportDottedName { + module: module.into(), + name: name.clone(), + upname: false, + }, + }, + + ParseErrorType::UnexpectedToken { + token: Token::UpName { ref name }, + .. + } => ParseError { + location: err.location, + error: ParseErrorType::ImportDottedName { + module: module.into(), + name: name.clone(), + upname: true, + }, + }, + + _ => err, + } + } + // [Name (as Name)? | UpName (as Name)? ](, [Name (as Name)? | UpName (as Name)?])*,? fn parse_unqualified_imports(&mut self) -> Result { let mut imports = ParsedUnqualifiedImports::default(); diff --git a/compiler-core/src/parse/error.rs b/compiler-core/src/parse/error.rs index e4e3af964ba..bdeaba80ae4 100644 --- a/compiler-core/src/parse/error.rs +++ b/compiler-core/src/parse/error.rs @@ -292,6 +292,36 @@ utf16_codepoint, utf32_codepoint, signed, unsigned, big, little, native, size, u "Unsupported expression", vec!["Functions cannot be called in clause guards.".into()], ), + ParseErrorType::ImportDottedName { + module, + name, + upname, + } => { + let mut hint = vec![wrap( + "This syntax for import is not correct. Probably, you meant:", + )]; + if *upname { + let l_name = name.to_lowercase(); + hint.push(wrap(format!( + "- `import {module}/{l_name}` to import module `{l_name}` from package `{module}`").as_str(), + )); + hint.push(wrap(format!( + "- `import {module}.{{{name}}}` to import value `{name}` from module `{module}`").as_str(), + )); + hint.push(wrap(format!( + "- `import {module}.{{type {name}}}` to import type `{name}` from module `{module}`").as_str(), + )); + } else { + hint.push(wrap(format!( + "- `import {module}/{name}` to import module `{name}` from package `{module}`").as_str(), + )); + hint.push(wrap(format!( + "- `import {module}.{{{name}}}` to import value `{name}` from module `{module}`").as_str(), + )); + } + + ("I was not expecting this", hint) + } } } } @@ -358,6 +388,12 @@ pub enum ParseErrorType { field_type: Option, }, CallInClauseGuard, // case x { _ if f() -> 1 } + ImportDottedName { + // import x.y + module: EcoString, + name: EcoString, + upname: bool, + }, } impl LexicalError { diff --git a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_dotted_name.snap b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_dotted_name.snap new file mode 100644 index 00000000000..0f031dcd1a6 --- /dev/null +++ b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_dotted_name.snap @@ -0,0 +1,13 @@ +--- +source: compiler-core/src/parse/tests.rs +expression: "\nimport one.two\n" +--- +error: Syntax error + ┌─ /src/parse/error.gleam:2:12 + │ +2 │ import one.two + │ ^^^ I was not expecting this + +This syntax for import is not correct. Probably, you meant: +- `import one/two` to import module `two` from package `one` +- `import one.{two}` to import value `two` from module `one` diff --git a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_dotted_name_multiple_dots.snap b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_dotted_name_multiple_dots.snap new file mode 100644 index 00000000000..a25dca16380 --- /dev/null +++ b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_dotted_name_multiple_dots.snap @@ -0,0 +1,13 @@ +--- +source: compiler-core/src/parse/tests.rs +expression: "\nimport one.two.three\n" +--- +error: Syntax error + ┌─ /src/parse/error.gleam:2:12 + │ +2 │ import one.two.three + │ ^^^ I was not expecting this + +This syntax for import is not correct. Probably, you meant: +- `import one/two` to import module `two` from package `one` +- `import one.{two}` to import value `two` from module `one` diff --git a/compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_dotted_upname.snap b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_dotted_upname.snap new file mode 100644 index 00000000000..b77685feecd --- /dev/null +++ b/compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_dotted_upname.snap @@ -0,0 +1,14 @@ +--- +source: compiler-core/src/parse/tests.rs +expression: "\nimport one.Two\n" +--- +error: Syntax error + ┌─ /src/parse/error.gleam:2:12 + │ +2 │ import one.Two + │ ^^^ I was not expecting this + +This syntax for import is not correct. Probably, you meant: +- `import one/two` to import module `two` from package `one` +- `import one.{Two}` to import value `Two` from module `one` +- `import one.{type Two}` to import type `Two` from module `one` diff --git a/compiler-core/src/parse/tests.rs b/compiler-core/src/parse/tests.rs index 9a65fd27391..e701d65f70a 100644 --- a/compiler-core/src/parse/tests.rs +++ b/compiler-core/src/parse/tests.rs @@ -1482,3 +1482,30 @@ type Wibble { "# ); } + +#[test] +fn import_dotted_name() { + assert_module_error!( + r#" +import one.two +"# + ); +} + +#[test] +fn import_dotted_name_multiple_dots() { + assert_module_error!( + r#" +import one.two.three +"# + ); +} + +#[test] +fn import_dotted_upname() { + assert_module_error!( + r#" +import one.Two +"# + ); +} diff --git a/compiler-core/src/type_/error.rs b/compiler-core/src/type_/error.rs index c36f7a7968f..40a0121f23b 100644 --- a/compiler-core/src/type_/error.rs +++ b/compiler-core/src/type_/error.rs @@ -109,6 +109,13 @@ pub enum Error { imported_modules: Vec, }, + UnknownModuleWithRichSuggestions { + location: SrcSpan, + name: EcoString, + name_parts: (EcoString, EcoString), + importable_modules: Vec, + }, + UnknownModuleType { location: SrcSpan, name: EcoString, @@ -775,6 +782,7 @@ impl Error { | Error::UnknownVariable { location, .. } | Error::UnknownType { location, .. } | Error::UnknownModule { location, .. } + | Error::UnknownModuleWithRichSuggestions { location, .. } | Error::UnknownModuleType { location, .. } | Error::UnknownModuleValue { location, .. } | Error::ModuleAliasUsedAsName { location, .. } diff --git a/compiler-core/src/type_/tests/imports.rs b/compiler-core/src/type_/tests/imports.rs index c7c104966b0..1d9fd4bd303 100644 --- a/compiler-core/src/type_/tests/imports.rs +++ b/compiler-core/src/type_/tests/imports.rs @@ -326,3 +326,49 @@ pub fn main() { " ); } + +#[test] +fn import_value_as_submodule() { + assert_with_module_error!(("one", "pub fn wibble() {}"), "import one/wibble"); +} + +#[test] +fn import_value_as_submodule2() { + assert_with_module_error!(("one/two", "pub fn wibble() {}"), "import one/two/wibble"); +} +#[test] +fn import_value_as_submodule3() { + // Two possible suggestions: `import one/wobble` or `import one.{wibble}` + // One via hint, one via `did_you_mean`. + assert_with_module_error!( + ("one", "pub fn wibble() {}"), + ("one/wobble", ""), + "import one/wibble" + ); +} + +#[test] +fn import_value_as_submodule_very_long_name() { + assert_with_module_error!( + ( + "this_is_a_very_long_name_for_a_module_even_hard_to_read", + "pub fn this_is_a_very_long_submodule_name() {}" + ), + "import this_is_a_very_long_name_for_a_module_even_hard_to_read/this_is_a_very_long_submodule_name" + ); +} + +#[test] +fn import_value_as_submodule_no_hint() { + assert_with_module_error!(("one/two", ""), "import one/two/wibble"); +} + +#[test] +fn import_value_as_submodule_no_hint2() { + assert_with_module_error!(("one/two", "pub fn wibble() {}"), "import some/other"); +} + +#[test] +fn import_value_as_submodule_no_hint3() { + assert_module_error!("import some/other"); +} diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_module.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_module.snap index 12a4f0dd3e2..e70c77af496 100644 --- a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_module.snap +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__unknown_module.snap @@ -6,6 +6,6 @@ error: Unknown module ┌─ /src/one/two.gleam:1:1 │ 1 │ import xpto - │ ^^^^^^^^^^^ + │ ^^^^^^^^^^^ Did you mean `gleam`? No module has been found with the name `xpto`. diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_errors_do_not_block_analysis.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_errors_do_not_block_analysis.snap index 6ee72e0b903..7691dafded2 100644 --- a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_errors_do_not_block_analysis.snap +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_errors_do_not_block_analysis.snap @@ -6,7 +6,7 @@ error: Unknown module ┌─ /src/one/two.gleam:1:1 │ 1 │ import unknown_module - │ ^^^^^^^^^^^^^^^^^^^^^ + │ ^^^^^^^^^^^^^^^^^^^^^ Did you mean `gleam`? No module has been found with the name `unknown_module`. diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule.snap new file mode 100644 index 00000000000..18e5d3b7895 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule.snap @@ -0,0 +1,14 @@ +--- +source: compiler-core/src/type_/tests/imports.rs +expression: import one/wibble +--- +error: Unknown module + ┌─ /src/one/two.gleam:1:1 + │ +1 │ import one/wibble + │ ^^^^^^^^^^^^^^^^^ + +No module has been found with the name `one/wibble`. +Hint: Did you mean `import one.{wibble}`? + +See: https://tour.gleam.run/basics/unqualified-imports diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule2.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule2.snap new file mode 100644 index 00000000000..ea4c29e5e84 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule2.snap @@ -0,0 +1,14 @@ +--- +source: compiler-core/src/type_/tests/imports.rs +expression: import one/two/wibble +--- +error: Unknown module + ┌─ /src/one/two.gleam:1:1 + │ +1 │ import one/two/wibble + │ ^^^^^^^^^^^^^^^^^^^^^ Did you mean `one/two`? + +No module has been found with the name `one/two/wibble`. +Hint: Did you mean `import one/two.{wibble}`? + +See: https://tour.gleam.run/basics/unqualified-imports diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule3.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule3.snap new file mode 100644 index 00000000000..d2abd4f6efa --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule3.snap @@ -0,0 +1,14 @@ +--- +source: compiler-core/src/type_/tests/imports.rs +expression: import one/wibble +--- +error: Unknown module + ┌─ /src/one/two.gleam:1:1 + │ +1 │ import one/wibble + │ ^^^^^^^^^^^^^^^^^ Did you mean `one/wobble`? + +No module has been found with the name `one/wibble`. +Hint: Did you mean `import one.{wibble}`? + +See: https://tour.gleam.run/basics/unqualified-imports diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_no_hint.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_no_hint.snap new file mode 100644 index 00000000000..dd416dbee90 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_no_hint.snap @@ -0,0 +1,11 @@ +--- +source: compiler-core/src/type_/tests/imports.rs +expression: import one/two/wibble +--- +error: Unknown module + ┌─ /src/one/two.gleam:1:1 + │ +1 │ import one/two/wibble + │ ^^^^^^^^^^^^^^^^^^^^^ Did you mean `one/two`? + +No module has been found with the name `one/two/wibble`. diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_no_hint2.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_no_hint2.snap new file mode 100644 index 00000000000..300530dea6d --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_no_hint2.snap @@ -0,0 +1,11 @@ +--- +source: compiler-core/src/type_/tests/imports.rs +expression: import some/other +--- +error: Unknown module + ┌─ /src/one/two.gleam:1:1 + │ +1 │ import some/other + │ ^^^^^^^^^^^^^^^^^ + +No module has been found with the name `some/other`. diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_no_hint3.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_no_hint3.snap new file mode 100644 index 00000000000..7bf399d39a4 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_no_hint3.snap @@ -0,0 +1,11 @@ +--- +source: compiler-core/src/type_/tests/imports.rs +expression: import some/other +--- +error: Unknown module + ┌─ /src/one/two.gleam:1:1 + │ +1 │ import some/other + │ ^^^^^^^^^^^^^^^^^ Did you mean `gleam`? + +No module has been found with the name `some/other`. diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_very_long_name.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_very_long_name.snap new file mode 100644 index 00000000000..c0af325e2b8 --- /dev/null +++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_very_long_name.snap @@ -0,0 +1,16 @@ +--- +source: compiler-core/src/type_/tests/imports.rs +expression: import this_is_a_very_long_name_for_a_module_even_hard_to_read/this_is_a_very_long_submodule_name +--- +error: Unknown module + ┌─ /src/one/two.gleam:1:1 + │ +1 │ import this_is_a_very_long_name_for_a_module_even_hard_to_read/this_is_a_very_long_submodule_name + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Did you mean `this_is_a_very_long_name_for_a_module_even_hard_to_read`? + +No module has been found with the name `this_is_a_very_long_name_for_a_module_even_hard_to_read/this_is_a_very_long_submodule_name`. +Hint: Did you mean `import +this_is_a_very_long_name_for_a_module_even_hard_to_read. +{this_is_a_very_long_submodule_name}`? + +See: https://tour.gleam.run/basics/unqualified-imports