Add more context to derivability errors when they happen

Cargo.lock

@@ -4037,6 +4037,7 @@ dependencies = [
  "roc_collections",
  "roc_constrain",
  "roc_derive",
+ "roc_error_macros",
  "roc_exhaustive",
  "roc_fmt",
  "roc_load",

crates/compiler/solve/src/ability.rs

@@ -4,7 +4,7 @@ use roc_collections::{VecMap, VecSet};
 use roc_error_macros::{internal_error, todo_abilities};
 use roc_module::symbol::Symbol;
 use roc_region::all::{Loc, Region};
-use roc_solve_problem::{TypeError, UnderivableReason, Unfulfilled};
+use roc_solve_problem::{NotDerivableContext, TypeError, UnderivableReason, Unfulfilled};
 use roc_types::num::NumericRange;
 use roc_types::subs::{
     instantiate_rigids, Content, FlatType, GetSubsSlice, Rank, RecordFields, Subs, Variable,
@@ -278,11 +278,14 @@ impl ObligationCache {
                 // can derive!
                 None
             }
-            Some(Err(DerivableError::NotDerivable(failure_var))) => Some(if failure_var == var {
+            Some(Err(NotDerivable {
-                UnderivableReason::SurfaceNotDerivable
+                var: failure_var,
+                context,
+            })) => Some(if failure_var == var {
+                UnderivableReason::SurfaceNotDerivable(context)
             } else {
                 let (error_type, _skeletons) = subs.var_to_error_type(failure_var);
-                UnderivableReason::NestedNotDerivable(error_type)
+                UnderivableReason::NestedNotDerivable(error_type, context)
             }),
             None => Some(UnderivableReason::NotABuiltin),
         };
@@ -430,8 +433,9 @@ fn is_builtin_number_alias(symbol: Symbol) -> bool {
     )
 }
 
-enum DerivableError {
+struct NotDerivable {
-    NotDerivable(Variable),
+    var: Variable,
+    context: NotDerivableContext,
 }
 
 struct Descend(bool);
@@ -445,36 +449,51 @@ trait DerivableVisitor {
     }
 
     #[inline(always)]
-    fn visit_flex_able(var: Variable, ability: Symbol) -> Result<(), DerivableError> {
+    fn visit_flex_able(var: Variable, ability: Symbol) -> Result<(), NotDerivable> {
         if ability != Self::ABILITY {
-            Err(DerivableError::NotDerivable(var))
+            Err(NotDerivable {
+                var,
+                context: NotDerivableContext::NoContext,
+            })
         } else {
             Ok(())
         }
     }
 
     #[inline(always)]
-    fn visit_rigid_able(var: Variable, ability: Symbol) -> Result<(), DerivableError> {
+    fn visit_rigid_able(var: Variable, ability: Symbol) -> Result<(), NotDerivable> {
         if ability != Self::ABILITY {
-            Err(DerivableError::NotDerivable(var))
+            Err(NotDerivable {
+                var,
+                context: NotDerivableContext::NoContext,
+            })
         } else {
             Ok(())
         }
     }
 
     #[inline(always)]
-    fn visit_recursion(var: Variable) -> Result<Descend, DerivableError> {
+    fn visit_recursion(var: Variable) -> Result<Descend, NotDerivable> {
-        Err(DerivableError::NotDerivable(var))
+        Err(NotDerivable {
+            var,
+            context: NotDerivableContext::NoContext,
+        })
     }
 
     #[inline(always)]
-    fn visit_apply(var: Variable, _symbol: Symbol) -> Result<Descend, DerivableError> {
+    fn visit_apply(var: Variable, _symbol: Symbol) -> Result<Descend, NotDerivable> {
-        Err(DerivableError::NotDerivable(var))
+        Err(NotDerivable {
+            var,
+            context: NotDerivableContext::NoContext,
+        })
     }
 
     #[inline(always)]
-    fn visit_func(var: Variable) -> Result<Descend, DerivableError> {
+    fn visit_func(var: Variable) -> Result<Descend, NotDerivable> {
-        Err(DerivableError::NotDerivable(var))
+        Err(NotDerivable {
+            var,
+            context: NotDerivableContext::Function,
+        })
     }
 
     #[inline(always)]
@@ -482,43 +501,67 @@ trait DerivableVisitor {
         _subs: &Subs,
         var: Variable,
         _fields: RecordFields,
-    ) -> Result<Descend, DerivableError> {
+    ) -> Result<Descend, NotDerivable> {
-        Err(DerivableError::NotDerivable(var))
+        Err(NotDerivable {
+            var,
+            context: NotDerivableContext::NoContext,
+        })
     }
 
     #[inline(always)]
-    fn visit_tag_union(var: Variable) -> Result<Descend, DerivableError> {
+    fn visit_tag_union(var: Variable) -> Result<Descend, NotDerivable> {
-        Err(DerivableError::NotDerivable(var))
+        Err(NotDerivable {
+            var,
+            context: NotDerivableContext::NoContext,
+        })
     }
 
     #[inline(always)]
-    fn visit_recursive_tag_union(var: Variable) -> Result<Descend, DerivableError> {
+    fn visit_recursive_tag_union(var: Variable) -> Result<Descend, NotDerivable> {
-        Err(DerivableError::NotDerivable(var))
+        Err(NotDerivable {
+            var,
+            context: NotDerivableContext::NoContext,
+        })
     }
 
     #[inline(always)]
-    fn visit_function_or_tag_union(var: Variable) -> Result<Descend, DerivableError> {
+    fn visit_function_or_tag_union(var: Variable) -> Result<Descend, NotDerivable> {
-        Err(DerivableError::NotDerivable(var))
+        Err(NotDerivable {
+            var,
+            context: NotDerivableContext::NoContext,
+        })
     }
 
     #[inline(always)]
-    fn visit_empty_record(var: Variable) -> Result<(), DerivableError> {
+    fn visit_empty_record(var: Variable) -> Result<(), NotDerivable> {
-        Err(DerivableError::NotDerivable(var))
+        Err(NotDerivable {
+            var,
+            context: NotDerivableContext::NoContext,
+        })
     }
 
     #[inline(always)]
-    fn visit_empty_tag_union(var: Variable) -> Result<(), DerivableError> {
+    fn visit_empty_tag_union(var: Variable) -> Result<(), NotDerivable> {
-        Err(DerivableError::NotDerivable(var))
+        Err(NotDerivable {
+            var,
+            context: NotDerivableContext::NoContext,
+        })
     }
 
     #[inline(always)]
-    fn visit_alias(var: Variable, _symbol: Symbol) -> Result<Descend, DerivableError> {
+    fn visit_alias(var: Variable, _symbol: Symbol) -> Result<Descend, NotDerivable> {
-        Err(DerivableError::NotDerivable(var))
+        Err(NotDerivable {
+            var,
+            context: NotDerivableContext::NoContext,
+        })
     }
 
     #[inline(always)]
-    fn visit_ranged_number(var: Variable, _range: NumericRange) -> Result<(), DerivableError> {
+    fn visit_ranged_number(var: Variable, _range: NumericRange) -> Result<(), NotDerivable> {
-        Err(DerivableError::NotDerivable(var))
+        Err(NotDerivable {
+            var,
+            context: NotDerivableContext::NoContext,
+        })
     }
 
     #[inline(always)]
@@ -527,7 +570,7 @@ trait DerivableVisitor {
         abilities_store: &AbilitiesStore,
         subs: &mut Subs,
         var: Variable,
-    ) -> Result<(), DerivableError> {
+    ) -> Result<(), NotDerivable> {
         let mut stack = vec![var];
         let mut seen_recursion_vars = vec![];
 
@@ -545,14 +588,18 @@ trait DerivableVisitor {
             let content = subs.get_content_without_compacting(var);
 
             use Content::*;
-            use DerivableError::*;
             use FlatType::*;
             match *content {
                 FlexVar(opt_name) => {
                     // Promote the flex var to be bound to the ability.
                     subs.set_content(var, Content::FlexAbleVar(opt_name, Self::ABILITY));
                 }
-                RigidVar(_) => return Err(NotDerivable(var)),
+                RigidVar(_) => {
+                    return Err(NotDerivable {
+                        var,
+                        context: NotDerivableContext::NoContext,
+                    })
+                }
                 FlexAbleVar(_, ability) => Self::visit_flex_able(var, ability)?,
                 RigidAbleVar(_, ability) => Self::visit_rigid_able(var, ability)?,
                 RecursionVar {
@@ -622,7 +669,12 @@ trait DerivableVisitor {
                     EmptyRecord => Self::visit_empty_record(var)?,
                     EmptyTagUnion => Self::visit_empty_tag_union(var)?,
 
-                    Erroneous(_) => return Err(NotDerivable(var)),
+                    Erroneous(_) => {
+                        return Err(NotDerivable {
+                            var,
+                            context: NotDerivableContext::NoContext,
+                        })
+                    }
                 },
                 Alias(
                     Symbol::NUM_NUM | Symbol::NUM_INTEGER | Symbol::NUM_FLOATINGPOINT,
@@ -639,7 +691,10 @@ trait DerivableVisitor {
                         .is_err()
                         && !Self::is_derivable_builtin_opaque(opaque)
                     {
-                        return Err(NotDerivable(var));
+                        return Err(NotDerivable {
+                            var,
+                            context: NotDerivableContext::Opaque(opaque),
+                        });
                     }
                 }
                 Alias(symbol, _alias_variables, real_var, AliasKind::Structural) => {
@@ -650,9 +705,17 @@ trait DerivableVisitor {
                 }
                 RangedNumber(range) => Self::visit_ranged_number(var, range)?,
 
-                LambdaSet(..) => return Err(NotDerivable(var)),
+                LambdaSet(..) => {
+                    return Err(NotDerivable {
+                        var,
+                        context: NotDerivableContext::NoContext,
+                    })
+                }
                 Error => {
-                    return Err(NotDerivable(var));
+                    return Err(NotDerivable {
+                        var,
+                        context: NotDerivableContext::NoContext,
+                    });
                 }
             }
         }
@@ -671,19 +734,22 @@ impl DerivableVisitor for DeriveEncoding {
     }
 
     #[inline(always)]
-    fn visit_recursion(_var: Variable) -> Result<Descend, DerivableError> {
+    fn visit_recursion(_var: Variable) -> Result<Descend, NotDerivable> {
         Ok(Descend(true))
     }
 
     #[inline(always)]
-    fn visit_apply(var: Variable, symbol: Symbol) -> Result<Descend, DerivableError> {
+    fn visit_apply(var: Variable, symbol: Symbol) -> Result<Descend, NotDerivable> {
         if matches!(
             symbol,
             Symbol::LIST_LIST | Symbol::SET_SET | Symbol::DICT_DICT | Symbol::STR_STR,
         ) {
             Ok(Descend(true))
         } else {
-            Err(DerivableError::NotDerivable(var))
+            Err(NotDerivable {
+                var,
+                context: NotDerivableContext::NoContext,
+            })
         }
     }
 
@@ -692,37 +758,37 @@ impl DerivableVisitor for DeriveEncoding {
         _subs: &Subs,
         _var: Variable,
         _fields: RecordFields,
-    ) -> Result<Descend, DerivableError> {
+    ) -> Result<Descend, NotDerivable> {
         Ok(Descend(true))
     }
 
     #[inline(always)]
-    fn visit_tag_union(_var: Variable) -> Result<Descend, DerivableError> {
+    fn visit_tag_union(_var: Variable) -> Result<Descend, NotDerivable> {
         Ok(Descend(true))
     }
 
     #[inline(always)]
-    fn visit_recursive_tag_union(_var: Variable) -> Result<Descend, DerivableError> {
+    fn visit_recursive_tag_union(_var: Variable) -> Result<Descend, NotDerivable> {
         Ok(Descend(true))
     }
 
     #[inline(always)]
-    fn visit_function_or_tag_union(_var: Variable) -> Result<Descend, DerivableError> {
+    fn visit_function_or_tag_union(_var: Variable) -> Result<Descend, NotDerivable> {
         Ok(Descend(true))
     }
 
     #[inline(always)]
-    fn visit_empty_record(_var: Variable) -> Result<(), DerivableError> {
+    fn visit_empty_record(_var: Variable) -> Result<(), NotDerivable> {
         Ok(())
     }
 
     #[inline(always)]
-    fn visit_empty_tag_union(_var: Variable) -> Result<(), DerivableError> {
+    fn visit_empty_tag_union(_var: Variable) -> Result<(), NotDerivable> {
         Ok(())
     }
 
     #[inline(always)]
-    fn visit_alias(_var: Variable, symbol: Symbol) -> Result<Descend, DerivableError> {
+    fn visit_alias(_var: Variable, symbol: Symbol) -> Result<Descend, NotDerivable> {
         if is_builtin_number_alias(symbol) {
             Ok(Descend(false))
         } else {
@@ -731,7 +797,7 @@ impl DerivableVisitor for DeriveEncoding {
     }
 
     #[inline(always)]
-    fn visit_ranged_number(_var: Variable, _range: NumericRange) -> Result<(), DerivableError> {
+    fn visit_ranged_number(_var: Variable, _range: NumericRange) -> Result<(), NotDerivable> {
         Ok(())
     }
 }
@@ -746,19 +812,22 @@ impl DerivableVisitor for DeriveDecoding {
     }
 
     #[inline(always)]
-    fn visit_recursion(_var: Variable) -> Result<Descend, DerivableError> {
+    fn visit_recursion(_var: Variable) -> Result<Descend, NotDerivable> {
         Ok(Descend(true))
     }
 
     #[inline(always)]
-    fn visit_apply(var: Variable, symbol: Symbol) -> Result<Descend, DerivableError> {
+    fn visit_apply(var: Variable, symbol: Symbol) -> Result<Descend, NotDerivable> {
         if matches!(
             symbol,
             Symbol::LIST_LIST | Symbol::SET_SET | Symbol::DICT_DICT | Symbol::STR_STR,
         ) {
             Ok(Descend(true))
         } else {
-            Err(DerivableError::NotDerivable(var))
+            Err(NotDerivable {
+                var,
+                context: NotDerivableContext::NoContext,
+            })
         }
     }
 
@@ -767,46 +836,49 @@ impl DerivableVisitor for DeriveDecoding {
         subs: &Subs,
         var: Variable,
         fields: RecordFields,
-    ) -> Result<Descend, DerivableError> {
+    ) -> Result<Descend, NotDerivable> {
         let has_optional_field = subs
             .get_subs_slice(fields.record_fields())
             .iter()
             .any(|field| matches!(field, RecordField::Optional(..)));
 
         if has_optional_field {
-            Err(DerivableError::NotDerivable(var))
+            Err(NotDerivable {
+                var,
+                context: NotDerivableContext::NoContext,
+            })
         } else {
             Ok(Descend(true))
         }
     }
 
     #[inline(always)]
-    fn visit_tag_union(_var: Variable) -> Result<Descend, DerivableError> {
+    fn visit_tag_union(_var: Variable) -> Result<Descend, NotDerivable> {
         Ok(Descend(true))
     }
 
     #[inline(always)]
-    fn visit_recursive_tag_union(_var: Variable) -> Result<Descend, DerivableError> {
+    fn visit_recursive_tag_union(_var: Variable) -> Result<Descend, NotDerivable> {
         Ok(Descend(true))
     }
 
     #[inline(always)]
-    fn visit_function_or_tag_union(_var: Variable) -> Result<Descend, DerivableError> {
+    fn visit_function_or_tag_union(_var: Variable) -> Result<Descend, NotDerivable> {
         Ok(Descend(true))
     }
 
     #[inline(always)]
-    fn visit_empty_record(_var: Variable) -> Result<(), DerivableError> {
+    fn visit_empty_record(_var: Variable) -> Result<(), NotDerivable> {
         Ok(())
     }
 
     #[inline(always)]
-    fn visit_empty_tag_union(_var: Variable) -> Result<(), DerivableError> {
+    fn visit_empty_tag_union(_var: Variable) -> Result<(), NotDerivable> {
         Ok(())
     }
 
     #[inline(always)]
-    fn visit_alias(_var: Variable, symbol: Symbol) -> Result<Descend, DerivableError> {
+    fn visit_alias(_var: Variable, symbol: Symbol) -> Result<Descend, NotDerivable> {
         if is_builtin_number_alias(symbol) {
             Ok(Descend(false))
         } else {
@@ -815,7 +887,7 @@ impl DerivableVisitor for DeriveDecoding {
     }
 
     #[inline(always)]
-    fn visit_ranged_number(_var: Variable, _range: NumericRange) -> Result<(), DerivableError> {
+    fn visit_ranged_number(_var: Variable, _range: NumericRange) -> Result<(), NotDerivable> {
         Ok(())
     }
 }

crates/compiler/solve_problem/src/lib.rs

@@ -1,5 +1,5 @@
 use roc_can::expected::{Expected, PExpected};
-use roc_module::symbol::Symbol;
+use roc_module::{ident::Lowercase, symbol::Symbol};
 use roc_problem::can::CycleEntry;
 use roc_region::all::Region;
 
@@ -55,7 +55,21 @@ pub enum Unfulfilled {
 pub enum UnderivableReason {
     NotABuiltin,
     /// The surface type is not derivable
-    SurfaceNotDerivable,
+    SurfaceNotDerivable(NotDerivableContext),
     /// A nested type is not derivable
-    NestedNotDerivable(ErrorType),
+    NestedNotDerivable(ErrorType, NotDerivableContext),
+}
+
+#[derive(PartialEq, Debug, Clone)]
+pub enum NotDerivableContext {
+    NoContext,
+    Function,
+    UnboundVar,
+    Opaque(Symbol),
+    Decode(NotDerivableDecode),
+}
+
+#[derive(PartialEq, Debug, Clone)]
+pub enum NotDerivableDecode {
+    OptionalRecordField(Lowercase),
 }

crates/reporting/Cargo.toml

@@ -7,6 +7,7 @@ edition = "2021"
 
 [dependencies]
 roc_collections = { path = "../compiler/collections" }
+roc_error_macros = { path = "../error_macros" }
 roc_exhaustive = { path = "../compiler/exhaustive" }
 roc_region = { path = "../compiler/region" }
 roc_module = { path = "../compiler/module" }

crates/reporting/src/error/type.rs

@@ -2,12 +2,15 @@ use crate::error::canonicalize::{to_circular_def_doc, CIRCULAR_DEF};
 use crate::report::{Annotation, Report, RocDocAllocator, RocDocBuilder, Severity};
 use roc_can::expected::{Expected, PExpected};
 use roc_collections::all::{HumanIndex, MutSet, SendMap};
+use roc_error_macros::internal_error;
 use roc_exhaustive::CtorName;
 use roc_module::called_via::{BinOp, CalledVia};
 use roc_module::ident::{Ident, IdentStr, Lowercase, TagName};
 use roc_module::symbol::Symbol;
 use roc_region::all::{LineInfo, Loc, Region};
-use roc_solve_problem::{TypeError, UnderivableReason, Unfulfilled};
+use roc_solve_problem::{
+    NotDerivableContext, NotDerivableDecode, TypeError, UnderivableReason, Unfulfilled,
+};
 use roc_std::RocDec;
 use roc_types::pretty_print::{Parens, WILDCARD};
 use roc_types::types::{
@@ -334,9 +337,11 @@ fn report_underivable_reason<'a>(
         UnderivableReason::NotABuiltin => {
             Some(alloc.reflow("Only builtin abilities can have generated implementations!"))
         }
-        UnderivableReason::SurfaceNotDerivable => underivable_hint(alloc, ability, typ),
+        UnderivableReason::SurfaceNotDerivable(context) => {
-        UnderivableReason::NestedNotDerivable(nested_typ) => {
+            underivable_hint(alloc, ability, context, typ)
-            let hint = underivable_hint(alloc, ability, &nested_typ);
+        }
+        UnderivableReason::NestedNotDerivable(nested_typ, context) => {
+            let hint = underivable_hint(alloc, ability, context, &nested_typ);
             let reason = alloc.stack(
                 [
                     alloc.reflow("In particular, an implementation for"),
@@ -354,31 +359,17 @@ fn report_underivable_reason<'a>(
 fn underivable_hint<'b>(
     alloc: &'b RocDocAllocator<'b>,
     ability: Symbol,
+    context: NotDerivableContext,
     typ: &ErrorType,
 ) -> Option<RocDocBuilder<'b>> {
-    match typ {
+    match context {
-        ErrorType::Function(..) => Some(alloc.note("").append(alloc.concat([
+        NotDerivableContext::NoContext => None,
+        NotDerivableContext::Function => Some(alloc.note("").append(alloc.concat([
             alloc.symbol_unqualified(ability),
             alloc.reflow(" cannot be generated for functions."),
         ]))),
-        ErrorType::FlexVar(v) | ErrorType::RigidVar(v) => Some(alloc.tip().append(alloc.concat([
+        NotDerivableContext::Opaque(symbol) => Some(alloc.tip().append(alloc.concat([
-            alloc.reflow("This type variable is not bound to "),
+            alloc.symbol_unqualified(symbol),
-            alloc.symbol_unqualified(ability),
-            alloc.reflow(". Consider adding a "),
-            alloc.keyword("has"),
-            alloc.reflow(" clause to bind the type variable, like "),
-            alloc.inline_type_block(alloc.concat([
-                alloc.string("| ".to_string()),
-                alloc.type_variable(v.clone()),
-                alloc.space(),
-                alloc.keyword("has"),
-                alloc.space(),
-                alloc.symbol_qualified(ability),
-            ])),
-        ]))),
-        ErrorType::Alias(symbol, _, _, AliasKind::Opaque) => {
-            Some(alloc.tip().append(alloc.concat([
-                alloc.symbol_unqualified(*symbol),
             alloc.reflow(" does not implement "),
             alloc.symbol_unqualified(ability),
             alloc.reflow("."),
@@ -394,7 +385,7 @@ fn underivable_hint<'b>(
                                 alloc.symbol_qualified(ability),
                             ])),
                             alloc.reflow(" to the definition of "),
-                                alloc.symbol_unqualified(*symbol),
+                            alloc.symbol_unqualified(symbol),
                         ])
                     } else {
                         alloc.nil()
@@ -404,9 +395,44 @@ fn underivable_hint<'b>(
             } else {
                 alloc.nil()
             },
+        ]))),
+        NotDerivableContext::UnboundVar => {
+            let v = match typ {
+                ErrorType::FlexVar(v) => v,
+                ErrorType::RigidVar(v) => v,
+                _ => internal_error!("unbound variable context only applicable for variables"),
+            };
+
+            Some(alloc.tip().append(alloc.concat([
+                alloc.reflow("This type variable is not bound to "),
+                alloc.symbol_unqualified(ability),
+                alloc.reflow(". Consider adding a "),
+                alloc.keyword("has"),
+                alloc.reflow(" clause to bind the type variable, like "),
+                alloc.inline_type_block(alloc.concat([
+                    alloc.string("| ".to_string()),
+                    alloc.type_variable(v.clone()),
+                    alloc.space(),
+                    alloc.keyword("has"),
+                    alloc.space(),
+                    alloc.symbol_qualified(ability),
+                ])),
             ])))
         }
-        _ => None,
+        NotDerivableContext::Decode(reason) => match reason {
+            NotDerivableDecode::OptionalRecordField(field) => {
+                Some(alloc.note("").append(alloc.concat([
+                    alloc.reflow("Roc cannot derive decoding for a record with an optional field, which in this case is "),
+                    alloc.record_field(field),
+                    alloc.reflow(". Optional record fields are polymorphic over records that may or may not contain them at compile time, "),
+                    alloc.reflow("but are not a concept that extends to runtime! That means Roc cannot derive a decoder for a record with an optional value "),
+                    alloc.reflow("by way of an optional record field. If you want to model the idea that a field may or may not be present at runtime, "),
+                    alloc.reflow("consider using a "),
+                    alloc.symbol_unqualified(Symbol::RESULT_RESULT),
+                    alloc.reflow("."),
+                ])))
+            }
+        },
     }
 }