Better assertion error messages

Author: sharkdp

book/src/examples/example-numbat_syntax.md

@@ -124,7 +124,7 @@ print("value of π ≈ {π:.3}")     # Format specifiers
 
 assert(1 yard < 1 meter)         # Assertion
 
-assert_eq(1 ft, 12 in)           # Assert that two quantities are equal
+assert_eq(12 in, 1 ft)           # Assert that two quantities are equal
 assert_eq(1 yd, 1 m, 10 cm)      # Assert that two quantities are equal, up to
                                  # the given precision
 type(2 m/s)                      # Print the type of an expression

examples/numbat_syntax.nbt

@@ -115,7 +115,7 @@ print("value of π ≈ {π:.3}")     # Format specifiers
 
 assert(1 yard < 1 meter)         # Assertion
 
-assert_eq(1 ft, 12 in)           # Assert that two quantities are equal
+assert_eq(12 in, 1 ft)           # Assert that two quantities are equal
 assert_eq(1 yd, 1 m, 10 cm)      # Assert that two quantities are equal, up to
                                  # the given precision
 type(2 m/s)                      # Print the type of an expression

examples/runtime_error/assert_eq_4.nbt

@@ -0,0 +1 @@
+assert_eq(2 + 2, 2 + 1)

examples/runtime_error/assert_eq_5.nbt

@@ -0,0 +1 @@
+assert_eq(2 + 2e-12, 2 + 1e-12)

numbat/src/diagnostic.rs

@@ -552,21 +552,25 @@ impl ErrorDiagnostic for ResolverDiagnostic<'_, RuntimeError> {
                             .with_message("assertion failed"),
                     ]),
             ),
