Repository: denoland/deno_core (rust)

Issue:
`op2` doesn't support `Option<JsBuffer>`
```rust
#[op2]
#[serde]
pub fn op_url_parse_search_params(
  #[string] args: Option<String>,
  #[buffer] zero_copy: Option<JsBuffer>,
) -> Result<Vec<(String, String)>, AnyError> {
  let params = match (args, zero_copy) {
    (None, Some(zero_copy)) => form_urlencoded::parse(&zero_copy)
      .into_iter()
      .map(|(k, v)| (k.as_ref().to_owned(), v.as_ref().to_owned()))
      .collect(),
    (Some(args), None) => form_urlencoded::parse(args.as_bytes())
      .into_iter()
      .map(|(k, v)| (k.as_ref().to_owned(), v.as_ref().to_owned()))
      .collect(),
    _ => return Err(type_error("invalid parameters")),
  };
  Ok(params)
}
```

Pull request:
feat(ops): support Option<*buffer type*> and Option<*smi*>
Fixes #188 and #176

Adds a small error fix for #189 

Patch:
diff --git a/ops/op2/dispatch_fast.rs b/ops/op2/dispatch_fast.rs
index c48406b..9697bb5 100644
--- a/ops/op2/dispatch_fast.rs
+++ b/ops/op2/dispatch_fast.rs
@@ -433,6 +433,7 @@ fn map_arg_to_v8_fastcall_type(
     Arg::OptionNumeric(_)
     | Arg::Option(_)
     | Arg::OptionString(_)
+    | Arg::OptionBuffer(_)
     | Arg::SerdeV8(_)
     | Arg::Ref(..) => return Ok(None),
     // We don't support v8 global arguments
diff --git a/ops/op2/dispatch_slow.rs b/ops/op2/dispatch_slow.rs
index 6b34c4b..ef8add3 100644
--- a/ops/op2/dispatch_slow.rs
+++ b/ops/op2/dispatch_slow.rs
@@ -298,7 +298,27 @@ pub fn from_arg(
       })
     }
     Arg::Buffer(buffer) => {
-      from_arg_buffer(generator_state, &arg_ident, buffer)?
+      // Explicit temporary lifetime extension so we can take a reference
+      let temp = format_ident!("{}_temp", arg_ident);
+      let buffer = from_arg_buffer(generator_state, &arg_ident, buffer, &temp)?;
+      quote! {
+        let mut #temp;
+        #buffer
+      }
+    }
+    Arg::OptionBuffer(buffer) => {
+      // Explicit temporary lifetime extension so we can take a reference
+      let temp = format_ident!("{}_temp", arg_ident);
+      let some = from_arg_buffer(generator_state, &arg_ident, buffer, &temp)?;
+      quote! {
+        let mut #temp;
+        let #arg_ident = if #arg_ident.is_null_or_undefined() {
+          None
+        } else {
+          #some
+          Some(#arg_ident)
+        };
+      }
     }
     Arg::External(External::Ptr(_)) => {
       from_arg_option(generator_state, &arg_ident, "external")?
@@ -431,6 +451,7 @@ pub fn from_arg_buffer(
   generator_state: &mut GeneratorState,
   arg_ident: &Ident,
   buffer: &Buffer,
+  temp: &Ident,
 ) -> Result<TokenStream, V8MappingError> {
   let err = format_ident!("{}_err", arg_ident);
   let throw_exception = throw_type_error_static_string(generator_state, &err)?;
@@ -449,7 +470,7 @@ pub fn from_arg_buffer(
   };
 
   let make_v8slice = gs_quote!(generator_state(deno_core, scope) => {
-    let mut #arg_ident = match unsafe { #deno_core::_ops::#to_v8_slice::<#deno_core::v8::#array>(&mut #scope, #arg_ident) } {
+    #temp = match unsafe { #deno_core::_ops::#to_v8_slice::<#deno_core::v8::#array>(&mut #scope, #arg_ident) } {
       Ok(#arg_ident) => #arg_ident,
       Err(#err) => {
         #throw_exception
@@ -458,20 +479,23 @@ pub fn from_arg_buffer(
   });
 
   let make_arg = match buffer {
-    Buffer::Slice(_, NumericArg::u8) => {
-      quote!(let #arg_ident = &mut #arg_ident;)
+    Buffer::Slice(RefType::Ref, NumericArg::u8) => {
+      quote!(let #arg_ident = #temp.as_ref();)
+    }
+    Buffer::Slice(RefType::Mut, NumericArg::u8) => {
+      quote!(let #arg_ident = #temp.as_mut();)
     }
     Buffer::Vec(NumericArg::u8) => {
-      quote!(let #arg_ident = #arg_ident.to_vec();)
+      quote!(let #arg_ident = #temp.to_vec();)
     }
     Buffer::BoxSlice(NumericArg::u8) => {
-      quote!(let #arg_ident = #arg_ident.to_boxed_slice();)
+      quote!(let #arg_ident = #temp.to_boxed_slice();)
     }
     Buffer::Bytes(BufferMode::Copy) => {
-      quote!(let #arg_ident = #arg_ident.to_vec().into();)
+      quote!(let #arg_ident = #temp.to_vec().into();)
     }
     Buffer::JsBuffer(BufferMode::Default | BufferMode::Detach) => {
-      gs_quote!(generator_state(deno_core) => (let #arg_ident = #deno_core::serde_v8::JsBuffer::from_parts(#arg_ident);))
+      gs_quote!(generator_state(deno_core) => (let #arg_ident = #deno_core::serde_v8::JsBuffer::from_parts(#temp);))
     }
     _ => {
       return Err(V8MappingError::NoMapping(
diff --git a/ops/op2/signature.rs b/ops/op2/signature.rs
index 443c70c..dfc2458 100644
--- a/ops/op2/signature.rs
+++ b/ops/op2/signature.rs
@@ -223,6 +223,7 @@ pub enum Arg {
   Option(Special),
   OptionString(Strings),
   OptionNumeric(NumericArg),
+  OptionBuffer(Buffer),
   OptionV8Local(V8Arg),
   OptionV8Global(V8Arg),
   V8Local(V8Arg),
@@ -296,6 +297,7 @@ impl Arg {
         | Arg::OptionNumeric(..)
         | Arg::Option(..)
         | Arg::OptionString(..)
+        | Arg::OptionBuffer(..)
         | Arg::OptionState(..)
     )
   }
@@ -309,6 +311,7 @@ impl Arg {
       Arg::OptionNumeric(t) => Arg::Numeric(*t),
       Arg::Option(t) => Arg::Special(*t),
       Arg::OptionString(t) => Arg::String(*t),
+      Arg::OptionBuffer(t) => Arg::Buffer(*t),
       Arg::OptionState(r, t) => Arg::State(*r, t.clone()),
       _ => return None,
     })
@@ -346,6 +349,12 @@ impl Arg {
           | Buffer::BoxSlice(NumericArg::u8)
           | Buffer::BytesMut(BufferMode::Default),
         ) => ArgSlowRetval::V8LocalFalliable,
+        Arg::OptionBuffer(
+          Buffer::JsBuffer(BufferMode::Default)
+          | Buffer::Vec(NumericArg::u8)
+          | Buffer::BoxSlice(NumericArg::u8)
+          | Buffer::BytesMut(BufferMode::Default),
+        ) => ArgSlowRetval::V8LocalFalliable,
         _ => ArgSlowRetval::None,
       }
     }
@@ -634,6 +643,8 @@ pub enum ArgError {
   InvalidType(String, &'static str),
   #[error("Invalid numeric argument type: {0}")]
   InvalidNumericType(String),
+  #[error("Invalid numeric #[smi] argument type: {0}")]
+  InvalidSmiType(String),
   #[error(
     "Invalid argument type path (should this be #[smi] or #[serde]?): {0}"
   )]
@@ -1159,7 +1170,8 @@ pub(crate) fn parse_type(
           let token = stringify_token(of.path.clone());
           if let Ok(Some(err)) = std::panic::catch_unwind(|| {
             rules!(ty => {
-              ( $( serde_v8:: )? Value $( < $_lifetime:lifetime >)? ) => Some("use v8::Value"),
+              ( serde_v8::Value $( < $_lifetime:lifetime >)? ) => Some("use v8::Value"),
+              ( Value $( < $_lifetime:lifetime >)? ) => Some("use a fully-qualified type: v8::Value or serde_json::Value"),
               ( $_ty:ty ) => None,
             })
           }) {
@@ -1181,9 +1193,20 @@ pub(crate) fn parse_type(
       | AttributeModifier::Global => {
         // We handle this as part of the normal parsing process
       }
-      AttributeModifier::Smi => {
-        return Ok(Arg::Numeric(NumericArg::__SMI__));
-      }
+      AttributeModifier::Smi => match ty {
+        Type::Path(of) => {
+          let is_option = rules!(of.into_token_stream() => {
+            ( Option < $_ty:ty > ) => true,
+            ( $_ty:ty ) => false,
+          });
+          if is_option {
+            return Ok(Arg::OptionNumeric(NumericArg::__SMI__));
+          } else {
+            return Ok(Arg::Numeric(NumericArg::__SMI__));
+          }
+        }
+        _ => return Err(ArgError::InvalidSmiType(stringify_token(ty))),
+      },
     }
   };
   match ty {
@@ -1262,6 +1285,7 @@ pub(crate) fn parse_type(
       COption(TNumeric(special)) => Ok(Arg::OptionNumeric(special)),
       COption(TSpecial(special)) => Ok(Arg::Option(special)),
       COption(TString(string)) => Ok(Arg::OptionString(string)),
+      COption(TBuffer(buffer)) => Ok(Arg::OptionBuffer(buffer)),
       CRcRefCell(TSpecial(special)) => Ok(Arg::RcRefCell(special)),
       COptionV8Local(TV8(v8)) => Ok(Arg::OptionV8Local(v8)),
       COptionV8Global(TV8(v8)) => Ok(Arg::OptionV8Global(v8)),
@@ -1427,6 +1451,10 @@ mod tests {
     fn op_option_numeric_result(state: &mut OpState) -> Result<Option<u32>, AnyError>;
     (Ref(Mut, OpState)) -> Result(OptionNumeric(u32))
   );
+  test!(
+    #[smi] fn op_option_numeric_smi_result(#[smi] a: Option<u32>) -> Result<Option<u32>, AnyError>;
+    (OptionNumeric(__SMI__)) -> Result(OptionNumeric(__SMI__))
+  );
   test!(
     fn op_ffi_read_f64(state: &mut OpState, ptr: *mut c_void, #[bigint] offset: isize) -> Result <f64, AnyError>;
     (Ref(Mut, OpState), External(Ptr(Mut)), Numeric(isize)) -> Result(Numeric(f64))


Test patch:
diff --git a/ops/op2/test_cases/async/async_jsbuffer.out b/ops/op2/test_cases/async/async_jsbuffer.out
index c5b664b..3600e6d 100644
--- a/ops/op2/test_cases/async/async_jsbuffer.out
+++ b/ops/op2/test_cases/async/async_jsbuffer.out
@@ -36,7 +36,8 @@ impl op_async_v8_buffer {
         };
         let result = {
             let arg0 = args.get(1usize as i32);
-            let mut arg0 = match unsafe {
+            let mut arg0_temp;
+            arg0_temp = match unsafe {
                 deno_core::_ops::to_v8_slice::<
                     deno_core::v8::Uint8Array,
                 >(&mut scope, arg0)
@@ -55,7 +56,7 @@ impl op_async_v8_buffer {
                     return;
                 }
             };
-            let arg0 = deno_core::serde_v8::JsBuffer::from_parts(arg0);
+            let arg0 = deno_core::serde_v8::JsBuffer::from_parts(arg0_temp);
             Self::call(arg0)
         };
         let promise_id = deno_core::_ops::to_i32_option(&args.get(0))
diff --git a/ops/op2/test_cases/sync/buffers.out b/ops/op2/test_cases/sync/buffers.out
index 2644caa..3694e2a 100644
--- a/ops/op2/test_cases/sync/buffers.out
+++ b/ops/op2/test_cases/sync/buffers.out
@@ -60,7 +60,8 @@ impl op_buffers {
         });
         let result = {
             let arg0 = args.get(0usize as i32);
-            let mut arg0 = match unsafe {
+            let mut arg0_temp;
+            arg0_temp = match unsafe {
                 deno_core::_ops::to_v8_slice::<
                     deno_core::v8::Uint8Array,
                 >(&mut scope, arg0)
@@ -79,9 +80,10 @@ impl op_buffers {
                     return;
                 }
             };
-            let arg0 = &mut arg0;
+            let arg0 = arg0_temp.as_ref();
             let arg1 = args.get(1usize as i32);
-            let mut arg1 = match unsafe {
+            let mut arg1_temp;
+            arg1_temp = match unsafe {
                 deno_core::_ops::to_v8_slice::<
                     deno_core::v8::Uint8Array,
                 >(&mut scope, arg1)
@@ -99,7 +101,7 @@ impl op_buffers {
                     return;
                 }
             };
-            let arg1 = &mut arg1;
+            let arg1 = arg1_temp.as_mut();
             Self::call(arg0, arg1)
         };
         deno_core::_ops::RustToV8RetVal::to_v8_rv(result, &mut rv)
@@ -107,3 +109,99 @@ impl op_buffers {
     #[inline(always)]
     fn call(_a: &[u8], _b: &mut [u8]) {}
 }
+
+#[allow(non_camel_case_types)]
+struct op_buffers_option {
+    _unconstructable: ::std::marker::PhantomData<()>,
+}
+impl deno_core::_ops::Op for op_buffers_option {
+    const NAME: &'static str = stringify!(op_buffers_option);
+    const DECL: deno_core::_ops::OpDecl = deno_core::_ops::OpDecl::new_internal(
+        stringify!(op_buffers_option),
+        false,
+        false,
+        false,
+        2usize as u8,
+        Self::v8_fn_ptr as _,
+        None,
+    );
+}
+impl op_buffers_option {
+    pub const fn name() -> &'static str {
+        stringify!(op_buffers_option)
+    }
+    #[deprecated(note = "Use the const op::DECL instead")]
+    pub const fn decl() -> deno_core::_ops::OpDecl {
+        <Self as deno_core::_ops::Op>::DECL
+    }
+    extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let mut scope = 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 result = {
+            let arg0 = args.get(0usize as i32);
+            let mut arg0_temp;
+            let arg0 = if arg0.is_null_or_undefined() {
+                None
+            } else {
+                arg0_temp = match unsafe {
+                    deno_core::_ops::to_v8_slice::<
+                        deno_core::v8::Uint8Array,
+                    >(&mut scope, arg0)
+                } {
+                    Ok(arg0) => arg0,
+                    Err(arg0_err) => {
+                        let mut scope = unsafe {
+                            deno_core::v8::CallbackScope::new(&*info)
+                        };
+                        let msg = deno_core::v8::String::new_from_one_byte(
+                                &mut scope,
+                                arg0_err.as_bytes(),
+                                deno_core::v8::NewStringType::Normal,
+                            )
+                            .unwrap();
+                        let exc = deno_core::v8::Exception::error(&mut scope, msg);
+                        scope.throw_exception(exc);
+                        return;
+                    }
+                };
+                let arg0 = arg0_temp.as_ref();
+                Some(arg0)
+            };
+            let arg1 = args.get(1usize as i32);
+            let mut arg1_temp;
+            let arg1 = if arg1.is_null_or_undefined() {
+                None
+            } else {
+                arg1_temp = match unsafe {
+                    deno_core::_ops::to_v8_slice::<
+                        deno_core::v8::Uint8Array,
+                    >(&mut scope, arg1)
+                } {
+                    Ok(arg1) => arg1,
+                    Err(arg1_err) => {
+                        let msg = deno_core::v8::String::new_from_one_byte(
+                                &mut scope,
+                                arg1_err.as_bytes(),
+                                deno_core::v8::NewStringType::Normal,
+                            )
+                            .unwrap();
+                        let exc = deno_core::v8::Exception::error(&mut scope, msg);
+                        scope.throw_exception(exc);
+                        return;
+                    }
+                };
+                let arg1 = deno_core::serde_v8::JsBuffer::from_parts(arg1_temp);
+                Some(arg1)
+            };
+            Self::call(arg0, arg1)
+        };
+        deno_core::_ops::RustToV8RetVal::to_v8_rv(result, &mut rv)
+    }
+    #[inline(always)]
+    fn call(_a: Option<&[u8]>, _b: Option<JsBuffer>) {}
+}
diff --git a/ops/op2/test_cases/sync/buffers.rs b/ops/op2/test_cases/sync/buffers.rs
index ee56204..b46cbbc 100644
--- a/ops/op2/test_cases/sync/buffers.rs
+++ b/ops/op2/test_cases/sync/buffers.rs
@@ -2,6 +2,12 @@
 #![deny(warnings)]
 deno_ops_compile_test_runner::prelude!();
 
+use deno_core::JsBuffer;
+
 #[op2(fast)]
 fn op_buffers(#[buffer] _a: &[u8], #[buffer] _b: &mut [u8]) {
 }
+
+#[op2]
+fn op_buffers_option(#[buffer] _a: Option<&[u8]>, #[buffer] _b: Option<JsBuffer>) {
+}
diff --git a/ops/op2/test_cases/sync/buffers_copy.out b/ops/op2/test_cases/sync/buffers_copy.out
index 8c4a947..fbae008 100644
--- a/ops/op2/test_cases/sync/buffers_copy.out
+++ b/ops/op2/test_cases/sync/buffers_copy.out
@@ -70,7 +70,8 @@ impl op_buffers {
         });
         let result = {
             let arg0 = args.get(0usize as i32);
-            let mut arg0 = match unsafe {
+            let mut arg0_temp;
+            arg0_temp = match unsafe {
                 deno_core::_ops::to_v8_slice::<
                     deno_core::v8::Uint8Array,
                 >(&mut scope, arg0)
@@ -89,9 +90,10 @@ impl op_buffers {
                     return;
                 }
             };
-            let arg0 = arg0.to_vec();
+            let arg0 = arg0_temp.to_vec();
             let arg1 = args.get(1usize as i32);
-            let mut arg1 = match unsafe {
+            let mut arg1_temp;
+            arg1_temp = match unsafe {
                 deno_core::_ops::to_v8_slice::<
                     deno_core::v8::Uint8Array,
                 >(&mut scope, arg1)
@@ -109,9 +111,10 @@ impl op_buffers {
                     return;
                 }
             };
-            let arg1 = arg1.to_boxed_slice();
+            let arg1 = arg1_temp.to_boxed_slice();
             let arg2 = args.get(2usize as i32);
-            let mut arg2 = match unsafe {
+            let mut arg2_temp;
+            arg2_temp = match unsafe {
                 deno_core::_ops::to_v8_slice::<
                     deno_core::v8::Uint8Array,
                 >(&mut scope, arg2)
@@ -129,7 +132,7 @@ impl op_buffers {
                     return;
                 }
             };
-            let arg2 = arg2.to_vec().into();
+            let arg2 = arg2_temp.to_vec().into();
             Self::call(arg0, arg1, arg2)
         };
         deno_core::_ops::RustToV8RetVal::to_v8_rv(result, &mut rv)
diff --git a/ops/op2/test_cases/sync/smi.out b/ops/op2/test_cases/sync/smi.out
index 0c0dca1..339b773 100644
--- a/ops/op2/test_cases/sync/smi.out
+++ b/ops/op2/test_cases/sync/smi.out
@@ -253,3 +253,64 @@ impl op_smi_signed_return {
         a as Int32 + b as Int32 + c as Int32 + d as Int32
     }
 }
+
+#[allow(non_camel_case_types)]
+struct op_smi_option {
+    _unconstructable: ::std::marker::PhantomData<()>,
+}
+impl deno_core::_ops::Op for op_smi_option {
+    const NAME: &'static str = stringify!(op_smi_option);
+    const DECL: deno_core::_ops::OpDecl = deno_core::_ops::OpDecl::new_internal(
+        stringify!(op_smi_option),
+        false,
+        false,
+        false,
+        1usize as u8,
+        Self::v8_fn_ptr as _,
+        None,
+    );
+}
+impl op_smi_option {
+    pub const fn name() -> &'static str {
+        stringify!(op_smi_option)
+    }
+    #[deprecated(note = "Use the const op::DECL instead")]
+    pub const fn decl() -> deno_core::_ops::OpDecl {
+        <Self as deno_core::_ops::Op>::DECL
+    }
+    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 result = {
+            let arg0 = args.get(0usize as i32);
+            let arg0 = if arg0.is_null_or_undefined() {
+                None
+            } else {
+                let Some(arg0) = deno_core::_ops::to_i32_option(&arg0) else {
+                    let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+                    let msg = deno_core::v8::String::new_from_one_byte(
+                            &mut scope,
+                            "expected i32".as_bytes(),
+                            deno_core::v8::NewStringType::Normal,
+                        )
+                        .unwrap();
+                    let exc = deno_core::v8::Exception::type_error(&mut scope, msg);
+                    scope.throw_exception(exc);
+                    return;
+                };
+                let arg0 = arg0 as _;
+                Some(arg0)
+            };
+            Self::call(arg0)
+        };
+        deno_core::_ops::RustToV8RetVal::to_v8_rv(result, &mut rv)
+    }
+    #[inline(always)]
+    fn call(a: Option<Uint32>) -> Option<Uint32> {
+        a
+    }
+}
diff --git a/ops/op2/test_cases/sync/smi.rs b/ops/op2/test_cases/sync/smi.rs
index f25af80..d2c79c2 100644
--- a/ops/op2/test_cases/sync/smi.rs
+++ b/ops/op2/test_cases/sync/smi.rs
@@ -18,3 +18,9 @@ fn op_smi_unsigned_return(#[smi] a: Int16, #[smi] b: Int32, #[smi] c: Uint16, #[
 fn op_smi_signed_return(#[smi] a: Int16, #[smi] b: Int32, #[smi] c: Uint16, #[smi] d: Uint32) -> Int32 {
   a as Int32 + b as Int32 + c as Int32 + d as Int32
 }
+
+#[op2]
+#[smi]
+fn op_smi_option(#[smi] a: Option<Uint32>) -> Option<Uint32> {
+  a
+}