remove &str special handling

anonrig · anonrig · commit 153ece0f9f10 · 2026-01-02T15:29:38.000-05:00
diff --git a/src/rust/api/dns.rs b/src/rust/api/dns.rs
@@ -136,7 +136,7 @@ impl DnsUtil {
     /// `DnsParserError::InvalidHexString`
     /// `DnsParserError::ParseIntError`
     #[jsg_method]
-    pub fn parse_caa_record(&self, record: &str) -> Result<CaaRecord, DnsParserError> {
+    pub fn parse_caa_record(&self, record: String) -> Result<CaaRecord, DnsParserError> {
         // Let's remove "\\#" and the length of data from the beginning of the record
         let data = record.split_ascii_whitespace().collect::<Vec<_>>()[2..].to_vec();
         let critical = data[0].parse::<u8>()?;
@@ -191,7 +191,7 @@ impl DnsUtil {
     /// `DnsParserError::InvalidHexString`
     /// `DnsParserError::ParseIntError`
     #[jsg_method]
-    pub fn parse_naptr_record(&self, record: &str) -> jsg::Result<NaptrRecord, DnsParserError> {
+    pub fn parse_naptr_record(&self, record: String) -> jsg::Result<NaptrRecord, DnsParserError> {
         let data = record.split_ascii_whitespace().collect::<Vec<_>>()[1..].to_vec();
 
         let order_str = data[1..3].to_vec();
@@ -262,7 +262,7 @@ mod tests {
             _state: ResourceState::default(),
         };
         let record = dns_util
-            .parse_caa_record("\\# 15 00 05 69 73 73 75 65 70 6b 69 2e 67 6f 6f 67")
+            .parse_caa_record("\\# 15 00 05 69 73 73 75 65 70 6b 69 2e 67 6f 6f 67".to_owned())
             .unwrap();
 
         assert_eq!(record.critical, 0);
@@ -277,7 +277,8 @@ mod tests {
         };
         let record = dns_util
             .parse_caa_record(
-                "\\# 21 00 09 69 73 73 75 65 77 69 6c 64 6c 65 74 73 65 6e 63 72 79 70 74",
+                "\\# 21 00 09 69 73 73 75 65 77 69 6c 64 6c 65 74 73 65 6e 63 72 79 70 74"
+                    .to_owned(),
             )
             .unwrap();
 
@@ -291,8 +292,9 @@ mod tests {
         let dns_util = DnsUtil {
             _state: ResourceState::default(),
         };
-        let result =
-            dns_util.parse_caa_record("\\# 15 00 05 69 6e 76 61 6c 69 64 70 6b 69 2e 67 6f 6f 67");
+        let result = dns_util.parse_caa_record(
+            "\\# 15 00 05 69 6e 76 61 6c 69 64 70 6b 69 2e 67 6f 6f 67".to_owned(),
+        );
 
         assert!(result.is_err());
     }
@@ -303,7 +305,7 @@ mod tests {
             _state: ResourceState::default(),
         };
         let record = dns_util
-            .parse_naptr_record("\\# 37 15 b3 08 ae 01 73 0a 6d 79 2d 73 65 72 76 69 63 65 06 72 65 67 65 78 70 0b 72 65 70 6c 61 63 65 6d 65 6e 74 00")
+            .parse_naptr_record("\\# 37 15 b3 08 ae 01 73 0a 6d 79 2d 73 65 72 76 69 63 65 06 72 65 67 65 78 70 0b 72 65 70 6c 61 63 65 6d 65 6e 74 00".to_owned())
             .unwrap();
 
         assert_eq!(record.flags, "s");
diff --git a/src/rust/jsg-macros/BUILD.bazel b/src/rust/jsg-macros/BUILD.bazel
@@ -4,7 +4,6 @@ wd_rust_proc_macro(
     name = "jsg-macros",
     visibility = ["//visibility:public"],
     deps = [
-        "@crates_vendor//:proc-macro2",
         "@crates_vendor//:quote",
         "@crates_vendor//:syn",
     ],
diff --git a/src/rust/jsg-macros/README.md b/src/rust/jsg-macros/README.md
@@ -9,7 +9,7 @@ Generates the `jsg::Struct` and `jsg::Type` implementations for data structures.
 ```rust
 #[jsg_struct]
 pub struct CaaRecord {
-    pub critical: u8,
+    pub critical: f64,
     pub field: String,
     pub value: String,
 }
@@ -29,7 +29,7 @@ Parameters and return values are handled via the `jsg::Wrappable` trait. Any typ
 ```rust
 impl DnsUtil {
     #[jsg_method(name = "parseCaaRecord")]
-    pub fn parse_caa_record(&self, record: &str) -> Result<CaaRecord, DnsParserError> {
+    pub fn parse_caa_record(&self, record: String) -> Result<CaaRecord, DnsParserError> {
         // Errors are thrown as JavaScript exceptions
     }
 
@@ -65,7 +65,7 @@ pub struct MyUtil {
 #[jsg_resource]
 impl DnsUtil {
     #[jsg_method]
-    pub fn parse_caa_record(&self, record: &str) -> Result<CaaRecord, DnsParserError> {
+    pub fn parse_caa_record(&self, record: String) -> Result<CaaRecord, DnsParserError> {
         // implementation
     }
 }
diff --git a/src/rust/jsg-macros/lib.rs b/src/rust/jsg-macros/lib.rs
@@ -1,7 +1,4 @@
-mod types;
-
 use proc_macro::TokenStream;
-use proc_macro2::TokenStream as TokenStream2;
 use quote::ToTokens;
 use quote::quote;
 use syn::Data;
@@ -10,10 +7,8 @@ use syn::Fields;
 use syn::FnArg;
 use syn::ItemFn;
 use syn::ItemImpl;
-use syn::Type;
 use syn::parse_macro_input;
 use syn::spanned::Spanned;
-use types::is_str_ref;
 
 /// Generates `jsg::Struct` and `jsg::Type` implementations for data structures.
 ///
@@ -51,27 +46,31 @@ pub fn jsg_struct(attr: TokenStream, item: TokenStream) -> TokenStream {
         #input
 
         impl jsg::Type for #name {
-            type This = Self;
-
             fn class_name() -> &'static str { #class_name }
 
-            fn wrap<'a, 'b>(this: Self::This, lock: &'a mut jsg::Lock) -> jsg::v8::Local<'b, jsg::v8::Value>
-            where 'b: 'a,
+            fn is_exact(value: &jsg::v8::Local<jsg::v8::Value>) -> bool {
+                value.is_object()
+            }
+        }
+
+        impl jsg::Wrappable for #name {
+            fn wrap<'a, 'b>(self, lock: &'a mut jsg::Lock) -> jsg::v8::Local<'b, jsg::v8::Value>
+            where
+                'b: 'a,
             {
                 // TODO(soon): Use a precached ObjectTemplate instance to create the object,
                 // similar to how C++ JSG optimizes object creation. This would avoid recreating
                 // the object shape on every wrap() call and improve performance.
                 unsafe {
+                    let this = self;
                     let mut obj = lock.new_object();
                     #(#field_assignments)*
                     obj.into()
                 }
             }
+        }
 
-            fn is_exact(value: &jsg::v8::Local<jsg::v8::Value>) -> bool {
-                value.is_object()
-            }
-
+        impl jsg::Unwrappable for #name {
             fn unwrap(_isolate: jsg::v8::IsolatePtr, _value: jsg::v8::Local<jsg::v8::Value>) -> Self {
                 unimplemented!("Struct unwrap is not yet supported")
             }
@@ -104,13 +103,15 @@ pub fn jsg_method(_attr: TokenStream, item: TokenStream) -> TokenStream {
         })
         .collect();
 
-    let (unwraps, arg_refs): (Vec<_>, Vec<_>) = params
+    let (unwraps, arg_names): (Vec<_>, Vec<_>) = params
         .iter()
         .enumerate()
         .map(|(i, ty)| {
             let arg = syn::Ident::new(&format!("arg{i}"), fn_name.span());
-            let (unwrap, arg_ref) = generate_unwrap(&arg, ty, i);
-            (unwrap, arg_ref)
+            let unwrap = quote! {
+                let Some(#arg) = <#ty as jsg::Unwrappable>::try_unwrap(&mut lock, args.get(#i)) else { return; };
+            };
+            (unwrap, arg)
         })
         .unzip();
 
@@ -124,29 +125,13 @@ pub fn jsg_method(_attr: TokenStream, item: TokenStream) -> TokenStream {
             #(#unwraps)*
             let this = args.this();
             let self_ = jsg::unwrap_resource::<Self>(&mut lock, this);
-            let result = self_.#fn_name(#(#arg_refs),*);
-            jsg::Wrappable::wrap_return(result, &mut lock, &mut args);
+            let result = self_.#fn_name(#(#arg_names),*);
+            args.set_return_value(jsg::Wrappable::wrap(result, &mut lock));
         }
     }
     .into()
 }
 
-fn generate_unwrap(arg: &syn::Ident, ty: &Type, index: usize) -> (TokenStream2, TokenStream2) {
-    // &str: unwrap as String and borrow
-    if is_str_ref(ty) {
-        let unwrap = quote! {
-            let Some(#arg) = <String as jsg::Wrappable>::try_unwrap(&mut lock, args.get(#index)) else { return; };
-        };
-        return (unwrap, quote! { &#arg });
-    }
-
-    // All types use Wrappable::try_unwrap
-    let unwrap = quote! {
-        let Some(#arg) = <#ty as jsg::Wrappable>::try_unwrap(&mut lock, args.get(#index)) else { return; };
-    };
-    (unwrap, quote! { #arg })
-}
-
 /// Generates boilerplate for JSG resources.
 ///
 /// On structs: generates `jsg::Type` and `ResourceTemplate`.
@@ -174,20 +159,25 @@ pub fn jsg_resource(attr: TokenStream, item: TokenStream) -> TokenStream {
 
         #[automatically_derived]
         impl jsg::Type for #name {
-            type This = jsg::Ref<Self>;
-
             fn class_name() -> &'static str { #class_name }
 
-            fn wrap<'a, 'b>(_this: Self::This, _lock: &'a mut jsg::Lock) -> jsg::v8::Local<'b, jsg::v8::Value>
-            where 'b: 'a,
-            {
-                todo!("Implement wrap for jsg::Resource")
-            }
-
             fn is_exact(value: &jsg::v8::Local<jsg::v8::Value>) -> bool {
                 value.is_object()
             }
+        }
 
+        #[automatically_derived]
+        impl jsg::Wrappable for #name {
+            fn wrap<'a, 'b>(self, _lock: &'a mut jsg::Lock) -> jsg::v8::Local<'b, jsg::v8::Value>
+            where
+                'b: 'a,
+            {
+                unimplemented!("Resource wrap is not yet supported")
+            }
+        }
+
+        #[automatically_derived]
+        impl jsg::Unwrappable for #name {
             fn unwrap(_isolate: jsg::v8::IsolatePtr, _value: jsg::v8::Local<jsg::v8::Value>) -> Self {
                 unimplemented!("Resource unwrap is not yet supported")
             }
diff --git a/src/rust/jsg-macros/types.rs b/src/rust/jsg-macros/types.rs
diff --git a/src/rust/jsg-test/tests/jsg_struct.rs b/src/rust/jsg-test/tests/jsg_struct.rs
@@ -1,7 +1,7 @@
 use jsg::Error;
 use jsg::ExceptionType;
 use jsg::Lock;
-use jsg::Type;
+use jsg::Wrappable;
 use jsg::v8;
 use jsg::v8::ToLocalValue;
 use jsg_macros::jsg_struct;
@@ -31,7 +31,7 @@ fn objects_can_be_wrapped_and_unwrapped() {
         let instance = TestStruct {
             str: "test".to_owned(),
         };
-        let wrapped = TestStruct::wrap(instance, &mut lock);
+        let wrapped = instance.wrap(&mut lock);
         let mut obj: v8::Local<'_, v8::Object> = wrapped.into();
         assert!(obj.has(&mut lock, "str"));
         let str_value = obj.get(&mut lock, "str");
@@ -54,7 +54,7 @@ fn struct_with_multiple_properties() {
             age: 30,
             active: "true".to_owned(),
         };
-        let wrapped = MultiPropertyStruct::wrap(instance, &mut lock);
+        let wrapped = instance.wrap(&mut lock);
         let obj: v8::Local<'_, v8::Object> = wrapped.into();
 
         assert!(obj.has(&mut lock, "name"));
@@ -142,7 +142,7 @@ fn nested_object_properties() {
         let inner_instance = NestedStruct {
             inner: "nested value".to_owned(),
         };
-        let inner_wrapped = NestedStruct::wrap(inner_instance, &mut lock);
+        let inner_wrapped = inner_instance.wrap(&mut lock);
         outer.set(&mut lock, "nested", inner_wrapped);
 
         assert!(outer.has(&mut lock, "nested"));
diff --git a/src/rust/jsg-test/tests/resource_callback.rs b/src/rust/jsg-test/tests/resource_callback.rs
@@ -26,7 +26,7 @@ struct EchoResource {
 #[expect(clippy::unnecessary_wraps)]
 impl EchoResource {
     #[jsg_method]
-    pub fn echo(&self, message: &str) -> Result<String, String> {
+    pub fn echo(&self, message: String) -> Result<String, String> {
         Ok(format!("{}{}", self.prefix, message))
     }
 }
diff --git a/src/rust/jsg/lib.rs b/src/rust/jsg/lib.rs
@@ -14,6 +14,7 @@ pub mod v8;
 mod wrappable;
 
 pub use v8::ffi::ExceptionType;
+pub use wrappable::Unwrappable;
 pub use wrappable::Wrappable;
 
 #[cxx::bridge(namespace = "workerd::rust::jsg")]
@@ -230,6 +231,11 @@ impl<T: Type> NonCoercible<T> {
     pub fn new(value: T) -> Self {
         Self { value }
     }
+
+    /// Consumes the wrapper and returns the inner value.
+    pub fn into_inner(self) -> T {
+        self.value
+    }
 }
 
 impl<T: Type> From<T> for NonCoercible<T> {
@@ -366,32 +372,30 @@ impl<T: Resource> Clone for Ref<T> {
     }
 }
 
+/// Provides metadata about Rust types exposed to JavaScript.
+///
+/// This trait provides type information used for error messages, memory tracking,
+/// and type validation (for `NonCoercible<T>`). The actual conversion logic is in
+/// `Wrappable` (Rust → JS) and `Unwrappable` (JS → Rust).
+///
 /// TODO: Implement `memory_info(jsg::MemoryTracker)`
 pub trait Type: Sized {
-    /// The input type for [`wrap()`](Self::wrap). For primitive types this is typically `Self`,
-    /// but resource types may use `Ref<Self>` or other wrapper types.
-    type This;
-
+    /// The JavaScript class name for this type (used in error messages).
     fn class_name() -> &'static str;
+
     /// Same as jsgGetMemoryName
     fn memory_name() -> &'static str {
         std::any::type_name::<Self>()
     }
+
     /// Same as jsgGetMemorySelfSize
     fn memory_self_size() -> usize {
         std::mem::size_of::<Self>()
     }
-    /// Wraps this struct as a JavaScript value by deep-copying its fields.
-    fn wrap<'a, 'b>(this: Self::This, lock: &'a mut Lock) -> v8::Local<'b, v8::Value>
-    where
-        'b: 'a;
 
     /// Returns true if the V8 value is exactly this type (no coercion).
     /// Used by `NonCoercible<T>` to reject values that would require coercion.
     fn is_exact(value: &v8::Local<v8::Value>) -> bool;
-
-    /// Unwraps a V8 value into this type without coercion.
-    fn unwrap(isolate: v8::IsolatePtr, value: v8::Local<v8::Value>) -> Self;
 }
 
 pub enum Member {
diff --git a/src/rust/jsg/wrappable.rs b/src/rust/jsg/wrappable.rs

Original file line number	Diff line number	Diff line change
@@ -9,7 +9,7 @@ Generates the `jsg::Struct` and `jsg::Type` implementations for data structures.
`9`	`9`	```rust
`10`	`10`	`#[jsg_struct]`
`11`	`11`	`pub struct CaaRecord {`
`12`		`- pub critical: u8,`
	`12`	`+ pub critical: f64,`
`13`	`13`	`pub field: String,`
`14`	`14`	`pub value: String,`
`15`	`15`	`}`
@@ -29,7 +29,7 @@ Parameters and return values are handled via the `jsg::Wrappable` trait. Any typ
`29`	`29`	```rust
`30`	`30`	`impl DnsUtil {`
`31`	`31`	`#[jsg_method(name = "parseCaaRecord")]`
`32`		`- pub fn parse_caa_record(&self, record: &str) -> Result<CaaRecord, DnsParserError> {`
	`32`	`+ pub fn parse_caa_record(&self, record: String) -> Result<CaaRecord, DnsParserError> {`
`33`	`33`	`// Errors are thrown as JavaScript exceptions`
`34`	`34`	`}`
`35`	`35`
`@@ -65,7 +65,7 @@ pub struct MyUtil {`
`65`	`65`	`#[jsg_resource]`
`66`	`66`	`impl DnsUtil {`
`67`	`67`	`#[jsg_method]`
`68`		`- pub fn parse_caa_record(&self, record: &str) -> Result<CaaRecord, DnsParserError> {`
	`68`	`+ pub fn parse_caa_record(&self, record: String) -> Result<CaaRecord, DnsParserError> {`
`69`	`69`	`// implementation`
`70`	`70`	`}`
`71`	`71`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ struct EchoResource {`
`26`	`26`	`#[expect(clippy::unnecessary_wraps)]`
`27`	`27`	`impl EchoResource {`
`28`	`28`	`#[jsg_method]`
`29`		`- pub fn echo(&self, message: &str) -> Result<String, String> {`
	`29`	`+ pub fn echo(&self, message: String) -> Result<String, String> {`
`30`	`30`	`Ok(format!("{}{}", self.prefix, message))`
`31`	`31`	`}`
`32`	`32`	`}`