fix(assert): test a RegExp matcher against String(thrown), and make it terminal

changelog.d/9228-assert-regexp-matcher-input.md

@@ -0,0 +1,32 @@
+`assert.throws(fn, /re/)` and friends now test the RegExp matcher the way Node
+does: against `String(thrown)` only, with a RegExp treated as a terminal
+matcher category.
+
+`expected_error_matches` used to give a non-matching pattern a second chance
+against `thrown.message`, then fall through to the `instanceof` /
+constructor-name / validation-function checks when that also failed. Two bugs
+came out of one block:
+
+- A pattern that matched the bare message but not the stringified error was
+  wrongly accepted. `assert.throws(() => { throw new Error("nope") }, /^nope/)`
+  passed, where Node reports an `AssertionError` because `String(err)` is
+  `"Error: nope"`. Same for a thrown non-error carrying a `message` property.
+- A pattern that matched nothing reached `js_instanceof_dynamic(thrown, regexp)`
+  and surfaced as a `TypeError` instead of an `AssertionError`, so
+  `assert.throws`/`rejects` reported the wrong error class and
+  `doesNotThrow`/`doesNotReject` failed to rethrow the original error.
+
+A RegExp *value on a validator key* (`{ message: /bad/ }`) is a different
+matcher and still tests against that property.
+
+The pre-existing `assert/errors/strict-throws-validation.ts` could not catch
+either bug: its pattern matched both the message and the stringified error, and
+it only printed `name`/`code`. `assert/errors/throws-regexp-matcher-input.ts`
+covers the two inputs separately across `throws`, `doesNotThrow`, `rejects`,
+and `doesNotReject`.
+
+Known remaining gap: the generated `AssertionError` message is Perry's generic
+"The thrown error did not match the expected matcher" rather than Node's
+"The input did not match the regular expression /x/. Input:\n\n'Error: nope'\n".
+That generic fallback is shared by every matcher category, so it is left for a
+separate change.

crates/perry-runtime/src/object/assert.rs

@@ -259,14 +259,18 @@ fn expected_error_matches(thrown: f64, expected: f64) -> bool {
     if is_null_or_undefined(expected) {
         return true;
     }
+    // A RegExp matcher is a complete, terminal matcher category: Node tests it
+    // against `String(thrown)` and nothing else, and a non-match is an
+    // AssertionError — never a fallthrough to the instanceof /
+    // constructor-name / validation-function checks below (`instanceof`
+    // against a RegExp throws a TypeError). Retrying the pattern against
+    // `thrown.message` was a second, non-Node chance that wrongly accepted
+    // anchored patterns: `/^nope/` matched `new Error("nope")` even though
+    // `String(err)` is `"Error: nope"`. A RegExp *value on a validator key*
+    // (`{ message: /bad/ }`) is a different thing and is still tested against
+    // that property in `object_matcher_matches`.
     if let Some(matches_thrown) = regex_test_value(expected, thrown) {
-        if matches_thrown {
-            return true;
-        }
-        let message = read_property(thrown, "message");
-        if !is_null_or_undefined(message) && regex_test_value(expected, message).unwrap_or(false) {
-            return true;
-        }
+        return matches_thrown;
     }
     // A plain object validator (e.g. `{ code: "ERR_X" }`) is a property-bag
     // matcher, never a constructor — its own enumerable keys must each equal
@@ -1304,3 +1308,61 @@ pub extern "C" fn js_assert_if_error(value: f64) -> f64 {
     }
     throw_assertion(if_error_message(value), value, null_f64(), "ifError", false)
 }
