Repository: denoland/deno_core
Pull request #976 (2024-11-21 18:04:21)

Add `#[required(<count>)]`
Equivalent to Deno's `webidl.requiredArguments` internal.

```rust
#[fast]
#[required(2)]
fn set_item(
  &self,
  state: &mut OpState,
  #[string] key: &str,
  #[string] value: &str,
) {
 // ...
}
```


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 7ac38fc..1f0bfa2 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -559,7 +559,7 @@ dependencies = [
 
 [[package]]
 name = "deno_core"
-version = "0.321.0"
+version = "0.322.0"
 dependencies = [
  "anyhow",
  "bencher",
@@ -631,7 +631,7 @@ dependencies = [
 
 [[package]]
 name = "deno_ops"
-version = "0.197.0"
+version = "0.198.0"
 dependencies = [
  "pretty_assertions",
  "prettyplease",
@@ -1843,7 +1843,7 @@ dependencies = [
 
 [[package]]
 name = "serde_v8"
-version = "0.230.0"
+version = "0.231.0"
 dependencies = [
  "bencher",
  "num-bigint",
diff --git a/Cargo.toml b/Cargo.toml
index af98a57..35041aa 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -20,9 +20,9 @@ repository = "https://github.com/denoland/deno_core"
 
 [workspace.dependencies]
 # Local dependencies
-deno_core = { version = "0.321.0", path = "./core" }
-deno_ops = { version = "0.197.0", path = "./ops" }
-serde_v8 = { version = "0.230.0", path = "./serde_v8" }
+deno_core = { version = "0.322.0", path = "./core" }
+deno_ops = { version = "0.198.0", path = "./ops" }
+serde_v8 = { version = "0.231.0", path = "./serde_v8" }
 deno_core_testing = { path = "./testing" }
 
 v8 = { version = "130.0.1", default-features = false }
diff --git a/core/Cargo.toml b/core/Cargo.toml
index bffae3f..011199b 100644
--- a/core/Cargo.toml
+++ b/core/Cargo.toml
@@ -2,7 +2,7 @@
 
 [package]
 name = "deno_core"
-version = "0.321.0"
+version = "0.322.0"
 authors.workspace = true
 edition.workspace = true
 license.workspace = true
diff --git a/core/modules/map.rs b/core/modules/map.rs
index 3175681..5a220b3 100644
--- a/core/modules/map.rs
+++ b/core/modules/map.rs
@@ -2035,21 +2035,28 @@ fn render_js_wasm_module(specifier: &str, wasm_deps: WasmDeps) -> String {
           // an export can't be found on the module
           .map(|(name_index, name)| format!(
             "\"{}\" as import_{}_{}",
-            name, i, name_index
+            name.escape_default(),
+            i,
+            name_index
           ))
           .collect::<Vec<_>>()
           .join(", "),
-        key
+        key.escape_default(),
       ));
     }
 
     src.push("const importsObject = {".to_string());
 
     for (i, (key, names)) in aggregated_imports.iter().enumerate() {
-      src.push(format!("  \"{}\": {{", key).to_string());
+      src.push(format!("  \"{}\": {{", key.escape_default()).to_string());
 
       for (name_index, name) in names.iter().enumerate() {
-        src.push(format!("    \"{0}\": import_{1}_{2},", name, i, name_index));
+        src.push(format!(
+          "    \"{0}\": import_{1}_{2},",
+          name.escape_default(),
+          i,
+          name_index
+        ));
       }
 
       src.push("  },".to_string());
@@ -2067,18 +2074,16 @@ fn render_js_wasm_module(specifier: &str, wasm_deps: WasmDeps) -> String {
   }
 
   if !wasm_deps.exports.is_empty() {
-    for export_desc in wasm_deps.exports.iter().filter(|e| {
-      matches!(e.export_type, wasm_dep_analyzer::ExportType::Function(_))
-    }) {
+    for (idx, export_desc) in wasm_deps.exports.iter().enumerate() {
       if export_desc.name == "default" {
         src.push(format!(
           "export default modInstance.exports.{};",
           export_desc.name
         ));
       } else {
+        let escaped_name = export_desc.name.escape_default();
         src.push(format!(
-          "export const {} = modInstance.exports.{};",
-          export_desc.name, export_desc.name
+          "const export{idx} = modInstance.exports[\"{escaped_name}\"];\nexport {{ export{idx} as \"{escaped_name}\" }};",
         ));
       }
     }
@@ -2197,7 +2202,50 @@ const importsObject = {
   },
 };
 const modInstance = new import.meta.WasmInstance(wasmMod, importsObject);
-export const export1 = modInstance.exports.export1;
+const export0 = modInstance.exports["export1"];
+export { export0 as "export1" };
+const export1 = modInstance.exports["export2"];
+export { export1 as "export2" };
+const export2 = modInstance.exports["export3"];
+export { export2 as "export3" };
+const export3 = modInstance.exports["export4"];
+export { export3 as "export4" };
+const export4 = modInstance.exports["export5"];
+export { export4 as "export5" };
+const export5 = modInstance.exports["export6"];
+export { export5 as "export6" };
 export default modInstance.exports.default;"#,
   );
+
+  let deps = WasmDeps {
+    imports: vec![wasm_dep_analyzer::Import {
+      name: "\n",
+      module: "\n",
+      import_type: wasm_dep_analyzer::ImportType::Function(1),
+    }],
+    exports: vec![wasm_dep_analyzer::Export {
+      name: "\n",
+      index: 0,
+      export_type: wasm_dep_analyzer::ExportType::Function(Ok(
+        wasm_dep_analyzer::FunctionSignature {
+          params: vec![],
+          returns: vec![],
+        },
+      )),
+    }],
+  };
+  let rendered = render_js_wasm_module("./bar.wasm", deps);
+  pretty_assertions::assert_eq!(
+    rendered,
+    r#"import wasmMod from "./bar.wasm" with { type: "$$deno-core-internal-wasm-module" };
+import { "\n" as import_0_0 } from "\n";
+const importsObject = {
+  "\n": {
+    "\n": import_0_0,
+  },
+};
+const modInstance = new import.meta.WasmInstance(wasmMod, importsObject);
+const export0 = modInstance.exports["\n"];
+export { export0 as "\n" };"#,
+  );
 }
diff --git a/ops/Cargo.toml b/ops/Cargo.toml
index c46f80e..2a3334c 100644
--- a/ops/Cargo.toml
+++ b/ops/Cargo.toml
@@ -2,7 +2,7 @@
 
 [package]
 name = "deno_ops"
-version = "0.197.0"
+version = "0.198.0"
 authors.workspace = true
 edition.workspace = true
 license.workspace = true
diff --git a/ops/op2/config.rs b/ops/op2/config.rs
index 1b822d2..150b998 100644
--- a/ops/op2/config.rs
+++ b/ops/op2/config.rs
@@ -24,6 +24,8 @@ pub(crate) struct MacroConfig {
   pub reentrant: bool,
   /// Marks an op as a method on a wrapped object.
   pub method: Option<String>,
+  /// Same as method name but also used by static and constructor ops.
+  pub self_name: Option<String>,
   /// Marks an op as a constructor
   pub constructor: bool,
   /// Marks an op as a static member
@@ -36,6 +38,8 @@ pub(crate) struct MacroConfig {
   pub setter: bool,
   /// Marks an op to have it collect stack trace of the call site in the OpState.
   pub stack_trace: bool,
+  /// Total required number of arguments for the op.
+  pub required: u8,
 }
 
 impl MacroConfig {
@@ -126,6 +130,13 @@ impl MacroConfig {
           })
         })
         .map_err(|_| Op2Error::PatternMatchFailed("attribute", flag))?;
+      } else if flag.starts_with("required(") {
+        let tokens = syn::parse_str::<TokenTree>(&flag[9..flag.len() - 1])?
+          .into_token_stream();
+        config.required = tokens
+          .to_string()
+          .parse()
+          .map_err(|_| Op2Error::InvalidAttribute(flag))?;
       } else {
         return Err(Op2Error::InvalidAttribute(flag));
       }
@@ -182,8 +193,12 @@ impl MacroConfig {
         ( $($flags:tt $( ( $( $args:ty ),* ) )? ),+ ) => {
           Self::from_token_trees(flags, args)
         }
-        ( # [ $($flags:tt),+ ] ) => {
-            Self::from_flags(flags.into_iter().map(|flag| flag.to_string()))
+        ( $( #[$attr:meta] )* ) => {
+          Self::from_flags(
+            attr.
+             into_iter().
+             map(|attr| attr.to_token_stream().to_string())
+          )
         }
       })
     })
diff --git a/ops/op2/dispatch_slow.rs b/ops/op2/dispatch_slow.rs
index cf9973c..967ceb3 100644
--- a/ops/op2/dispatch_slow.rs
+++ b/ops/op2/dispatch_slow.rs
@@ -120,6 +120,13 @@ pub(crate) fn generate_dispatch_slow(
     quote!()
   };
 
+  let with_required_check = if generator_state.needs_args && config.required > 0
+  {
+    with_required_check(generator_state, config.required)
+  } else {
+    quote!()
+  };
+
   let with_self = if generator_state.needs_self {
     with_self(generator_state, &signature.ret_val)
       .map_err(V8SignatureMappingError::NoSelfMapping)?
@@ -143,6 +150,7 @@ pub(crate) fn generate_dispatch_slow(
         #with_scope
         #with_retval
         #with_args
+        #with_required_check
         #with_opctx
         #with_self
         #with_isolate
@@ -227,6 +235,48 @@ pub(crate) fn with_fn_args(
   )
 }
 
+pub(crate) fn with_required_check(
+  generator_state: &mut GeneratorState,
+  required: u8,
+) -> TokenStream {
+  generator_state.needs_scope = true;
+  let arguments_lit = if required > 1 {
+    "arguments"
+  } else {
+    "argument"
+  };
+
+  let prefix = if generator_state.needs_self {
+    format!(
+      "Failed to execute '{}' on '{}': ",
+      generator_state.name, generator_state.self_ty
+    )
+  } else if generator_state.use_this_cppgc {
+    format!("Failed to construct '{}': ", generator_state.self_ty)
+  } else {
+    format!(
+      "Failed to execute '{}.{}': ",
+      generator_state.self_ty, generator_state.name
+    )
+  };
+
+  gs_quote!(generator_state(fn_args, scope) =>
+    (if #fn_args.length() < #required as i32 {
+      let msg = format!(
+        "{}{} {} required, but only {} present",
+        #prefix,
+        #required,
+        #arguments_lit,
+        #fn_args.length()
+      );
+      let msg = deno_core::v8::String::new(&mut #scope, &msg).unwrap();
+      let exception = deno_core::v8::Exception::type_error(&mut #scope, msg.into());
+      #scope.throw_exception(exception);
+      return 1;
+    })
+  )
+}
+
 pub(crate) fn with_opctx(generator_state: &mut GeneratorState) -> TokenStream {
   generator_state.needs_args = true;
   gs_quote!(generator_state(opctx, fn_args) =>
diff --git a/ops/op2/mod.rs b/ops/op2/mod.rs
index 0826442..a0855d1 100644
--- a/ops/op2/mod.rs
+++ b/ops/op2/mod.rs
@@ -160,7 +160,7 @@ pub(crate) fn generate_op2(
     Ident::new("v8_fn_ptr_fast_metrics", Span::call_site());
   let fast_api_callback_options =
     Ident::new("fast_api_callback_options", Span::call_site());
-  let self_ty = if let Some(ref ty) = config.method {
+  let self_ty = if let Some(ref ty) = config.self_name {
     format_ident!("{ty}")
   } else {
     Ident::new("UNINIT", Span::call_site())
diff --git a/ops/op2/object_wrap.rs b/ops/op2/object_wrap.rs
index fc16f49..a9e0a4c 100644
--- a/ops/op2/object_wrap.rs
+++ b/ops/op2/object_wrap.rs
@@ -12,6 +12,8 @@ use crate::op2::generate_op2;
 use crate::op2::MacroConfig;
 use crate::op2::Op2Error;
 
+use super::signature::is_attribute_special;
+
 // Object wrap for Cppgc-backed objects
 //
 // This module generates the glue code declarations
@@ -82,8 +84,8 @@ pub(crate) fn generate_impl_ops(
 
   for item in item.items {
     if let ImplItem::Fn(mut method) = item {
-      /* First attribute idents, for all functions in block */
-      let attrs = method.attrs.swap_remove(0);
+      let (item_fn_attrs, attrs) =
+        method.attrs.into_iter().partition(is_attribute_special);
 
       /* Convert snake_case to camelCase */
       method.sig.ident = format_ident!(
@@ -93,15 +95,16 @@ pub(crate) fn generate_impl_ops(
 
       let ident = method.sig.ident.clone();
       let func = ItemFn {
-        attrs: method.attrs,
+        attrs: item_fn_attrs,
         vis: method.vis,
         sig: method.sig,
         block: Box::new(method.block),
       };
 
       let mut config = MacroConfig::from_tokens(quote! {
-        #attrs
+        #(#attrs)*
       })?;
+
       if config.constructor {
         if constructor.is_some() {
           return Err(Op2Error::MultipleConstructors);
@@ -120,6 +123,8 @@ pub(crate) fn generate_impl_ops(
         config.method = Some(self_ty_ident.clone());
       }
 
+      config.self_name = Some(self_ty_ident.clone());
+
       let op = generate_op2(config, func)?;
       tokens.extend(op);
     }
diff --git a/ops/op2/signature.rs b/ops/op2/signature.rs
index 3d035e2..9352f32 100644
--- a/ops/op2/signature.rs
+++ b/ops/op2/signature.rs
@@ -755,6 +755,8 @@ pub enum AttributeModifier {
   Number,
   /// #[cppgc], for a resource backed managed by cppgc.
   CppGcResource,
+  /// Any attribute that we may want to omit if not syntactically valid.
+  Ignore,
 }
 
 impl AttributeModifier {
@@ -771,6 +773,7 @@ impl AttributeModifier {
       AttributeModifier::State => "state",
       AttributeModifier::Global => "global",
       AttributeModifier::CppGcResource => "cppgc",
+      AttributeModifier::Ignore => "ignore",
     }
   }
 }
@@ -809,8 +812,6 @@ pub enum AttributeError {
   InvalidAttribute(String),
   #[error("Invalid inner attribute (#![attr]) in this position. Use an equivalent outer attribute (#[attr]) on the function instead.")]
   InvalidInnerAttribute,
-  #[error("Too many attributes")]
-  TooManyAttributes,
 }
 
 #[derive(Error, Debug)]
@@ -1181,6 +1182,9 @@ fn parse_attributes(
   let mut attrs = vec![];
   for attr in attributes {
     if let Some(attr) = parse_attribute(attr)? {
+      if attr == AttributeModifier::Ignore {
+        continue;
+      }
       attrs.push(attr)
     }
   }
@@ -1188,9 +1192,6 @@ fn parse_attributes(
   if attrs.is_empty() {
     return Ok(Attributes::default());
   }
-  if attrs.len() > 1 {
-    return Err(AttributeError::TooManyAttributes);
-  }
   Ok(Attributes {
     primary: Some(*attrs.first().unwrap()),
   })
@@ -1198,7 +1199,13 @@ fn parse_attributes(
 
 /// Is this a special attribute that we understand?
 pub fn is_attribute_special(attr: &Attribute) -> bool {
-  parse_attribute(attr).unwrap_or_default().is_some()
+  parse_attribute(attr)
+      .unwrap_or_default()
+      .and_then(|attr| match attr {
+        AttributeModifier::Ignore => None,
+        _ => Some(()),
+      })
+      .is_some()
     // this is kind of ugly, but #[meta(..)] is the only
     // attribute that we want to omit from the generated code
     // that doesn't have a semantic meaning
@@ -1239,10 +1246,18 @@ fn parse_attribute(
       (#[cppgc]) => Some(AttributeModifier::CppGcResource),
       (#[to_v8]) => Some(AttributeModifier::ToV8),
       (#[from_v8]) => Some(AttributeModifier::FromV8),
+      (#[required ($_attr:literal)]) => Some(AttributeModifier::Ignore),
+      (#[method ($_attr:literal)]) => Some(AttributeModifier::Ignore),
+      (#[method]) => Some(AttributeModifier::Ignore),
+      (#[getter]) => Some(AttributeModifier::Ignore),
+      (#[setter]) => Some(AttributeModifier::Ignore),
+      (#[fast]) => Some(AttributeModifier::Ignore),
+      (#[static_method]) => Some(AttributeModifier::Ignore),
+      (#[constructor]) => Some(AttributeModifier::Ignore),
       (#[allow ($_rule:path)]) => None,
       (#[doc = $_attr:literal]) => None,
       (#[cfg $_cfg:tt]) => None,
-      (#[meta ($($_key: ident = $_value: literal),*)]) => None,
+      (#[meta ($($_key: ident = $_value: literal),*)]) => Some(AttributeModifier::Ignore),
     })
   }).map_err(|_| AttributeError::InvalidAttribute(stringify_token(attr)))?;
   Ok(res)
@@ -1506,6 +1521,9 @@ pub(crate) fn parse_type(
 
   if let Some(primary) = attrs.primary {
     match primary {
+      AttributeModifier::Ignore => {
+        unreachable!();
+      }
       AttributeModifier::CppGcResource => return parse_cppgc(position, ty),
       AttributeModifier::FromV8 if position == Position::RetVal => {
         return Err(ArgError::InvalidAttributePosition(
diff --git a/serde_v8/Cargo.toml b/serde_v8/Cargo.toml
index 3568c39..0e6551f 100644
--- a/serde_v8/Cargo.toml
+++ b/serde_v8/Cargo.toml
@@ -2,7 +2,7 @@
 
 [package]
 name = "serde_v8"
-version = "0.230.0"
+version = "0.231.0"
 authors.workspace = true
 edition.workspace = true
 license.workspace = true


Test patch:
diff --git a/testing/checkin/runner/ops.rs b/testing/checkin/runner/ops.rs
index 8ac41b1..042f49e 100644
--- a/testing/checkin/runner/ops.rs
+++ b/testing/checkin/runner/ops.rs
@@ -97,6 +97,7 @@ impl DOMPoint {
     }
   }
 
+  #[required(1)]
   #[static_method]
   #[cppgc]
   fn from_point(