-            RuntimeErrorKind::AssertEq2Failed(assert_eq2_error) => diag.push(
-                Diagnostic::error()
-                    .with_message("Assertion failed")
-                    .with_labels(vec![
-                        assert_eq2_error
-                            .span_lhs
-                            .diagnostic_label(LabelStyle::Secondary)
-                            .with_message(format!("{}", assert_eq2_error.lhs)),
-                        assert_eq2_error
-                            .span_rhs
-                            .diagnostic_label(LabelStyle::Primary)
-                            .with_message(format!("{}", assert_eq2_error.rhs)),
-                    ])
-                    .with_notes(vec![inner]),
-            ),
+            RuntimeErrorKind::AssertEq2Failed(assert_eq2_error) => {
+                let (lhs, rhs) = assert_eq2_error.fmt_values();
+
+                diag.push(
+                    Diagnostic::error()
+                        .with_message("Assertion failed")
+                        .with_labels(vec![
+                            assert_eq2_error
+                                .span_lhs
+                                .diagnostic_label(LabelStyle::Secondary)
+                                .with_message(lhs),
+                            assert_eq2_error
+                                .span_rhs
+                                .diagnostic_label(LabelStyle::Primary)
+                                .with_message(rhs),
+                        ])
+                        .with_notes(vec![inner]),
+                )
+            }
             RuntimeErrorKind::AssertEq3Failed(assert_eq3_error) => {
                 let (lhs, rhs) = assert_eq3_error.fmt_comparands();
 

numbat/src/ffi/procedures.rs

@@ -85,30 +85,38 @@ fn assert_eq(_: &mut ExecutionContext, mut args: Args) -> ControlFlow {
         let lhs = lhs_arg.value;
         let rhs = rhs_arg.value;
 
-        let error = ControlFlow::Break(RuntimeErrorKind::AssertEq2Failed(AssertEq2Error {
-            span_lhs: lhs_arg.span,
-            lhs: lhs.clone(),
-            span_rhs: rhs_arg.span,
-            rhs: rhs.clone(),
-        }));
-
-        if lhs.is_quantity() {
-            let lhs = lhs.unsafe_as_quantity();
-            let rhs = rhs.unsafe_as_quantity();
-
-            if let Ok(args1_converted) = rhs.convert_to(lhs.unit()) {
-                if lhs == args1_converted {
-                    ControlFlow::Continue(())
-                } else {
-                    error
+        let mut lhs_converted = None;
+        let mut diff = None;
+
+        let values_equal = if lhs.is_quantity() {
+            let lhs_q = lhs.clone().unsafe_as_quantity();
+            let rhs_q = rhs.clone().unsafe_as_quantity();
+
+            if let Ok(converted) = lhs_q.convert_to(rhs_q.unit()) {
+                let equal = converted == rhs_q;
+                if !equal {
+                    diff = (&converted - &rhs_q).ok().map(|d| d.abs());
                 }
+                lhs_converted = Some(converted);
+                equal
             } else {
-                error
+                false
             }
-        } else if lhs == rhs {
+        } else {
+            lhs == rhs
+        };
+
+        if values_equal {
             ControlFlow::Continue(())
         } else {
-            error
+            ControlFlow::Break(RuntimeErrorKind::AssertEq2Failed(AssertEq2Error {
+                span_lhs: lhs_arg.span,
+                lhs,
+                span_rhs: rhs_arg.span,
+                rhs,
+                lhs_converted,
+                diff,
+            }))
         }
     } else {
         let lhs_original = lhs_arg.value.unsafe_as_quantity();

numbat/src/interpreter/assert_eq.rs

@@ -9,22 +9,73 @@ pub struct AssertEq2Error {
     pub lhs: Value,
     pub span_rhs: Span,
     pub rhs: Value,
+    /// When both values are quantities, stores lhs converted to rhs's unit.
+    pub lhs_converted: Option<Quantity>,
+    /// When both values are quantities, stores the absolute difference (in rhs's unit).
+    pub diff: Option<Quantity>,
+}
+
+impl AssertEq2Error {
+    /// Returns formatted display strings for lhs and rhs values.
+    /// When both values are quantities with different units, lhs is shown
+    /// converted to rhs's unit with the original in parentheses.
+    pub fn fmt_values(&self) -> (String, String) {
+        let rhs_str = format!("{}", self.rhs);
+        let lhs_str = if let Some(ref lhs_converted) = self.lhs_converted {
+            let lhs_converted_str = format!("{lhs_converted}");
+            if let Value::Quantity(ref lhs_q) = self.lhs
+                && lhs_converted.unit() != lhs_q.unit()
+            {
+                return (format!("{lhs_converted_str} ({})", self.lhs), rhs_str);
+            }
+            lhs_converted_str
+        } else {
+            format!("{}", self.lhs)
+        };
+        (lhs_str, rhs_str)
+    }
+
+    fn is_floating_point_inaccuracy(&self) -> bool {
+        if let Some(ref diff) = self.diff {
+            if let Some(ref lhs_converted) = self.lhs_converted {
+                let diff_val = diff.unsafe_value().to_f64().abs();
+                let lhs_val = lhs_converted.unsafe_value().to_f64().abs();
+
+                if let Value::Quantity(ref rhs_q) = self.rhs {
+                    let rhs_val = rhs_q.unsafe_value().to_f64().abs();
+                    let max_val = lhs_val.max(rhs_val);
+                    return max_val > 0.0 && diff_val / max_val < 1e-9;
+                }
+            }
+        }
+        false
+    }
 }
 
 impl Display for AssertEq2Error {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let optional_message = if format!("{}", self.lhs) == format!("{}", self.rhs) {
-            "\nNote: The two printed values appear to be the same, this may be due to floating point precision errors.\n      \
-            For dimension types you may want to test approximate equality instead: assert_eq(q1, q2, ε)."
+        let (lhs_str, rhs_str) = self.fmt_values();
+
+        let fp_note = if self.is_floating_point_inaccuracy() {
+            "\nNote: this is likely due to floating point inaccuracy. \
+            Consider testing approximate equality instead: assert_eq(q1, q2, ε)."
         } else {
             ""
         };
 
-        write!(
-            f,
-            "Assertion failed because the following two values are not the same:\n  {}\n  {}{}",
-            self.lhs, self.rhs, optional_message
-        )
+        if let Some(ref diff) = self.diff {
+            write!(
+                f,
+                "Assertion failed because the following two quantities differ by {diff}:\
+                \n  {lhs_str}\n  {rhs_str}{fp_note}",
+            )
+        } else {
+            write!(
+                f,
+                "Assertion failed because the following two values are not the same:\
+                \n  {lhs_str}\n  {rhs_str}",
+            )
+        }
     }
 }
 

numbat/tests/interpreter.rs

@@ -1337,21 +1337,5 @@ mod tests {
               -77.0089°
             "###);
         }
-
-        #[test]
-        fn test_floating_point_warning() {
-            insta::assert_snapshot!(fail("assert_eq(2+ 2, 2 + 1)"), @r###"
-            Assertion failed because the following two values are not the same:
-              4
-              3
-            "###);
-            insta::assert_snapshot!(fail("assert_eq(2 + 2e-12, 2 + 1e-12)"), @r###"
-            Assertion failed because the following two values are not the same:
-              2.0
-              2.0
-            Note: The two printed values appear to be the same, this may be due to floating point precision errors.
-                  For dimension types you may want to test approximate equality instead: assert_eq(q1, q2, ε).
-            "###);
-        }
     }
 }

numbat/tests/snapshots/prelude_and_examples__runtime_error_snapshots@assert_eq_1.nbt.snap

@@ -11,7 +11,7 @@ error: Assertion failed
   │           │         
   │           2 m
   │
-  = Assertion failed because the following two values are not the same:
+  = Assertion failed because the following two quantities differ by 0.1 m:
       2 m
       2.1 m
 

numbat/tests/snapshots/prelude_and_examples__runtime_error_snapshots@assert_eq_4.nbt.snap

@@ -0,0 +1,20 @@
+---
+source: numbat/tests/prelude_and_examples.rs
+expression: output
+input_file: examples/runtime_error/assert_eq_4.nbt
+---
+error: Assertion failed
+  ┌─ <internal:3>:1:18
+  │
+1 │ assert_eq(2 + 2, 2 + 1)
+  │           -----  ^^^^^ 3
+  │           │       
+  │           4
+  │
+  = Assertion failed because the following two quantities differ by 1:
+      4
+      3
+
+help: Backtrace:
+ = 0: assert_eq(2 + 2, 2 + 1)
+   	at <main> - <internal:3>:1:0