Address review comments

crates/javy-test-macros/README.md

@@ -0,0 +1,3 @@
+# Macro helpers for testing Javy
+
+See `src/lib.rs` for more details and usage.

crates/javy-test-macros/src/lib.rs

@@ -1,9 +1,10 @@
+use anyhow::bail;
 /// Macros for testing Javy.
 ///
-/// Helper macros to define 262 tests or tests that exercise different
+/// Helper macros to define Test262 tests or tests that exercise different
 /// configuration combinations.
 ///
-/// Currently only defining 262 tests for JSON is supported.
+/// Currently only defining Test262 tests for JSON is supported.
 ///
 /// Usage
 ///
@@ -26,7 +27,7 @@ impl Config262 {
         if path.is_dir() {
             Ok(())
         } else {
-            Err(anyhow::anyhow!("Invalid path"))
+            bail!("Invalid path")
         }
     }
 }
@@ -76,7 +77,7 @@ fn ignore(test_name: &str) -> bool {
         "test_stringify_replacer_array_proxy_revoked_realm",
         "test_stringify_value_bigint_cross_realm",
         // TODO
-        // Currenlty the conversion between non-utf8 string encodings is lossy.
+        // Currently the conversion between non-utf8 string encodings is lossy.
         // There's probably a way to improve the interop.
         "test_stringify_value_string_escape_unicode",
     ]

crates/javy/src/apis/json.rs

@@ -32,7 +32,7 @@ use crate::serde::de::get_to_json;
 
 use simd_json::Error as SError;
 
-use anyhow::{anyhow, Result};
+use anyhow::{anyhow, bail, Result};
 use std::{
     io::{Read, Write},
     ptr::NonNull,
@@ -71,6 +71,8 @@ fn register<'js>(this: Ctx<'js>) -> Result<()> {
     )?;
 
     // Explicitly set the function's name and length properties.
+    // In both the parse and the stringify case below, the spec tests
+    // assert that the name and length properties must be  correctly set.
     parse.set_length(2)?;
     parse.set_name("parse")?;
 
@@ -97,10 +99,10 @@ fn call_json_parse<'a>(args: Args<'a>, default: Function<'a>) -> Result<Value<'a
     let (this, args) = args.release();
 
     match args.len() {
-        0 => Err(anyhow!(Exception::throw_syntax(
+        0 => bail!(Exception::throw_syntax(
             &this,
             "undefined\" is not valid JSON"
-        ))),
+        )),
         1 => {
             let val = args[0].clone();
             // Fast path. Number and null are treated as identity.
@@ -109,10 +111,7 @@ fn call_json_parse<'a>(args: Args<'a>, default: Function<'a>) -> Result<Value<'a
             }
 
             if val.is_symbol() {
-                return Err(anyhow!(Exception::throw_type(
-                    &this,
-                    "Expected string primitive"
-                )));
+                bail!(Exception::throw_type(&this, "Expected string primitive"));
             }
 
             let mut string = val_to_string(this.clone(), args[0].clone())?;
@@ -122,9 +121,11 @@ fn call_json_parse<'a>(args: Args<'a>, default: Function<'a>) -> Result<Value<'a
                     return original;
                 }
 
-                // TODO: Use the actual result here.
-                let _e = original.downcast::<SError>().unwrap();
-                anyhow!(Exception::throw_syntax(&this, ""))
+                let e = match original.downcast_ref::<SError>() {
+                    Some(e) => e.to_string(),
+                    None => "JSON parse error".into(),
+                };
+                anyhow!(Exception::throw_syntax(&this, &e))
             })
         }
         _ => {

crates/javy/src/serde/de.rs

@@ -213,7 +213,6 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
             if let Some(f) = get_to_json(&self.value) {
                 let v: Value = f.call((This(self.value.clone()),))?;
 
-                // TODO: Must find a way to discard.
                 if v.is_undefined() {
                     self.value = Value::new_undefined(v.ctx().clone());
                 } else {