Merge branch 'trunk' of github.com:rtfeldman/roc into wasm-repl-fixes

ast/src/lang/env.rs

@@ -160,12 +160,17 @@ impl<'a> Env<'a> {
                                 })
                             }
                         },
-                        None => {
+                        None => Err(RuntimeError::ModuleNotImported {
-                            panic!(
+                            module_name,
-                                "Module {} exists, but is not recorded in dep_idents",
+                            imported_modules: self
-                                module_name
+                                .dep_idents
-                            )
+                                .keys()
-                        }
+                                .filter_map(|module_id| self.module_ids.get_name(*module_id))
+                                .map(|module_name| module_name.as_ref().into())
+                                .collect(),
+                            region,
+                            module_exists: true,
+                        }),
                     }
                 }
             }
@@ -177,6 +182,7 @@ impl<'a> Env<'a> {
                     .map(|string| string.as_ref().into())
                     .collect(),
                 region,
+                module_exists: false,
             }),
         }
     }

compiler/can/src/annotation.rs

@@ -130,7 +130,9 @@ fn make_apply_symbol(
                 // it was imported but it doesn't expose this ident.
                 env.problem(roc_problem::can::Problem::RuntimeError(problem));
 
-                Err(Type::Erroneous(Problem::UnrecognizedIdent((*ident).into())))
+                // A failed import should have already been reported through
+                // roc_can::env::Env::qualified_lookup's checks
+                Err(Type::Erroneous(Problem::SolvedTypeError))
             }
         }
     }

compiler/can/src/env.rs

@@ -124,12 +124,17 @@ impl<'a> Env<'a> {
                                 })
                             }
                         },
-                        None => {
+                        None => Err(RuntimeError::ModuleNotImported {
-                            panic!(
+                            module_name,
-                                "Module {} exists, but is not recorded in dep_idents",
+                            imported_modules: self
-                                module_name
+                                .dep_idents
-                            )
+                                .keys()
-                        }
+                                .filter_map(|module_id| self.module_ids.get_name(*module_id))
+                                .map(|module_name| module_name.as_ref().into())
+                                .collect(),
+                            region,
+                            module_exists: true,
+                        }),
                     }
                 }
             }
@@ -141,6 +146,7 @@ impl<'a> Env<'a> {
                     .map(|string| string.as_ref().into())
                     .collect(),
                 region,
+                module_exists: false,
             }),
         }
     }

compiler/problem/src/can.rs

@@ -171,10 +171,37 @@ pub enum RuntimeError {
         region: Region,
         exposed_values: Vec<Lowercase>,
     },
+    /// A module was referenced, but hasn't been imported anywhere in the program
+    ///
+    /// An example would be:
+    /// ```roc
+    /// app "hello"
+    ///     packages { pf: "platform" }
+    ///     imports [ pf.Stdout]
+    ///     provides [ main ] to pf
+    ///
+    /// main : Task.Task {} [] // Task isn't imported!
+    /// main = Stdout.line "I'm a Roc application!"
+    /// ```
     ModuleNotImported {
+        /// The name of the module that was referenced
         module_name: ModuleName,
+        /// A list of modules which *have* been imported
         imported_modules: MutSet<Box<str>>,
+        /// Where the problem occurred
         region: Region,
+        /// Whether or not the module exists at all
+        ///
+        /// This is used to suggest that the user import the module, as opposed to fix a
+        /// typo in the spelling.  For example, if the user typed `Task`, and the platform
+        /// exposes a `Task` module that hasn't been imported, we can sugguest that they
+        /// add the import statement.
+        ///
+        /// On the other hand, if the user typed `Tesk`, they might want to check their
+        /// spelling.
+        ///
+        /// If unsure, this should be set to `false`
+        module_exists: bool,
     },
     InvalidPrecedence(PrecedenceProblem, Region),
     MalformedIdentifier(Box<str>, roc_parse::ident::BadIdent, Region),

compiler/types/src/subs.rs

@@ -1862,6 +1862,13 @@ impl UnionTags {
         slice.length == 1
     }
 
