From a56bdff85c10d52e6b3aa91dd9d0378816536450 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Tue, 27 Oct 2026 13:41:17 -0700 Subject: [PATCH] Add NullOrUndefined to Rust JSG Represent optional nullable values as Some(T), Null, or Undefined without nesting Option and Nullable. Support JavaScript conversion, omitted method arguments, struct-field omission, and GC tracing. Add state predicates, borrowing and mapping helpers, and conversions preserving all three states. Integrate the type with Lenient, including its to_parts() return value, while retaining Option> conversions. Recommend NullOrUndefined throughout the API docs, macro reference, review checklist, and agent guidance. Use consuming, fallible ToJS conversions for struct fields and preserve upstream field tracing, Dict support, numeric conversions, and Web IDL function lengths. Cover non-Clone payload conversion errors and optional-argument lengths under both compatibility settings. Validation: Rust formatting and Clippy pass for all Rust rule targets under src/rust, including unit and integration tests. The full Rust suite passes all 117 applicable test targets; two sanitizer targets are skipped. --- docs/reference/rust-review-checklist.md | 17 +- src/rust/AGENTS.md | 3 +- src/rust/jsg-macros/README.md | 15 +- src/rust/jsg-macros/lib.rs | 54 +++-- src/rust/jsg-macros/utils-test.rs | 14 ++ src/rust/jsg-macros/utils.rs | 6 + src/rust/jsg-test/tests/jsg_struct.rs | 30 +++ src/rust/jsg-test/tests/mod.rs | 1 + src/rust/jsg-test/tests/null_or_undefined.rs | 217 +++++++++++++++++++ src/rust/jsg-test/tests/nullable.rs | 4 +- src/rust/jsg/README.md | 33 +++ src/rust/jsg/lenient.rs | 49 ++++- src/rust/jsg/lib.rs | 2 + src/rust/jsg/null_or_undefined-test.rs | 86 ++++++++ src/rust/jsg/null_or_undefined.rs | 115 ++++++++++ src/rust/jsg/nullable.rs | 24 +- src/rust/jsg/wrappable-test.rs | 1 + src/rust/jsg/wrappable.rs | 71 +++++- 18 files changed, 682 insertions(+), 60 deletions(-) create mode 100644 src/rust/jsg-test/tests/null_or_undefined.rs create mode 100644 src/rust/jsg/null_or_undefined-test.rs create mode 100644 src/rust/jsg/null_or_undefined.rs diff --git a/docs/reference/rust-review-checklist.md b/docs/reference/rust-review-checklist.md index 23fe507c509..e12d0952674 100644 --- a/docs/reference/rust-review-checklist.md +++ b/docs/reference/rust-review-checklist.md @@ -104,8 +104,12 @@ Rust types exposed to JavaScript via the JSG bindings follow these patterns: `v8::Local`. `Option` maps to `T | undefined`; like `jsg::Optional`, `null` converts as `T`. `Nullable` maps to `T | null` and, like `kj::Maybe`, coerces `undefined` to `null` on - input, but a method argument is still required. Use `Option>` for an optional - `T | null | undefined` that keeps `undefined` distinct from `null`. + input, but a method argument is still required. Prefer `NullOrUndefined` to + `Option>` for an optional `T | null | undefined`, with distinct `Some(T)`, + `Null`, and `Undefined` states. Omitted method arguments become `Undefined`. Use + `Lenient` only when shallow type mismatches should also be accepted; converting it to + `NullOrUndefined` maps `Unconvertable` to `Undefined`, while nested conversion errors + still propagate. `String`/`&str` map to JS strings. Integers and `#[jsg_struct]`s convert like C++ JSG's integers and dictionaries: out-of-range integers throw, and an absent struct field reads as `undefined`. @@ -118,10 +122,11 @@ Rust types exposed to JavaScript via the JSG bindings follow these patterns: which GC never visits and which keeps its handle inline. A handle that a resource owns is a `jsg::v8::TracedGlobal`, which GC traces so that a cycle through it can be collected, and which keeps its handles in a box. `Global` has no `Traced` impl, so it cannot be a resource field. -- **GC tracing**: `Ref`, `Option>`, and `Nullable>` fields on `#[jsg_resource]` - structs are automatically traced during GC. `WeakRef` fields are not traced (no-op). Verify - that any resource holding a `Ref` or `Nullable>` to another resource is properly - traced — missing traces cause use-after-free when the child is collected prematurely. +- **GC tracing**: `jsg::Rc` fields on `#[jsg_resource]` structs are automatically traced + during GC, including through `Option`, `Nullable`, and `NullOrUndefined` wrappers when + `Some`. `Lenient` traces both `Some` and `Unconvertable`. `jsg::Weak` fields are not + traced (no-op). Verify that every resource reference is properly traced — missing traces + cause use-after-free when the child is collected prematurely. ### Rust JSG Safety Rules diff --git a/src/rust/AGENTS.md b/src/rust/AGENTS.md index 65cb9fd2310..59dec286970 100644 --- a/src/rust/AGENTS.md +++ b/src/rust/AGENTS.md @@ -34,13 +34,14 @@ _Snapshot — the set drifts as crates come and go; `bazel query //src/rust/...` - **CXX bridge**: `#[cxx::bridge(namespace = "workerd::rust::")]` with companion `ffi.c++`/`ffi.h` files - **Namespace**: `workerd::rust::*` for crates in this directory, except `python-parser` → `edgeworker::rust::python_parser`; a crate living beside C++ uses that C++'s namespace (e.g. `workerd::server::cli`) - **Errors**: `thiserror` for library crates; `jsg::Error` with `ExceptionType` for JSG-facing crates +- **JSG nullish values**: `Option` is `T | undefined`; `Nullable` is `T | null` (coerces `undefined` to `Null`, but method arguments remain required). Prefer `NullOrUndefined` to `Option>` for three distinct `Some(T)`, `Null`, and `Undefined` states; omitted arguments become `Undefined`. `Lenient` additionally preserves shallow mismatches as `Unconvertable`; converting it to `NullOrUndefined` maps those to `Undefined`, and `Lenient::to_parts()` also returns the original JS value. See [Rust JSG nullish values](jsg/README.md#nullablet-and-nullorundefinedt). - **JSG resources**: `#[jsg_resource]` on struct + impl block; `#[jsg_method]` auto-converts `snake_case` → `camelCase`; methods with `&self` become instance methods, methods without a receiver become static methods, and any other receiver is a compile error (JavaScript can re-enter a resource, so state that methods change goes in `Cell`/`RefCell` fields; never hold a `RefCell` borrow across a call that can run JavaScript, nor a `borrow_mut()` of one holding GC references across a call that can allocate on the V8 heap, wrap the resource or create or drop a `jsg::Rc` to it, since tracing skips a mutably borrowed `RefCell`); a method or accessor can take `this: jsg::Rc` as its first parameter instead of `&self` to get a `jsg::Rc` of its resource (like `JSG_THIS`) to hand to a callback; don't share the resource's state with a callback through an `std::rc::Rc` (see `jsg/README.md` §Reaching the resource from a callback); an eligible `#[jsg_method]` also gets a V8 Fast API callback (types in `jsg/fast_api.rs`) and must not call into V8; a `#[jsg_constructor]` returns `Self` or `Result` (an `Err` is thrown) and throws `TypeError` when called without `new`; `#[jsg_static_constant]` on `const` items exposes read-only numeric constants on both constructor and prototype (name kept as-is, no camelCase); `#[jsg_resource(nested_types(Foo, Name = Bar))]` on the impl block puts other resources' constructors on the prototype (`JSG_NESTED_TYPE`); resources integrate with GC via `Traced` + `GarbageCollected`: every named field is traced via `Traced::trace(&self.field, visitor)` and all non-traceable types use no-op `Traced` impls - **Resources created by C++**: C++ creates a Rust resource through an `extern "Rust"` function of the crate that defines it, which returns the wrapper through `jsg::v8::run_ffi_callback`; C++ holds the wrapper (a `jsg::JsRef`), since it has no typed reference to a Rust resource, and checks a value's type through one that returns `jsg::resource::ffi_is::`, a `kj::Maybe` that is none when an exception is pending. See `jsg/README.md` §Creating resources from C++. - **Custom JSG registration**: `#[jsg_resource(custom_members)]` on an impl generates member factories but leaves `impl Resource` to the author. `Resource::members(flags)` uses ordinary Rust to select members, order, and property placement. Factories are `_member()` (constant names lowercased); ordinary property factories are `_member(PropertyPlacement)` (`Prototype`, `Instance` or `LazyInstance`) and include the paired setter. See `jsg-macros/README.md` §Custom member registration. - **JSG properties**: three property macros on `#[jsg_resource]` impl blocks — `#[jsg_property(prototype|instance|lazy_instance [, name = "..."] [, readonly])]` (`prototype` registers an accessor on the prototype, equivalent to `JSG_PROTOTYPE_PROPERTY`; `instance` registers an own data property via V8 `SetNativeDataProperty`, equivalent to `JSG_INSTANCE_PROPERTY`; `lazy_instance` registers an own data property via V8 `SetLazyDataProperty` whose getter runs once, on first access, equivalent to `JSG_LAZY_INSTANCE_PROPERTY`: it has no setter, is writable unless `readonly` (`JSG_LAZY_READONLY_INSTANCE_PROPERTY`); declaration order sets `Object.keys()` order for automatic registration; `readonly` is a compile-time check preventing a paired setter; `name = "..."` overrides the JS name; prefer `prototype` in almost all cases), `#[jsg_inspect_property]` (registered under a unique symbol, invisible to normal enumeration and string-key lookup, surfaced by `node:util` `inspect()`, equivalent to `JSG_INSPECT_PROPERTY`), and `#[jsg_wildcard_property]` (answers string-named properties the object and its prototypes lack, at most one per type, equivalent to `JSG_WILDCARD_PROPERTY`; see `jsg/README.md`); setter auto-detected from `set_` prefix; read-only when no setter present; getter/setter `.length` and `.name` are set correctly when `spec_compliant_property_attributes` compat flag is enabled - **`jsg::v8::Global` / `TracedGlobal`**: persistent handles. A `Global` is for handles Rust owns outside resources: GC never visits it, so it is always strong, holds its handle inline (no box), has no `Traced` impl (it cannot be a resource field), and is `Send` and `Sync`. A `TracedGlobal` is for resource fields, which GC traces to collect cycles; it is `Send` (not `Sync`). Either may be dropped on any thread. Under the isolate lock its strong handle is reset at once; without the lock it is queued and reset at the next `IsolateBase::Lock`, through `IsolateBase::applyDeferredActions()`; once the `Realm` is dropped (under the lock, before `Dispose`) the handles are left to `Dispose`. See `jsg/isolate_state.rs`. Hand-written FFI that resets a V8 handle directly must hold the lock. Rust never resets a `TracedGlobal`'s traced reference: every GC visit lets go of it and makes the strong handle strong again (`ClearWeak`) before C++ installs a new one if needed, and a drop just lets go of it; V8 frees untraced nodes at the next full GC. See `GcVisitor::visit_traced_global`. A `TracedGlobal` keeps its handles in a `Box`, since V8 holds the address of a weak handle's slot; the slot of a `TracedGlobal` dropped after the `Realm` is kept until the isolate is disposed - **JSG parameters**: function `.length` is the Web IDL required-argument count under `spec_compliant_property_attributes` (`this: jsg::Rc` and a leading `&mut Lock` are not counted; counting stops at the first type whose `FromJS::is_optional()` returns true), so put optional parameters last -- **`Traced`**: core tracing trait in `jsg::wrappable`; built-ins include no-op impls for primitives/value types and delegating impls for wrappers/collections (`Option`, `Nullable`, `Vec`, maps/sets, `Cell`, `jsg::Rc`, `jsg::Weak`, `jsg::v8::TracedGlobal`); `jsg::v8::Global` deliberately has none +- **`Traced`**: core tracing trait in `jsg::wrappable`; built-ins include no-op impls for primitives/value types and delegating impls for wrappers/collections (`Option`, `Nullable`, `NullOrUndefined`, `Lenient`, `Vec`, maps/sets, `Cell`, `jsg::Rc`, `jsg::Weak`, `jsg::v8::TracedGlobal`); `jsg::v8::Global` deliberately has none - **Sequences to JavaScript**: `Vec` converts to an `Array`, numbers included (`vec![1u8, 2]` makes `[1, 2]`). A typed array or an `ArrayBuffer` is an explicit conversion of a slice: `to_js_typed_array(lock)` makes the typed array of the element type, and `to_js_array_buffer(lock)` makes an `ArrayBuffer` of bytes (`[u8]` only), as C++ JSG returns for a `kj::Array`. See `jsg/README.md` §Arrays, typed arrays and `ArrayBuffer`s - **`#[jsg_resource(custom_trace)]`**: suppresses the auto-generated `Traced` impl so the user can write their own; `GarbageCollected` (`memory_name`), `jsg::Type`, `jsg::ToJS`, and `jsg::FromJS` are still generated - **Rust JSG safety rules** (full text and examples in `src/rust/jsg/README.md` §Safety rules): diff --git a/src/rust/jsg-macros/README.md b/src/rust/jsg-macros/README.md index 3a3113af3ad..1a6acd93d34 100644 --- a/src/rust/jsg-macros/README.md +++ b/src/rust/jsg-macros/README.md @@ -25,6 +25,13 @@ it, the fields are defined as data properties, an absent optional field as `unde JavaScript runs; without it, they are set one by one, an absent optional field is omitted, and a setter on `Object.prototype` runs, and what it throws is passed on. +Optional fields can use `Option` (`None` is absent) or `jsg::NullOrUndefined` +(`Undefined` is absent). Prefer `NullOrUndefined` to `Option>` +when a field must distinguish `null` from `undefined`; its `Null` state always +produces a property whose value is `null`. An absent input property becomes `None` +or `Undefined`, respectively. If all public fields are optional, a `null` or +`undefined` input is accepted as an empty dictionary. + ```rust #[jsg_struct] pub struct CaaRecord { @@ -69,6 +76,11 @@ Generates a V8 `FunctionCallback` for a method on a `#[jsg_resource]` type. - The first typed parameter may be `&mut Lock` / `&mut jsg::Lock` to receive the isolate lock directly — it is not exposed as a JavaScript argument. In a method that takes `this`, the lock comes right after `this`. +- `Option`, `jsg::NullOrUndefined`, and `jsg::Lenient` arguments may be omitted. + Prefer `NullOrUndefined` to `Option>` for three distinct + `Some(T)`, `Null`, and `Undefined` states; omission produces `Undefined`. + A `jsg::Nullable` argument remains required, although explicitly passing + `undefined` converts to `Null`. See [nullish value conversions](../jsg/README.md#nullablet-and-nullorundefinedt). - To take or return handles, name the callback's scope: `fn f<'s>(&self, lock: &mut Lock<'s>, v: Local<'s, Value>) -> Local<'s, Value>`. The generated callback ties `'s` to its own scope. @@ -468,7 +480,8 @@ use no-op `Traced` impls; containers/wrappers recurse to inner values. | `jsg::v8::TracedGlobal` | Dual strong/traced — `visitor.visit_traced_global` (enables cycle collection) | | `jsg::Weak` | **Not traced** — does not keep the target alive | | `jsg::v8::Global` | **Not allowed** — no `Traced` impl; use `jsg::v8::TracedGlobal` | -| `Option` / `jsg::Nullable` | Delegates to `T` when present | +| `Option` / `jsg::Nullable` / `jsg::NullOrUndefined` | Delegates to `T` when `Some` | +| `jsg::Lenient` | Traces `T` when `Some`, or the original JS value when `Unconvertable` | | `Vec`, `jsg::Dict`, `HashMap`, `BTreeMap`, `HashSet`, `BTreeSet` | Delegates recursively to contained values | | `Cell` / `std::cell::Cell` | Delegates via `as_ptr()` read (safe under single-threaded, non-reentrant GC tracing) | | `RefCell` | Delegates via `try_borrow()`; skipped while mutably borrowed (debug builds abort if `T` can hold GC references) | diff --git a/src/rust/jsg-macros/lib.rs b/src/rust/jsg-macros/lib.rs index a88c892fe61..b797950519d 100644 --- a/src/rust/jsg-macros/lib.rs +++ b/src/rust/jsg-macros/lib.rs @@ -46,6 +46,7 @@ use utils::extract_named_fields; use utils::fast_api_signature; use utils::is_lock_ref; use utils::is_named_this; +use utils::is_null_or_undefined_type; use utils::is_option_type; use utils::is_rc_self; use utils::is_result_type; @@ -62,6 +63,8 @@ use utils::this_param; /// Only `pub` fields are projected into the JavaScript object. Each converts with its `ToJS` /// impl, and a field whose conversion fails fails the struct's conversion with that error. /// The generated `Traced` impl visits every field, so a field can be a `jsg::Rc`. +/// Optional fields use `Option` or `jsg::NullOrUndefined`; prefer the latter to +/// `Option>` to distinguish `null` from `undefined`. /// Use `#[jsg_struct(name = "MyName")]` to override `Type::class_name()` /// metadata (used in diagnostics/type reporting), not to define a JS class. #[proc_macro_attribute] @@ -92,10 +95,16 @@ pub fn jsg_struct(attr: TokenStream, item: TokenStream) -> TokenStream { }; let field_name_str = field_name.to_string(); let field_type = &field.ty; - let optional = is_option_type(field_type); - all_optional &= optional; + let present = if is_option_type(field_type) { + Some(quote! { this.#field_name.is_some() }) + } else if is_null_or_undefined_type(field_type) { + Some(quote! { !this.#field_name.is_undefined() }) + } else { + None + }; + all_optional &= present.is_some(); - to_js_fields.push((field_name, field_name_str.clone(), optional)); + to_js_fields.push((field_name, field_name_str.clone(), present)); // Like C++ JSG, an absent property reads as undefined. field_extractions.push(quote! { let prop = obj.get(lock, #field_name_str)?; @@ -107,7 +116,7 @@ pub fn jsg_struct(attr: TokenStream, item: TokenStream) -> TokenStream { // Like C++ JSG, null or undefined is an empty dictionary, which is an // error if a field is required. let empty = if all_optional { - quote! { return Ok(Self { #(#field_names: None),* }); } + quote! { return Ok(Self { #(#field_names: Default::default()),* }); } } else { quote! { return Err(jsg::Error::new_type_error(format!( @@ -195,31 +204,30 @@ fn struct_traced_impl( } /// The body of a `#[jsg_struct]`'s `ToJS::to_js`, given each public field's name, its -/// JavaScript name and whether it is an `Option`. Like C++ JSG's `JSG_STRUCT`, it follows the -/// `fast_jsg_struct` flag. -fn to_js_fn_body(fields: &[(&syn::Ident, String, bool)]) -> proc_macro2::TokenStream { +/// JavaScript name and the condition for its present state, if optional. Like C++ JSG's +/// `JSG_STRUCT`, it follows the `fast_jsg_struct` flag. +fn to_js_fn_body( + fields: &[(&syn::Ident, String, Option)], +) -> proc_macro2::TokenStream { // A DictionaryTemplate instance has every field, so an absent optional field is undefined. - let data_properties = fields.iter().map(|(field_name, js_name, optional)| { - let value = if *optional { - quote! { - match this.#field_name { - Some(value) => jsg::ToJS::to_js(value, lock)?, - None => lock.undefined(), - } - } - } else { - quote! { jsg::ToJS::to_js(this.#field_name, lock)? } - }; - quote! { properties.push((#js_name, #value)); } + let data_properties = fields.iter().map(|(field_name, js_name, _)| { + quote! { + properties.push((#js_name, jsg::ToJS::to_js(this.#field_name, lock)?)); + } }); // Set() skips an absent optional field rather than setting it to undefined. - let field_sets = fields.iter().map(|(field_name, js_name, optional)| { + let field_sets = fields.iter().map(|(field_name, js_name, present)| { let set = quote! { let value = jsg::ToJS::to_js(value, lock)?; obj.set(lock, #js_name, value)?; }; - if *optional { - quote! { if let Some(value) = this.#field_name { #set } } + if let Some(present) = present { + quote! { + if #present { + let value = this.#field_name; + #set + } + } } else { quote! { { let value = this.#field_name; #set } } } @@ -528,7 +536,7 @@ fn unwrap_argument( }; // Like C++ JSG (TypeWrapper::unwrap() in type-wrapper.h), a missing // required argument or a top-level type mismatch is "Failed to execute - // ...". Only Option and Lenient arguments may be omitted. + // ...". FromJS::is_optional() determines whether an argument may be omitted. quote! { let not_of_type = || jsg::Error::new_type_error(format!( "Failed to execute '{}' on '{}': parameter {} is not of type '{}'.", diff --git a/src/rust/jsg-macros/utils-test.rs b/src/rust/jsg-macros/utils-test.rs index c531f33bbb9..0ae7d3f977a 100644 --- a/src/rust/jsg-macros/utils-test.rs +++ b/src/rust/jsg-macros/utils-test.rs @@ -116,6 +116,20 @@ fn is_attr_cases() { assert!(is_attr(&qualified.attrs[0], "jsg_method")); } +#[test] +fn is_null_or_undefined_type_cases() { + assert!(is_null_or_undefined_type(&parse_quote!( + NullOrUndefined + ))); + assert!(is_null_or_undefined_type(&parse_quote!( + jsg::NullOrUndefined + ))); + assert!(!is_null_or_undefined_type(&parse_quote!(String))); + assert!(!is_null_or_undefined_type(&parse_quote!( + &NullOrUndefined + ))); +} + #[test] fn is_option_type_cases() { assert!(is_option_type(&parse_quote!(Option))); diff --git a/src/rust/jsg-macros/utils.rs b/src/rust/jsg-macros/utils.rs index 0f6160f93c9..99966bc38e6 100644 --- a/src/rust/jsg-macros/utils.rs +++ b/src/rust/jsg-macros/utils.rs @@ -187,6 +187,12 @@ pub fn this_param(sig: &syn::Signature) -> Option<&syn::PatType> { } } +/// Returns true if `ty` is `NullOrUndefined`. +pub fn is_null_or_undefined_type(ty: &syn::Type) -> bool { + matches!(ty, syn::Type::Path(p) + if p.path.segments.last().is_some_and(|s| s.ident == "NullOrUndefined")) +} + /// Returns true if `ty` is `Option`. pub fn is_option_type(ty: &syn::Type) -> bool { matches!(ty, syn::Type::Path(p) diff --git a/src/rust/jsg-test/tests/jsg_struct.rs b/src/rust/jsg-test/tests/jsg_struct.rs index 589ec3e2640..9908ddebfce 100644 --- a/src/rust/jsg-test/tests/jsg_struct.rs +++ b/src/rust/jsg-test/tests/jsg_struct.rs @@ -70,6 +70,11 @@ struct FailingFieldStruct { pub bad: Unconvertible, } +#[jsg_struct] +struct FailingNullishFieldStruct { + pub bad: jsg::NullOrUndefined, +} + #[test] fn objects_can_be_wrapped_and_unwrapped() { let harness = crate::Harness::new(); @@ -395,6 +400,31 @@ fn struct_to_js_fails_with_a_fields_error() { } } +#[test] +fn nullish_struct_fields_use_consuming_fallible_conversion() { + for fast in [false, true] { + let harness = crate::Harness::with_flags(|mut flags| flags.set_fast_jsg_struct(fast)); + harness.run_in_context(|lock, _ctx| { + let err = FailingNullishFieldStruct { + bad: jsg::NullOrUndefined::Some(Unconvertible), + } + .to_js(lock) + .unwrap_err(); + assert_eq!(err.name, ExceptionType::TypeError); + assert_eq!(err.message, "field does not convert"); + + for bad in [jsg::NullOrUndefined::Null, jsg::NullOrUndefined::Undefined] { + let is_null = bad.is_null(); + let value = FailingNullishFieldStruct { bad }.to_js(lock)?; + let obj: v8::Local<'_, v8::Object> = value.try_into().unwrap(); + assert_eq!(obj.has(lock, "bad")?, fast || is_null); + assert_eq!(obj.get(lock, "bad")?.is_null(), is_null); + } + Ok(()) + }); + } +} + /// A `Vec` field converts to an `Array` of numbers, like any other `Vec`, with and without /// `fast_jsg_struct`. #[test] diff --git a/src/rust/jsg-test/tests/mod.rs b/src/rust/jsg-test/tests/mod.rs index d1bbdbd0ca4..c6f9809d222 100644 --- a/src/rust/jsg-test/tests/mod.rs +++ b/src/rust/jsg-test/tests/mod.rs @@ -28,6 +28,7 @@ mod local_value; mod name; mod nested_type; mod non_coercible; +mod null_or_undefined; mod nullable; mod number; mod option; diff --git a/src/rust/jsg-test/tests/null_or_undefined.rs b/src/rust/jsg-test/tests/null_or_undefined.rs new file mode 100644 index 00000000000..c385a20f459 --- /dev/null +++ b/src/rust/jsg-test/tests/null_or_undefined.rs @@ -0,0 +1,217 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +use jsg::FromJS; +use jsg::NullOrUndefined; +use jsg::ToJS; +use jsg_macros::jsg_method; +use jsg_macros::jsg_resource; +use jsg_macros::jsg_struct; + +#[jsg_resource] +struct Args {} + +#[jsg_resource] +impl Args { + #[jsg_method] + fn echo(&self, value: NullOrUndefined) -> NullOrUndefined { + value + } + + #[jsg_method] + fn with_prefix( + &self, + _prefix: String, + value: NullOrUndefined, + ) -> NullOrUndefined { + value + } + + #[jsg_method] + fn strict(&self, value: NullOrUndefined>) -> bool { + value.is_some() + } +} + +#[jsg_struct] +#[derive(Debug, PartialEq)] +struct Fields { + pub value: NullOrUndefined, +} + +#[jsg_resource] +struct Child {} + +#[jsg_resource] +impl Child {} + +#[jsg_resource] +struct Parent { + child: NullOrUndefined>, +} + +#[jsg_resource] +impl Parent {} + +#[test] +fn gc_traces_the_contained_resource() { + let harness = crate::Harness::new(); + let mut weak_child = None; + harness.run_in_context(|lock, ctx| { + let child = jsg::Rc::new(Child {}); + weak_child = Some(child.downgrade()); + let parent = jsg::Rc::new(Parent { + child: NullOrUndefined::Some(child), + }); + let wrapped = parent.to_js(lock)?; + ctx.set_global(lock, "parent", wrapped); + crate::Harness::request_gc(lock); + assert!(weak_child.as_ref().unwrap().is_alive()); + Ok(()) + }); + // The parent global is scoped to its context; it and its child can now be collected. + harness.run_in_context(|lock, _ctx| { + crate::Harness::request_gc(lock); + assert!(!weak_child.as_ref().unwrap().is_alive()); + for child in [NullOrUndefined::Null, NullOrUndefined::Undefined] { + let parent = jsg::Rc::new(Parent { child }); + let _wrapped = parent.to_js(lock)?; + crate::Harness::request_gc(lock); + } + Ok(()) + }); +} + +#[test] +fn conversion_preserves_all_states() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + for (expr, expected) in [ + ("'x'", NullOrUndefined::Some("x".to_owned())), + ("null", NullOrUndefined::Null), + ("undefined", NullOrUndefined::Undefined), + ] { + let input = ctx.eval_raw(lock, expr).unwrap(); + let value = NullOrUndefined::::from_js(lock, input)?; + assert_eq!(value, expected); + let output = value.to_js(lock)?; + ctx.set_global(lock, "output", output); + assert!( + ctx.eval::(lock, &format!("output === {expr}")) + .unwrap() + ); + assert_eq!( + NullOrUndefined::::try_from_js(lock, input)?, + Some(expected) + ); + } + // The inner conversion still coerces non-nullish values normally. + let input = ctx.eval_raw(lock, "123").unwrap(); + assert_eq!( + NullOrUndefined::::from_js(lock, input)?, + NullOrUndefined::Some("123".to_owned()) + ); + Ok(()) + }); +} + +#[test] +fn methods_accept_omission_and_report_inner_type_errors() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + let args = jsg::Rc::new(Args {}).to_js(lock)?; + ctx.set_global(lock, "args", args); + assert!(ctx.eval::(lock, "args.echo() === undefined && args.echo(undefined) === undefined && args.echo(null) === null && args.echo('x') === 'x'").unwrap()); + assert_eq!(ctx.eval::(lock, "try { args.strict(123) } catch(e) { e.message }").unwrap(), + "Failed to execute 'strict' on 'Args': parameter 1 is not of type 'string'."); + Ok(()) + }); +} + +#[test] +fn optional_arguments_do_not_count_toward_web_idl_length() { + for enabled in [false, true] { + let harness = crate::Harness::with_flags(|mut flags| { + flags.set_spec_compliant_property_attributes(enabled); + }); + harness.run_in_context(|lock, ctx| { + let args = jsg::Rc::new(Args {}).to_js(lock)?; + ctx.set_global(lock, "args", args); + assert_eq!( + ctx.eval::(lock, "args.withPrefix.length").unwrap(), + u32::from(enabled) + ); + assert_eq!(ctx.eval::(lock, "args.echo.length").unwrap(), 0); + assert!( + ctx.eval::(lock, "args.withPrefix('p') === undefined") + .unwrap() + ); + Ok(()) + }); + } +} + +#[test] +fn struct_fields_preserve_states_with_both_wrapping_modes() { + for fast in [false, true] { + let harness = crate::Harness::with_flags(|mut flags| flags.set_fast_jsg_struct(fast)); + harness.run_in_context(|lock, ctx| { + for (expr, expected) in [ + ("({value: 'x'})", NullOrUndefined::Some("x".to_owned())), + ("({value: null})", NullOrUndefined::Null), + ("({value: undefined})", NullOrUndefined::Undefined), + ("({})", NullOrUndefined::Undefined), + ("undefined", NullOrUndefined::Undefined), + ("null", NullOrUndefined::Undefined), + ] { + let input = ctx.eval_raw(lock, expr).unwrap(); + assert_eq!( + Fields::from_js(lock, input)?, + Fields { + value: expected.clone() + } + ); + let output = Fields { + value: expected.clone(), + } + .to_js(lock)?; + let obj: jsg::v8::Local<'_, jsg::v8::Object> = output.try_into().unwrap(); + assert_eq!(obj.has(lock, "value")?, fast || !expected.is_undefined()); + let property = obj.get(lock, "value")?; + assert_eq!( + NullOrUndefined::::from_js(lock, property)?, + expected + ); + } + Ok(()) + }); + } +} + +#[test] +fn lenient_conversion_only_swallows_shallow_mismatches() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + type Strict = NullOrUndefined>; + let input = ctx.eval_raw(lock, "123").unwrap(); + assert!(Strict::from_js(lock, input).is_err()); + assert!(Strict::try_from_js(lock, input)?.is_none()); + + let lenient = jsg::Lenient::::from_js(lock, input)?; + let (value, original) = lenient.to_parts(); + assert!(value.is_undefined()); + assert!(original.is_some()); + let lenient = jsg::Lenient::::from_js(lock, input)?; + assert!(NullOrUndefined::from(lenient).is_undefined()); + + let malformed = ctx.eval_raw(lock, "[123]").unwrap(); + assert!(jsg::Lenient::>>::from_js(lock, malformed).is_err()); + let throwing = ctx + .eval_raw(lock, "({ toString() { throw new Error('coercion'); } })") + .unwrap(); + let error = lock.try_catch(|lock| NullOrUndefined::::from_js(lock, throwing)); + assert!(error?.is_err()); + Ok(()) + }); +} diff --git a/src/rust/jsg-test/tests/nullable.rs b/src/rust/jsg-test/tests/nullable.rs index 154d7ef2460..6b3e0b8ae92 100644 --- a/src/rust/jsg-test/tests/nullable.rs +++ b/src/rust/jsg-test/tests/nullable.rs @@ -3,7 +3,7 @@ // https://opensource.org/licenses/Apache-2.0 //! `Nullable` is `T | null`, with Web IDL's `undefined`-to-`null` coercion. -//! `Option>` is `T | null | undefined`, and keeps the three apart. +//! `NullOrUndefined` is `T | null | undefined`, and keeps the three apart. use jsg::FromJS; use jsg::Nullable; @@ -247,7 +247,7 @@ fn lenient_converts_to_optional_nullable() { let lenient = convert::>>(lock, ctx, "1")?; let (value, unconvertable) = lenient.to_parts(); - assert!(value.is_none()); + assert!(value.is_undefined()); assert!(unconvertable.is_some()); assert!(jsg::Lenient::::from(None::>).is_undefined_strict()); diff --git a/src/rust/jsg/README.md b/src/rust/jsg/README.md index fbff207386f..21329c11c7c 100644 --- a/src/rust/jsg/README.md +++ b/src/rust/jsg/README.md @@ -20,6 +20,35 @@ Weak reference that doesn't prevent GC collection. Use `upgrade()` to get a `Rc< Per-isolate state for Rust resources exposed to JavaScript. Stores cached function templates. +### `Nullable` and `NullOrUndefined` + +Use `Nullable` for `T | null`: it coerces `undefined` to `null` on input and +method arguments remain required. Use `NullOrUndefined` for `T | null | undefined` +with distinct `Some(T)`, `Null`, and `Undefined` states and optional method arguments. +Prefer it to `Option>`. Both wrappers trace their contained value. + +| Rust type | Nullish input conversion | Omitted method argument | +|---|---|---| +| `Option` | `undefined` → `None`; `null` is converted by `T` | `None` | +| `Nullable` | `null` or `undefined` → `Null` | `TypeError` | +| `NullOrUndefined` | `null` → `Null`; `undefined` → `Undefined` | `Undefined` | + +`NullOrUndefined::default()` is `Undefined`, without requiring `T: Default`. +`as_ref()`, `as_mut()`, and `map()` preserve `Null` and `Undefined`. +Conversions to and from `Option>` preserve all three states; +`Nullable` also converts into `NullOrUndefined`. + +In a `#[jsg_struct]`, a missing `NullOrUndefined` field reads as `Undefined`. +When converting back to JavaScript, `Null` produces a `null` property; `Undefined` +is omitted without `fast_jsg_struct`, or becomes an own `undefined` property with +that flag, just as `None` does for an `Option` field. + +`Lenient` additionally preserves shallow conversion mismatches as `Unconvertable`. +Converting it to `NullOrUndefined` maps those mismatches to `Undefined`; +`Lenient::to_parts()` returns `(NullOrUndefined, Option>)`, +with the original mismatched JavaScript value in the second element when present. +Errors from nested conversions or JavaScript execution still propagate. + ## Safety rules Every change to Rust JSG, and to the C++ shims under it, must follow these four rules. Each rule @@ -153,6 +182,7 @@ struct MyResource { child: jsg::Rc, maybe_child: Option>, nullable_child: jsg::Nullable>, + optional_nullable_child: jsg::NullOrUndefined>, // jsg::Weak fields — weak reference, does not keep the target alive observer: jsg::Weak, @@ -239,10 +269,12 @@ does for `&self` methods. A method that takes `this` has no Fast API callback. | `jsg::Rc` | Yes — strong edge | Keeps target alive through GC | | `Option>` | Yes — when `Some` | | | `jsg::Nullable>` | Yes — when `Some` | | +| `jsg::NullOrUndefined>` | Yes — when `Some` | | | `jsg::Lenient>` | Yes — when `Some` or `Unconvertable` | | | `Cell>` | Yes — strong edge | Use `Cell` when field needs interior mutability | | `Cell>>` | Yes — when `Some` | | | `Cell>>` | Yes — when `Some` | | +| `Cell>>` | Yes — when `Some` | | | `Vec>` | Yes — each element | Iterates and visits every `Rc` in the vec | | `HashMap>` | Yes — each value | Iterates `.values()` and visits each `Rc` | | `BTreeMap>` | Yes — each value | Iterates `.values()` and visits each `Rc` | @@ -255,6 +287,7 @@ does for `&self` methods. A method that takes `this` has no Fast API callback. | `jsg::v8::TracedGlobal` | Yes — dual strong/traced | Enables cycle collection; see below | | `Option>` | Yes — when `Some` | | | `jsg::Nullable>` | Yes — when `Some` | | +| `jsg::NullOrUndefined>` | Yes — when `Some` | | | `Cell>` | Yes — dual strong/traced | Required when set after construction | | `Cell>>` | Yes — when `Some` | | | `jsg::Weak` | No | Doesn't keep target alive | diff --git a/src/rust/jsg/lenient.rs b/src/rust/jsg/lenient.rs index ebff1d4a660..42dd0da0d01 100644 --- a/src/rust/jsg/lenient.rs +++ b/src/rust/jsg/lenient.rs @@ -2,6 +2,7 @@ // Licensed under the Apache 2.0 license found in the LICENSE file or at: // https://opensource.org/licenses/Apache-2.0 +use crate::NullOrUndefined; use crate::Nullable; use crate::v8::TracedGlobal; use crate::v8::Value; @@ -9,15 +10,16 @@ use crate::v8::Value; /// [`Lenient`] is a type for lenient conversion from JavaScript, akin to the /// C++ type `LenientOptional`. /// -/// It is an alternative to `Option>` that treats type mismatches as `undefined` +/// It is an alternative to [`NullOrUndefined`] that treats type mismatches as `undefined` /// instead of erroring. When `T::try_from_js` reports a shallow conversion mismatch, the /// original JS value is preserved as [`Lenient::Unconvertable`] rather than propagating the error. /// [`Lenient::Unconvertable`] is considered `undefined` when passed to JavaScript or when -/// converted into an `Option>`. +/// converted into a [`NullOrUndefined`]. Errors from nested conversions or JavaScript +/// execution still propagate. /// /// [`Lenient`] is intentionally minimal — it is designed for crossing the Rust<->C++ FFI -/// boundary, with the possibility of inspecting the JS value. For general-purpose Rust, -/// code, prefer converting to `Option>`. +/// boundary, with the possibility of inspecting the JS value. For general-purpose Rust +/// code, prefer converting to [`NullOrUndefined`]. pub enum Lenient { Some(T), Unconvertable(TracedGlobal), @@ -56,15 +58,38 @@ impl Lenient { matches!(self, Self::Unconvertable(_)) } - /// Converts `self` into an `Option>`, where `None` is - /// `undefined`, while also providing the value passed to a failed - /// conversion as an optional argument in the case that it exists. - pub fn to_parts(self) -> (Option>, Option>) { + /// Splits `self` into a [`NullOrUndefined`] and the original value of a + /// failed shallow conversion, if any. + /// + /// `Some`, `Null`, and `Undefined` preserve their states, with no original + /// value in the second element. `Unconvertable` becomes `Undefined`, with + /// its original JavaScript value in the second element. + pub fn to_parts(self) -> (NullOrUndefined, Option>) { match self { - Self::Some(v) => (Some(Nullable::Some(v)), None), - Self::Unconvertable(global) => (None, Some(global)), - Self::Null => (Some(Nullable::Null), None), - Self::Undefined => (None, None), + Self::Some(v) => (NullOrUndefined::Some(v), None), + Self::Unconvertable(global) => (NullOrUndefined::Undefined, Some(global)), + Self::Null => (NullOrUndefined::Null, None), + Self::Undefined => (NullOrUndefined::Undefined, None), + } + } +} + +impl From> for NullOrUndefined { + fn from(value: Lenient) -> Self { + match value { + Lenient::Some(v) => Self::Some(v), + Lenient::Null => Self::Null, + Lenient::Unconvertable(_) | Lenient::Undefined => Self::Undefined, + } + } +} + +impl From> for Lenient { + fn from(value: NullOrUndefined) -> Self { + match value { + NullOrUndefined::Some(v) => Self::Some(v), + NullOrUndefined::Null => Self::Null, + NullOrUndefined::Undefined => Self::Undefined, } } } diff --git a/src/rust/jsg/lib.rs b/src/rust/jsg/lib.rs index 7432af5cec6..34ac1e14ed8 100644 --- a/src/rust/jsg/lib.rs +++ b/src/rust/jsg/lib.rs @@ -16,6 +16,7 @@ mod isolate_state; pub mod lenient; pub mod macros; pub mod modules; +pub mod null_or_undefined; pub mod nullable; mod promise; pub mod resource; @@ -30,6 +31,7 @@ pub use dict::Dict; pub use feature_flags::FeatureFlags; use isolate_state::IsolateState; pub use lenient::Lenient; +pub use null_or_undefined::NullOrUndefined; pub use nullable::Nullable; pub use resource::NestedType; pub use resource::Rc; diff --git a/src/rust/jsg/null_or_undefined-test.rs b/src/rust/jsg/null_or_undefined-test.rs new file mode 100644 index 00000000000..947e0a02f57 --- /dev/null +++ b/src/rust/jsg/null_or_undefined-test.rs @@ -0,0 +1,86 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +use super::*; + +#[test] +fn states_and_mapping() { + struct NoDefault; + + let mut value = NullOrUndefined::Some(String::from("x")); + assert!(value.is_some()); + assert!(!value.is_null()); + assert!(!value.is_undefined()); + assert_eq!( + value.as_ref().map(String::as_str), + NullOrUndefined::Some("x") + ); + if let NullOrUndefined::Some(s) = value.as_mut() { + s.push('y'); + } + assert_eq!(value.map(|s| s.len()), NullOrUndefined::Some(2)); + + for absent in [NullOrUndefined::::Null, NullOrUndefined::Undefined] { + assert!(!absent.is_some()); + assert_ne!(absent.is_null(), absent.is_undefined()); + assert_eq!(absent.as_ref().map(|_| panic!("absent")), absent); + assert_eq!(absent.clone().map(|_| panic!("absent")), absent); + } + // No T: Default bound is needed. + assert!(NullOrUndefined::::default().is_undefined()); +} + +#[test] +fn optional_nullable_conversions_preserve_all_states() { + for value in [None, Some(Nullable::Null), Some(Nullable::Some("x"))] { + let converted = NullOrUndefined::from(value.clone()); + assert_eq!(Option::>::from(converted), value); + } + assert_eq!( + NullOrUndefined::from(Nullable::Some(1)), + NullOrUndefined::Some(1) + ); + assert_eq!( + NullOrUndefined::::from(Nullable::Null), + NullOrUndefined::Null + ); +} + +#[test] +fn lenient_conversions_preserve_convertible_states() { + for value in [ + NullOrUndefined::Some(1), + NullOrUndefined::Null, + NullOrUndefined::Undefined, + ] { + let lenient = crate::Lenient::from(value); + assert_eq!(NullOrUndefined::from(lenient), value); + let (parts, original) = crate::Lenient::from(value).to_parts(); + assert_eq!(parts, value); + assert!(original.is_none()); + } +} + +#[test] +fn tracing_visits_only_some() { + use std::cell::Cell; + + use crate::Traced; + use crate::v8; + + struct Edge<'a>(&'a Cell); + impl Traced for Edge<'_> { + fn trace(&self, _visitor: &mut v8::GcVisitor) { + self.0.set(self.0.get() + 1); + } + } + let visits = Cell::new(0); + let mut visitor = v8::GcVisitor { + handle: v8::ffi::GcVisitor { ptr: 0 }, + }; + NullOrUndefined::Some(Edge(&visits)).trace(&mut visitor); + NullOrUndefined::>::Null.trace(&mut visitor); + NullOrUndefined::>::Undefined.trace(&mut visitor); + assert_eq!(visits.get(), 1); +} diff --git a/src/rust/jsg/null_or_undefined.rs b/src/rust/jsg/null_or_undefined.rs new file mode 100644 index 00000000000..084b2d1f2fb --- /dev/null +++ b/src/rust/jsg/null_or_undefined.rs @@ -0,0 +1,115 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +use crate::Nullable; + +/// A value of type `T`, `null`, or `undefined`, preserving all three states. +/// +/// Prefer this to `Option>` for optional nullable values. Unlike +/// [`Nullable`], input conversion keeps `undefined` distinct from `null`. +/// Method arguments may be omitted, producing [`NullOrUndefined::Undefined`]. +/// Non-nullish values use `T`'s normal conversion rules; use [`crate::Lenient`] +/// when shallow type mismatches should also become `undefined`. +/// +/// The default is [`NullOrUndefined::Undefined`], without requiring `T: Default`. +/// Conversions to and from `Option>` preserve all three states. +/// [`Nullable`] also converts into this type, preserving `Some` and `Null`. +/// +/// In a `#[jsg_struct]`, missing fields become `Undefined`. On output, `Null` +/// produces a `null` property. `Undefined` is omitted unless `fast_jsg_struct` +/// is enabled, in which case it produces an own `undefined` property. +/// +/// ```ignore +/// #[jsg_method] +/// pub fn process(&self, value: jsg::NullOrUndefined) { +/// match value { +/// jsg::NullOrUndefined::Some(s) => println!("Got value: {s}"), +/// jsg::NullOrUndefined::Null => println!("Got null"), +/// jsg::NullOrUndefined::Undefined => println!("Got undefined"), +/// } +/// } +/// ``` +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash)] +pub enum NullOrUndefined { + /// A value of type `T`. + Some(T), + /// JavaScript `null`. + Null, + /// JavaScript `undefined`, including an omitted argument or missing field. + #[default] + Undefined, +} + +impl NullOrUndefined { + /// Returns `true` if this contains a value. + pub fn is_some(&self) -> bool { + matches!(self, Self::Some(_)) + } + + /// Returns `true` if this is `Null`. + pub fn is_null(&self) -> bool { + matches!(self, Self::Null) + } + + /// Returns `true` if this is `Undefined`. + pub fn is_undefined(&self) -> bool { + matches!(self, Self::Undefined) + } + + /// Borrows the contained value, preserving `Null` and `Undefined`. + pub fn as_ref(&self) -> NullOrUndefined<&T> { + match self { + Self::Some(v) => NullOrUndefined::Some(v), + Self::Null => NullOrUndefined::Null, + Self::Undefined => NullOrUndefined::Undefined, + } + } + + /// Mutably borrows the contained value, preserving `Null` and `Undefined`. + pub fn as_mut(&mut self) -> NullOrUndefined<&mut T> { + match self { + Self::Some(v) => NullOrUndefined::Some(v), + Self::Null => NullOrUndefined::Null, + Self::Undefined => NullOrUndefined::Undefined, + } + } + + /// Transforms the contained value, preserving `Null` and `Undefined`. + pub fn map(self, f: impl FnOnce(T) -> U) -> NullOrUndefined { + match self { + Self::Some(v) => NullOrUndefined::Some(f(v)), + Self::Null => NullOrUndefined::Null, + Self::Undefined => NullOrUndefined::Undefined, + } + } +} + +impl From> for NullOrUndefined { + fn from(value: Nullable) -> Self { + match value { + Nullable::Some(v) => Self::Some(v), + Nullable::Null => Self::Null, + } + } +} + +impl From>> for NullOrUndefined { + fn from(value: Option>) -> Self { + value.map_or(Self::Undefined, Self::from) + } +} + +impl From> for Option> { + fn from(value: NullOrUndefined) -> Self { + match value { + NullOrUndefined::Some(v) => Some(Nullable::Some(v)), + NullOrUndefined::Null => Some(Nullable::Null), + NullOrUndefined::Undefined => None, + } + } +} + +#[cfg(test)] +#[path = "null_or_undefined-test.rs"] +mod tests; diff --git a/src/rust/jsg/nullable.rs b/src/rust/jsg/nullable.rs index 95fd196c5c4..ece184a43f3 100644 --- a/src/rust/jsg/nullable.rs +++ b/src/rust/jsg/nullable.rs @@ -15,17 +15,17 @@ use crate::Result; /// passing `undefined` produces `Nullable::Null`, but omitting the argument /// is a `TypeError`. /// -/// Use `Option>` for an optional nullable that keeps `undefined` -/// distinct from `null`, since [`Option`] is `T | undefined`: +/// Use [`NullOrUndefined`](crate::NullOrUndefined) for an optional nullable +/// that keeps `undefined` distinct from `null`: /// -/// | JavaScript | `Nullable` | `Option>` | -/// |-------------|---------------------|---------------------------| -/// | a `T` | `Nullable::Some(v)` | `Some(Nullable::Some(v))` | -/// | `null` | `Nullable::Null` | `Some(Nullable::Null)` | -/// | `undefined` | `Nullable::Null` | `None` | +/// | JavaScript | `Nullable` | `NullOrUndefined` | +/// |-------------|---------------------|------------------------------| +/// | a `T` | `Nullable::Some(v)` | `NullOrUndefined::Some(v)` | +/// | `null` | `Nullable::Null` | `NullOrUndefined::Null` | +/// | `undefined` | `Nullable::Null` | `NullOrUndefined::Undefined` | /// /// Output conversion produces only `T` or `null`. The TypeScript type is -/// `T | null`; `Option>` adds `undefined` and permits omission. +/// `T | null`; `NullOrUndefined` adds `undefined` and permits omission. /// /// # Examples /// @@ -41,11 +41,11 @@ use crate::Result; /// } /// /// #[jsg_method] -/// pub fn process_optional(&self, value: Option>) { +/// pub fn process_optional(&self, value: jsg::NullOrUndefined) { /// match value { -/// Some(Nullable::Some(s)) => println!("Got value: {s}"), -/// Some(Nullable::Null) => println!("Got null"), -/// None => println!("Got undefined"), +/// jsg::NullOrUndefined::Some(s) => println!("Got value: {s}"), +/// jsg::NullOrUndefined::Null => println!("Got null"), +/// jsg::NullOrUndefined::Undefined => println!("Got undefined"), /// } /// } /// diff --git a/src/rust/jsg/wrappable-test.rs b/src/rust/jsg/wrappable-test.rs index d3a117e7aec..1c18eb2fc4a 100644 --- a/src/rust/jsg/wrappable-test.rs +++ b/src/rust/jsg/wrappable-test.rs @@ -34,6 +34,7 @@ fn holds_gc_references_follows_the_contents() { assert!(>::holds_gc_references()); assert_forwards!(Option); assert_forwards!(Nullable); + assert_forwards!(NullOrUndefined); assert_forwards!(NonCoercible); assert_forwards!(Vec); assert_forwards!(HashSet); diff --git a/src/rust/jsg/wrappable.rs b/src/rust/jsg/wrappable.rs index 0efa5fba13a..62f5e9358a9 100644 --- a/src/rust/jsg/wrappable.rs +++ b/src/rust/jsg/wrappable.rs @@ -27,7 +27,7 @@ //! | `SystemTime` | `Date`; from a `Date` or `number` of milliseconds | //! | `Option` | `T` or `undefined` | //! | `Nullable` | `T` or `null` (`undefined` coerces to `null` on input) | -//! | `Option>` | `T`, `null`, or `undefined` | +//! | `NullOrUndefined` | `T`, `null`, or `undefined` | //! | `Lenient` | `T`, `null`, or `undefined` (shallow type mismatches silently become `undefined`) | //! | `Result` | `T` or throws | //! | `NonCoercible` | `T` (strict type checking) | @@ -36,6 +36,10 @@ //! | `&[T]` | `Array` (parameter only) | //! | `Dict` | `Record`: an object other than an array | //! +//! [`Option`], [`NullOrUndefined`], and [`Lenient`] method arguments may be omitted. +//! [`Nullable`] arguments remain required even though explicitly passing `undefined` converts to `Null`. +//! Prefer [`NullOrUndefined`] to `Option>` to preserve all three states. +//! //! ## Arrays, Typed Arrays and `ArrayBuffer`s //! //! A `Vec` converts to an `Array` of the converted elements, numbers included: @@ -73,6 +77,7 @@ use crate::Error; use crate::Lenient; use crate::Lock; use crate::NonCoercible; +use crate::NullOrUndefined; use crate::Nullable; use crate::Type; use crate::v8; @@ -91,7 +96,7 @@ use crate::v8; /// # Built-in implementations /// /// - **Primitives** (`bool`, `String`, integers, floats, `()`) — no-op. -/// - **`Option`**, **`Nullable`** — delegates to the inner value if present. +/// - **`Option`**, **`Nullable`**, **`NullOrUndefined`** — delegates to the inner value if present. /// - **Collections** (`Vec`, `Dict`, `HashMap`, `BTreeMap`, `HashSet`, /// `BTreeSet`) — iterates elements/values and traces each. /// - **`Cell`** — reads through `as_ptr()` (sound under single-threaded GC). @@ -217,6 +222,18 @@ impl Traced for Nullable { } } +impl Traced for NullOrUndefined { + fn holds_gc_references() -> bool { + T::holds_gc_references() + } + + fn trace(&self, visitor: &mut crate::v8::GcVisitor) { + if let Self::Some(inner) = self { + inner.trace(visitor); + } + } +} + impl Traced for NonCoercible { fn holds_gc_references() -> bool { T::holds_gc_references() @@ -1005,6 +1022,16 @@ impl<'s, T: ToJS<'s>> ToJS<'s> for Nullable { } } +impl<'s, T: ToJS<'s>> ToJS<'s> for NullOrUndefined { + fn to_js(self, lock: &mut Lock<'s>) -> Result, Error> { + match self { + Self::Some(value) => value.to_js(lock), + Self::Null => Ok(lock.null()), + Self::Undefined => Ok(lock.undefined()), + } + } +} + impl<'s, T: ToJS<'s>> ToJS<'s> for Lenient { fn to_js(self, lock: &mut Lock<'s>) -> Result, Error> { match self { @@ -1082,7 +1109,7 @@ impl<'s, T: Type + FromJS<'s>> FromJS<'s> for NonCoercible { } // Like C++ `kj::Maybe`, Web IDL nullable conversion reads `undefined` as `null`. -// Nullable arguments remain required; only an outer `Option` permits omission. +// Nullable arguments remain required; use `NullOrUndefined` to permit omission. impl<'s, T: FromJS<'s>> FromJS<'s> for Nullable { type ResultType = Nullable; @@ -1113,6 +1140,44 @@ impl<'s, T: FromJS<'s>> FromJS<'s> for Nullable { } } +impl<'s, T: FromJS<'s>> FromJS<'s> for NullOrUndefined { + type ResultType = NullOrUndefined; + + fn type_name() -> String { + T::type_name() + } + + fn is_optional() -> bool { + true + } + + fn from_js( + lock: &mut Lock<'s>, + value: v8::Local<'s, v8::Value>, + ) -> Result { + if value.is_null() { + Ok(NullOrUndefined::Null) + } else if value.is_undefined() { + Ok(NullOrUndefined::Undefined) + } else { + Ok(NullOrUndefined::Some(T::from_js(lock, value)?)) + } + } + + fn try_from_js( + lock: &mut Lock<'s>, + value: v8::Local<'s, v8::Value>, + ) -> Result, Error> { + if value.is_null() { + Ok(Some(NullOrUndefined::Null)) + } else if value.is_undefined() { + Ok(Some(NullOrUndefined::Undefined)) + } else { + Ok(T::try_from_js(lock, value)?.map(NullOrUndefined::Some)) + } + } +} + impl<'s, T: FromJS<'s>> FromJS<'s> for Lenient { type ResultType = Lenient;