Fix string prototype trim methods (#583)

* Made trim methods ECMAScript specification compliant * Added tests
boa-dev · Jul 21, 2020 · cf25305 · cf25305
1 parent 5d4d8fe
commit cf25305
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 6 deletions.
diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs
@@ -722,6 +722,7 @@ impl String {
     }
 
     /// Helper function to check if a `char` is trimmable.
+    #[inline]
     fn is_trimmable_whitespace(c: char) -> bool {
         // The rust implementation of `trim` does not regard the same characters whitespace as ecma standard does
         //
@@ -753,9 +754,10 @@ impl String {
     /// [spec]: https://tc39.es/ecma262/#sec-string.prototype.trim
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/trim
     pub(crate) fn trim(this: &Value, _: &[Value], ctx: &mut Interpreter) -> ResultValue {
-        let this_str = ctx.to_string(this)?;
+        let this = ctx.require_object_coercible(this)?;
+        let string = ctx.to_string(this)?;
         Ok(Value::from(
-            this_str.trim_matches(Self::is_trimmable_whitespace),
+            string.trim_matches(Self::is_trimmable_whitespace),
         ))
     }
 
@@ -772,9 +774,10 @@ impl String {
     /// [spec]: https://tc39.es/ecma262/#sec-string.prototype.trimstart
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/trimStart
     pub(crate) fn trim_start(this: &Value, _: &[Value], ctx: &mut Interpreter) -> ResultValue {
-        let this_str = ctx.to_string(this)?;
+        let this = ctx.require_object_coercible(this)?;
+        let string = ctx.to_string(this)?;
         Ok(Value::from(
-            this_str.trim_start_matches(Self::is_trimmable_whitespace),
+            string.trim_start_matches(Self::is_trimmable_whitespace),
         ))
     }
 
@@ -791,9 +794,10 @@ impl String {
     /// [spec]: https://tc39.es/ecma262/#sec-string.prototype.trimend
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/trimEnd
     pub(crate) fn trim_end(this: &Value, _: &[Value], ctx: &mut Interpreter) -> ResultValue {
-        let this_str = ctx.to_string(this)?;
+        let this = ctx.require_object_coercible(this)?;
+        let string = ctx.to_string(this)?;
         Ok(Value::from(
-            this_str.trim_end_matches(Self::is_trimmable_whitespace),
+            string.trim_end_matches(Self::is_trimmable_whitespace),
         ))
     }
 

diff --git a/boa/src/builtins/string/tests.rs b/boa/src/builtins/string/tests.rs
@@ -304,3 +304,33 @@ fn test_match() {
     );
     assert_eq!(forward(&mut engine, "result4[0]"), "B");
 }
+
+#[test]
+fn trim() {
+    let realm = Realm::create();
+    let mut engine = Interpreter::new(realm);
+    assert_eq!(forward(&mut engine, "'Hello'.trim()"), "Hello");
+    assert_eq!(forward(&mut engine, "' \nHello'.trim()"), "Hello");
+    assert_eq!(forward(&mut engine, "'Hello \n\r'.trim()"), "Hello");
+    assert_eq!(forward(&mut engine, "' Hello '.trim()"), "Hello");
+}
+
+#[test]
+fn trim_start() {
+    let realm = Realm::create();
+    let mut engine = Interpreter::new(realm);
+    assert_eq!(forward(&mut engine, "'Hello'.trimStart()"), "Hello");
+    assert_eq!(forward(&mut engine, "' \nHello'.trimStart()"), "Hello");
+    assert_eq!(forward(&mut engine, "'Hello \n'.trimStart()"), "Hello \n");
+    assert_eq!(forward(&mut engine, "' Hello '.trimStart()"), "Hello ");
+}
+
+#[test]
+fn trim_end() {
+    let realm = Realm::create();
+    let mut engine = Interpreter::new(realm);
+    assert_eq!(forward(&mut engine, "'Hello'.trimEnd()"), "Hello");
+    assert_eq!(forward(&mut engine, "' \nHello'.trimEnd()"), " \nHello");
+    assert_eq!(forward(&mut engine, "'Hello \n'.trimEnd()"), "Hello");
+    assert_eq!(forward(&mut engine, "' Hello '.trimEnd()"), " Hello");
+}