feat(core): op2 #[serde] parameter support (denoland#20)

Supports #[serde] parameters in argument and return position.

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;

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;
 
@@ -336,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,
     ]
   );
 
@@ -769,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(())
+  }
 }

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)
@@ -435,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",
@@ -542,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;
+  })
+}

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 >")

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 {}
+}

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 {}