implement jsg::NonCoercible in Rust (#5730)

* implement jsg::NonCoercible in Rust

* add resource callback tests

* simplify implementation and improve error message

* rename into_ffi() to as_ffi()

* avoid unnecessary string copies in resources

* address pr review

Author: anonrig

src/rust/api/lib.rs

@@ -48,7 +48,7 @@ mod tests {
     #[test]
     fn test_wrap_resource_equality() {
         let harness = Harness::new();
-        harness.run_in_context(|isolate| unsafe {
+        harness.run_in_context(|isolate, _ctx| unsafe {
             let mut lock = jsg::Lock::from_isolate_ptr(isolate);
             let dns_util = jsg::Ref::new(DnsUtil {
                 _state: ResourceState::default(),

src/rust/jsg-macros/README.md

@@ -20,24 +20,6 @@ pub struct MyRecord {
 }
 ```
 
-## `#[jsg_method]`
-
-Generates FFI callback functions for JSG resource methods. The `name` parameter is optional and defaults to converting the method name from `snake_case` to `camelCase`.
-
-```rust
-impl DnsUtil {
-    #[jsg_method(name = "parseCaaRecord")]
-    pub fn parse_caa_record(&self, record: &str) -> Result<CaaRecord, DnsParserError> {
-        // implementation
-    }
-
-    #[jsg_method]
-    pub fn parse_naptr_record(&self, record: &str) -> Result<NaptrRecord, DnsParserError> {
-        // Exposed as "parseNaptrRecord" in JavaScript
-    }
-}
-```
-
 ## `#[jsg_resource]`
 
 Generates boilerplate for JSG resources. Applied to both struct definitions and impl blocks. Automatically implements `jsg::Type::class_name()` using the struct name, or a custom name if provided via the `name` parameter.

src/rust/jsg-macros/lib.rs

@@ -75,7 +75,7 @@ pub fn jsg_struct(attr: TokenStream, item: TokenStream) -> TokenStream {
                 .expect("Named fields must have identifiers")
                 .to_string();
             Some(quote! {
-                let #field_name = jsg::v8::ToLocalValue::to_local(&self.#field_name, lock);
+                let #field_name = jsg::v8::ToLocalValue::to_local(&this.#field_name, lock);
                 obj.set(lock, #field_name_str, #field_name);
             })
         } else {
@@ -87,11 +87,13 @@ 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>(&self, lock: &'a mut jsg::Lock) -> jsg::v8::Local<'b, jsg::v8::Value>
+            fn wrap<'a, 'b>(this: Self::This, lock: &'a mut jsg::Lock) -> jsg::v8::Local<'b, jsg::v8::Value>
             where
                 'b: 'a,
             {
@@ -104,6 +106,15 @@ pub fn jsg_struct(attr: TokenStream, item: TokenStream) -> TokenStream {
                     obj.into()
                 }
             }
+
+            fn is_exact(value: &jsg::v8::Local<jsg::v8::Value>) -> bool {
+                value.is_object()
+            }
+
+            fn unwrap(_isolate: jsg::v8::IsolatePtr, _value: jsg::v8::Local<jsg::v8::Value>) -> Self {
+                // TODO(soon): Implement proper unwrapping for struct types
+                unimplemented!("Struct unwrap is not yet supported")
+            }
         }
 
         impl jsg::Struct for #name {
@@ -119,16 +130,47 @@ pub fn jsg_struct(attr: TokenStream, item: TokenStream) -> TokenStream {
 /// Creates a `{method_name}_callback` extern "C" function that bridges JavaScript and Rust.
 /// If no name is provided, automatically converts `snake_case` to `camelCase`.
 ///
+/// # Supported Parameter Types
+///
+/// | Type       | `T`           | `NonCoercible<T>` |
+/// |------------|---------------|-------------------|
+/// | `&str`     | ✅ Supported  | ❌                |
+/// | `T: Type`  | ❌            | ✅ Supported      |
+///
+/// Any type implementing [`jsg::Type`] can be used with `NonCoercible<T>`.
+/// Built-in types include `String`, `bool`, and `f64`.
+///
+/// # `NonCoercible<T>` Parameters
+///
+/// Use `NonCoercible<T>` when you want to accept a value only if it's already the expected
+/// type, without JavaScript's automatic type coercion:
+///
+/// ```ignore
+/// #[jsg_method]
+/// pub fn strict_string(&self, param: NonCoercible<String>) -> Result<String, Error> {
+///     // Only accepts actual strings - passing null/undefined/numbers will throw
+///     // Access via Deref: *param, or via AsRef: param.as_ref()
+///     Ok(param.as_ref().clone())
+/// }
+///
+/// #[jsg_method]
+/// pub fn strict_bool(&self, param: NonCoercible<bool>) -> Result<bool, Error> {
+///     // Only accepts actual booleans
+///     Ok(*param)
+/// }
+/// ```
+///
 /// # Example
-/// ```rust
+///
+/// ```ignore
 /// // With explicit name
-/// #[jsg::method(name = "parseRecord")]
+/// #[jsg_method(name = "parseRecord")]
 /// pub fn parse_record(&self, data: &str) -> Result<Record, Error> {
 ///     // implementation
 /// }
 ///
 /// // Without name - automatically becomes "parseRecord"
-/// #[jsg::method]
+/// #[jsg_method]
 /// pub fn parse_record(&self, data: &str) -> Result<Record, Error> {
 ///     // implementation
 /// }
@@ -210,11 +252,34 @@ fn is_str_reference(ty: &Type) -> bool {
     }
 }
 
+/// Returns true if the type is `NonCoercible<T>`.
+fn is_non_coercible(ty: &Type) -> bool {
+    let Type::Path(type_path) = ty else {
+        return false;
+    };
+    type_path
+        .path
+        .segments
+        .last()
+        .is_some_and(|seg| seg.ident == "NonCoercible")
+}
+
 fn generate_unwrap_code(
     arg_name: &syn::Ident,
     ty: &Type,
     index: usize,
 ) -> quote::__private::TokenStream {
+    // Check for NonCoercible<T> types
+    if is_non_coercible(ty) {
+        return quote! {
+            let Some(#arg_name) = (unsafe {
+                <#ty>::unwrap(&mut lock, args.get(#index))
+            }) else {
+                return;
+            };
+        };
+    }
+
     if is_str_reference(ty) {
         quote! {
             let #arg_name = unsafe {
@@ -223,7 +288,7 @@ fn generate_unwrap_code(
         }
     } else {
         quote! {
-            compile_error!("Unsupported parameter type for jsg::method. Currently only &str is supported.");
+            compile_error!("Unsupported parameter type for jsg::method. Currently only &str and NonCoercible<T> are supported.");
         }
     }
 }
@@ -294,16 +359,27 @@ 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>(&self, lock: &'a mut jsg::Lock) -> jsg::v8::Local<'b, jsg::v8::Value>
+            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()
+            }
+
+            fn unwrap(_isolate: jsg::v8::IsolatePtr, _value: jsg::v8::Local<jsg::v8::Value>) -> Self {
+                // TODO(soon): Implement proper unwrapping for resource types
+                unimplemented!("Resource unwrap is not yet supported")
+            }
         }
 
         #[automatically_derived]
@@ -353,7 +429,7 @@ fn generate_resource_impl(impl_block: &ItemImpl) -> TokenStream {
 
                     method_registrations.push(quote! {
                         jsg::Member::Method {
-                            name: #js_name,
+                            name: #js_name.to_owned(),
                             callback: Self::#callback_name,
                         }
                     });

src/rust/jsg-test/ffi.c++

@@ -1,6 +1,8 @@
 #include "ffi.h"
 
 #include <workerd/jsg/setup.h>
+#include <workerd/rust/jsg-test/lib.rs.h>
+#include <workerd/rust/jsg/ffi-inl.h>
 #include <workerd/rust/jsg/lib.rs.h>
 #include <workerd/rust/jsg/v8.rs.h>
 
@@ -44,11 +46,81 @@ kj::Own<TestHarness> create_test_harness() {
       [](::workerd::jsg::V8StackScope& stackScope) { return kj::heap<TestHarness>(stackScope); });
 }
 
-void TestHarness::run_in_context(::rust::Fn<void(Isolate*)> callback) const {
+EvalContext::EvalContext(v8::Isolate* isolate, v8::Local<v8::Context> context)
+    : v8Isolate(isolate),
+      v8Context(isolate, context) {}
+
+void EvalContext::set_global(::rust::Str name, ::workerd::rust::jsg::Local value) const {
+  auto ctx = v8Context.Get(v8Isolate);
+  auto key = ::workerd::jsg::check(v8::String::NewFromUtf8(
+      v8Isolate, name.data(), v8::NewStringType::kNormal, static_cast<int>(name.size())));
+  auto v8Value = ::workerd::rust::jsg::local_from_ffi<v8::Value>(kj::mv(value));
+  ::workerd::jsg::check(ctx->Global()->Set(ctx, key, v8Value));
+}
+
+EvalResult EvalContext::eval(::rust::Str code) const {
+  EvalResult result;
+  result.success = false;
+
+  v8::Local<v8::Context> ctx = v8Context.Get(v8Isolate);
+
+  v8::Local<v8::String> source = ::workerd::jsg::check(v8::String::NewFromUtf8(
+      v8Isolate, code.data(), v8::NewStringType::kNormal, static_cast<int>(code.size())));
+
+  v8::Local<v8::Script> script;
+  if (!v8::Script::Compile(ctx, source).ToLocal(&script)) {
+    result.success = false;
+    result.result_type = "CompileError";
+    result.result_value = "Failed to compile script";
+    return result;
+  }
+
+  v8::TryCatch catcher(v8Isolate);
+
+  v8::Local<v8::Value> value;
+  if (script->Run(ctx).ToLocal(&value)) {
+    v8::String::Utf8Value type(v8Isolate, value->TypeOf(v8Isolate));
+    v8::String::Utf8Value valueStr(v8Isolate, value);
+
+    result.success = true;
+    result.result_type = *type;
+    result.result_value = *valueStr;
+  } else if (catcher.HasCaught()) {
+    v8::String::Utf8Value message(v8Isolate, catcher.Exception());
+
+    result.success = false;
+    result.result_type = "throws";
+    result.result_value = *message ? *message : "Unknown error";
+  } else {
+    result.success = false;
+    result.result_type = "error";
+    result.result_value = "Returned empty handle but didn't throw exception";
+  }
+
+  return result;
+}
+
+void TestHarness::run_in_context(::rust::Fn<void(Isolate*, EvalContext&)> callback) const {
+  isolate->runInLockScope([&](TestIsolate::Lock& lock) {
+    auto context = lock.newContext<TestContext>();
+    v8::Local<v8::Context> v8Context = context.getHandle(lock.v8Isolate);
+    v8::Context::Scope contextScope(v8Context);
+
+    EvalContext evalContext(lock.v8Isolate, v8Context);
+    callback(lock.v8Isolate, evalContext);
+  });
+}
+
+void TestHarness::set_global(::rust::Str name, ::workerd::rust::jsg::Local value) const {
   isolate->runInLockScope([&](TestIsolate::Lock& lock) {
     auto context = lock.newContext<TestContext>();
-    v8::Context::Scope contextScope(context.getHandle(isolate->getIsolate()));
-    callback(isolate->getIsolate());
+    v8::Local<v8::Context> v8Context = context.getHandle(lock.v8Isolate);
+    v8::Context::Scope contextScope(v8Context);
+
+    v8::Local<v8::String> key = ::workerd::jsg::check(v8::String::NewFromUtf8(
+        lock.v8Isolate, name.data(), v8::NewStringType::kNormal, static_cast<int>(name.size())));
+    v8::Local<v8::Value> v8Value = ::workerd::rust::jsg::local_from_ffi<v8::Value>(kj::mv(value));
+    ::workerd::jsg::check(v8Context->Global()->Set(v8Context, key, v8Value));
   });
 }
 

src/rust/jsg-test/ffi.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <workerd/jsg/jsg.h>
+#include <workerd/rust/jsg/ffi.h>
 #include <workerd/rust/jsg/v8.rs.h>
 
 #include <kj-rs/kj-rs.h>
@@ -17,14 +18,31 @@ namespace rust::jsg_test {
 
 using Isolate = v8::Isolate;
 
+struct EvalResult;
+
+class EvalContext {
+ public:
+  EvalContext(v8::Isolate* isolate, v8::Local<v8::Context> context);
+
+  EvalResult eval(::rust::Str code) const;
+  void set_global(::rust::Str name, ::workerd::rust::jsg::Local value) const;
+
+  v8::Isolate* v8Isolate;
+  v8::Global<v8::Context> v8Context;
+};
+
 // Testing harness that provides a simple V8 isolate for Rust JSG testing
 class TestHarness {
  public:
   // Use create_test_harness() instead - it ensures proper V8 stack scope
   TestHarness(::workerd::jsg::V8StackScope& stackScope);
 
   // Runs a callback within a proper V8 context and stack scope
-  void run_in_context(::rust::Fn<void(Isolate*)> callback) const;
+  // The callback receives both the isolate and a context that can be used with eval
+  void run_in_context(::rust::Fn<void(Isolate*, EvalContext&)> callback) const;
+
+  // Sets a global variable on the context
+  void set_global(::rust::Str name, ::workerd::rust::jsg::Local value) const;
 
  private:
   mutable kj::Own<TestIsolate> isolate;