feat(webidl): add WebIdlInterfaceConverter, add .as_str() for enu…

…m converters, and fix optional types on dictionary (#1045)
denoland · Jan 16, 2025 · dd0b9a0 · dd0b9a0
1 parent be588b8
commit dd0b9a0
Show file tree

Hide file tree

Showing 12 changed files with 365 additions and 57 deletions.
diff --git a/core/cppgc.rs b/core/cppgc.rs
@@ -82,7 +82,6 @@ pub fn wrap_object<'a, T: GarbageCollected + 'static>(
   obj
 }
 
-#[doc(hidden)]
 pub struct Ptr<T: GarbageCollected> {
   inner: v8::cppgc::Ptr<CppGcObject<T>>,
   root: Option<v8::cppgc::Persistent<CppGcObject<T>>>,
@@ -202,12 +201,13 @@ impl<T: GarbageCollected + 'static> SameObject<T> {
     f: F,
   ) -> v8::Global<v8::Object>
   where
-    F: FnOnce() -> T,
+    F: FnOnce(&mut v8::HandleScope) -> T,
   {
     self
       .cell
       .get_or_init(|| {
-        let obj = make_cppgc_object(scope, f());
+        let v = f(scope);
+        let obj = make_cppgc_object(scope, v);
         v8::Global::new(scope, obj)
       })
       .clone()

diff --git a/core/webidl.rs b/core/webidl.rs
@@ -840,6 +840,37 @@ impl<'a> WebIdlConverter<'a> for ByteString {
   }
 }
 
+pub trait WebIdlInterfaceConverter:
+  v8::cppgc::GarbageCollected + 'static
+{
+  const NAME: &'static str;
+}
+
+impl<'a, T: WebIdlInterfaceConverter> WebIdlConverter<'a>
+  for crate::cppgc::Ptr<T>
+{
+  type Options = ();
+
+  fn convert<'b>(
+    scope: &mut HandleScope<'a>,
+    value: Local<'a, Value>,
+    prefix: Cow<'static, str>,
+    context: ContextFn<'b>,
+    _options: &Self::Options,
+  ) -> Result<Self, WebIdlError> {
+    if let Some(ptr) = crate::cppgc::try_unwrap_cppgc_object::<T>(scope, value)
+    {
+      Ok(ptr)
+    } else {
+      Err(WebIdlError::new(
+        prefix,
+        context,
+        WebIdlErrorKind::ConvertToConverterType(T::NAME),
+      ))
+    }
+  }
+}
+
 // TODO:
 //  object
 //  ArrayBuffer