+
+#[cfg(all(test, feature = "regex-engine"))]
+mod regexp_matcher_tests {
+    use super::*;
+
+    fn jsstr(bytes: &[u8]) -> *mut crate::StringHeader {
+        crate::string::js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32)
+    }
+
+    fn error_value(message: &[u8]) -> f64 {
+        let err = crate::error::js_error_new_with_message(jsstr(message));
+        crate::value::js_nanbox_pointer(err as i64)
+    }
+
+    fn regexp_value(pattern: &[u8]) -> f64 {
+        let re = crate::regex::js_regexp_new(jsstr(pattern), jsstr(b""));
+        crate::value::js_nanbox_pointer(re as i64)
+    }
+
+    /// Node tests a RegExp matcher against `String(thrown)` only. An anchored
+    /// pattern that matches the bare message must NOT match, because
+    /// `String(new Error("nope"))` is `"Error: nope"`.
+    #[test]
+    fn regexp_matcher_tests_the_stringified_error_not_the_message() {
+        let thrown = error_value(b"nope");
+        assert!(!expected_error_matches(thrown, regexp_value(b"^nope")));
+        assert!(expected_error_matches(
+            thrown,
+            regexp_value(b"^Error: nope$")
+        ));
+    }
+
+    /// A RegExp is a terminal matcher category: a non-match returns `false` so
+    /// the caller reports an `AssertionError`, instead of falling through to
+    /// `js_instanceof_dynamic(thrown, regexp)`, which throws a `TypeError`.
+    #[test]
+    fn non_matching_regexp_is_terminal() {
+        assert!(!expected_error_matches(
+            error_value(b"nope"),
+            regexp_value(b"will-not-match")
+        ));
+    }
+
+    /// A RegExp *value on a validator key* is a different matcher and is still
+    /// tested against that property, not against the stringified error.
+    #[test]
+    fn validator_key_regexp_still_tests_the_property() {
+        let thrown = error_value(b"bad value");
+        let validator = crate::object::js_object_alloc(0, 1);
+        crate::object::js_object_set_field_by_name(
+            validator,
+            jsstr(b"message"),
+            regexp_value(b"^bad"),
+        );
+        let validator = crate::value::js_nanbox_pointer(validator as i64);
+        assert!(expected_error_matches(thrown, validator));
+    }
+}

test-parity/node-suite/assert/errors/throws-regexp-matcher-input.ts

@@ -0,0 +1,57 @@
+// A RegExp matcher is tested against `String(thrown)` and nothing else, and it
+// is a terminal matcher category: a non-match is an AssertionError, never a
+// fallthrough to the instanceof / constructor / validation-function checks.
+// The pre-existing `strict-throws-validation.ts` only exercised a pattern that
+// matches the message *and* the stringified error, so it could not tell the
+// two inputs apart.
+import assert from "node:assert";
+
+function show(label: string, fn: () => void): void {
+  try {
+    fn();
+    console.log(label + ": pass");
+  } catch (err: any) {
+    console.log(label + ":", err?.name, err?.code ?? err?.operator ?? "no-code");
+  }
+}
+
+// `String(new Error("nope"))` is "Error: nope", so an anchored pattern that
+// only matches the bare message must NOT match.
+show("anchored message-only pattern", () =>
+  assert.throws(() => { throw new Error("nope"); }, /^nope/));
+
+// The same pattern anchored against the stringified error does match.
+show("anchored stringified pattern", () =>
+  assert.throws(() => { throw new Error("nope"); }, /^Error: nope$/));
+
+// A plain object is stringified to "[object Object]"; its `message` property
+// is not a second chance for the pattern.
+show("plain object with message prop", () =>
+  assert.throws(() => { throw { message: "nope" }; }, /^nope/));
+
+// A thrown primitive stringifies to itself.
+show("thrown string", () =>
+  assert.throws(() => { throw "nope"; }, /^nope$/));
+
+// doesNotThrow with a non-matching RegExp rethrows the original error rather
+// than reporting an AssertionError (and must not raise a TypeError from a
+// fallthrough into `instanceof`).
+show("doesNotThrow non-matching rethrows", () =>
+  assert.doesNotThrow(() => { throw new Error("nope"); }, /will-not-match/));
+
+show("doesNotThrow matching reports", () =>
+  assert.doesNotThrow(() => { throw new Error("nope"); }, /nope/));
+
+// A RegExp value on a validator *key* is a different matcher: it is tested
+// against that property, so an anchored message pattern does match there.
+show("validator key regexp", () =>
+  assert.throws(() => { throw new TypeError("bad value"); }, { message: /^bad/ }));
+
+// Async paths route through the same matcher.
+await assert.rejects(async () => { throw new Error("nope"); }, /^nope/).then(
+  () => console.log("rejects anchored: pass"),
+  (err: any) => console.log("rejects anchored:", err?.name, err?.code ?? err?.operator));
+
+await assert.doesNotReject(async () => { throw new Error("nope"); }, /will-not-match/).then(
+  () => console.log("doesNotReject non-matching: pass"),
+  (err: any) => console.log("doesNotReject non-matching:", err?.name, err?.code ?? "no-code"));