From ba2ba444e1059b90fcfca8894c977cc2ec756fab Mon Sep 17 00:00:00 2001 From: Kevin Velasco Date: Tue, 8 Jun 2021 11:53:15 +0800 Subject: [PATCH 1/6] Do the naive thing for questions --- boa/src/value/mod.rs | 67 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 3 deletions(-) diff --git a/boa/src/value/mod.rs b/boa/src/value/mod.rs index 51325674885..84afa76541f 100644 --- a/boa/src/value/mod.rs +++ b/boa/src/value/mod.rs @@ -612,9 +612,70 @@ impl Value { Value::String(string) => Ok(string.clone()), Value::Symbol(_) => Err(context.construct_type_error("can't convert symbol to string")), Value::BigInt(ref bigint) => Ok(bigint.to_string().into()), - Value::Object(_) => { - let primitive = self.to_primitive(context, PreferredType::String)?; - primitive.to_string(context) + Value::Object(gc_object) => { + if let Some(function) = gc_object.borrow().as_function() { + use crate::builtins::function::Function; + let name = { + // Is there a case here where if there is no name field on a value + // We it should default to None? Do all functions have names set? + let value = self.get_field("name", &mut *context)?; + if value.is_null_or_undefined() { + None + } else { + Some(value.to_string(context)?) + } + }; + + match (function, name) { + (Function::BuiltIn(_, _), Some(name)) => { + Ok(format!("function {}() {{\n [native Code]\n}}", &name).into()) + } + (Function::Ordinary { body, params, .. }, Some(name)) => { + let arguments: String = params + .iter() + .map(|param| param.name()) + .collect::>() + .join(", "); + + let statement_list = &**body; + // This is a kluge. The implementaion in browser seems to suggest that + // the value here is printed exactly as defined in source. I'm not sure if + // that's possible here, but for now here's a dumb heuristic that prints functions + let is_multiline = { + let value = statement_list.to_string(); + value.lines().count() > 1 + }; + if is_multiline { + Ok( + // ?? For some reason statement_list string implementation + // sticks a \n at the end no matter what + format!( + "{}({}) {{\n{}}}", + &name, + arguments, + statement_list.to_string() + ) + .into(), + ) + } else { + Ok(format!( + "{}({}) {{{}}}", + &name, + arguments, + // The trim here is to remove a \n stuck at the end + // of the statement_list to_string method + statement_list.to_string().trim() + ) + .into()) + } + } + + _ => Ok("TODO".into()), + } + } else { + let primitive = self.to_primitive(context, PreferredType::String)?; + primitive.to_string(context) + } } } } From 14b381aae4f59d4940c0425422c58cf0dc98159f Mon Sep 17 00:00:00 2001 From: Kevin Velasco Date: Mon, 14 Jun 2021 09:38:11 +0800 Subject: [PATCH 2/6] Move all the rendering code to the builtin function struct --- boa/src/builtins/function/mod.rs | 73 ++++++++++++++++++++++++++++++++ boa/src/value/mod.rs | 67 ++--------------------------- 2 files changed, 76 insertions(+), 64 deletions(-) diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index 036f0c324a0..4a3957ad46d 100644 --- a/boa/src/builtins/function/mod.rs +++ b/boa/src/builtins/function/mod.rs @@ -329,6 +329,78 @@ impl BuiltInFunctionObject { // TODO?: 5. PrepareForTailCall context.call(this, &this_arg, &arg_list) } + + fn to_string(this: &Value, _: &[Value], context: &mut Context) -> Result { + let name = { + // Is there a case here where if there is no name field on a value + // We it should default to None? Do all functions have names set? + let value = this.get_field("name", &mut *context)?; + if value.is_null_or_undefined() { + None + } else { + Some(value.to_string(context)?) + } + }; + + let function = { + let object = this.clone().as_object().expect("Should be an object"); + + let func = object + .borrow() + .as_function() + .map(|f| f.clone()) + .expect("Should be a function"); + + func + }; + + match (&function, name) { + (Function::BuiltIn(_, _), Some(name)) => { + Ok(format!("function {}() {{\n [native Code]\n}}", &name).into()) + } + (Function::Ordinary { body, params, .. }, Some(name)) => { + let arguments: String = params + .iter() + .map(|param| param.name()) + .collect::>() + .join(", "); + + let statement_list = &*body; + // This is a kluge. The implementaion in browser seems to suggest that + // the value here is printed exactly as defined in source. I'm not sure if + // that's possible here, but for now here's a dumb heuristic that prints functions + let is_multiline = { + let value = statement_list.to_string(); + value.lines().count() > 1 + }; + if is_multiline { + Ok( + // ?? For some reason statement_list string implementation + // sticks a \n at the end no matter what + format!( + "{}({}) {{\n{}}}", + &name, + arguments, + statement_list.to_string() + ) + .into(), + ) + } else { + Ok(format!( + "{}({}) {{{}}}", + &name, + arguments, + // The trim here is to remove a \n stuck at the end + // of the statement_list to_string method + statement_list.to_string().trim() + ) + .into()) + } + } + + _ => Ok("TODO".into()), + } + } } impl BuiltIn for BuiltInFunctionObject { @@ -358,6 +430,7 @@ impl BuiltIn for BuiltInFunctionObject { .length(Self::LENGTH) .method(Self::call, "call", 1) .method(Self::apply, "apply", 1) + .method(Self::to_string, "toString", 0) .build(); (Self::NAME, function_object.into(), Self::attribute()) diff --git a/boa/src/value/mod.rs b/boa/src/value/mod.rs index 84afa76541f..51325674885 100644 --- a/boa/src/value/mod.rs +++ b/boa/src/value/mod.rs @@ -612,70 +612,9 @@ impl Value { Value::String(string) => Ok(string.clone()), Value::Symbol(_) => Err(context.construct_type_error("can't convert symbol to string")), Value::BigInt(ref bigint) => Ok(bigint.to_string().into()), - Value::Object(gc_object) => { - if let Some(function) = gc_object.borrow().as_function() { - use crate::builtins::function::Function; - let name = { - // Is there a case here where if there is no name field on a value - // We it should default to None? Do all functions have names set? - let value = self.get_field("name", &mut *context)?; - if value.is_null_or_undefined() { - None - } else { - Some(value.to_string(context)?) - } - }; - - match (function, name) { - (Function::BuiltIn(_, _), Some(name)) => { - Ok(format!("function {}() {{\n [native Code]\n}}", &name).into()) - } - (Function::Ordinary { body, params, .. }, Some(name)) => { - let arguments: String = params - .iter() - .map(|param| param.name()) - .collect::>() - .join(", "); - - let statement_list = &**body; - // This is a kluge. The implementaion in browser seems to suggest that - // the value here is printed exactly as defined in source. I'm not sure if - // that's possible here, but for now here's a dumb heuristic that prints functions - let is_multiline = { - let value = statement_list.to_string(); - value.lines().count() > 1 - }; - if is_multiline { - Ok( - // ?? For some reason statement_list string implementation - // sticks a \n at the end no matter what - format!( - "{}({}) {{\n{}}}", - &name, - arguments, - statement_list.to_string() - ) - .into(), - ) - } else { - Ok(format!( - "{}({}) {{{}}}", - &name, - arguments, - // The trim here is to remove a \n stuck at the end - // of the statement_list to_string method - statement_list.to_string().trim() - ) - .into()) - } - } - - _ => Ok("TODO".into()), - } - } else { - let primitive = self.to_primitive(context, PreferredType::String)?; - primitive.to_string(context) - } + Value::Object(_) => { + let primitive = self.to_primitive(context, PreferredType::String)?; + primitive.to_string(context) } } } From 1f93af06e385f47cbb82e377a152f3aacf6ac305 Mon Sep 17 00:00:00 2001 From: Kevin Velasco Date: Mon, 14 Jun 2021 10:14:31 +0800 Subject: [PATCH 3/6] prevent panics when called unconventionally --- boa/src/builtins/function/mod.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index 4a3957ad46d..c9446d70d8f 100644 --- a/boa/src/builtins/function/mod.rs +++ b/boa/src/builtins/function/mod.rs @@ -343,15 +343,16 @@ impl BuiltInFunctionObject { }; let function = { - let object = this.clone().as_object().expect("Should be an object"); + let object = this.clone().as_object().map(|object| { + let func = object.borrow().as_function().map(|f| f.clone()); + func + }); - let func = object - .borrow() - .as_function() - .map(|f| f.clone()) - .expect("Should be a function"); - - func + if let Some(Some(function)) = object { + function + } else { + return context.throw_type_error("Not a function"); + } }; match (&function, name) { From 467496608f30caa7e438dabd230fe34b9901c542 Mon Sep 17 00:00:00 2001 From: Kevin Velasco Date: Mon, 14 Jun 2021 10:23:00 +0800 Subject: [PATCH 4/6] remove unnecessary clone --- boa/src/builtins/function/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index c9446d70d8f..d0c072cb92f 100644 --- a/boa/src/builtins/function/mod.rs +++ b/boa/src/builtins/function/mod.rs @@ -343,7 +343,7 @@ impl BuiltInFunctionObject { }; let function = { - let object = this.clone().as_object().map(|object| { + let object = this.as_object().map(|object| { let func = object.borrow().as_function().map(|f| f.clone()); func }); From 739ed62dc81819c7b4ed6326c3e9ca1a4985d6ed Mon Sep 17 00:00:00 2001 From: Kevin Velasco Date: Tue, 15 Jun 2021 08:32:13 +0800 Subject: [PATCH 5/6] fix clippy issues --- boa/src/builtins/function/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index d0c072cb92f..474b86fdd57 100644 --- a/boa/src/builtins/function/mod.rs +++ b/boa/src/builtins/function/mod.rs @@ -330,6 +330,7 @@ impl BuiltInFunctionObject { context.call(this, &this_arg, &arg_list) } + #[allow(clippy::wrong_self_convention)] fn to_string(this: &Value, _: &[Value], context: &mut Context) -> Result { let name = { // Is there a case here where if there is no name field on a value @@ -343,10 +344,9 @@ impl BuiltInFunctionObject { }; let function = { - let object = this.as_object().map(|object| { - let func = object.borrow().as_function().map(|f| f.clone()); - func - }); + let object = this + .as_object() + .map(|object| object.borrow().as_function().cloned()); if let Some(Some(function)) = object { function From f4cd7770554fee81251dcbfb7246f9a0decb530d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Borges?= Date: Wed, 8 Sep 2021 16:51:49 +0200 Subject: [PATCH 6/6] Docs: minor phrasing fix --- boa/src/builtins/function/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index 474b86fdd57..b1bf6574113 100644 --- a/boa/src/builtins/function/mod.rs +++ b/boa/src/builtins/function/mod.rs @@ -334,7 +334,7 @@ impl BuiltInFunctionObject { fn to_string(this: &Value, _: &[Value], context: &mut Context) -> Result { let name = { // Is there a case here where if there is no name field on a value - // We it should default to None? Do all functions have names set? + // name should default to None? Do all functions have names set? let value = this.get_field("name", &mut *context)?; if value.is_null_or_undefined() { None