+    pub fn is_newtype_wrapper_of_global_tag(&self, subs: &Subs) -> bool {
+        self.is_newtype_wrapper(subs) && {
+            let tags = &subs.tag_names[self.tag_names().indices()];
+            matches!(tags[0], TagName::Global(_))
+        }
+    }
+
     pub fn from_tag_name_index(index: SubsIndex<TagName>) -> Self {
         Self::from_slices(
             SubsSlice::new(index.index, 1),

repl_eval/src/eval.rs

@@ -5,11 +5,11 @@ use std::cmp::{max_by_key, min_by_key};
 use roc_builtins::bitcode::{FloatWidth, IntWidth};
 use roc_collections::all::MutMap;
 use roc_module::called_via::CalledVia;
-use roc_module::ident::TagName;
+use roc_module::ident::{Lowercase, TagName};
 use roc_module::symbol::{Interns, ModuleId, Symbol};
 use roc_mono::ir::ProcLayout;
 use roc_mono::layout::{
-    union_sorted_tags_help, Builtin, Layout, UnionLayout, UnionVariant, WrappedVariant,
+    union_sorted_tags_help, Builtin, Layout, LayoutCache, UnionLayout, UnionVariant, WrappedVariant,
 };
 use roc_parse::ast::{AssignedField, Collection, Expr, StrLiteral};
 use roc_region::all::{Loc, Region};
@@ -70,6 +70,7 @@ pub fn jit_to_ast<'a, A: ReplApp<'a>>(
     }
 }
 
+#[derive(Debug)]
 enum NewtypeKind<'a> {
     Tag(&'a TagName),
     RecordField(&'a str),
@@ -89,10 +90,11 @@ fn unroll_newtypes<'a>(
     mut content: &'a Content,
 ) -> (Vec<'a, NewtypeKind<'a>>, &'a Content) {
     let mut newtype_containers = Vec::with_capacity_in(1, env.arena);
+    let mut force_alias_content = None;
     loop {
         match content {
             Content::Structure(FlatType::TagUnion(tags, _))
-                if tags.is_newtype_wrapper(env.subs) =>
+                if tags.is_newtype_wrapper_of_global_tag(env.subs) =>
             {
                 let (tag_name, vars): (&TagName, &[Variable]) = tags
                     .unsorted_iterator(env.subs, Variable::EMPTY_TAG_UNION)
@@ -113,7 +115,20 @@ fn unroll_newtypes<'a>(
                 let field_var = *field.as_inner();
                 content = env.subs.get_content_without_compacting(field_var);
             }
-            _ => return (newtype_containers, content),
+            Content::Alias(_, _, real_var) => {
+                // We need to pass through aliases too, because their underlying types may have
+                // unrolled newtypes. In such cases return the list of unrolled newtypes, but keep
+                // the content as the alias for readability. For example,
+                //   T : { a : Str }
+                //   v : T
+                //   v = { a : "value" }
+                //   v
+                // Here we need the newtype container to be `[RecordField(a)]`, but the content to
+                // remain as the alias `T`.
+                force_alias_content = Some(content);
+                content = env.subs.get_content_without_compacting(*real_var);
+            }
+            _ => return (newtype_containers, force_alias_content.unwrap_or(content)),
         }
     }
 }
@@ -340,15 +355,11 @@ fn jit_to_ast_help<'a, A: ReplApp<'a>>(
         Layout::Struct { field_layouts, .. } => {
             let struct_addr_to_ast = |mem: &'a A::Memory, addr: usize| match content {
                 Content::Structure(FlatType::Record(fields, _)) => {
-                    Ok(struct_to_ast(env, mem, addr, field_layouts, *fields))
+                    Ok(struct_to_ast(env, mem, addr, *fields))
+                }
+                Content::Structure(FlatType::EmptyRecord) => {
+                    Ok(struct_to_ast(env, mem, addr, RecordFields::empty()))
                 }
-                Content::Structure(FlatType::EmptyRecord) => Ok(struct_to_ast(
-                    env,
-                    mem,
-                    addr,
-                    field_layouts,
-                    RecordFields::empty(),
-                )),
                 Content::Structure(FlatType::TagUnion(tags, _)) => {
                     debug_assert_eq!(tags.len(), 1);
 
@@ -524,7 +535,7 @@ fn addr_to_ast<'a, M: ReplAppMemory>(
         }
         (_, Layout::Struct{field_layouts, ..}) => match content {
             Content::Structure(FlatType::Record(fields, _)) => {
-                struct_to_ast(env, mem, addr, field_layouts, *fields)
+                struct_to_ast(env, mem, addr, *fields)
             }
             Content::Structure(FlatType::TagUnion(tags, _)) => {
                 debug_assert_eq!(tags.len(), 1);
@@ -537,7 +548,7 @@ fn addr_to_ast<'a, M: ReplAppMemory>(
                 single_tag_union_to_ast(env, mem, addr, field_layouts, tag_name, &[])
             }
             Content::Structure(FlatType::EmptyRecord) => {
-                struct_to_ast(env, mem, addr, &[], RecordFields::empty())
+                struct_to_ast(env, mem, addr, RecordFields::empty())
             }
             other => {
                 unreachable!(
@@ -847,30 +858,46 @@ fn struct_to_ast<'a, M: ReplAppMemory>(
     env: &Env<'a, 'a>,
     mem: &'a M,
     addr: usize,
-    field_layouts: &'a [Layout<'a>],
     record_fields: RecordFields,
 ) -> Expr<'a> {
     let arena = env.arena;
     let subs = env.subs;
-    let mut output = Vec::with_capacity_in(field_layouts.len(), arena);
+    let mut output = Vec::with_capacity_in(record_fields.len(), arena);
 
     let sorted_fields: Vec<_> = Vec::from_iter_in(
         record_fields.sorted_iterator(env.subs, Variable::EMPTY_RECORD),
-        env.arena,
+        arena,
     );
 
+    let mut layout_cache = LayoutCache::new(env.target_info);
+    // We recalculate the layouts here because we will have compiled the record so that its fields
+    // are sorted by descending alignment, and then alphabetic, but the type of the record is
+    // always only sorted alphabetically. We want to arrange the rendered record in the order of
+    // the type.
+    let field_to_layout: MutMap<Lowercase, Layout> = sorted_fields
+        .iter()
+        .map(|(label, field)| {
+            let layout = layout_cache
+                .from_var(arena, *field.as_inner(), env.subs)
+                .unwrap();
+            (label.clone(), layout)
+        })
+        .collect();
+
     if sorted_fields.len() == 1 {
         // this is a 1-field wrapper record around another record or 1-tag tag union
         let (label, field) = sorted_fields.into_iter().next().unwrap();
 
         let inner_content = env.subs.get_content_without_compacting(field.into_inner());
+        debug_assert_eq!(field_to_layout.len(), 1);
+        let inner_layouts = arena.alloc([field_to_layout.into_values().next().unwrap()]);
 
         let loc_expr = &*arena.alloc(Loc {
             value: addr_to_ast(
                 env,
                 mem,
                 addr,
-                &Layout::struct_no_name_order(field_layouts),
+                &Layout::struct_no_name_order(inner_layouts),
                 WhenRecursive::Unreachable,
                 inner_content,
             ),
@@ -886,19 +913,20 @@ fn struct_to_ast<'a, M: ReplAppMemory>(
             region: Region::zero(),
         };
 
-        let output = env.arena.alloc([loc_field]);
+        let output = arena.alloc([loc_field]);
 
         Expr::Record(Collection::with_items(output))
     } else {
-        debug_assert_eq!(sorted_fields.len(), field_layouts.len());
+        debug_assert_eq!(sorted_fields.len(), field_to_layout.len());
 
         // We'll advance this as we iterate through the fields
         let mut field_addr = addr;
 
-        for ((label, field), field_layout) in sorted_fields.into_iter().zip(field_layouts.iter()) {
+        for (label, field) in sorted_fields.into_iter() {
             let var = field.into_inner();
 
             let content = subs.get_content_without_compacting(var);
+            let field_layout = field_to_layout.get(&label).unwrap();
 
             let loc_expr = &*arena.alloc(Loc {
                 value: addr_to_ast(

repl_test/src/tests.rs

@@ -813,7 +813,7 @@ fn function_in_list() {
 fn function_in_record() {
     expect_success(
         r#"{ n: 1, adder: \x -> x + 1 }"#,
-        r#"{ adder: <function>, n: <function> } : { adder : Num a -> Num a, n : Num * }"#,
+        r#"{ adder: <function>, n: 1 } : { adder : Num a -> Num a, n : Num * }"#,
     )
 }
 
@@ -948,3 +948,48 @@ fn issue_2343_complete_mono_with_shadowed_vars() {
         ),
     );
 }
+
+#[test]
+fn record_with_type_behind_alias() {
+    expect_success(
+        indoc!(
+            r#"
+            T : { a: Str }
+            v : T
+            v = { a: "value" }
+            v
+            "#
+        ),
+        r#"{ a: "value" } : T"#,
+    );
+}
+
+#[test]
+fn tag_with_type_behind_alias() {
+    expect_success(
+        indoc!(
+            r#"
+            T : [ A Str ]
+            v : T
+            v = A "value"
+            v
+            "#
+        ),
+        r#"A "value" : T"#,
+    );
+}
+
+#[cfg(not(feature = "wasm"))]
+#[test]
+fn issue_2588_record_with_function_and_nonfunction() {
+    expect_success(
+        indoc!(
+            r#"
+            x = 1
+            f = \n -> n * 2
+            { y: f x, f }
+            "#
+        ),
+        r#"{ f: <function>, y: 2 } : { f : Num a -> Num a, y : Num * }"#,
+    )
+}

reporting/src/error/canonicalize.rs

@@ -1015,8 +1015,16 @@ fn pretty_runtime_error<'b>(
             module_name,
             imported_modules,
             region,
+            module_exists,
         } => {
-            doc = module_not_found(alloc, lines, region, &module_name, imported_modules);
+            doc = module_not_found(
+                alloc,
+                lines,
+                region,
+                &module_name,
+                imported_modules,
+                module_exists,
+            );
 
             title = MODULE_NOT_IMPORTED;
         }
@@ -1501,34 +1509,39 @@ fn not_found<'b>(
     ])
 }
 
+/// Generate a message informing the user that a module was referenced, but not found
+///
+/// See [`roc_problem::can::ModuleNotImported`]
 fn module_not_found<'b>(
     alloc: &'b RocDocAllocator<'b>,
     lines: &LineInfo,
     region: roc_region::all::Region,
     name: &ModuleName,
     options: MutSet<Box<str>>,
+    module_exists: bool,
 ) -> RocDocBuilder<'b> {
-    let mut suggestions =
+    // If the module exists, sugguest that the user import it
-        suggest::sort(name.as_str(), options.iter().map(|v| v.as_ref()).collect());
+    let details = if module_exists {
-    suggestions.truncate(4);
+        // TODO:  Maybe give an example of how to do that
+        alloc.reflow("Did you mean to import it?")
+    } else {
+        // If the module might not exist, sugguest that it's a typo
+        let mut suggestions =
+            suggest::sort(name.as_str(), options.iter().map(|v| v.as_ref()).collect());
+        suggestions.truncate(4);
 
-    let default_no = alloc.concat(vec![
-        alloc.reflow("Is there an "),
-        alloc.keyword("import"),
-        alloc.reflow(" or "),
-        alloc.keyword("exposing"),
-        alloc.reflow(" missing up-top"),
-    ]);
-
-    let default_yes = alloc
-        .reflow("Is there an import missing? Perhaps there is a typo. Did you mean one of these?");
-
-    let to_details = |no_suggestion_details, yes_suggestion_details| {
         if suggestions.is_empty() {
-            no_suggestion_details
+            // We don't have any recommended spelling corrections
+            alloc.concat(vec![
+                alloc.reflow("Is there an "),
+                alloc.keyword("import"),
+                alloc.reflow(" or "),
+                alloc.keyword("exposing"),
+                alloc.reflow(" missing up-top"),
+            ])
         } else {
             alloc.stack(vec![
-                yes_suggestion_details,
+                alloc.reflow("Is there an import missing? Perhaps there is a typo. Did you mean one of these?"),
                 alloc
                     .vcat(suggestions.into_iter().map(|v| alloc.string(v.to_string())))
                     .indent(4),
@@ -1543,6 +1556,6 @@ fn module_not_found<'b>(
             alloc.reflow("` module is not imported:"),
         ]),
         alloc.region(lines.convert_region(region)),
-        to_details(default_no, default_yes),
+        details,
     ])
 }

reporting/tests/test_reporting.rs

@@ -8274,4 +8274,35 @@ I need all branches in an `if` to have the same type!
             ),
         )
     }
+
+    #[test]
+    fn unimported_modules_reported() {
+        report_problem_as(
+            indoc!(
+                r#"
+                main : Task.Task {} []
+                main = "whatever man you don't even know my type"
+                main
+                "#
+            ),
+            indoc!(
+                r#"
+                ── MODULE NOT IMPORTED ─────────────────────────────────────────────────────────
+
+                The `Task` module is not imported:
+
+                1│  main : Task.Task {} []
+                           ^^^^^^^^^^^^^^^
+
+                Is there an import missing? Perhaps there is a typo. Did you mean one
+                of these?
+
+                    Test
+                    List
+                    Num
+                    Set
+                "#
+            ),
+        )
+    }
 }