Report missing params

crates/compiler/can/src/constraint.rs

@@ -689,11 +689,11 @@ impl Constraints {
 
     pub fn import_params(
         &mut self,
-        type_index: TypeOrVar,
+        opt_type_index: Option<TypeOrVar>,
         module_id: ModuleId,
         region: Region,
     ) -> Constraint {
-        Constraint::ImportParams(type_index, module_id, region)
+        Constraint::ImportParams(opt_type_index, module_id, region)
     }
 }
 
@@ -797,7 +797,7 @@ pub enum Constraint {
     CheckCycle(Index<Cycle>, IllegalCycleMark),
 
     IngestedFile(TypeOrVar, Box<PathBuf>, Arc<Vec<u8>>),
-    ImportParams(TypeOrVar, ModuleId, Region),
+    ImportParams(Option<TypeOrVar>, ModuleId, Region),
 }
 
 #[derive(Debug, Clone, Copy, Default)]

crates/compiler/can/src/copy.rs

@@ -302,6 +302,7 @@ fn deep_copy_expr_help<C: CopyEnv>(env: &mut C, copied: &mut Vec<Variable>, expr
             sub!(*var),
             *module_id,
         ),
+        MissingImportParams(module_id, region) => MissingImportParams(*module_id, *region),
         &AbilityMember(sym, specialization, specialization_var) => {
             AbilityMember(sym, specialization, sub!(specialization_var))
         }

crates/compiler/can/src/debug/pretty_print.rs

@@ -210,6 +210,9 @@ fn expr<'a>(c: &Ctx, p: EPrec, f: &'a Arena<'a>, e: &'a Expr) -> DocBuilder<'a,
             pp_sym(c, f, *sym)
         }
         ImportParams(loc_expr, _, _) => expr(c, p, f, &loc_expr.value),
+        MissingImportParams(module_id, _) => {
+            text!(f, "<missing params for {:?}>", module_id)
+        }
         When {
             loc_cond, branches, ..
         } => maybe_paren!(

crates/compiler/can/src/def.rs

@@ -171,7 +171,7 @@ enum PendingValueDef<'a> {
         symbol: Symbol,
         loc_pattern: Loc<Pattern>,
         module_id: ModuleId,
-        params: ast::Collection<'a, Loc<AssignedField<'a, ast::Expr<'a>>>>,
+        opt_provided: Option<ast::Collection<'a, Loc<AssignedField<'a, ast::Expr<'a>>>>>,
     },
     /// Ingested file
     IngestedFile(
@@ -191,7 +191,7 @@ impl PendingValueDef<'_> {
                 loc_pattern,
                 symbol: _,
                 module_id: _,
-                params: _,
+                opt_provided: _,
             } => loc_pattern,
             PendingValueDef::IngestedFile(loc_pattern, _, _) => loc_pattern,
         }
@@ -1141,27 +1141,34 @@ fn canonicalize_value_defs<'a>(
             PendingValue::ExpectFx(pending_expect) => {
                 pending_expect_fx.push(pending_expect);
             }
