From 821b15f85eb0e9e34f2f0aa25f59871db8076d1f Mon Sep 17 00:00:00 2001 From: Jimmy Bourassa Date: Thu, 9 May 2024 14:46:59 -0400 Subject: [PATCH] Leverage QuickJS's `JS_ToString` in logging (#644) * Leverage QuickJS's toString * Revamp console tests * Handle invalid UTF8 in Console * Support Symbol in console --- crates/apis/src/console/mod.rs | 133 ++++++++++++++++++++++++--------- crates/javy/src/lib.rs | 43 +---------- 2 files changed, 100 insertions(+), 76 deletions(-) diff --git a/crates/apis/src/console/mod.rs b/crates/apis/src/console/mod.rs index d4a0be17..970c27bd 100644 --- a/crates/apis/src/console/mod.rs +++ b/crates/apis/src/console/mod.rs @@ -2,9 +2,11 @@ use std::io::Write; use anyhow::{Error, Result}; use javy::{ - hold, hold_and_release, print, - quickjs::{prelude::MutFn, Context, Function, Object, Value}, - to_js_error, Args, Runtime, + hold, hold_and_release, + quickjs::{ + convert, prelude::MutFn, Context, FromJs, Function, Object, String as JSString, Value, + }, + to_js_error, to_string_lossy, Args, Runtime, }; use crate::{APIConfig, JSApiSet}; @@ -71,15 +73,31 @@ where fn log<'js, T: Write>(args: Args<'js>, stream: &mut T) -> Result> { let (ctx, args) = args.release(); - let mut buf = String::new(); - for (i, arg) in args.iter().enumerate() { + for (i, arg) in args.into_inner().into_iter().enumerate() { if i != 0 { - buf.push(' '); + write!(stream, " ")?; } - print(arg, &mut buf)?; - } - writeln!(stream, "{buf}")?; + if let Some(symbol) = arg.as_symbol() { + if let Some(description) = symbol.description()?.into_string() { + let description = description + .to_string() + .unwrap_or_else(|e| to_string_lossy(&ctx, &description, e)); + write!(stream, "Symbol({description})")?; + } else { + write!(stream, "Symbol()")?; + } + } else { + let stringified = + >::from_js(&ctx, arg.clone()).map(|string| { + string + .to_string() + .unwrap_or_else(|e| to_string_lossy(&ctx, &string.0, e)) + })?; + write!(stream, "{stringified}")?; + }; + } + writeln!(stream)?; Ok(Value::new_undefined(ctx.clone())) } @@ -115,7 +133,7 @@ mod tests { } #[test] - fn test_console_log() -> Result<()> { + fn test_value_serialization() -> Result<()> { let mut stream = SharedStream::default(); let runtime = Runtime::default(); @@ -123,21 +141,70 @@ mod tests { register_console(ctx, stream.clone(), stream.clone())?; ctx.with(|this| { - this.eval("console.log(\"hello world\");")?; - assert_eq!(b"hello world\n", stream.buffer.borrow().as_slice()); - stream.clear(); + macro_rules! test_console_log { + ($js:expr, $expected:expr) => {{ + this.eval($js)?; + assert_eq!( + $expected, + std::str::from_utf8(stream.buffer.borrow().as_slice()).unwrap() + ); + stream.clear(); + }}; + } + + test_console_log!("console.log(\"hello world\");", "hello world\n"); + + // Invalid UTF-16 surrogate pair + test_console_log!("console.log(\"\\uD800\");", "�\n"); + + test_console_log!( + "console.log(function(){ return 1 })", + "function(){ return 1 }\n" + ); - this.eval("console.log(\"bonjour\", \"le\", \"monde\")")?; - assert_eq!(b"bonjour le monde\n", stream.buffer.borrow().as_slice()); + test_console_log!( + "console.log([1, \"two\", 3.42, null, 5])", + "1,two,3.42,,5\n" + ); - stream.clear(); + test_console_log!( + "console.log(2.3, true, { foo: 'bar' }, null, undefined)", + "2.3 true [object Object] null undefined\n" + ); - this.eval("console.log(2.3, true, { foo: 'bar' }, null, undefined)")?; - assert_eq!( - b"2.3 true [object Object] null undefined\n", - stream.buffer.borrow().as_slice() + test_console_log!( + "console.log(new Date(0))", + "Thu Jan 01 1970 00:00:00 GMT+0000\n" ); + test_console_log!("console.log(new ArrayBuffer())", "[object ArrayBuffer]\n"); + + test_console_log!("console.log(NaN)", "NaN\n"); + + test_console_log!("console.log(new Set())", "[object Set]\n"); + + test_console_log!("console.log(new Map())", "[object Map]\n"); + + test_console_log!( + "function Foo(){}; console.log(new Foo())", + "[object Object]\n" + ); + + test_console_log!("console.log(Symbol())", "Symbol()\n"); + + test_console_log!("console.log(Symbol(''))", "Symbol()\n"); + + test_console_log!("console.log(Symbol('foo'))", "Symbol(foo)\n"); + + test_console_log!("console.log(Symbol(null))", "Symbol(null)\n"); + + test_console_log!("console.log(Symbol(undefined))", "Symbol()\n"); + + test_console_log!("console.log(Symbol([]))", "Symbol()\n"); + + // Invalid UTF-16 surrogate pair + test_console_log!("console.log(Symbol(\"\\uD800\"))", "Symbol(�)\n"); + Ok::<_, Error>(()) })?; @@ -145,29 +212,25 @@ mod tests { } #[test] - fn test_console_error() -> Result<()> { - let mut stream = SharedStream::default(); + fn test_console_streams() -> Result<()> { + let mut log_stream = SharedStream::default(); + let error_stream = SharedStream::default(); let runtime = Runtime::default(); let ctx = runtime.context(); - register_console(ctx, stream.clone(), stream.clone())?; + register_console(ctx, log_stream.clone(), error_stream.clone())?; ctx.with(|this| { - this.eval("console.error(\"hello world\");")?; - assert_eq!(b"hello world\n", stream.buffer.borrow().as_slice()); - - stream.clear(); + this.eval("console.log(\"hello world\");")?; + assert_eq!(b"hello world\n", log_stream.buffer.borrow().as_slice()); + assert!(error_stream.buffer.borrow().is_empty()); - this.eval("console.error(\"bonjour\", \"le\", \"monde\")")?; - assert_eq!(b"bonjour le monde\n", stream.buffer.borrow().as_slice()); + log_stream.clear(); - stream.clear(); + this.eval("console.error(\"hello world\");")?; + assert_eq!(b"hello world\n", error_stream.buffer.borrow().as_slice()); + assert!(log_stream.buffer.borrow().is_empty()); - this.eval("console.error(2.3, true, { foo: 'bar' }, null, undefined)")?; - assert_eq!( - b"2.3 true [object Object] null undefined\n", - stream.buffer.borrow().as_slice() - ); Ok::<_, Error>(()) })?; diff --git a/crates/javy/src/lib.rs b/crates/javy/src/lib.rs index 3471a110..3a4bbbea 100644 --- a/crates/javy/src/lib.rs +++ b/crates/javy/src/lib.rs @@ -50,11 +50,8 @@ mod config; mod runtime; mod serde; -use anyhow::{anyhow, Error, Result}; -use rquickjs::{ - prelude::Rest, qjs, Ctx, Error as JSError, Exception, String as JSString, Type, Value, -}; -use std::fmt::Write; +use anyhow::{anyhow, Error}; +use rquickjs::{prelude::Rest, qjs, Ctx, Error as JSError, Exception, String as JSString, Value}; #[cfg(feature = "messagepack")] pub mod messagepack; @@ -62,42 +59,6 @@ pub mod messagepack; #[cfg(feature = "json")] pub mod json; -/// Print the given JS value. -/// -/// The implementation matches the default JavaScript display format for each value. -pub fn print(val: &Value, sink: &mut String) -> Result<()> { - match val.type_of() { - Type::Undefined => write!(sink, "undefined").map_err(Into::into), - Type::Null => write!(sink, "null").map_err(Into::into), - Type::Bool => { - let b = val.as_bool().unwrap(); - write!(sink, "{}", b).map_err(Into::into) - } - Type::Int => { - let i = val.as_int().unwrap(); - write!(sink, "{}", i).map_err(Into::into) - } - Type::Float => { - let f = val.as_float().unwrap(); - write!(sink, "{}", f).map_err(Into::into) - } - Type::String => { - let s = val.as_string().unwrap(); - write!(sink, "{}", s.to_string()?).map_err(Into::into) - } - Type::Array => { - let inner = val.as_array().unwrap(); - for e in inner.iter() { - print(&e?, sink)? - } - Ok(()) - } - Type::Object => write!(sink, "[object Object]").map_err(Into::into), - // TODO: Implement the rest. - x => unimplemented!("{x}"), - } -} - /// A struct to hold the current [`Ctx`] and [`Value`]s passed as arguments to Rust /// functions. /// A struct here is used to explicitly tie these values with a particular