`RegExp` constructor should call `IsRegExp()` (#2881)

Make the `RegExp` constructor call the `IsRegExp` function to check, not just internal slot check.

boa_engine/src/builtins/regexp/mod.rs

@@ -165,7 +165,7 @@ impl BuiltInConstructor for RegExp {
         let flags = args.get_or_undefined(1);
 
         // 1. Let patternIsRegExp be ? IsRegExp(pattern).
-        let pattern_is_regexp = pattern.as_object().filter(|obj| obj.is_regexp());
+        let pattern_is_regexp = Self::is_reg_exp(pattern, context)?;
 
         // 2. If NewTarget is undefined, then
         // 3. Else, let newTarget be NewTarget.
@@ -192,24 +192,25 @@ impl BuiltInConstructor for RegExp {
         }
 
         // 4. If Type(pattern) is Object and pattern has a [[RegExpMatcher]] internal slot, then
-        // 6. Else,
         let (p, f) = if let Some(pattern) = pattern_is_regexp {
-            let obj = pattern.borrow();
+            let mut original_source = JsValue::undefined();
-            let regexp = obj
+            let mut original_flags = JsValue::undefined();
-                .as_regexp()
+
-                .expect("already checked that IsRegExp returns true");
+            if let Some(regexp) = pattern.borrow().as_regexp() {
+                original_source = regexp.original_source.clone().into();
+                original_flags = regexp.original_flags.clone().into();
+            };
 
             // a. Let P be pattern.[[OriginalSource]].
             // b. If flags is undefined, let F be pattern.[[OriginalFlags]].
             // c. Else, let F be flags.
             if flags.is_undefined() {
-                (
+                (original_source, original_flags)
-                    JsValue::new(regexp.original_source.clone()),
-                    JsValue::new(regexp.original_flags.clone()),
-                )
             } else {
-                (JsValue::new(regexp.original_source.clone()), flags.clone())
+                (original_source, flags.clone())
             }
+
+        // 6. Else,
         } else {
             // a. Let P be pattern.
             // b. Let F be flags.
@@ -225,6 +226,40 @@ impl BuiltInConstructor for RegExp {
 }
 
 impl RegExp {
+    /// `7.2.8 IsRegExp ( argument )`
+    ///
+    /// This modified to return the object if it's `true`, [`None`] otherwise.
+    ///
+    /// More information:
+    ///  - [ECMAScript reference][spec]
+    ///
+    /// [spec]: https://tc39.es/ecma262/#sec-isregexp
+    pub(crate) fn is_reg_exp<'a>(
+        argument: &'a JsValue,
+        context: &mut Context<'_>,
+    ) -> JsResult<Option<&'a JsObject>> {
+        // 1. If argument is not an Object, return false.
+        let Some(argument) = argument.as_object() else {
+            return Ok(None);
+        };
+
+        // 2. Let matcher be ? Get(argument, @@match).
+        let matcher = argument.get(JsSymbol::r#match(), context)?;
+
+        // 3. If matcher is not undefined, return ToBoolean(matcher).
+        if !matcher.is_undefined() {
+            return Ok(matcher.to_boolean().then_some(argument));
+        }
+
+        // 4. If argument has a [[RegExpMatcher]] internal slot, return true.
+        if argument.is_regexp() {
+            return Ok(Some(argument));
+        }
+
+        // 5. Return false.
+        Ok(None)
+    }
+
     /// `22.2.3.2.1 RegExpAlloc ( newTarget )`
     ///
     /// More information:

boa_engine/src/builtins/string/mod.rs

@@ -825,7 +825,7 @@ impl String {
 
         // 3. Let isRegExp be ? IsRegExp(searchString).
         // 4. If isRegExp is true, throw a TypeError exception.
-        if is_reg_exp(search_string, context)? {
+        if RegExp::is_reg_exp(search_string, context)?.is_some() {
             return Err(JsNativeError::typ().with_message(
                 "First argument to String.prototype.startsWith must not be a regular expression",
             ).into());
@@ -893,7 +893,7 @@ impl String {
         let search_str = match args.get_or_undefined(0) {
             // 3. Let isRegExp be ? IsRegExp(searchString).
             // 4. If isRegExp is true, throw a TypeError exception.
-            search_string if is_reg_exp(search_string, context)? => {
+            search_string if RegExp::is_reg_exp(search_string, context)?.is_some() => {
                 return Err(JsNativeError::typ().with_message(
                     "First argument to String.prototype.endsWith must not be a regular expression",
                 ).into());
@@ -958,7 +958,7 @@ impl String {
 
         let search_str = match args.get_or_undefined(0) {
             // 3. Let isRegExp be ? IsRegExp(searchString).
-            search_string if is_reg_exp(search_string, context)? => {
+            search_string if RegExp::is_reg_exp(search_string, context)?.is_some() => {
                 return Err(JsNativeError::typ().with_message(
                     // 4. If isRegExp is true, throw a TypeError exception.
                     "First argument to String.prototype.includes must not be a regular expression",
@@ -1120,21 +1120,21 @@ impl String {
         // 2. If searchValue is neither undefined nor null, then
         if !search_value.is_null_or_undefined() {
             // a. Let isRegExp be ? IsRegExp(searchValue).
-            if let Some(obj) = search_value.as_object() {
+            // b. If isRegExp is true, then
-                // b. If isRegExp is true, then
+            if let Some(obj) = RegExp::is_reg_exp(search_value, context)? {
-                if is_reg_exp_object(obj, context)? {
+                // i. Let flags be ? Get(searchValue, "flags").
-                    // i. Let flags be ? Get(searchValue, "flags").
+                let flags = obj.get(utf16!("flags"), context)?;
-                    let flags = obj.get(utf16!("flags"), context)?;
+
-
+                // ii. Perform ? RequireObjectCoercible(flags).
-                    // ii. Perform ? RequireObjectCoercible(flags).
+                flags.require_object_coercible()?;
-                    flags.require_object_coercible()?;
+
-
+                // iii. If ? ToString(flags) does not contain "g", throw a TypeError exception.
-                    // iii. If ? ToString(flags) does not contain "g", throw a TypeError exception.
+                if !flags.to_string(context)?.contains(&u16::from(b'g')) {
-                    if !flags.to_string(context)?.contains(&u16::from(b'g')) {
+                    return Err(JsNativeError::typ()
-                        return Err(JsNativeError::typ().with_message(
+                        .with_message(
                             "String.prototype.replaceAll called with a non-global RegExp argument",
-                        ).into());
+                        )
-                    }
+                        .into());
                 }
             }
 
@@ -1985,22 +1985,20 @@ impl String {
         if !regexp.is_null_or_undefined() {
             // a. Let isRegExp be ? IsRegExp(regexp).
             // b. If isRegExp is true, then
-            if let Some(regexp_obj) = regexp.as_object() {
+            if let Some(regexp) = RegExp::is_reg_exp(regexp, context)? {
-                if is_reg_exp_object(regexp_obj, context)? {
+                // i. Let flags be ? Get(regexp, "flags").
-                    // i. Let flags be ? Get(regexp, "flags").
+                let flags = regexp.get(utf16!("flags"), context)?;
-                    let flags = regexp_obj.get(utf16!("flags"), context)?;
 
-                    // ii. Perform ? RequireObjectCoercible(flags).
+                // ii. Perform ? RequireObjectCoercible(flags).
-                    flags.require_object_coercible()?;
+                flags.require_object_coercible()?;
 
-                    // iii. If ? ToString(flags) does not contain "g", throw a TypeError exception.
+                // iii. If ? ToString(flags) does not contain "g", throw a TypeError exception.
-                    if !flags.to_string(context)?.contains(&u16::from(b'g')) {
+                if !flags.to_string(context)?.contains(&u16::from(b'g')) {
-                        return Err(JsNativeError::typ()
+                    return Err(JsNativeError::typ()
                         .with_message(
                             "String.prototype.matchAll called with a non-global RegExp argument",
                         )
                         .into());
-                    }
                 }
             }
             // c. Let matcher be ? GetMethod(regexp, @@matchAll).
@@ -2760,31 +2758,3 @@ pub(crate) fn get_substitution(
     // 11. Return result.
     Ok(js_string!(result))
 }
-
-/// Abstract operation `IsRegExp( argument )`
-///
-/// More information:
-/// [ECMAScript reference][spec]
-///
-/// [spec]: https://tc39.es/ecma262/#sec-isregexp
-fn is_reg_exp(argument: &JsValue, context: &mut Context<'_>) -> JsResult<bool> {
-    // 1. If Type(argument) is not Object, return false.
-    let JsValue::Object(argument) = argument else {
-        return Ok(false);
-    };
-
-    is_reg_exp_object(argument, context)
-}
-fn is_reg_exp_object(argument: &JsObject, context: &mut Context<'_>) -> JsResult<bool> {
-    // 2. Let matcher be ? Get(argument, @@match).
-    let matcher = argument.get(JsSymbol::r#match(), context)?;
-
-    // 3. If matcher is not undefined, return ! ToBoolean(matcher).
-    if !matcher.is_undefined() {
-        return Ok(matcher.to_boolean());
-    }
-
-    // 4. If argument has a [[RegExpMatcher]] internal slot, return true.
-    // 5. Return false.
-    Ok(argument.is_regexp())
-}