From a1667d43a010afbe016aca90c6839f0fe04756c2 Mon Sep 17 00:00:00 2001
From: sobolevn <mail@sobolevn.me>
Date: Wed, 21 Aug 2024 18:19:18 +0300
Subject: [PATCH 1/6] Improve import error suggestions
---
CHANGELOG.md | 6 ++++
compiler-core/src/analyse/imports.rs | 20 +++++++++++
compiler-core/src/error.rs | 3 +-
compiler-core/src/parse.rs | 35 ++++++++++++++++++-
compiler-core/src/parse/error.rs | 35 +++++++++++++++++++
...ore__parse__tests__import_dotted_name.snap | 13 +++++++
...e__parse__tests__import_dotted_upname.snap | 14 ++++++++
compiler-core/src/parse/tests.rs | 18 ++++++++++
compiler-core/src/type_/error.rs | 3 ++
compiler-core/src/type_/expression.rs | 2 ++
compiler-core/src/type_/tests/imports.rs | 20 +++++++++++
...s__imports__import_value_as_submodule.snap | 12 +++++++
...__imports__import_value_as_submodule2.snap | 12 +++++++
13 files changed, 191 insertions(+), 2 deletions(-)
create mode 100644 compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_dotted_name.snap
create mode 100644 compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_dotted_upname.snap
create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule.snap
create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule2.snap
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 53c89c1277b..b2fc10b7428 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -106,6 +106,12 @@
- Adds a hint to syntax error when defining a function inside a type.
([sobolevn](https://github.com/sobolevn))
+- Improve import error messages.
+
+ Now we recommend fixes for `import a.b`
+ and have better suggestions for `import a/b` when `b` is acutally a value.
+ ([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..d16f661cde2 100644
--- a/compiler-core/src/analyse/imports.rs
+++ b/compiler-core/src/analyse/imports.rs
@@ -1,4 +1,5 @@
use ecow::EcoString;
+use itertools::Itertools;
use crate::{
ast::{SrcSpan, UnqualifiedImport, UntypedImport},
@@ -48,9 +49,28 @@ impl<'context, 'problems> Importer<'context, 'problems> {
// Find imported module
let Some(module_info) = self.environment.importable_modules.get(&name) else {
+ // Improve error message when users confuse `import one/two`
+ // with `import one.{two}`.
+ let mut modpath = name.split("/").collect_vec();
+ let last_part = match modpath.last() {
+ Some(name) => name,
+ None => "",
+ };
+ modpath.truncate(modpath.len() - 1);
+ let basename = EcoString::from(modpath.join("/"));
+
+ let mut hint = None;
+ if let Some(module) = self.environment.importable_modules.get(&basename) {
+ if module.get_public_value(last_part).is_some() {
+ hint = Some(format!(
+ "Maybe you meant `import {basename}.{{{last_part}}}`?"
+ ));
+ }
+ }
self.problems.error(Error::UnknownModule {
location,
name: name.clone(),
+ hint,
imported_modules: self.environment.imported_modules.keys().cloned().collect(),
});
return;
diff --git a/compiler-core/src/error.rs b/compiler-core/src/error.rs
index 8f9f1db0d3e..e3e5d82796f 100644
--- a/compiler-core/src/error.rs
+++ b/compiler-core/src/error.rs
@@ -1958,10 +1958,11 @@ Private types can only be used within the module that defines them.",
location,
name,
imported_modules,
+ hint,
} => Diagnostic {
title: "Unknown module".into(),
text: format!("No module has been found with the name `{name}`."),
- hint: None,
+ hint: hint.clone(),
level: Level::Error,
location: Some(Location {
label: Label {
diff --git a/compiler-core/src/parse.rs b/compiler-core/src/parse.rs
index b484c8bffda..b12f9c36cf8 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<Option<UntypedDefinition>, 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::InvalidImportSyntax {
+ module: module.into(),
+ name: name.clone(),
+ upname: false,
+ },
+ },
+
+ ParseErrorType::UnexpectedToken {
+ token: Token::UpName { ref name },
+ ..
+ } => ParseError {
+ location: err.location,
+ error: ParseErrorType::InvalidImportSyntax {
+ 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<ParsedUnqualifiedImports, ParseError> {
let mut imports = ParsedUnqualifiedImports::default();
diff --git a/compiler-core/src/parse/error.rs b/compiler-core/src/parse/error.rs
index e4e3af964ba..9e184383b35 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::InvalidImportSyntax {
+ 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,11 @@ pub enum ParseErrorType {
field_type: Option<TypeAst>,
},
CallInClauseGuard, // case x { _ if f() -> 1 }
+ InvalidImportSyntax {
+ 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_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..8a040a76646 100644
--- a/compiler-core/src/parse/tests.rs
+++ b/compiler-core/src/parse/tests.rs
@@ -1482,3 +1482,21 @@ type Wibble {
"#
);
}
+
+#[test]
+fn import_dotted_name() {
+ assert_module_error!(
+ r#"
+import one.two
+"#
+ );
+}
+
+#[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..1750a18a3ee 100644
--- a/compiler-core/src/type_/error.rs
+++ b/compiler-core/src/type_/error.rs
@@ -107,6 +107,7 @@ pub enum Error {
location: SrcSpan,
name: EcoString,
imported_modules: Vec<EcoString>,
+ hint: Option<String>,
},
UnknownModuleType {
@@ -958,6 +959,7 @@ pub fn convert_get_value_constructor_error(
location,
name,
imported_modules,
+ hint: None,
},
UnknownValueConstructorError::ModuleValue {
@@ -1019,6 +1021,7 @@ pub fn convert_get_type_constructor_error(
location: *location,
name,
imported_modules,
+ hint: None,
},
UnknownTypeConstructorError::ModuleType {
diff --git a/compiler-core/src/type_/expression.rs b/compiler-core/src/type_/expression.rs
index 09df07cc3e8..85fd858388a 100644
--- a/compiler-core/src/type_/expression.rs
+++ b/compiler-core/src/type_/expression.rs
@@ -2098,6 +2098,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
name: module_alias.clone(),
location: *module_location,
imported_modules: self.environment.imported_modules.keys().cloned().collect(),
+ hint: None,
})?;
let constructor =
@@ -2407,6 +2408,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
.ok_or_else(|| Error::UnknownModule {
location: *location,
name: module_name.clone(),
+ hint: None,
imported_modules: self
.environment
.imported_modules
diff --git a/compiler-core/src/type_/tests/imports.rs b/compiler-core/src/type_/tests/imports.rs
index c7c104966b0..e7acc0b9c4d 100644
--- a/compiler-core/src/type_/tests/imports.rs
+++ b/compiler-core/src/type_/tests/imports.rs
@@ -326,3 +326,23 @@ 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
+ "
+ );
+}
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..b0284becc4f
--- /dev/null
+++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule.snap
@@ -0,0 +1,12 @@
+---
+source: compiler-core/src/type_/tests/imports.rs
+expression: "\n import one/wibble\n "
+---
+error: Unknown module
+ ┌─ /src/one/two.gleam:2:9
+ │
+2 │ import one/wibble
+ │ ^^^^^^^^^^^^^^^^^
+
+No module has been found with the name `one/wibble`.
+Hint: Maybe you meant `import one.{wibble}`?
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..083c5123f09
--- /dev/null
+++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule2.snap
@@ -0,0 +1,12 @@
+---
+source: compiler-core/src/type_/tests/imports.rs
+expression: "\n import one/two/wibble\n "
+---
+error: Unknown module
+ ┌─ /src/one/two.gleam:2:9
+ │
+2 │ import one/two/wibble
+ │ ^^^^^^^^^^^^^^^^^^^^^
+
+No module has been found with the name `one/two/wibble`.
+Hint: Maybe you meant `import one/two.{wibble}`?
From 21afdcd679da966d66da0d23e358782a0a8958f4 Mon Sep 17 00:00:00 2001
From: sobolevn <mail@sobolevn.me>
Date: Thu, 22 Aug 2024 10:41:05 +0300
Subject: [PATCH 2/6] More suggestions
---
compiler-core/src/analyse/imports.rs | 31 +++++++++++++---
compiler-core/src/type_/tests/imports.rs | 35 +++++++++++++------
...s__imports__import_value_as_submodule.snap | 12 ++++---
...__imports__import_value_as_submodule2.snap | 12 ++++---
...__imports__import_value_as_submodule3.snap | 14 ++++++++
...ts__import_value_as_submodule_no_hint.snap | 11 ++++++
...s__import_value_as_submodule_no_hint2.snap | 11 ++++++
...s__import_value_as_submodule_no_hint3.snap | 11 ++++++
8 files changed, 113 insertions(+), 24 deletions(-)
create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule3.snap
create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_no_hint.snap
create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_no_hint2.snap
create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_no_hint3.snap
diff --git a/compiler-core/src/analyse/imports.rs b/compiler-core/src/analyse/imports.rs
index d16f661cde2..c133330de8b 100644
--- a/compiler-core/src/analyse/imports.rs
+++ b/compiler-core/src/analyse/imports.rs
@@ -4,6 +4,7 @@ use itertools::Itertools;
use crate::{
ast::{SrcSpan, UnqualifiedImport, UntypedImport},
build::Origin,
+ error::wrap,
type_::{
EntityKind, Environment, Error, ModuleInterface, Problems, UnusedModuleAlias,
ValueConstructorVariant,
@@ -62,16 +63,38 @@ impl<'context, 'problems> Importer<'context, 'problems> {
let mut hint = None;
if let Some(module) = self.environment.importable_modules.get(&basename) {
if module.get_public_value(last_part).is_some() {
- hint = Some(format!(
- "Maybe you meant `import {basename}.{{{last_part}}}`?"
- ));
+ hint = Some(
+ wrap(
+ format!(
+ "Did you mean `import {basename}.{{{last_part}}}`?
+
+See: https://tour.gleam.run/basics/unqualified-imports
+ "
+ )
+ .as_str(),
+ )
+ .to_string(),
+ );
}
}
+ let importable_modules = self
+ .environment
+ .importable_modules
+ .keys()
+ .cloned()
+ .collect_vec();
+ // If we have a single module here, it means that it is `gleam` module.
+ // We don't want to suggest that.
+ let modules_to_suggest = if importable_modules.len() == 1 {
+ vec![]
+ } else {
+ importable_modules
+ };
self.problems.error(Error::UnknownModule {
location,
name: name.clone(),
hint,
- imported_modules: self.environment.imported_modules.keys().cloned().collect(),
+ imported_modules: modules_to_suggest,
});
return;
};
diff --git a/compiler-core/src/type_/tests/imports.rs b/compiler-core/src/type_/tests/imports.rs
index e7acc0b9c4d..6f17512b1ae 100644
--- a/compiler-core/src/type_/tests/imports.rs
+++ b/compiler-core/src/type_/tests/imports.rs
@@ -329,20 +329,35 @@ pub fn main() {
#[test]
fn import_value_as_submodule() {
- assert_with_module_error!(
- ("one", "pub fn wibble() {}"),
- "
- import one/wibble
- "
- );
+ 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/two", "pub fn wibble() {}"),
- "
- import one/two/wibble
- "
+ ("one", "pub fn wibble() {}"),
+ ("one/wobble", ""),
+ "import one/wibble"
);
}
+
+#[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__imports__import_value_as_submodule.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule.snap
index b0284becc4f..18e5d3b7895 100644
--- 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
@@ -1,12 +1,14 @@
---
source: compiler-core/src/type_/tests/imports.rs
-expression: "\n import one/wibble\n "
+expression: import one/wibble
---
error: Unknown module
- ┌─ /src/one/two.gleam:2:9
+ ┌─ /src/one/two.gleam:1:1
│
-2 │ import one/wibble
- │ ^^^^^^^^^^^^^^^^^
+1 │ import one/wibble
+ │ ^^^^^^^^^^^^^^^^^
No module has been found with the name `one/wibble`.
-Hint: Maybe you meant `import 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
index 083c5123f09..ea4c29e5e84 100644
--- 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
@@ -1,12 +1,14 @@
---
source: compiler-core/src/type_/tests/imports.rs
-expression: "\n import one/two/wibble\n "
+expression: import one/two/wibble
---
error: Unknown module
- ┌─ /src/one/two.gleam:2:9
+ ┌─ /src/one/two.gleam:1:1
│
-2 │ import one/two/wibble
- │ ^^^^^^^^^^^^^^^^^^^^^
+1 │ import one/two/wibble
+ │ ^^^^^^^^^^^^^^^^^^^^^ Did you mean `one/two`?
No module has been found with the name `one/two/wibble`.
-Hint: Maybe you meant `import 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..300530dea6d
--- /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
+ │ ^^^^^^^^^^^^^^^^^
+
+No module has been found with the name `some/other`.
From bb98f794407f7d55e394320c56711d4c49cad530 Mon Sep 17 00:00:00 2001
From: sobolevn <mail@sobolevn.me>
Date: Thu, 22 Aug 2024 10:49:09 +0300
Subject: [PATCH 3/6] Better changelog
---
CHANGELOG.md | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b2fc10b7428..1569097fd66 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -106,10 +106,15 @@
- Adds a hint to syntax error when defining a function inside a type.
([sobolevn](https://github.com/sobolevn))
-- Improve import error messages.
+- Improve import error messages. Now this code produces:
- Now we recommend fixes for `import a.b`
- and have better suggestions for `import a/b` when `b` is acutally a value.
+ ```gleam
+ import gleam.io
+ ^^
+ I was not expecting this.
+
+ Hint: did you mean `import gleam/io`?
+ ```
([sobolevn](https://github.com/sobolevn))
### Formatter
From 0c7640854e6349c077a9543a43911a827e0fda0d Mon Sep 17 00:00:00 2001
From: sobolevn <mail@sobolevn.me>
Date: Fri, 23 Aug 2024 14:53:54 +0300
Subject: [PATCH 4/6] Address review
---
compiler-core/src/analyse/imports.rs | 80 ++++++++-----------
compiler-core/src/error.rs | 65 +++++++++++----
compiler-core/src/parse.rs | 4 +-
compiler-core/src/parse/error.rs | 5 +-
...sts__import_dotted_name_multiple_dots.snap | 13 +++
compiler-core/src/parse/tests.rs | 9 +++
compiler-core/src/type_/error.rs | 11 ++-
compiler-core/src/type_/expression.rs | 2 -
..._type___tests__errors__unknown_module.snap | 2 +-
...__import_errors_do_not_block_analysis.snap | 2 +-
...s__import_value_as_submodule_no_hint3.snap | 2 +-
11 files changed, 121 insertions(+), 74 deletions(-)
create mode 100644 compiler-core/src/parse/snapshots/gleam_core__parse__tests__import_dotted_name_multiple_dots.snap
diff --git a/compiler-core/src/analyse/imports.rs b/compiler-core/src/analyse/imports.rs
index c133330de8b..cda9ab4b5ad 100644
--- a/compiler-core/src/analyse/imports.rs
+++ b/compiler-core/src/analyse/imports.rs
@@ -1,10 +1,8 @@
use ecow::EcoString;
-use itertools::Itertools;
use crate::{
ast::{SrcSpan, UnqualifiedImport, UntypedImport},
build::Origin,
- error::wrap,
type_::{
EntityKind, Environment, Error, ModuleInterface, Problems, UnusedModuleAlias,
ValueConstructorVariant,
@@ -50,52 +48,7 @@ impl<'context, 'problems> Importer<'context, 'problems> {
// Find imported module
let Some(module_info) = self.environment.importable_modules.get(&name) else {
- // Improve error message when users confuse `import one/two`
- // with `import one.{two}`.
- let mut modpath = name.split("/").collect_vec();
- let last_part = match modpath.last() {
- Some(name) => name,
- None => "",
- };
- modpath.truncate(modpath.len() - 1);
- let basename = EcoString::from(modpath.join("/"));
-
- let mut hint = None;
- if let Some(module) = self.environment.importable_modules.get(&basename) {
- if module.get_public_value(last_part).is_some() {
- hint = Some(
- wrap(
- format!(
- "Did you mean `import {basename}.{{{last_part}}}`?
-
-See: https://tour.gleam.run/basics/unqualified-imports
- "
- )
- .as_str(),
- )
- .to_string(),
- );
- }
- }
- let importable_modules = self
- .environment
- .importable_modules
- .keys()
- .cloned()
- .collect_vec();
- // If we have a single module here, it means that it is `gleam` module.
- // We don't want to suggest that.
- let modules_to_suggest = if importable_modules.len() == 1 {
- vec![]
- } else {
- importable_modules
- };
- self.problems.error(Error::UnknownModule {
- location,
- name: name.clone(),
- hint,
- imported_modules: modules_to_suggest,
- });
+ self.module_not_found_error(name, location);
return;
};
@@ -118,6 +71,37 @@ See: https://tour.gleam.run/basics/unqualified-imports
}
}
+ 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 e3e5d82796f..2e77ec9bdcd 100644
--- a/compiler-core/src/error.rs
+++ b/compiler-core/src/error.rs
@@ -1958,21 +1958,58 @@ Private types can only be used within the module that defines them.",
location,
name,
imported_modules,
- hint,
} => Diagnostic {
- title: "Unknown module".into(),
- text: format!("No module has been found with the name `{name}`."),
- hint: hint.clone(),
- 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 b12f9c36cf8..be8fe092016 100644
--- a/compiler-core/src/parse.rs
+++ b/compiler-core/src/parse.rs
@@ -2497,7 +2497,7 @@ where
..
} => ParseError {
location: err.location,
- error: ParseErrorType::InvalidImportSyntax {
+ error: ParseErrorType::ImportDottedName {
module: module.into(),
name: name.clone(),
upname: false,
@@ -2509,7 +2509,7 @@ where
..
} => ParseError {
location: err.location,
- error: ParseErrorType::InvalidImportSyntax {
+ error: ParseErrorType::ImportDottedName {
module: module.into(),
name: name.clone(),
upname: true,
diff --git a/compiler-core/src/parse/error.rs b/compiler-core/src/parse/error.rs
index 9e184383b35..bdeaba80ae4 100644
--- a/compiler-core/src/parse/error.rs
+++ b/compiler-core/src/parse/error.rs
@@ -292,7 +292,7 @@ utf16_codepoint, utf32_codepoint, signed, unsigned, big, little, native, size, u
"Unsupported expression",
vec!["Functions cannot be called in clause guards.".into()],
),
- ParseErrorType::InvalidImportSyntax {
+ ParseErrorType::ImportDottedName {
module,
name,
upname,
@@ -388,7 +388,8 @@ pub enum ParseErrorType {
field_type: Option<TypeAst>,
},
CallInClauseGuard, // case x { _ if f() -> 1 }
- InvalidImportSyntax {
+ ImportDottedName {
+ // import x.y
module: EcoString,
name: EcoString,
upname: bool,
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/tests.rs b/compiler-core/src/parse/tests.rs
index 8a040a76646..e701d65f70a 100644
--- a/compiler-core/src/parse/tests.rs
+++ b/compiler-core/src/parse/tests.rs
@@ -1492,6 +1492,15 @@ 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!(
diff --git a/compiler-core/src/type_/error.rs b/compiler-core/src/type_/error.rs
index 1750a18a3ee..40a0121f23b 100644
--- a/compiler-core/src/type_/error.rs
+++ b/compiler-core/src/type_/error.rs
@@ -107,7 +107,13 @@ pub enum Error {
location: SrcSpan,
name: EcoString,
imported_modules: Vec<EcoString>,
- hint: Option<String>,
+ },
+
+ UnknownModuleWithRichSuggestions {
+ location: SrcSpan,
+ name: EcoString,
+ name_parts: (EcoString, EcoString),
+ importable_modules: Vec<EcoString>,
},
UnknownModuleType {
@@ -776,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, .. }
@@ -959,7 +966,6 @@ pub fn convert_get_value_constructor_error(
location,
name,
imported_modules,
- hint: None,
},
UnknownValueConstructorError::ModuleValue {
@@ -1021,7 +1027,6 @@ pub fn convert_get_type_constructor_error(
location: *location,
name,
imported_modules,
- hint: None,
},
UnknownTypeConstructorError::ModuleType {
diff --git a/compiler-core/src/type_/expression.rs b/compiler-core/src/type_/expression.rs
index 85fd858388a..09df07cc3e8 100644
--- a/compiler-core/src/type_/expression.rs
+++ b/compiler-core/src/type_/expression.rs
@@ -2098,7 +2098,6 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
name: module_alias.clone(),
location: *module_location,
imported_modules: self.environment.imported_modules.keys().cloned().collect(),
- hint: None,
})?;
let constructor =
@@ -2408,7 +2407,6 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
.ok_or_else(|| Error::UnknownModule {
location: *location,
name: module_name.clone(),
- hint: None,
imported_modules: self
.environment
.imported_modules
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_no_hint3.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_no_hint3.snap
index 300530dea6d..7bf399d39a4 100644
--- 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
@@ -6,6 +6,6 @@ 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`.
From 1679c5790b858dddc27cf318eefaaf8d379bed4f Mon Sep 17 00:00:00 2001
From: sobolevn <mail@sobolevn.me>
Date: Fri, 23 Aug 2024 14:57:08 +0300
Subject: [PATCH 5/6] Add long name to the test
---
compiler-core/src/type_/tests/imports.rs | 11 +++++++++++
...import_value_as_submodule_very_long_name.snap | 16 ++++++++++++++++
2 files changed, 27 insertions(+)
create mode 100644 compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__imports__import_value_as_submodule_very_long_name.snap
diff --git a/compiler-core/src/type_/tests/imports.rs b/compiler-core/src/type_/tests/imports.rs
index 6f17512b1ae..1d9fd4bd303 100644
--- a/compiler-core/src/type_/tests/imports.rs
+++ b/compiler-core/src/type_/tests/imports.rs
@@ -347,6 +347,17 @@ fn import_value_as_submodule3() {
);
}
+#[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");
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
From 6699cd36a82240fbbc507cce99756dec9483aa43 Mon Sep 17 00:00:00 2001
From: sobolevn <mail@sobolevn.me>
Date: Fri, 23 Aug 2024 15:01:38 +0300
Subject: [PATCH 6/6] Use real output in changelog
---
CHANGELOG.md | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1569097fd66..c84dc515dc6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -113,7 +113,9 @@
^^
I was not expecting this.
- Hint: did you mean `import gleam/io`?
+ 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))