From 816e1bb6481a76494b195bdd831e79162873c9b2 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Sun, 21 Jul 2024 10:11:07 -0700 Subject: [PATCH] Add a way to add setters/getters in js_class! (#3911) * Add a way to add setters/getters in js_class! * Address cargo clippies * Add new interface and update doc * Setters can now return a value, which can be a JsResult * Remove clippy warning * Fix having both get and set with a return type fails to match --- core/interop/src/lib.rs | 28 +++ core/interop/src/macros.rs | 351 ++++++++++++++++++++++++++++++++++++- 2 files changed, 376 insertions(+), 3 deletions(-) diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 154e7d4a1c..9196a27602 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -616,6 +616,7 @@ fn can_throw_exception() { #[test] fn class() { use boa_engine::class::{Class, ClassBuilder}; + use boa_engine::property::Attribute; use boa_engine::{js_string, JsValue, Source}; use boa_macros::{Finalize, JsData, Trace}; use std::rc::Rc; @@ -645,6 +646,26 @@ fn class() { class.method(js_string!("getValue"), 0, get_value); let set_value = Self::set_value.into_js_function_copied(class.context()); class.method(js_string!("setValue"), 1, set_value); + + let get_value_getter = Self::get_value + .into_js_function_copied(class.context()) + .to_js_function(class.context().realm()); + let set_value_setter = Self::set_value + .into_js_function_copied(class.context()) + .to_js_function(class.context().realm()); + class.accessor( + js_string!("value_get"), + Some(get_value_getter), + None, + Attribute::CONFIGURABLE | Attribute::NON_ENUMERABLE, + ); + class.accessor( + js_string!("value_set"), + None, + Some(set_value_setter), + Attribute::CONFIGURABLE | Attribute::NON_ENUMERABLE, + ); + Ok(()) } @@ -675,6 +696,13 @@ fn class() { if (t.getValue() != 456) { throw 'invalid value 456'; } + if (t.value_get != 456) { + throw 'invalid value 456'; + } + t.value_set = 789; + if (t.getValue() != 789) { + throw 'invalid value 789'; + } ", ); let root_module = Module::parse(source, None, &mut context).unwrap(); diff --git a/core/interop/src/macros.rs b/core/interop/src/macros.rs index ed5c280024..38d871c9f8 100644 --- a/core/interop/src/macros.rs +++ b/core/interop/src/macros.rs @@ -2,7 +2,7 @@ /// Declare a JavaScript class, in a simpler way. /// -/// This can make declaration of JavaScript classes easier by using an hybrid +/// This can make declaration of JavaScript classes easier by using a hybrid /// declarative approach. The class itself follows a closer syntax to JavaScript /// while the method arguments/results and bodies are written in Rust. /// @@ -10,8 +10,57 @@ /// be declared separately as a Rust type, along with necessary derives and /// traits. /// -/// Here's an example using the animal class declared in [`boa_engine::class`]: +/// # Allowed declarations (in order): +/// +/// ## Any number of JS fields +/// ```ignore +/// public () -> { } +/// ``` +/// Declare public fields on the JavaScript prototype at construction. This is optional. +/// Those fields can be overwritten on the object itself. +/// +/// ## Any number of properties +/// ```ignore +/// property [as ""] { +/// get() -> { } +/// set() [-> JsResult<()>] { } +/// } +/// ``` +/// Declare a getter and/or a setter on a JavaScript class property. This is optional. +/// Both get and set are optional, but at least one must be present. The `set` method +/// must either return the unit type or a `JsResult<...>`. The value returned will be +/// ignored, only errors will be used. +/// +/// Using the `as` keyword, you can set the name of the property in JavaScript that +/// would otherwise not be possible in Rust. +/// +/// ## Required JavaScript Constructor +/// ```ignore +/// constructor() { } +/// ``` +/// Declares the JS constructor for the class. This is required, but could throw if creating +/// the object fails. +/// The body MUST return `JsResult`. +/// +/// ## An optional init function +/// ```ignore +/// fn init(class: &mut ClassBuilder) -> JsResult<()> { } +/// ``` +/// Declare a block of code to add at the end of the implementation's init function. +/// +/// ## Any number of methods +/// ```ignore +/// fn [as ]() -> { } +/// ``` +/// Declare methods on the class. This is optional. +/// +/// Using the `as` keyword, you can set the name of the property in JavaScript that +/// would otherwise not be possible in Rust. +/// +/// ---- /// # Example +/// +/// Here's an example using the animal class declared in [`boa_engine::class`]: /// ``` /// # use boa_engine::{JsString, JsData, js_string}; /// # use boa_gc::{Finalize, Trace}; @@ -90,10 +139,35 @@ macro_rules! js_class { $field_body: block )* + $( + $(#[$field_prop_attr: meta])* + property $field_prop_name: ident $(as $field_prop_js_name: literal)? { + $( + $(#[$field_prop_get_attr: meta])* + $(fn)? get( $( $field_prop_get_arg: ident: $field_prop_get_arg_type: ty ),* ) -> $field_prop_get_ty: ty + $field_prop_get_body: block + )? + + $( + $(#[$field_prop_set_attr: meta])* + $(fn)? set( $( $field_prop_set_arg: ident: $field_prop_set_arg_type: ty ),* ) + $( -> $field_prop_set_ty: ty )? + $field_prop_set_body: block + )? + } + )* + + $(#[$constructor_attr: meta])* constructor( $( $ctor_arg: ident: $ctor_arg_ty: ty ),* ) $constructor_body: block + $( + $(#[$init_attr: meta])* + init($init_class_builder_name: ident : &mut ClassBuilder) -> JsResult<()> + $init_body: block + )? + $( $(#[$method_attr: meta])* fn $method_name: ident $( as $method_js_name: literal )? @@ -109,7 +183,30 @@ macro_rules! js_class { const LENGTH: usize = $crate::__count!( $( $ctor_arg )* ); + #[allow(clippy::items_after_statements)] fn init(class: &mut $crate::boa_engine::class::ClassBuilder<'_>) -> $crate::boa_engine::JsResult<()> { + // Add properties. + $( + // Declare a function so that the compiler prevents duplicated names. + #[allow(dead_code)] + fn $field_prop_name() {} + + $crate::__get_set_decl!( + class, + $field_prop_name, + $($field_prop_js_name)?, + $( + @get( $( $field_prop_get_arg: $field_prop_get_arg_type ),* ) -> $field_prop_get_ty + $field_prop_get_body, + )? + $( + @set( $( $field_prop_set_arg: $field_prop_set_arg_type ),* ) + $( -> $field_prop_set_ty )? + $field_prop_set_body + )? + ); + )* + // Add all methods to the class. $( fn $method_name ( $($fn_arg: $fn_arg_type),* ) -> $( $result_type )? @@ -129,9 +226,18 @@ macro_rules! js_class { ); )* + // Add the init body, if any. + $({ + let $init_class_builder_name = class; + let result: $crate::boa_engine::JsResult<()> = $init_body; + + result?; + })? + Ok(()) } + #[allow(unused_variables)] fn data_constructor( new_target: &$crate::boa_engine::JsValue, args: &[$crate::boa_engine::JsValue], @@ -150,6 +256,7 @@ macro_rules! js_class { args: &[$crate::boa_engine::JsValue], context: &mut $crate::boa_engine::Context ) -> $crate::boa_engine::JsResult<()> { + // Public JS fields first. $( fn $field_name ( $($field_arg: $field_arg_type),* ) -> $field_ty $field_body @@ -164,7 +271,7 @@ macro_rules! js_class { function.call(&$crate::boa_engine::JsValue::undefined(), args, context)?, false, context - ); + )?; )* Ok(()) @@ -193,3 +300,241 @@ macro_rules! __count { 1 + $crate::__count!($($rest)*) }; } + +/// Internal macro to declare a getter/setter name. +#[macro_export] +macro_rules! __get_set_decl { + ( + $class: ident, + $field_name: ident, + $( $js_field_name: literal )?, + @get( $( $get_arg: ident: $get_arg_type: ty ),* ) -> $get_ty: ty + $get_body: block, + ) => { + let function = |$( $get_arg: $get_arg_type ),*| -> $get_ty { $get_body }; + let function_get = + $crate::IntoJsFunctionCopied::into_js_function_copied(function, $class.context()) + .to_js_function($class.context().realm()); + + let field_name = $crate::__js_class_name!($field_name, $($js_field_name)?); + $class.accessor( + $crate::boa_engine::JsString::from(field_name), + Some(function_get), + None, + $crate::boa_engine::property::Attribute::CONFIGURABLE + | $crate::boa_engine::property::Attribute::NON_ENUMERABLE, + ); + }; + ( + $class: ident, + $field_name: ident, + $( $js_field_name: literal )?, + @set( $( $set_arg: ident: $set_arg_type: ty ),* ) + $( -> $field_prop_set_ty: ty )? + $set_body: block + ) => { + let function = |$( $set_arg: $set_arg_type ),*| $(-> $field_prop_set_ty)? { $set_body }; + let function_set = + $crate::IntoJsFunctionCopied::into_js_function_copied(function, $class.context()) + .to_js_function($class.context().realm()); + + let field_name = $crate::__js_class_name!($field_name, $($js_field_name)?); + $class.accessor( + $crate::boa_engine::JsString::from(field_name), + None, + Some(function_set), + $crate::boa_engine::property::Attribute::CONFIGURABLE + | $crate::boa_engine::property::Attribute::NON_ENUMERABLE, + ); + }; + ( + $class: ident, + $field_name: ident, + $( $js_field_name: literal )?, + @get( $( $get_arg: ident: $get_arg_type: ty ),* ) -> $get_ty: ty + $get_body: block, + @set( $( $set_arg: ident: $set_arg_type: ty ),* ) + $( -> $field_prop_set_ty: ty )? + $set_body: block + ) => { + let function_get = + $crate::IntoJsFunctionCopied::into_js_function_copied( + |$( $get_arg: $get_arg_type ),*| -> $get_ty { $get_body }, + $class.context() + ).to_js_function($class.context().realm()); + let function_set = + $crate::IntoJsFunctionCopied::into_js_function_copied( + |$( $set_arg: $set_arg_type ),*| $(-> $field_prop_set_ty)? { $set_body }, + $class.context() + ).to_js_function($class.context().realm()); + + let field_name = $crate::__js_class_name!($field_name, $($js_field_name)?); + $class.accessor( + $crate::boa_engine::JsString::from(field_name), + Some(function_get), + Some(function_set), + $crate::boa_engine::property::Attribute::CONFIGURABLE + | $crate::boa_engine::property::Attribute::NON_ENUMERABLE, + ); + + }; + ( + $class: ident, + $field_name: ident, + $( $js_field_name: literal )?, + ) => { + compile_error!("Property must have at least a getter or a setter"); + }; +} + +// We allow too many lines. This test is straightforward but has a lot of boilerplate +// still. +#[test] +#[allow(clippy::too_many_lines)] +fn js_class_test() { + use crate::IntoJsFunctionCopied; + use crate::{js_class, loaders, JsClass}; + use boa_engine::property::Attribute; + use boa_engine::{js_string, Context, JsData, JsError, JsResult, Module, Source}; + use boa_gc::{Finalize, Trace}; + use std::rc::Rc; + + #[derive(Debug, Clone, Default, Trace, Finalize, JsData)] + struct Test { + f1: u32, + f2: u32, + f3: u32, + } + + js_class! { + class Test { + public fp() -> u32 { + 10 + } + + property f1 { + get(this: JsClass) -> u32 { + this.borrow().f1 + } + } + + property f2 { + set(this: JsClass, new_value: u32) { + this.borrow_mut().f1 = new_value; + } + } + + property f3 { + get(this: JsClass) -> u32 { + this.borrow().f3 + } + + set(this: JsClass, new_value: u32) { + this.borrow_mut().f3 = new_value; + } + } + + property f4 { + set() -> JsResult<()> { + Err(JsError::from_opaque(boa_engine::JsString::from("Cannot set f4.").into())) + } + } + + // Just to test the branch with both get, set and return value. + property f5 { + get() -> u8 { 1 } + set() -> () { } + } + + constructor() { + Ok(Test::default()) + } + + init(class: &mut ClassBuilder) -> JsResult<()> { + let get_value_getter = (|this: JsClass| { + this.borrow().f2 + }) + .into_js_function_copied(class.context()) + .to_js_function(class.context().realm()); + + let set_value_setter = (|this: JsClass, new_value: u32| { + this.borrow_mut().f2 = new_value; + }) + .into_js_function_copied(class.context()) + .to_js_function(class.context().realm()); + + class.accessor( + js_string!("value2"), + Some(get_value_getter), + Some(set_value_setter), + Attribute::CONFIGURABLE | Attribute::NON_ENUMERABLE, + ); + Ok(()) + } + } + } + + let loader = Rc::new(loaders::HashMapModuleLoader::new()); + let mut context = Context::builder() + .module_loader(loader.clone()) + .build() + .unwrap(); + + context.register_global_class::().unwrap(); + + let source = Source::from_bytes( + r" + function assert_eq(name, actual, expected) { + if (expected !== actual) { + throw `Assertion failed: ${name} - expected ${expected}, got ${actual}`; + } + } + + let t = new Test(); + assert_eq('fp', t.fp, 10); + + assert_eq('value2', t.value2, 0); + t.value2 = 123; + assert_eq('value2', t.value2, 123); + + // This should do nothing in Rust as this is a JS property and not a getter/setter. + t.fp = 123; + assert_eq('fp (set)', t.fp, 123); + + // Test separate getter/setter. + assert_eq('f1', t.f1, 0); + t.f2 = 1; + assert_eq('f2', t.f1, 1); + + // Test same getter/setter. + assert_eq('f3', t.f3, 0); + t.f3 = 456; + assert_eq('f3 (set)', t.f3, 456); + + // Test exception on setter. + try { + t.f4 = 123; + throw 'Expected an exception'; + } catch (e) { + if (e !== 'Cannot set f4.') { + throw e; + } + } + ", + ); + let root_module = Module::parse(source, None, &mut context).unwrap(); + + let promise_result = root_module.load_link_evaluate(&mut context); + context.run_jobs(); + + // Checking if the final promise didn't return an error. + assert!( + promise_result.state().as_fulfilled().is_some(), + "module didn't execute successfully! Promise: {:?} ({:?})", + promise_result.state(), + promise_result + .state() + .as_rejected() + .map(|r| r.to_json(&mut context)) + ); +}