Improve import error suggestions

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

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

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 {

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::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<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::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<TypeAst>,
     },
     CallInClauseGuard, // case x { _ if f() -> 1 }
+    ImportDottedName {
+        // import x.y
+        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_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`

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

compiler-core/src/type_/error.rs

@@ -109,6 +109,13 @@ pub enum Error {
         imported_modules: Vec<EcoString>,
     },
 
+    UnknownModuleWithRichSuggestions {
+        location: SrcSpan,
+        name: EcoString,
+        name_parts: (EcoString, EcoString),
+        importable_modules: Vec<EcoString>,
+    },
+
     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, .. }

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