-            PendingValue::ModuleImport {
+            PendingValue::ModuleImport(PendingModuleImport {
                 module_id,
                 region,
                 exposed_symbols,
                 params,
-            } => {
+            }) => {
                 imports_introduced.push(IntroducedImport {
                     module_id,
                     region,
                     exposed_symbols,
                 });
 
-                if let Some((symbol, loc_pattern, params)) = params {
+                match params {
+                    PendingModuleImportParams::None => {}
+                    PendingModuleImportParams::Required {
+                        symbol,
+                        loc_pattern,
+                        opt_provided,
+                    } => {
                         pending_value_defs.push(PendingValueDef::ImportParams {
                             symbol,
                             loc_pattern,
                             module_id,
-                        params,
+                            opt_provided,
                         });
                     }
                 }
+            }
             PendingValue::InvalidIngestedFile => { /* skip */ }
             PendingValue::ImportNameConflict => { /* skip */ }
         }
@@ -2406,26 +2413,36 @@ fn canonicalize_pending_value_def<'a>(
             symbol,
             loc_pattern,
             module_id,
-            params,
+            opt_provided,
         } => {
-            let (record, can_output) =
-                canonicalize_record(env, var_store, scope, loc_pattern.region, params);
-
             // Insert a reference to the record so that we don't report it as unused
             // If the whole module is unused, we'll report that separately
             output
                 .references
                 .insert_value_lookup(symbol, QualifiedReference::Unqualified);
 
-            let references = DefReferences::Value(can_output.references.clone());
+            let (expr, references) = match opt_provided {
+                Some(params) => {
+                    let (record, can_output) =
+                        canonicalize_record(env, var_store, scope, loc_pattern.region, params);
+
+                    let references = can_output.references.clone();
                     output.union(can_output);
 
                     let loc_record = Loc::at(loc_pattern.region, record);
 
-            let loc_expr = Loc::at(
-                loc_pattern.region,
-                Expr::ImportParams(Box::new(loc_record), var_store.fresh(), module_id),
-            );
+                    let expr =
+                        Expr::ImportParams(Box::new(loc_record), var_store.fresh(), module_id);
+
+                    (expr, references)
+                }
+                None => (
+                    Expr::MissingImportParams(module_id, loc_pattern.region),
+                    References::new(),
+                ),
+            };
+
+            let loc_expr = Loc::at(loc_pattern.region, expr);
 
             let def = single_can_def(
                 loc_pattern,
@@ -2437,7 +2454,7 @@ fn canonicalize_pending_value_def<'a>(
 
             DefOutput {
                 output,
-                references,
+                references: DefReferences::Value(references),
                 def,
             }
         }
@@ -2982,16 +2999,7 @@ enum PendingValue<'a> {
     Dbg(PendingExpectOrDbg<'a>),
     Expect(PendingExpectOrDbg<'a>),
     ExpectFx(PendingExpectOrDbg<'a>),
-    ModuleImport {
-        module_id: ModuleId,
-        region: Region,
-        exposed_symbols: Vec<(Symbol, Region)>,
-        params: Option<(
-            Symbol,
-            Loc<Pattern>,
-            ast::Collection<'a, Loc<AssignedField<'a, ast::Expr<'a>>>>,
-        )>,
-    },
+    ModuleImport(PendingModuleImport<'a>),
     SignatureDefMismatch,
     InvalidIngestedFile,
     ImportNameConflict,
@@ -3002,6 +3010,24 @@ struct PendingExpectOrDbg<'a> {
     preceding_comment: Region,
 }
 
+struct PendingModuleImport<'a> {
+    module_id: ModuleId,
+    region: Region,
+    exposed_symbols: Vec<(Symbol, Region)>,
+    params: PendingModuleImportParams<'a>,
+}
+
+enum PendingModuleImportParams<'a> {
+    /// The module does not require any params
+    None,
+    /// The module requires params, they may or may not have been provided
+    Required {
+        symbol: Symbol,
+        loc_pattern: Loc<Pattern>,
+        opt_provided: Option<ast::Collection<'a, Loc<AssignedField<'a, ast::Expr<'a>>>>>,
+    },
+}
+
 pub struct IntroducedImport {
     module_id: ModuleId,
     region: Region,
@@ -3152,42 +3178,40 @@ fn to_pending_value_def<'a>(
                 None => module_name.clone(),
             };
 
-            let mut params_sym = None;
-
-            let params = module_import.params.and_then(|params| {
+            let params = if env.modules_expecting_params.contains(&module_id) {
                 let name_str = name_with_alias.as_str();
+                let sym_region = module_import.params.map(|p| p.params.region).unwrap_or(region);
 
-                let params_region = params.params.region;
+                match scope.introduce_str(format!("#{name_str}").as_str(), sym_region) {
+                    Ok(symbol) =>  {
+                        let loc_pattern = Loc::at(sym_region, Pattern::Identifier(symbol));
 
-                match scope.introduce_str(format!("#{name_str}").as_str(), params_region) {
-                    Ok(sym) => {
-                        params_sym = Some(sym);
-
-                        let loc_pattern = Loc::at(params_region, Pattern::Identifier(sym));
-                        Some((sym, loc_pattern, params.params.value))
+                        PendingModuleImportParams::Required {
+                            symbol,
+                            loc_pattern,
+                            opt_provided: module_import.params.map(|p| p.params.value),
+                        }
                     },
                     Err(_) =>  {
                         // Ignore conflict, it will be handled as a duplicate import
-                        None
+                        PendingModuleImportParams::None
+                    },
                 }
-                }
-            });
-
-            match (module_import.params, env.modules_expecting_params.contains(&module_id)) {
-                (None, true) => {
-                    env.problems.push(Problem::MissingParams {
-                        module_id,
-                        region,
-                    });
-                }
-                (Some(import_params), false) => {
+            } else {
+                if let Some(params) = module_import.params {
                     env.problems.push(Problem::UnexpectedParams {
                         module_id,
-                        region: import_params.params.region,
+                        region: params.params.region,
                     });
                 }
-                (None, false) | (Some(_), true) => { /* All good */}
-            }
+
+                PendingModuleImportParams::None
+            };
+
+            let params_sym = match params {
+                PendingModuleImportParams::Required { symbol, .. } => Some(symbol),
+                PendingModuleImportParams::None => None,
+            };
 
             if let Err(existing_import) =
                 scope
@@ -3257,12 +3281,12 @@ fn to_pending_value_def<'a>(
                 }
             }
 
-            PendingValue::ModuleImport {
+            PendingValue::ModuleImport(PendingModuleImport {
                 module_id,
                 region,
                 exposed_symbols,
                 params,
-            }
+            })
         }
         IngestedFileImport(ingested_file) => {
             let loc_name = ingested_file.name.item;

crates/compiler/can/src/expr.rs

@@ -186,6 +186,9 @@ pub enum Expr {
 
     /// Module params expression in import
     ImportParams(Box<Loc<Expr>>, Variable, ModuleId),
+    /// The imported module requires params but none were provided
+    /// We delay this error until solve, so we can report the expected type
+    MissingImportParams(ModuleId, Region),
 
     /// The "crash" keyword
     Crash {
@@ -340,6 +343,7 @@ impl Expr {
             Self::TupleAccess { index, .. } => Category::TupleAccess(*index),
             Self::RecordUpdate { .. } => Category::Record,
             Self::ImportParams(loc_expr, _, _) => loc_expr.value.category(),
+            Self::MissingImportParams(_, _) => Category::Unknown,
             Self::Tag {
                 name, arguments, ..
             } => Category::TagApply {
@@ -1966,6 +1970,7 @@ pub fn inline_calls(var_store: &mut VarStore, expr: Expr) -> Expr {
         | other @ TypedHole { .. }
         | other @ ForeignCall { .. }
         | other @ OpaqueWrapFunction(_)
+        | other @ MissingImportParams(_, _)
         | other @ Crash { .. } => other,
 
         List {
@@ -3270,6 +3275,7 @@ pub(crate) fn get_lookup_symbols(expr: &Expr) -> Vec<ExpectLookup> {
             | Expr::EmptyRecord
             | Expr::TypedHole(_)
             | Expr::RuntimeError(_)
+            | Expr::MissingImportParams(_, _)
             | Expr::OpaqueWrapFunction(_) => {}
         }
     }

crates/compiler/can/src/module.rs

@@ -1149,6 +1149,7 @@ fn fix_values_captured_in_closure_expr(
         | TypedHole { .. }
         | RuntimeError(_)
         | ZeroArgumentTag { .. }
+        | MissingImportParams(_, _)
         | RecordAccessor { .. } => {}
 
         List { loc_elems, .. } => {

crates/compiler/can/src/traverse.rs

@@ -404,6 +404,7 @@ pub fn walk_expr<V: Visitor>(visitor: &mut V, expr: &Expr, var: Variable) {
         }
         Expr::TypedHole(_) => { /* terminal */ }
         Expr::RuntimeError(..) => { /* terminal */ }
+        Expr::MissingImportParams(..) => { /* terminal */ }
     }
 }
 

crates/compiler/constrain/src/expr.rs

@@ -595,10 +595,13 @@ pub fn constrain_expr(
                 &params.value,
                 expected_params,
             );
-            let params_con = constraints.import_params(index, *module_id, params.region);
+            let params_con = constraints.import_params(Some(index), *module_id, params.region);
             let expr_and_params = constraints.and_constraint([expr_con, params_con]);
             constraints.exists([*var], expr_and_params)
         }
+        MissingImportParams(module_id, region) => {
+            constraints.import_params(None, *module_id, *region)
+        }
         &AbilityMember(symbol, specialization_id, specialization_var) => {
             // Save the expectation in the `specialization_var` so we know what to specialize, then
             // lookup the member in the environment.
@@ -4136,6 +4139,7 @@ fn is_generalizable_expr(mut expr: &Expr) -> bool {
             | ExpectFx { .. }
             | Dbg { .. }
             | TypedHole(_)
+            | MissingImportParams(_, _)
             | RuntimeError(..)
             | ZeroArgumentTag { .. }
             | Tag { .. }

crates/compiler/load/tests/test_reporting.rs

@@ -5005,6 +5005,27 @@ mod test_reporting {
     "###
     );
 
+    test_report!(
+        unexpected_module_params,
+        indoc!(
+            r#"
+            import Dict { key: "abc" } exposing [empty]
+
+            empty {}
+            "#
+        ),@r###"
+    ── UNEXPECTED PARAMS in /code/proj/Main.roc ────────────────────────────────────
+
+    This import specifies module params:
+
+    4│      import Dict { key: "abc" } exposing [empty]
+                        ^^^^^^^^^^^^^^
+
+    However, Dict does not expect any. Did you intend to import a
+    different module?
+    "###
+    );
+
     test_report!(
         unfinished_import_as_or_exposing,
         indoc!(

crates/compiler/load_internal/tests/test_load.rs

@@ -27,10 +27,11 @@ use roc_module::symbol::{Interns, ModuleId};
 use roc_packaging::cache::RocCacheDir;
 use roc_problem::can::Problem;
 use roc_region::all::LineInfo;
-use roc_reporting::report::RocDocAllocator;
 use roc_reporting::report::{can_problem, DEFAULT_PALETTE};
 use roc_reporting::report::{strip_colors, RenderTarget};
+use roc_reporting::report::{type_problem, RocDocAllocator};
 use roc_solve::FunctionKind;
+use roc_solve_problem::TypeError;
 use roc_target::Target;
 use roc_test_utils_dir::TmpDir;
 use roc_types::pretty_print::name_and_print_var;
@@ -107,6 +108,33 @@ fn format_can_problems(
     buf
 }
 
+fn format_type_problems(
+    problems: Vec<TypeError>,
+    home: ModuleId,
+    interns: &Interns,
+    filename: PathBuf,
+    src: &str,
+) -> String {
+    use ven_pretty::DocAllocator;
+
+    let src_lines: Vec<&str> = src.split('\n').collect();
+    let lines = LineInfo::new(src);
+    let alloc = RocDocAllocator::new(&src_lines, home, interns);
+    let reports = problems
+        .into_iter()
+        .flat_map(|problem| type_problem(&alloc, &lines, filename.clone(), problem))
+        .map(|report| report.pretty(&alloc));
+
+    let mut buf = String::new();
+    alloc
+        .stack(reports)
+        .append(alloc.line())
+        .1
+        .render_raw(70, &mut roc_reporting::report::CiWrite::new(&mut buf))
+        .unwrap();
+    buf
+}
+
 fn multiple_modules(subdir: &str, files: Vec<(&str, &str)>) -> Result<LoadedModule, String> {
     let arena = Bump::new();
     let arena = &arena;
@@ -130,11 +158,19 @@ fn multiple_modules(subdir: &str, files: Vec<(&str, &str)>) -> Result<LoadedModu
                 ));
             }
 
-            assert!(loaded_module
+            let type_problems = loaded_module
                 .type_problems
                 .remove(&home)
-                .unwrap_or_default()
-                .is_empty(),);
+                .unwrap_or_default();
+            if !type_problems.is_empty() {
+                return Err(format_type_problems(
+                    type_problems,
+                    home,
+                    &loaded_module.interns,
+                    filepath.clone(),
+                    src,
+                ));
+            }
 
             Ok(loaded_module)
         }
@@ -1735,6 +1771,60 @@ fn module_params_unexpected() {
     )
 }
 
+#[test]
+fn module_params_missing() {
+    let modules = vec![
+        (
+            "Api.roc",
+            indoc!(
+                r#"
+            module { key, exp } -> [url]
+
+            url = "example.com/$(key)?exp=$(Num.toStr exp)"
+            "#
+            ),
+        ),
+        (
+            "Main.roc",
+            indoc!(
+                r#"
+        module [example]
+
+        import Api
+
+        example = Api.url
+            "#
+            ),
+        ),
+    ];
+
+    let err = multiple_modules("module_params_missing", modules).unwrap_err();
+    assert_eq!(
+        err,
+        indoc!(
+            r#"
+            ── MISSING PARAMS in tmp/module_params_missing/Main.roc ────────────────────────
+
+            This import specifies no module params:
+
+            3│  import Api
+                ^^^^^^^^^^
+
+            However, Api expects the following to be provided:
+
+                {
+                    exp : Num *,
+                    key : Str,
+                }
+
+            You can provide params after the module name, like:
+
+                import Menu { echo, read }
+            "#
+        )
+    )
+}
+
 #[test]
 fn issue_2863_module_type_does_not_exist() {
     let modules = vec![

crates/compiler/mono/src/ir.rs

@@ -5883,6 +5883,7 @@ pub fn with_hole<'a>(
         }
         TypedHole(_) => runtime_error(env, "Hit a blank"),
         RuntimeError(e) => runtime_error(env, env.arena.alloc(e.runtime_message())),
+        MissingImportParams(_, _) => runtime_error(env, env.arena.alloc("Missing import params")),
         Crash { msg, ret_var: _ } => {
             let msg_sym = possible_reuse_symbol_or_specialize(
                 env,

crates/compiler/problem/src/can.rs

@@ -236,10 +236,6 @@ pub enum Problem {
         one_occurrence: Region,
         kind: AliasKind,
     },
-    MissingParams {
-        module_id: ModuleId,
-        region: Region,
-    },
     UnexpectedParams {
         module_id: ModuleId,
         region: Region,
@@ -316,7 +312,6 @@ impl Problem {
             Problem::OverAppliedCrash { .. } => RuntimeError,
             Problem::DefsOnlyUsedInRecursion(_, _) => Warning,
             Problem::FileProblem { .. } => Fatal,
-            Problem::MissingParams { .. } => Warning,
             Problem::UnexpectedParams { .. } => Warning,
         }
     }
@@ -475,7 +470,6 @@ impl Problem {
             | Problem::OverAppliedCrash { region }
             | Problem::UnappliedCrash { region }
             | Problem::DefsOnlyUsedInRecursion(_, region)
-            | Problem::MissingParams { region, .. }
             | Problem::UnexpectedParams { region, .. } => Some(*region),
             Problem::RuntimeError(RuntimeError::CircularDef(cycle_entries))
             | Problem::BadRecursion(cycle_entries) => {

crates/compiler/solve/src/solve.rs

@@ -1401,7 +1401,9 @@ fn solve(
                     }
                 }
             }
-            ImportParams(type_index, module_id, _region) => {
+            ImportParams(opt_provided, module_id, region) => {
+                match (module_params_vars.get(module_id), opt_provided) {
+                    (Some(expected), Some(provided)) => {
                         let actual = either_type_index_to_var(
                             env,
                             rank,
@@ -1410,15 +1412,9 @@ fn solve(
                             obligation_cache,
                             &mut can_types,
                             aliases,
-                    *type_index,
+                            *provided,
                         );
 
-                match module_params_vars.get(module_id) {
-                    None => {
-                        // Module does not expect params. This will be reported by can.
-                        state
-                    }
-                    Some(expected) => {
                         match unify(
                             &mut env.uenv(),
                             actual,
@@ -1444,6 +1440,22 @@ fn solve(
                             }
                         }
                     }
+                    (Some(expected), None) => {
+                        let err_type = env.uenv().var_to_error_type(*expected, Polarity::Neg);
+
+                        problems.push(TypeError::MissingImportParams {
+                            module_id: *module_id,
+                            region: *region,
+                            expected: err_type,
+                        });
+
+                        state
+                    }
+                    (None, Some(_)) | (None, None) => {
+                        // Module does not expect params.
+                        // If provided still, canonicalization will produce a warning.
+                        state
+                    }
                 }
             }
         };

crates/compiler/solve_problem/src/lib.rs

@@ -2,7 +2,10 @@
 use std::{path::PathBuf, str::Utf8Error};
 
 use roc_can::expected::{Expected, PExpected};
-use roc_module::{ident::Lowercase, symbol::Symbol};
+use roc_module::{
+    ident::Lowercase,
+    symbol::{ModuleId, Symbol},
+};
 use roc_problem::{can::CycleEntry, Severity};
 use roc_region::all::Region;
 
@@ -33,6 +36,11 @@ pub enum TypeError {
     },
     IngestedFileBadUtf8(Box<PathBuf>, Utf8Error),
     IngestedFileUnsupportedType(Box<PathBuf>, ErrorType),
+    MissingImportParams {
+        module_id: ModuleId,
+        region: Region,
+        expected: ErrorType,
+    },
 }
 
 impl TypeError {
@@ -52,6 +60,7 @@ impl TypeError {
             TypeError::Exhaustive(exhtv) => exhtv.severity(),
             TypeError::StructuralSpecialization { .. } => RuntimeError,
             TypeError::WrongSpecialization { .. } => RuntimeError,
+            TypeError::MissingImportParams { .. } => RuntimeError,
             TypeError::IngestedFileBadUtf8(..) => Fatal,
             TypeError::IngestedFileUnsupportedType(..) => Fatal,
         }
@@ -66,7 +75,8 @@ impl TypeError {
             | TypeError::BadExprMissingAbility(region, ..)
             | TypeError::StructuralSpecialization { region, .. }
             | TypeError::WrongSpecialization { region, .. }
-            | TypeError::BadPatternMissingAbility(region, ..) => Some(*region),
+            | TypeError::BadPatternMissingAbility(region, ..)
+            | TypeError::MissingImportParams { region, .. } => Some(*region),
             TypeError::UnfulfilledAbility(ab, ..) => ab.region(),
             TypeError::Exhaustive(e) => Some(e.region()),
             TypeError::CircularDef(c) => c.first().map(|ce| ce.symbol_region),

crates/reporting/src/error/canonicalize.rs

@@ -1313,7 +1313,6 @@ pub fn can_problem<'b>(
             doc = report.doc;
             title = report.title;
         }
-        Problem::MissingParams { .. } => todo!("agus"),
         Problem::UnexpectedParams { module_id, region } => {
             doc = alloc.stack([
                 alloc.reflow("This import specifies module params:"),

crates/reporting/src/error/type.rs

@@ -247,6 +247,32 @@ pub fn type_problem<'b>(
                 severity,
             })
         }
+        MissingImportParams {
+            module_id,
+            region,
+            expected,
+        } => {
+            let stack = [
+                alloc.reflow("This import specifies no module params:"),
+                alloc.region(lines.convert_region(region), severity),
+                alloc.concat([
+                    alloc.reflow("However, "),
+                    alloc.module(module_id),
+                    alloc.reflow(" expects the following to be provided:"),
+                ]),
+                alloc.type_block(error_type_to_doc(alloc, expected)),
+                alloc.reflow("You can provide params after the module name, like:"),
+                alloc
+                    .parser_suggestion("import Menu { echo, read }")
+                    .indent(4),
+            ];
+            Some(Report {
+                title: "MISSING PARAMS".to_string(),
+                filename,
+                doc: alloc.stack(stack),
+                severity,
+            })
+        }
     }
 }