@@ -1451,8 +1482,10 @@ mod tests {
       "prefix".into(),
       (|| "context".into()).into(),
       &Default::default(),
-    );
-    assert_eq!(converted.unwrap(), Enumeration::FooBar);
+    )
+    .unwrap();
+    assert_eq!(converted, Enumeration::FooBar);
+    assert_eq!(converted.as_str(), "foo-bar");
 
     let val = v8::String::new(scope, "baz").unwrap();
     let converted = Enumeration::convert(
@@ -1461,8 +1494,10 @@ mod tests {
       "prefix".into(),
       (|| "context".into()).into(),
       &Default::default(),
-    );
-    assert_eq!(converted.unwrap(), Enumeration::Baz);
+    )
+    .unwrap();
+    assert_eq!(converted, Enumeration::Baz);
+    assert_eq!(converted.as_str(), "baz");
 
     let val = v8::String::new(scope, "hello").unwrap();
     let converted = Enumeration::convert(
@@ -1471,8 +1506,10 @@ mod tests {
       "prefix".into(),
       (|| "context".into()).into(),
       &Default::default(),
-    );
-    assert_eq!(converted.unwrap(), Enumeration::World);
+    )
+    .unwrap();
+    assert_eq!(converted, Enumeration::World);
+    assert_eq!(converted.as_str(), "hello");
 
     let val = v8::String::new(scope, "unknown").unwrap();
     let converted = Enumeration::convert(

diff --git a/ops/op2/dispatch_slow.rs b/ops/op2/dispatch_slow.rs
@@ -667,34 +667,49 @@ pub fn from_arg(
       let err = format_ident!("{}_err", arg_ident);
       let throw_exception = throw_type_error_string(generator_state, &err);
       let prefix = get_prefix(generator_state);
-      let context = format!("Argument {}", index + 1);
+      let context = format!("Argument {}", index);
 
-      let options = if options.is_empty() {
-        quote!(Default::default())
+      let (alias, options) = if options.is_empty() {
+        (None, quote!(Default::default()))
       } else {
         let inner = options
           .iter()
           .map(|WebIDLPairs(k, v)| quote!(#k: #v))
           .collect::<Vec<_>>();
 
-        quote! {
-          <#ty as deno_core::webidl::WebIdlConverter>::Options {
-            #(#inner),*
-            ..Default::default()
-          }
-        }
+        let alias = format_ident!("{arg_ident}_webidl_alias");
+        // Type-alias to workaround https://github.com/rust-lang/rust/issues/86935
+        (
+          Some(quote! {
+            type #alias<'a> = <#ty as ::deno_core::webidl::WebIdlConverter<'a>>::Options;
+          }),
+          quote! {
+            #alias {
+              #(#inner),*,
+              ..Default::default()
+            }
+          },
+        )
       };
 
       let default = if let Some(default) = default {
         let tokens = default.0.to_token_stream();
-        let default = if let Ok(lit) = parse2::<syn::LitStr>(tokens) {
+        let default = if let Ok(lit) = parse2::<syn::LitStr>(tokens.clone()) {
           if lit.value().is_empty() {
             quote! {
-              v8::String::empty(&mut #scope)
+              deno_core::v8::String::empty(&mut #scope)
             }
           } else {
             return Err("unsupported WebIDL default value");
           }
+        } else if let Ok(lit) = parse2::<syn::LitInt>(tokens.clone()) {
+          quote! {
+            deno_core::v8::Number::new(&mut #scope, #lit as _)
+          }
+        } else if let Ok(lit) = parse2::<syn::LitFloat>(tokens) {
+          quote! {
+            deno_core::v8::Number::new(&mut #scope, #lit)
+          }
         } else {
           return Err("unsupported WebIDL default value");
         };
@@ -712,6 +727,7 @@ pub fn from_arg(
 
       quote! {
         #default
+        #alias
         let #arg_ident = match <#ty as deno_core::webidl::WebIdlConverter>::convert(
           &mut #scope,
           #arg_ident,

diff --git a/ops/op2/signature.rs b/ops/op2/signature.rs
@@ -1,7 +1,6 @@
 // Copyright 2018-2025 the Deno authors. MIT license.
 
 use proc_macro2::Ident;
-use proc_macro2::Literal;
 use proc_macro2::Span;
 use proc_macro2::TokenStream;
 use proc_macro_rules::rules;
@@ -251,10 +250,10 @@ pub enum NumericFlag {
 
 // its own struct to facility Eq & PartialEq on other structs
 #[derive(Clone, Debug)]
-pub struct WebIDLPairs(pub Ident, pub Literal);
+pub struct WebIDLPairs(pub Ident, pub syn::Expr);
 impl PartialEq for WebIDLPairs {
   fn eq(&self, other: &Self) -> bool {
-    self.0 == other.0 && self.1.to_string() == other.1.to_string()
+    self.0 == other.0
   }
 }
 impl Eq for WebIDLPairs {}
@@ -1260,7 +1259,7 @@ fn parse_attribute(
       (#[number]) => Some(AttributeModifier::Number),
       (#[serde]) => Some(AttributeModifier::Serde),
       (#[webidl]) => Some(AttributeModifier::WebIDL { options: vec![],default: None }),
-      (#[webidl($(default = $default:expr)?$($(,)? options($($key:ident = $value:literal),*))?)]) => Some(AttributeModifier::WebIDL { options: key.map(|key| key.into_iter().zip(value.unwrap().into_iter()).map(|v| WebIDLPairs(v.0, v.1)).collect()).unwrap_or_default(), default: default.map(WebIDLDefault) }),
+      (#[webidl($(default = $default:expr)?$($(, )?options($($key:ident = $value:expr),*))?)]) => Some(AttributeModifier::WebIDL { options: key.map(|key| key.into_iter().zip(value.unwrap().into_iter()).map(|v| WebIDLPairs(v.0, v.1)).collect()).unwrap_or_default(), default: default.map(WebIDLDefault) }),
       (#[smi]) => Some(AttributeModifier::Smi),
       (#[string]) => Some(AttributeModifier::String(StringMode::Default)),
       (#[string(onebyte)]) => Some(AttributeModifier::String(StringMode::OneByte)),

diff --git a/ops/op2/test_cases/sync/webidl.out b/ops/op2/test_cases/sync/webidl.out
diff --git a/ops/webidl/dictionary.rs b/ops/webidl/dictionary.rs
@@ -128,8 +128,6 @@ pub fn get_body(
       }
     };
 
-    let new_context = format!("'{string_name}' of '{ident_string}'");
-
     quote! {
       let #original_name = {
         let __key = #v8_eternal_name
@@ -151,7 +149,7 @@ pub fn get_body(
             __scope,
             __value,
             __prefix.clone(),
-            ::deno_core::webidl::ContextFn::new_borrowed(&|| format!("{} ({})", #new_context, __context.call()).into()),
+            ::deno_core::webidl::ContextFn::new_borrowed(&|| format!("'{}' of '{}' ({})", #string_name, #ident_string, __context.call()).into()),
             &#options,
           )?
         } else {
@@ -256,6 +254,22 @@ impl TryFrom<Field> for DictionaryField {
       }
     }
 
+    if default_value.is_none() {
+      let is_option = if let Type::Path(path) = &value.ty {
+        if let Some(last) = path.path.segments.last() {
+          last.ident == "Option"
+        } else {
+          false
+        }
+      } else {
+        false
+      };
+
+      if is_option {
+        default_value = Some(syn::parse_quote!(None));
+      }
+    }
+
     Ok(Self {
       span,
       name: value.ident.unwrap(),

diff --git a/ops/webidl/enum.rs b/ops/webidl/enum.rs
@@ -16,20 +16,19 @@ use syn::Variant;
 
 pub fn get_body(
   ident_string: String,
+  ident: &Ident,
   data: DataEnum,
-) -> Result<TokenStream, Error> {
+) -> Result<(TokenStream, TokenStream), Error> {
   let variants = data
     .variants
     .into_iter()
     .map(get_variant_name)
     .collect::<Result<indexmap::IndexMap<_, _>, _>>()?;
 
-  let variants = variants
-    .into_iter()
-    .map(|(name, ident)| quote!(#name => Ok(Self::#ident)))
-    .collect::<Vec<_>>();
+  let names = variants.keys();
+  let idents = variants.values();
 
-  Ok(quote! {
+  let impl_body = quote! {
     let Ok(str) = __value.try_cast::<::deno_core::v8::String>() else {
       return Err(::deno_core::webidl::WebIdlError::new(
         __prefix,
@@ -39,10 +38,25 @@ pub fn get_body(
     };
 
     match str.to_rust_string_lossy(__scope).as_str() {
-      #(#variants),*,
+      #(#names => Ok(Self::#idents)),*,
       s => Err(::deno_core::webidl::WebIdlError::new(__prefix, __context, ::deno_core::webidl::WebIdlErrorKind::InvalidEnumVariant { converter: #ident_string, variant: s.to_string() }))
     }
-  })
+  };
+
+  let names = variants.keys();
+  let idents = variants.values();
+
+  let as_str = quote! {
+    impl #ident {
+      pub fn as_str(&self) -> &'static str {
+        match self {
+          #(Self::#idents => #names),*,
+        }
+      }
+    }
+  };
+
+  Ok((impl_body, as_str))
 }
 
 fn get_variant_name(value: Variant) -> Result<(String, Ident), Error> {

diff --git a/ops/webidl/mod.rs b/ops/webidl/mod.rs
@@ -3,9 +3,9 @@
 mod dictionary;
 mod r#enum;
 
-use proc_macro2::Ident;
 use proc_macro2::TokenStream;
 use quote::quote;
+use quote::ToTokens;
 use syn::parse::Parse;
 use syn::parse::ParseStream;
 use syn::parse2;
@@ -25,7 +25,9 @@ pub fn webidl(item: TokenStream) -> Result<TokenStream, Error> {
     .attrs
     .into_iter()
     .find_map(|attr| ConverterType::from_attribute(attr).transpose())
-    .ok_or_else(|| Error::new(span, "missing #[webidl] attribute"))??;
+    .ok_or_else(|| {
+      Error::new(span, "missing top-level #[webidl] attribute")
+    })??;
 
   let out = match input.data {
     Data::Struct(data) => match converter {
@@ -44,7 +46,13 @@ pub fn webidl(item: TokenStream) -> Result<TokenStream, Error> {
         ));
       }
       ConverterType::Enum => {
-        create_impl(ident, r#enum::get_body(ident_string, data)?)
+        let (body, as_str) = r#enum::get_body(ident_string, &ident, data)?;
+        let implementation = create_impl(ident, body);
+
+        quote! {
+          #implementation
+          #as_str
+        }
       }
     },
     Data::Union(_) => return Err(Error::new(span, "Unions are not supported")),
@@ -93,7 +101,7 @@ impl Parse for ConverterType {
   }
 }
 
-fn create_impl(ident: Ident, body: TokenStream) -> TokenStream {
+fn create_impl(ident: impl ToTokens, body: TokenStream) -> TokenStream {
   quote! {
     impl<'a> ::deno_core::webidl::WebIdlConverter<'a> for #ident {
       type Options = ();
@@ -115,7 +123,7 @@ fn create_impl(ident: Ident, body: TokenStream) -> TokenStream {
 #[cfg(test)]
 mod tests {
   use super::*;
-  use quote::ToTokens;
+  use proc_macro2::Ident;
   use std::path::PathBuf;
   use syn::punctuated::Punctuated;
   use syn::Item;