From 7f2ea6f2d1ae0ece2b676957bf8983e3c4f8f4a3 Mon Sep 17 00:00:00 2001 From: HalidOdat Date: Sat, 1 Aug 2020 11:34:44 +0200 Subject: [PATCH] Made `String.prototype.lastIndexOf()` spec compliant (#598) --- boa/src/builtins/string/mod.rs | 55 +++++------- boa/src/builtins/string/tests.rs | 139 +++++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+), 32 deletions(-) diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index 657edca6b4..4b83ba13fa 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -532,9 +532,10 @@ impl String { /// `String.prototype.indexOf( searchValue[, fromIndex] )` /// - /// The `indexOf()` method returns the index within the calling `String` object of the first occurrence of the specified value, starting the search at `fromIndex`. + /// The `indexOf()` method returns the index within the calling `String` object of the first occurrence + /// of the specified value, starting the search at `fromIndex`. /// - /// Returns -1 if the value is not found. + /// Returns `-1` if the value is not found. /// /// More information: /// - [ECMAScript reference][spec] @@ -571,9 +572,10 @@ impl String { /// `String.prototype.lastIndexOf( searchValue[, fromIndex] )` /// - /// The `lastIndexOf()` method returns the index within the calling `String` object of the last occurrence of the specified value, searching backwards from `fromIndex`. + /// The `lastIndexOf()` method returns the index within the calling `String` object of the last occurrence + /// of the specified value, searching backwards from `fromIndex`. /// - /// Returns -1 if the value is not found. + /// Returns `-1` if the value is not found. /// /// More information: /// - [ECMAScript reference][spec] @@ -586,41 +588,30 @@ impl String { args: &[Value], ctx: &mut Interpreter, ) -> ResultValue { - // First we get it the actual string a private field stored on the object only the engine has access to. - // Then we convert it into a Rust String by wrapping it in from_value - let primitive_val = ctx.to_string(this)?; - - // TODO: Should throw TypeError if search_string is regular expression - let search_string = ctx.to_string( - args.get(0) - .expect("failed to get argument for String method"), - )?; + let this = ctx.require_object_coercible(this)?; + let string = ctx.to_string(this)?; - let length = primitive_val.chars().count() as i32; + let search_string = + ctx.to_string(&args.get(0).cloned().unwrap_or_else(Value::undefined))?; - // If less than 2 args specified, position is 'undefined', defaults to 0 - let position = if args.len() < 2 { - 0 - } else { - i32::from(args.get(1).expect("Could not get argument")) - }; + let length = string.chars().count(); + let start = args + .get(1) + .map(|position| ctx.to_integer(position)) + .transpose()? + .map_or(0, |position| position.max(0.0).min(length as f64) as usize); - let start = min(max(position, 0), length); + if search_string.is_empty() { + return Ok(start.min(length).into()); + } - // Here cannot use the &str method "rfind", because this returns the last - // byte index: we need to return the last char index in the JS String - // Instead, iterate over the part we're checking keeping track of the higher - // index we found that "starts with" the search string - let mut highest_index = -1; - for index in start..length { - let this_string: StdString = primitive_val.chars().skip(index as usize).collect(); - if this_string.starts_with(search_string.as_str()) { - highest_index = index; + if start < length { + if let Some(position) = string.rfind(search_string.as_str()) { + return Ok(string[..position].chars().count().into()); } } - // This will still be -1 if no matches were found, else with be >= 0 - Ok(Value::from(highest_index)) + Ok(Value::from(-1)) } /// `String.prototype.match( regexp )` diff --git a/boa/src/builtins/string/tests.rs b/boa/src/builtins/string/tests.rs index 719ce84298..2ba8b048d6 100644 --- a/boa/src/builtins/string/tests.rs +++ b/boa/src/builtins/string/tests.rs @@ -509,3 +509,142 @@ fn index_of_empty_search_string() { assert_eq!(forward(&mut engine, "'ABC'.indexOf('', 2)"), "2"); assert_eq!(forward(&mut engine, "'ABC'.indexOf('', 10)"), "3"); } + +#[test] +fn last_index_of_with_no_arguments() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + assert_eq!(forward(&mut engine, "''.lastIndexOf()"), "-1"); + assert_eq!(forward(&mut engine, "'undefined'.lastIndexOf()"), "0"); + assert_eq!(forward(&mut engine, "'a1undefined'.lastIndexOf()"), "2"); + assert_eq!( + forward(&mut engine, "'a1undefined1aundefined'.lastIndexOf()"), + "13" + ); + assert_eq!( + forward(&mut engine, "'µµµundefinedundefined'.lastIndexOf()"), + "12" + ); + assert_eq!( + forward(&mut engine, "'µµµundefinedµµµundefined'.lastIndexOf()"), + "15" + ); +} + +#[test] +fn last_index_of_with_string_search_string_argument() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + assert_eq!(forward(&mut engine, "''.lastIndexOf('hello')"), "-1"); + assert_eq!( + forward(&mut engine, "'undefined'.lastIndexOf('undefined')"), + "0" + ); + assert_eq!( + forward(&mut engine, "'a1undefined'.lastIndexOf('undefined')"), + "2" + ); + assert_eq!( + forward( + &mut engine, + "'a1undefined1aundefined'.lastIndexOf('undefined')" + ), + "13" + ); + assert_eq!( + forward( + &mut engine, + "'µµµundefinedundefined'.lastIndexOf('undefined')" + ), + "12" + ); + assert_eq!( + forward( + &mut engine, + "'µµµundefinedµµµundefined'.lastIndexOf('undefined')" + ), + "15" + ); +} + +#[test] +fn last_index_of_with_non_string_search_string_argument() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + assert_eq!(forward(&mut engine, "''.lastIndexOf(1)"), "-1"); + assert_eq!(forward(&mut engine, "'1'.lastIndexOf(1)"), "0"); + assert_eq!(forward(&mut engine, "'11'.lastIndexOf(1)"), "1"); + assert_eq!( + forward(&mut engine, "'truefalsetrue'.lastIndexOf(true)"), + "9" + ); + assert_eq!(forward(&mut engine, "'ab100ba'.lastIndexOf(100)"), "2"); + assert_eq!(forward(&mut engine, "'µµµfalse'.lastIndexOf(true)"), "-1"); + assert_eq!(forward(&mut engine, "'µµµ5µµµ65µ'.lastIndexOf(5)"), "8"); +} + +#[test] +fn last_index_of_with_from_index_argument() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + assert_eq!(forward(&mut engine, "''.lastIndexOf('x', 2)"), "-1"); + assert_eq!(forward(&mut engine, "'x'.lastIndexOf('x', 2)"), "-1"); + assert_eq!(forward(&mut engine, "'abcxx'.lastIndexOf('x', 2)"), "4"); + assert_eq!(forward(&mut engine, "'x'.lastIndexOf('x', 2)"), "-1"); + assert_eq!(forward(&mut engine, "'µµµxµµµ'.lastIndexOf('x', 2)"), "3"); + + assert_eq!( + forward(&mut engine, "'µµµxµµµ'.lastIndexOf('x', 10000000)"), + "-1" + ); +} + +#[test] +fn last_index_with_empty_search_string() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + assert_eq!(forward(&mut engine, "''.lastIndexOf('')"), "0"); + assert_eq!(forward(&mut engine, "'x'.lastIndexOf('', 2)"), "1"); + assert_eq!(forward(&mut engine, "'abcxx'.lastIndexOf('', 4)"), "4"); + assert_eq!(forward(&mut engine, "'µµµxµµµ'.lastIndexOf('', 2)"), "2"); + + assert_eq!(forward(&mut engine, "'abc'.lastIndexOf('', 10000000)"), "3"); +} + +#[test] +fn generic_last_index_of() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + forward_val( + &mut engine, + "Number.prototype.lastIndexOf = String.prototype.lastIndexOf", + ) + .unwrap(); + + assert_eq!(forward(&mut engine, "(1001).lastIndexOf(9)"), "-1"); + assert_eq!(forward(&mut engine, "(1001).lastIndexOf(0)"), "2"); + assert_eq!(forward(&mut engine, "(1001).lastIndexOf('0')"), "2"); +} + +#[test] +fn last_index_non_integer_position_argument() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + assert_eq!( + forward(&mut engine, "''.lastIndexOf('x', new Number(4))"), + "-1" + ); + assert_eq!( + forward(&mut engine, "'abc'.lastIndexOf('b', new Number(1))"), + "1" + ); + assert_eq!( + forward(&mut engine, "'abcx'.lastIndexOf('x', new String('1'))"), + "3" + ); + assert_eq!( + forward(&mut engine, "'abcx'.lastIndexOf('x', new String('100'))"), + "-1" + ); + assert_eq!(forward(&mut engine, "'abcx'.lastIndexOf('x', null)"), "3"); +}