Repository: denoland/deno_core
Pull request #20 (2023-07-08 02:08:18)

feat(core): op2 #[serde] parameter support
Supports `#[serde]` parameters in argument and return position.


Patch:
diff --git a/core/lib.rs b/core/lib.rs
index 1ddae93..eef9b8f 100644
--- a/core/lib.rs
+++ b/core/lib.rs
@@ -144,6 +144,8 @@ pub mod _ops {
   pub use super::runtime::ops::map_async_op4;
   pub use super::runtime::ops::queue_async_op;
   pub use super::runtime::ops::queue_fast_async_op;
+  pub use super::runtime::ops::serde_rust_to_v8;
+  pub use super::runtime::ops::serde_v8_to_rust;
   pub use super::runtime::ops::to_i32;
   pub use super::runtime::ops::to_str;
   pub use super::runtime::ops::to_str_ptr;
diff --git a/core/runtime/ops.rs b/core/runtime/ops.rs
index a02750d..a910112 100644
--- a/core/runtime/ops.rs
+++ b/core/runtime/ops.rs
@@ -7,6 +7,10 @@ use futures::future::Either;
 use futures::future::Future;
 use futures::future::FutureExt;
 use futures::task::noop_waker_ref;
+use serde::Deserialize;
+use serde::Serialize;
+use serde_v8::from_v8;
+use serde_v8::to_v8;
 use std::borrow::Cow;
 use std::cell::RefCell;
 use std::future::ready;
@@ -297,6 +301,20 @@ pub fn to_str<'a, const N: usize>(
   string.to_rust_cow_lossy(scope, buffer)
 }
 
+pub fn serde_rust_to_v8<'a, T: Serialize>(
+  scope: &mut v8::HandleScope<'a>,
+  input: T,
+) -> serde_v8::Result<v8::Local<'a, v8::Value>> {
+  to_v8(scope, input)
+}
+
+pub fn serde_v8_to_rust<'a, T: Deserialize<'a>>(
+  scope: &mut v8::HandleScope,
+  input: v8::Local<v8::Value>,
+) -> serde_v8::Result<T> {
+  from_v8(scope, input)
+}
+
 #[cfg(test)]
 mod tests {
   use crate::error::generic_error;
@@ -306,6 +324,8 @@ mod tests {
   use crate::JsRuntime;
   use crate::RuntimeOptions;
   use deno_ops::op2;
+  use serde::Deserialize;
+  use serde::Serialize;
   use std::borrow::Cow;
   use std::cell::Cell;
 
@@ -320,6 +340,8 @@ mod tests {
       op_test_result_void_err,
       op_test_result_primitive_ok,
       op_test_result_primitive_err,
+      op_test_bool,
+      op_test_bool_result,
       op_test_string_owned,
       op_test_string_ref,
       op_test_string_cow,
@@ -334,6 +356,7 @@ mod tests {
       op_test_v8_type_return_option,
       op_test_v8_type_handle_scope,
       op_test_v8_type_handle_scope_obj,
+      op_test_serde_v8,
     ]
   );
 
@@ -513,6 +536,40 @@ mod tests {
     Ok(())
   }
 
+  #[op2(core, fast)]
+  pub fn op_test_bool(b: bool) -> bool {
+    b
+  }
+
+  #[op2(core, fast)]
+  pub fn op_test_bool_result(b: bool) -> Result<bool, AnyError> {
+    if b {
+      Ok(true)
+    } else {
+      Err(generic_error("false!!!"))
+    }
+  }
+
+  #[tokio::test]
+  pub async fn test_op_bool() -> Result<(), Box<dyn std::error::Error>> {
+    run_test2(
+      10000,
+      "op_test_bool",
+      "assert(op_test_bool(true) === true && op_test_bool(false) === false)",
+    )?;
+    run_test2(
+      10000,
+      "op_test_bool_result",
+      "assert(op_test_bool_result(true) === true)",
+    )?;
+    run_test2(
+      1,
+      "op_test_bool_result",
+      "try { op_test_bool_result(false); assert(false) } catch (e) {}",
+    )?;
+    Ok(())
+  }
+
   #[op2(core, fast)]
   pub fn op_test_string_owned(#[string] s: String) -> u32 {
     s.len() as _
@@ -733,4 +790,31 @@ mod tests {
     }
     Ok(())
   }
+
+  #[derive(Serialize, Deserialize)]
+  pub struct Serde {
+    pub s: String,
+  }
+
+  #[op2(core)]
+  #[serde]
+  pub fn op_test_serde_v8(#[serde] mut serde: Serde) -> Serde {
+    serde.s += "!";
+    serde
+  }
+
+  #[tokio::test]
+  pub async fn test_op_serde_v8() -> Result<(), Box<dyn std::error::Error>> {
+    run_test2(
+      1,
+      "op_test_serde_v8",
+      "assert(op_test_serde_v8({s: 'abc'}).s == 'abc!')",
+    )?;
+    run_test2(
+      1,
+      "op_test_serde_v8",
+      "try { op_test_serde_v8({}); assert(false) } catch (e) { assert(String(e).indexOf('missing field') != -1) }",
+    )?;
+    Ok(())
+  }
 }
diff --git a/ops/op2/dispatch_slow.rs b/ops/op2/dispatch_slow.rs
index 4708037..d79cd19 100644
--- a/ops/op2/dispatch_slow.rs
+++ b/ops/op2/dispatch_slow.rs
@@ -294,6 +294,22 @@ pub fn from_arg(
         };
       }
     }
