From aad78154cfa2ae5b0701d7db8eaeea597ef695a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Juli=C3=A1n=20Espina?= Date: Sat, 5 Nov 2022 16:08:39 +0000 Subject: [PATCH] Pretty print promise objects (#2407) Right now our promises print `{ }` on display. This PR improves a bit the display and ergonomics of promises in general. Now, promises will print... - When pending: `Promise { }` - When fulfilled: `Promise { "hi" }` - When rejected: `Promise { ReferenceError: x is not initialized }` --- boa_engine/src/builtins/promise/mod.rs | 60 ++++++++------------- boa_engine/src/object/jsobject.rs | 13 +++++ boa_engine/src/object/mod.rs | 2 +- boa_engine/src/tests.rs | 2 +- boa_engine/src/value/display.rs | 75 ++++++++++++++++++-------- 5 files changed, 92 insertions(+), 60 deletions(-) diff --git a/boa_engine/src/builtins/promise/mod.rs b/boa_engine/src/builtins/promise/mod.rs index e127fff001..ae4ebef81e 100644 --- a/boa_engine/src/builtins/promise/mod.rs +++ b/boa_engine/src/builtins/promise/mod.rs @@ -58,17 +58,15 @@ macro_rules! if_abrupt_reject_promise { pub(crate) use if_abrupt_reject_promise; -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum PromiseState { +#[derive(Debug, Clone, Trace, Finalize)] +pub(crate) enum PromiseState { Pending, - Fulfilled, - Rejected, + Fulfilled(JsValue), + Rejected(JsValue), } #[derive(Debug, Clone, Trace, Finalize)] pub struct Promise { - promise_result: Option, - #[unsafe_ignore_trace] promise_state: PromiseState, promise_fulfill_reactions: Vec, promise_reject_reactions: Vec, @@ -277,6 +275,11 @@ struct ResolvingFunctionsRecord { impl Promise { const LENGTH: usize = 1; + /// Gets the current state of the promise. + pub(crate) fn state(&self) -> &PromiseState { + &self.promise_state + } + /// `Promise ( executor )` /// /// More information: @@ -311,7 +314,6 @@ impl Promise { let promise = JsObject::from_proto_and_data( promise, ObjectData::promise(Self { - promise_result: None, // 4. Set promise.[[PromiseState]] to pending. promise_state: PromiseState::Pending, // 5. Set promise.[[PromiseFulfillReactions]] to a new empty List. @@ -1406,9 +1408,8 @@ impl Promise { /// [spec]: https://tc39.es/ecma262/#sec-fulfillpromise pub fn fulfill_promise(&mut self, value: &JsValue, context: &mut Context) -> JsResult<()> { // 1. Assert: The value of promise.[[PromiseState]] is pending. - assert_eq!( - self.promise_state, - PromiseState::Pending, + assert!( + matches!(self.promise_state, PromiseState::Pending), "promise was not pending" ); @@ -1419,17 +1420,15 @@ impl Promise { Self::trigger_promise_reactions(reactions, value, context); // reordering this statement does not affect the semantics - // 3. Set promise.[[PromiseResult]] to value. - self.promise_result = Some(value.clone()); - // 4. Set promise.[[PromiseFulfillReactions]] to undefined. self.promise_fulfill_reactions = Vec::new(); // 5. Set promise.[[PromiseRejectReactions]] to undefined. self.promise_reject_reactions = Vec::new(); + // 3. Set promise.[[PromiseResult]] to value. // 6. Set promise.[[PromiseState]] to fulfilled. - self.promise_state = PromiseState::Fulfilled; + self.promise_state = PromiseState::Fulfilled(value.clone()); // 8. Return unused. Ok(()) @@ -1446,9 +1445,8 @@ impl Promise { /// [spec]: https://tc39.es/ecma262/#sec-rejectpromise pub fn reject_promise(&mut self, reason: &JsValue, context: &mut Context) { // 1. Assert: The value of promise.[[PromiseState]] is pending. - assert_eq!( - self.promise_state, - PromiseState::Pending, + assert!( + matches!(self.promise_state, PromiseState::Pending), "Expected promise.[[PromiseState]] to be pending" ); @@ -1459,17 +1457,15 @@ impl Promise { Self::trigger_promise_reactions(reactions, reason, context); // reordering this statement does not affect the semantics - // 3. Set promise.[[PromiseResult]] to reason. - self.promise_result = Some(reason.clone()); - // 4. Set promise.[[PromiseFulfillReactions]] to undefined. self.promise_fulfill_reactions = Vec::new(); // 5. Set promise.[[PromiseRejectReactions]] to undefined. self.promise_reject_reactions = Vec::new(); + // 3. Set promise.[[PromiseResult]] to reason. // 6. Set promise.[[PromiseState]] to rejected. - self.promise_state = PromiseState::Rejected; + self.promise_state = PromiseState::Rejected(reason.clone()); // 7. If promise.[[PromiseIsHandled]] is false, perform HostPromiseRejectionTracker(promise, "reject"). if !self.promise_is_handled { @@ -1969,16 +1965,11 @@ impl Promise { } // 10. Else if promise.[[PromiseState]] is fulfilled, then - PromiseState::Fulfilled => { - // a. Let value be promise.[[PromiseResult]]. - let value = self - .promise_result - .clone() - .expect("promise.[[PromiseResult]] cannot be empty"); - + // a. Let value be promise.[[PromiseResult]]. + PromiseState::Fulfilled(ref value) => { // b. Let fulfillJob be NewPromiseReactionJob(fulfillReaction, value). let fulfill_job = - PromiseJob::new_promise_reaction_job(fulfill_reaction, value, context); + PromiseJob::new_promise_reaction_job(fulfill_reaction, value.clone(), context); // c. Perform HostEnqueuePromiseJob(fulfillJob.[[Job]], fulfillJob.[[Realm]]). context.host_enqueue_promise_job(fulfill_job); @@ -1986,13 +1977,8 @@ impl Promise { // 11. Else, // a. Assert: The value of promise.[[PromiseState]] is rejected. - PromiseState::Rejected => { - // b. Let reason be promise.[[PromiseResult]]. - let reason = self - .promise_result - .clone() - .expect("promise.[[PromiseResult]] cannot be empty"); - + // b. Let reason be promise.[[PromiseResult]]. + PromiseState::Rejected(ref reason) => { // c. If promise.[[PromiseIsHandled]] is false, perform HostPromiseRejectionTracker(promise, "handle"). if !self.promise_is_handled { // HostPromiseRejectionTracker(promise, "handle") @@ -2001,7 +1987,7 @@ impl Promise { // d. Let rejectJob be NewPromiseReactionJob(rejectReaction, reason). let reject_job = - PromiseJob::new_promise_reaction_job(reject_reaction, reason, context); + PromiseJob::new_promise_reaction_job(reject_reaction, reason.clone(), context); // e. Perform HostEnqueuePromiseJob(rejectJob.[[Job]], rejectJob.[[Realm]]). context.host_enqueue_promise_job(reject_job); diff --git a/boa_engine/src/object/jsobject.rs b/boa_engine/src/object/jsobject.rs index b0e7500118..d1afe8f99a 100644 --- a/boa_engine/src/object/jsobject.rs +++ b/boa_engine/src/object/jsobject.rs @@ -665,6 +665,19 @@ Cannot both specify accessors and a value or writable attribute", Ok(()) } + #[inline] + pub(crate) fn get_property(&self, key: &PropertyKey) -> Option { + let mut obj = Some(self.clone()); + + while let Some(o) = obj { + if let Some(v) = o.borrow().properties.get(key) { + return Some(v); + } + obj = o.borrow().prototype().clone(); + } + None + } + /// Helper function for property insertion. #[inline] #[track_caller] diff --git a/boa_engine/src/object/mod.rs b/boa_engine/src/object/mod.rs index dbcd422588..8cbc7fe7a9 100644 --- a/boa_engine/src/object/mod.rs +++ b/boa_engine/src/object/mod.rs @@ -192,9 +192,9 @@ pub enum ObjectKind { Arguments(Arguments), NativeObject(Box), IntegerIndexed(IntegerIndexed), + Promise(Promise), #[cfg(feature = "intl")] DateTimeFormat(Box), - Promise(Promise), } unsafe impl Trace for ObjectKind { diff --git a/boa_engine/src/tests.rs b/boa_engine/src/tests.rs index 4a0cda6ada..e5febd0d3f 100644 --- a/boa_engine/src/tests.rs +++ b/boa_engine/src/tests.rs @@ -2645,7 +2645,7 @@ fn spread_getters_in_object() { var a = { x: 42 }; var aWithXGetter = { ...a, ... { get x() { throw new Error('not thrown yet') } } }; "#; - assert_eq!(&exec(scenario), "\"Error\": \"not thrown yet\""); + assert_eq!(&exec(scenario), "Error: not thrown yet"); } #[test] diff --git a/boa_engine/src/value/display.rs b/boa_engine/src/value/display.rs index 8431557a95..d9efe58c99 100644 --- a/boa_engine/src/value/display.rs +++ b/boa_engine/src/value/display.rs @@ -1,4 +1,9 @@ -use crate::{js_string, object::ObjectKind, property::PropertyDescriptor}; +use std::borrow::Cow; + +use crate::{ + builtins::promise::PromiseState, js_string, object::ObjectKind, property::PropertyDescriptor, + JsError, JsString, +}; use super::{fmt, Display, HashSet, JsValue, PropertyKey}; @@ -195,6 +200,54 @@ pub(crate) fn log_string_from(x: &JsValue, print_internals: bool, print_children format!("Set({size})") } } + ObjectKind::Error(_) => { + let name: Cow<'static, str> = v + .get_property(&"name".into()) + .as_ref() + .and_then(PropertyDescriptor::value) + .map_or_else( + || "".into(), + |v| { + v.as_string() + .map_or_else( + || v.display().to_string(), + JsString::to_std_string_escaped, + ) + .into() + }, + ); + let message = v + .get_property(&"message".into()) + .as_ref() + .and_then(PropertyDescriptor::value) + .map(|v| { + v.as_string().map_or_else( + || v.display().to_string(), + JsString::to_std_string_escaped, + ) + }) + .unwrap_or_default(); + if name.is_empty() { + message + } else if message.is_empty() { + name.to_string() + } else { + format!("{name}: {message}") + } + } + ObjectKind::Promise(ref promise) => { + format!( + "Promise {{ {} }}", + match promise.state() { + PromiseState::Pending => Cow::Borrowed(""), + PromiseState::Fulfilled(val) => Cow::Owned(val.display().to_string()), + PromiseState::Rejected(reason) => Cow::Owned(format!( + " {}", + JsError::from_opaque(reason.clone()) + )), + } + ) + } _ => display_obj(x, print_internals), } } @@ -255,26 +308,6 @@ pub(crate) fn display_obj(v: &JsValue, print_internals: bool) -> String { // in-memory address in this set let mut encounters = HashSet::new(); - if let JsValue::Object(object) = v { - if object.borrow().is_error() { - let name = v - .get_property("name") - .as_ref() - .and_then(PropertyDescriptor::value) - .unwrap_or(&JsValue::Undefined) - .display() - .to_string(); - let message = v - .get_property("message") - .as_ref() - .and_then(PropertyDescriptor::value) - .unwrap_or(&JsValue::Undefined) - .display() - .to_string(); - return format!("{name}: {message}"); - } - } - display_obj_internal(v, &mut encounters, 4, print_internals) }