Improve import error suggestions

CHANGELOG.md

@@ -106,6 +106,17 @@
 - 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.
+
+  Hint: did you mean `import gleam/io`?
+  ```
+  ([sobolevn](https://github.com/sobolevn))
+
 ### Formatter
 
 ### Language Server

compiler-core/src/analyse/imports.rs

@@ -1,8 +1,10 @@
 use ecow::EcoString;
+use itertools::Itertools;
 
 use crate::{
     ast::{SrcSpan, UnqualifiedImport, UntypedImport},
     build::Origin,
+    error::wrap,
     type_::{
         EntityKind, Environment, Error, ModuleInterface, Problems, UnusedModuleAlias,
         ValueConstructorVariant,
@@ -48,10 +50,51 @@ 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(),
-                imported_modules: self.environment.imported_modules.keys().cloned().collect(),
+                hint,
+                imported_modules: modules_to_suggest,
             });
             return;
         };

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 {

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();

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 {

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`

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`

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
+"#
+    );
+}

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 {

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

compiler-core/src/type_/tests/imports.rs

@@ -326,3 +326,38 @@ 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_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");
+}

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

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

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

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`.

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`.