+    Arg::SerdeV8(_class) => {
+      *needs_scope = true;
+      let arg_ident = arg_ident.clone();
+      let deno_core = deno_core.clone();
+      let scope = scope.clone();
+      let err = format_ident!("{}_err", arg_ident);
+      let throw_exception = throw_type_error_string(generator_state, &err)?;
+      quote! {
+        let #arg_ident = match #deno_core::_ops::serde_v8_to_rust(#scope, #arg_ident) {
+          Ok(t) => t,
+          Err(#err) => {
+            #throw_exception;
+          }
+        };
+      }
+    }
     _ => return Err(V8MappingError::NoMapping("a slow argument", arg.clone())),
   };
   Ok(res)
@@ -369,6 +385,10 @@ pub fn return_value_infallible(
     Arg::Void => {
       quote! {/* void */}
     }
+    Arg::Numeric(NumericArg::bool) => {
+      *needs_retval = true;
+      quote!(#retval.set_bool(#result as bool);)
+    }
     Arg::Numeric(NumericArg::u8)
     | Arg::Numeric(NumericArg::u16)
     | Arg::Numeric(NumericArg::u32) => {
@@ -431,6 +451,27 @@ pub fn return_value_infallible(
         #retval.set(#result.into())
       }
     }
+    Arg::SerdeV8(_class) => {
+      *needs_retval = true;
+      *needs_scope = true;
+
+      let deno_core = deno_core.clone();
+      let scope = scope.clone();
+      let result = result.clone();
+      let retval = retval.clone();
+      let err = format_ident!("{}_err", retval);
+      let throw_exception = throw_type_error_string(generator_state, &err)?;
+
+      quote! {
+        let #result = match #deno_core::_ops::serde_rust_to_v8(#scope, #result) {
+          Ok(t) => t,
+          Err(#err) => {
+            #throw_exception
+          }
+        };
+        #retval.set(#result.into())
+      }
+    }
     _ => {
       return Err(V8MappingError::NoMapping(
         "a slow return value",
@@ -538,3 +579,28 @@ fn throw_type_error(
     return;
   })
 }
+
+/// Generates code to throw an exception from a string variable, adding required additional dependencies as needed.
+fn throw_type_error_string(
+  generator_state: &mut GeneratorState,
+  message: &Ident,
+) -> Result<TokenStream, V8MappingError> {
+  let maybe_scope = if generator_state.needs_scope {
+    quote!()
+  } else {
+    with_scope(generator_state)
+  };
+
+  let GeneratorState {
+    deno_core, scope, ..
+  } = &generator_state;
+
+  Ok(quote! {
+    #maybe_scope
+    // TODO(mmastrac): This might be allocating too much, even if it's on the error path
+    let msg = #deno_core::v8::String::new(#scope, &format!("{}", #deno_core::anyhow::Error::from(#message))).unwrap();
+    let exc = #deno_core::v8::Exception::error(#scope, msg);
+    #scope.throw_exception(exc);
+    return;
+  })
+}
diff --git a/ops/op2/signature.rs b/ops/op2/signature.rs
index dddcc13..63ec1f1 100644
--- a/ops/op2/signature.rs
+++ b/ops/op2/signature.rs
@@ -247,8 +247,8 @@ pub enum ArgError {
   InvalidReference(String),
   #[error("The type {0} must be a reference")]
   MissingReference(String),
-  #[error("Invalid #[serde] type: {0}")]
-  InvalidSerdeType(String),
+  #[error("Invalid or deprecated #[serde] type '{0}': {1}")]
+  InvalidSerdeType(String, &'static str),
   #[error("Invalid #[string] type: {0}")]
   InvalidStringType(String),
   #[error("Cannot use #[serde] for type: {0}")]
@@ -266,6 +266,14 @@ struct Attributes {
   primary: Option<AttributeModifier>,
 }
 
+impl Attributes {
+  pub fn string() -> Self {
+    Self {
+      primary: Some(AttributeModifier::String),
+    }
+  }
+}
+
 fn stringify_token(tokens: impl ToTokens) -> String {
   tokens
     .into_token_stream()
@@ -583,15 +591,36 @@ fn parse_type(attrs: Attributes, ty: &Type) -> Result<Arg, ArgError> {
     match primary {
       AttributeModifier::Serde => match ty {
         Type::Path(of) => {
-          // If this type will parse without #[serde], it is illegal to use this type with #[serde]
+          // If this type will parse without #[serde] (or with #[string]), it is illegal to use this type with #[serde]
           if parse_type_path(Attributes::default(), false, of).is_ok() {
             return Err(ArgError::InvalidSerdeAttributeType(stringify_token(
               ty,
             )));
           }
-          return Ok(Arg::SerdeV8(stringify_token(of.path.clone())));
+          // If this type will parse without #[serde] (or with #[string]), it is illegal to use this type with #[serde]
+          if parse_type_path(Attributes::string(), false, of).is_ok() {
+            return Err(ArgError::InvalidSerdeAttributeType(stringify_token(
+              ty,
+            )));
+          }
+
+          // Denylist of serde_v8 types with better alternatives
+          let ty = of.into_token_stream();
+          let token = stringify_token(of.path.clone());
+          if let Ok(err) = std::panic::catch_unwind(|| {
+            use syn2 as syn;
+            rules!(ty => {
+              ( $( serde_v8:: )? Value $( < $_lifetime:lifetime >)? ) => "use v8::Value",
+            })
+          }) {
+            return Err(ArgError::InvalidSerdeType(stringify_token(ty), err));
+          }
+
+          return Ok(Arg::SerdeV8(token));
+        }
+        _ => {
+          return Err(ArgError::InvalidSerdeAttributeType(stringify_token(ty)))
         }
-        _ => return Err(ArgError::InvalidSerdeType(stringify_token(ty))),
       },
       AttributeModifier::String => {
         // We handle this as part of the normal parsing process
@@ -873,6 +902,22 @@ mod tests {
     }
   );
 
+  expect_fail!(
+    op_with_bad_serde_string,
+    ArgError("s", InvalidSerdeAttributeType("String")),
+    fn f(#[serde] s: String) {}
+  );
+  expect_fail!(
+    op_with_bad_serde_str,
+    ArgError("s", InvalidSerdeAttributeType("&str")),
+    fn f(#[serde] s: &str) {}
+  );
+  expect_fail!(
+    op_with_bad_serde_value,
+    ArgError("v", InvalidSerdeType("serde_v8::Value", "use v8::Value")),
+    fn f(#[serde] v: serde_v8::Value) {}
+  );
+
   #[test]
   fn test_parse_result() {
     let rt = parse_str::<ReturnType>("-> Result < (), Error >")


Test patch:
diff --git a/ops/op2/test_cases/sync/bool.out b/ops/op2/test_cases/sync/bool.out
new file mode 100644
index 0000000..f9ca48f
--- /dev/null
+++ b/ops/op2/test_cases/sync/bool.out
@@ -0,0 +1,72 @@
+#[allow(non_camel_case_types)]
+pub struct op_bool {
+    _unconstructable: ::std::marker::PhantomData<()>,
+}
+impl deno_core::_ops::Op for op_bool {
+    const NAME: &'static str = stringify!(op_bool);
+    const DECL: deno_core::_ops::OpDecl = deno_core::_ops::OpDecl {
+        name: stringify!(op_bool),
+        v8_fn_ptr: Self::v8_fn_ptr as _,
+        enabled: true,
+        fast_fn: Some({
+            use deno_core::v8::fast_api::Type;
+            use deno_core::v8::fast_api::CType;
+            deno_core::v8::fast_api::FastFunction::new(
+                &[Type::V8Value, Type::Bool],
+                CType::Bool,
+                Self::v8_fn_ptr_fast as *const ::std::ffi::c_void,
+            )
+        }),
+        is_async: false,
+        is_unstable: false,
+        is_v8: false,
+        arg_count: 1usize as u8,
+    };
+}
+impl op_bool {
+    pub const fn name() -> &'static str {
+        stringify!(op_bool)
+    }
+    pub const fn decl() -> deno_core::_ops::OpDecl {
+        deno_core::_ops::OpDecl {
+            name: stringify!(op_bool),
+            v8_fn_ptr: Self::v8_fn_ptr as _,
+            enabled: true,
+            fast_fn: Some({
+                use deno_core::v8::fast_api::Type;
+                use deno_core::v8::fast_api::CType;
+                deno_core::v8::fast_api::FastFunction::new(
+                    &[Type::V8Value, Type::Bool],
+                    CType::Bool,
+                    Self::v8_fn_ptr_fast as *const ::std::ffi::c_void,
+                )
+            }),
+            is_async: false,
+            is_unstable: false,
+            is_v8: false,
+            arg_count: 1usize as u8,
+        }
+    }
+    fn v8_fn_ptr_fast(
+        _: deno_core::v8::Local<deno_core::v8::Object>,
+        arg0: bool,
+    ) -> bool {
+        let arg0 = arg0 as _;
+        let result = Self::call(arg0);
+        result
+    }
+    extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
+        let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
+            &*info
+        });
+        let arg0 = args.get(0usize as i32);
+        let arg0 = arg0.is_true();
+        let result = Self::call(arg0);
+        rv.set_bool(result as bool);
+    }
+    #[inline(always)]
+    pub fn call(arg: bool) -> bool {}
+}
diff --git a/ops/op2/test_cases/sync/bool.rs b/ops/op2/test_cases/sync/bool.rs
new file mode 100644
index 0000000..8e036cc
--- /dev/null
+++ b/ops/op2/test_cases/sync/bool.rs
@@ -0,0 +1,5 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+
+#[op2(fast)]
+pub fn op_bool(arg: bool) -> bool {
+}
diff --git a/ops/op2/test_cases/sync/bool_result.out b/ops/op2/test_cases/sync/bool_result.out
new file mode 100644
index 0000000..e996848
--- /dev/null
+++ b/ops/op2/test_cases/sync/bool_result.out
@@ -0,0 +1,120 @@
+#[allow(non_camel_case_types)]
+pub struct op_bool {
+    _unconstructable: ::std::marker::PhantomData<()>,
+}
+impl deno_core::_ops::Op for op_bool {
+    const NAME: &'static str = stringify!(op_bool);
+    const DECL: deno_core::_ops::OpDecl = deno_core::_ops::OpDecl {
+        name: stringify!(op_bool),
+        v8_fn_ptr: Self::v8_fn_ptr as _,
+        enabled: true,
+        fast_fn: Some({
+            use deno_core::v8::fast_api::Type;
+            use deno_core::v8::fast_api::CType;
+            deno_core::v8::fast_api::FastFunction::new(
+                &[Type::V8Value, Type::Bool, Type::CallbackOptions],
+                CType::Bool,
+                Self::v8_fn_ptr_fast as *const ::std::ffi::c_void,
+            )
+        }),
+        is_async: false,
+        is_unstable: false,
+        is_v8: false,
+        arg_count: 1usize as u8,
+    };
+}
+impl op_bool {
+    pub const fn name() -> &'static str {
+        stringify!(op_bool)
+    }
+    pub const fn decl() -> deno_core::_ops::OpDecl {
+        deno_core::_ops::OpDecl {
+            name: stringify!(op_bool),
+            v8_fn_ptr: Self::v8_fn_ptr as _,
+            enabled: true,
+            fast_fn: Some({
+                use deno_core::v8::fast_api::Type;
+                use deno_core::v8::fast_api::CType;
+                deno_core::v8::fast_api::FastFunction::new(
+                    &[Type::V8Value, Type::Bool, Type::CallbackOptions],
+                    CType::Bool,
+                    Self::v8_fn_ptr_fast as *const ::std::ffi::c_void,
+                )
+            }),
+            is_async: false,
+            is_unstable: false,
+            is_v8: false,
+            arg_count: 1usize as u8,
+        }
+    }
+    fn v8_fn_ptr_fast(
+        _: deno_core::v8::Local<deno_core::v8::Object>,
+        arg0: bool,
+        fast_api_callback_options: *mut deno_core::v8::fast_api::FastApiCallbackOptions,
+    ) -> bool {
+        let fast_api_callback_options = unsafe { &mut *fast_api_callback_options };
+        let opctx = unsafe {
+            &*(deno_core::v8::Local::<
+                v8::External,
+            >::cast(unsafe { fast_api_callback_options.data.data })
+                .value() as *const deno_core::_ops::OpCtx)
+        };
+        let arg0 = arg0 as _;
+        let result = Self::call(arg0);
+        let result = match result {
+            Ok(result) => result,
+            Err(err) => {
+                unsafe {
+                    opctx.unsafely_set_last_error_for_ops_only(err);
+                }
+                fast_api_callback_options.fallback = true;
+                return ::std::default::Default::default();
+            }
+        };
+        result
+    }
+    extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
+        let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
+            &*info
+        });
+        let opctx = unsafe {
+            &*(deno_core::v8::Local::<deno_core::v8::External>::cast(args.data()).value()
+                as *const deno_core::_ops::OpCtx)
+        };
+        if let Some(err) = unsafe { opctx.unsafely_take_last_error_for_ops_only() } {
+            let scope = &mut unsafe { deno_core::v8::CallbackScope::new(&*info) };
+            let opstate = ::std::cell::RefCell::borrow(&*opctx.state);
+            let exception = deno_core::error::to_v8_error(
+                scope,
+                opstate.get_error_class_fn,
+                &err,
+            );
+            scope.throw_exception(exception);
+            return;
+        }
+        let arg0 = args.get(0usize as i32);
+        let arg0 = arg0.is_true();
+        let result = Self::call(arg0);
+        match result {
+            Ok(result) => {
+                rv.set_bool(result as bool);
+            }
+            Err(err) => {
+                let scope = &mut unsafe { deno_core::v8::CallbackScope::new(&*info) };
+                let opstate = ::std::cell::RefCell::borrow(&*opctx.state);
+                let exception = deno_core::error::to_v8_error(
+                    scope,
+                    opstate.get_error_class_fn,
+                    &err,
+                );
+                scope.throw_exception(exception);
+                return;
+            }
+        };
+    }
+    #[inline(always)]
+    pub fn call(arg: bool) -> Result<bool, AnyError> {}
+}
diff --git a/ops/op2/test_cases/sync/bool_result.rs b/ops/op2/test_cases/sync/bool_result.rs
new file mode 100644
index 0000000..6268b2e
--- /dev/null
+++ b/ops/op2/test_cases/sync/bool_result.rs
@@ -0,0 +1,5 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+
+#[op2(fast)]
+pub fn op_bool(arg: bool) -> Result<bool, AnyError> {
+}
diff --git a/ops/op2/test_cases/sync/serde_v8.out b/ops/op2/test_cases/sync/serde_v8.out
new file mode 100644
index 0000000..b2240ed
--- /dev/null
+++ b/ops/op2/test_cases/sync/serde_v8.out
@@ -0,0 +1,74 @@
+#[allow(non_camel_case_types)]
+pub struct op_serde_v8 {
+    _unconstructable: ::std::marker::PhantomData<()>,
+}
+impl deno_core::_ops::Op for op_serde_v8 {
+    const NAME: &'static str = stringify!(op_serde_v8);
+    const DECL: deno_core::_ops::OpDecl = deno_core::_ops::OpDecl {
+        name: stringify!(op_serde_v8),
+        v8_fn_ptr: Self::v8_fn_ptr as _,
+        enabled: true,
+        fast_fn: None,
+        is_async: false,
+        is_unstable: false,
+        is_v8: false,
+        arg_count: 1usize as u8,
+    };
+}
+impl op_serde_v8 {
+    pub const fn name() -> &'static str {
+        stringify!(op_serde_v8)
+    }
+    pub const fn decl() -> deno_core::_ops::OpDecl {
+        deno_core::_ops::OpDecl {
+            name: stringify!(op_serde_v8),
+            v8_fn_ptr: Self::v8_fn_ptr as _,
+            enabled: true,
+            fast_fn: None,
+            is_async: false,
+            is_unstable: false,
+            is_v8: false,
+            arg_count: 1usize as u8,
+        }
+    }
+    extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let scope = &mut unsafe { deno_core::v8::CallbackScope::new(&*info) };
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
+        let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
+            &*info
+        });
+        let arg0 = args.get(0usize as i32);
+        let arg0 = match deno_core::_ops::serde_v8_to_rust(scope, arg0) {
+            Ok(t) => t,
+            Err(arg0_err) => {
+                let msg = deno_core::v8::String::new(
+                        scope,
+                        &format!("{}", deno_core::anyhow::Error::from(arg0_err)),
+                    )
+                    .unwrap();
+                let exc = deno_core::v8::Exception::error(scope, msg);
+                scope.throw_exception(exc);
+                return;
+            }
+        };
+        let result = Self::call(arg0);
+        let result = match deno_core::_ops::serde_rust_to_v8(scope, result) {
+            Ok(t) => t,
+            Err(rv_err) => {
+                let msg = deno_core::v8::String::new(
+                        scope,
+                        &format!("{}", deno_core::anyhow::Error::from(rv_err)),
+                    )
+                    .unwrap();
+                let exc = deno_core::v8::Exception::error(scope, msg);
+                scope.throw_exception(exc);
+                return;
+            }
+        };
+        rv.set(result.into())
+    }
+    #[inline(always)]
+    pub fn call(input: Input) -> Output {}
+}
diff --git a/ops/op2/test_cases/sync/serde_v8.rs b/ops/op2/test_cases/sync/serde_v8.rs
new file mode 100644
index 0000000..90d166e
--- /dev/null
+++ b/ops/op2/test_cases/sync/serde_v8.rs
@@ -0,0 +1,5 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+
+#[op2]
+#[serde]
+pub fn op_serde_v8(#[serde] input: Input) -> Output {}