From 1c3f5478e18ce2824fd54f3ca987b93cb6157aa5 Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Sun, 30 Apr 2023 14:30:46 +0000 Subject: [PATCH] `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 | 57 +++++++++++++++---- boa_engine/src/builtins/string/mod.rs | 80 +++++++++------------------ 2 files changed, 71 insertions(+), 66 deletions(-) diff --git a/boa_engine/src/builtins/regexp/mod.rs b/boa_engine/src/builtins/regexp/mod.rs index 73bfec57e5a..95e1daff127 100644 --- a/boa_engine/src/builtins/regexp/mod.rs +++ b/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 regexp = obj - .as_regexp() - .expect("already checked that IsRegExp returns true"); + let mut original_source = JsValue::undefined(); + let mut original_flags = JsValue::undefined(); + + 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() { - ( - JsValue::new(regexp.original_source.clone()), - JsValue::new(regexp.original_flags.clone()), - ) + (original_source, original_flags) } 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> { + // 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: diff --git a/boa_engine/src/builtins/string/mod.rs b/boa_engine/src/builtins/string/mod.rs index 274b5346cea..c8380283ecf 100644 --- a/boa_engine/src/builtins/string/mod.rs +++ b/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 - if is_reg_exp_object(obj, context)? { - // i. Let flags be ? Get(searchValue, "flags"). - let flags = obj.get(utf16!("flags"), context)?; - - // ii. Perform ? RequireObjectCoercible(flags). - flags.require_object_coercible()?; - - // iii. If ? ToString(flags) does not contain "g", throw a TypeError exception. - if !flags.to_string(context)?.contains(&u16::from(b'g')) { - return Err(JsNativeError::typ().with_message( + // b. If isRegExp is true, then + if let Some(obj) = RegExp::is_reg_exp(search_value, context)? { + // i. Let flags be ? Get(searchValue, "flags"). + let flags = obj.get(utf16!("flags"), context)?; + + // ii. Perform ? RequireObjectCoercible(flags). + flags.require_object_coercible()?; + + // iii. If ? ToString(flags) does not contain "g", throw a TypeError exception. + if !flags.to_string(context)?.contains(&u16::from(b'g')) { + return Err(JsNativeError::typ() + .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 is_reg_exp_object(regexp_obj, context)? { - // i. Let flags be ? Get(regexp, "flags"). - let flags = regexp_obj.get(utf16!("flags"), context)?; + if let Some(regexp) = RegExp::is_reg_exp(regexp, context)? { + // i. Let flags be ? Get(regexp, "flags"). + let flags = regexp.get(utf16!("flags"), context)?; - // ii. Perform ? RequireObjectCoercible(flags). - flags.require_object_coercible()?; + // ii. Perform ? RequireObjectCoercible(flags). + flags.require_object_coercible()?; - // iii. If ? ToString(flags) does not contain "g", throw a TypeError exception. - if !flags.to_string(context)?.contains(&u16::from(b'g')) { - return Err(JsNativeError::typ() + // iii. If ? ToString(flags) does not contain "g", throw a TypeError exception. + if !flags.to_string(context)?.contains(&u16::from(b'g')) { + 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 { - // 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 { - // 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()) -}