Skip to content

Commit

Permalink
RegExp constructor should call IsRegExp() (#2881)
Browse files Browse the repository at this point in the history 
Make the `RegExp` constructor call the `IsRegExp` function to check, not just internal slot check.
  • Loading branch information
@HalidOdat
HalidOdat committed Apr 30, 2023
1 parent c738b50 commit 1c3f547
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 66 deletions.
57 changes: 46 additions & 11 deletions boa_engine/src/builtins/regexp/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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:
Expand Down
80 changes: 25 additions & 55 deletions boa_engine/src/builtins/string/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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());
}
}

Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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())
}

0 comments on commit 1c3f547

Please sign in to comment.