Made `String.prototype.lastIndexOf()` spec compliant (#598)

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 )`

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");
+}