Merge pull request #6346 from cloudflare/yagiz/enable-unsafe-safety-comments

enable undocumented_unsafe_blocks clippy rule

Author: anonrig

build/rust_lint.bazelrc

@@ -23,7 +23,7 @@ build:rust-enable-clippy-checks --output_groups=+clippy_checks
 # - clippy::non_std_lazy_statics – triggers on lazy_static, non-trivial to replace
 # - clippy::format_push_string – avoids single memory allocation, but makes code less readable
 # - clippy::cast_possible_truncation - usize/u64 conversion warning is unbelievably noisy
-build:rust-enable-clippy-checks --@rules_rust//:clippy_flags=-Wclippy::pedantic,-Wclippy::redundant_clone,-Wclippy::str_to_string,-Wclippy::to_string_in_format_args,-Wclippy::unnecessary_to_owned,-Wclippy::implicit_clone,-Wclippy::suspicious_to_owned,-Wclippy::unnecessary_to_owned,-Wclippy::nursery,-Wclippy::dbg_macro,-Wclippy::unwrap_used,-Wclippy::allow_attributes,-Aclippy::missing_const_for_fn,-Aclippy::cognitive_complexity,-Aclippy::trait_duplication_in_bounds,-Aclippy::non_send_fields_in_send_ty,-Aclippy::option_if_let_else,-Aclippy::missing_errors_doc,-Aclippy::must_use_candidate,-Aclippy::future_not_send,-Aclippy::trivial_regex,-Aclippy::literal_string_with_formatting_args,-Aclippy::non_std_lazy_statics,-Aclippy::format_push_string,-Aclippy::cast_possible_truncation,-Dwarnings
+build:rust-enable-clippy-checks --@rules_rust//:clippy_flags=-Wclippy::pedantic,-Wclippy::redundant_clone,-Wclippy::str_to_string,-Wclippy::to_string_in_format_args,-Wclippy::unnecessary_to_owned,-Wclippy::implicit_clone,-Wclippy::suspicious_to_owned,-Wclippy::unnecessary_to_owned,-Wclippy::nursery,-Wclippy::dbg_macro,-Wclippy::unwrap_used,-Wclippy::allow_attributes,-Wclippy::undocumented_unsafe_blocks,-Aclippy::missing_const_for_fn,-Aclippy::cognitive_complexity,-Aclippy::trait_duplication_in_bounds,-Aclippy::non_send_fields_in_send_ty,-Aclippy::option_if_let_else,-Aclippy::missing_errors_doc,-Aclippy::must_use_candidate,-Aclippy::future_not_send,-Aclippy::trivial_regex,-Aclippy::literal_string_with_formatting_args,-Aclippy::non_std_lazy_statics,-Aclippy::format_push_string,-Aclippy::cast_possible_truncation,-Dwarnings
 build --@rules_rust//rust/settings:clippy.toml=//src/rust:clippy.toml
 
 # enable rustfmt checks

src/rust/api/lib.rs

@@ -25,6 +25,7 @@ pub fn register_nodejs_modules(registry: Pin<&mut ffi::ModuleRegistry>) {
     jsg::modules::add_builtin(
         registry,
         "node-internal:dns",
+        // SAFETY: isolate is a valid pointer provided by V8 during module resolution.
         |isolate| unsafe {
             let mut lock = jsg::Lock::from_isolate_ptr(isolate);
             let dns_util = jsg::Ref::new(DnsUtil {
@@ -48,6 +49,7 @@ mod tests {
     #[test]
     fn test_wrap_resource_equality() {
         let harness = Harness::new();
+        // SAFETY: Harness guarantees lock and context are valid within run_in_context.
         harness.run_in_context(|lock, _ctx| unsafe {
             let dns_util = jsg::Ref::new(DnsUtil {
                 _state: ResourceState::default(),

src/rust/cxx-integration-test/lib.rs

@@ -134,11 +134,14 @@ fn pass_shared_struct_as_mut_ref(s: &mut ffi::SharedStruct) {
 
 unsafe fn pass_shared_struct_as_const_ptr(s: *const ffi::SharedStruct) -> i32 {
     assert!(!s.is_null());
+    // SAFETY: Null check above ensures s is valid; the SharedStruct is valid for reads.
     unsafe { (*s).a + (*s).b }
 }
 
 unsafe fn pass_shared_struct_as_mut_ptr(s: *mut ffi::SharedStruct) {
+    // SAFETY: Caller guarantees s is a valid, non-null mutable pointer.
     unsafe { (*s).a = 0 };
+    // SAFETY: Caller guarantees s is a valid, non-null mutable pointer.
     unsafe { (*s).b = 0 };
 }
 
@@ -180,7 +183,9 @@ fn get_str() -> &'static str {
     "rust_str"
 }
 
+// SAFETY: UsizeCallback is only accessed from the Tokio runtime thread after being moved there.
 unsafe impl Send for ffi::UsizeCallback {}
+// SAFETY: UsizeCallback is only accessed from one thread at a time via Pin<&mut>.
 unsafe impl Sync for ffi::UsizeCallback {}
 
 fn async_immediate(callback: Pin<&'static mut ffi::UsizeCallback>) {
@@ -205,6 +210,7 @@ mod tests {
     fn asan_stack_buffer_overflow() {
         expect_signal!(Signal::SIGABRT, {
             let xs = [0, 1, 2, 3];
+            // SAFETY: Intentional out-of-bounds access to trigger ASAN detection.
             let _y = unsafe { *xs.as_ptr().offset(4) };
         });
     }

src/rust/jsg-macros/lib.rs

@@ -86,7 +86,7 @@ pub fn jsg_struct(attr: TokenStream, item: TokenStream) -> TokenStream {
                 // 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)*
@@ -215,6 +215,7 @@ pub fn jsg_method(_attr: TokenStream, item: TokenStream) -> TokenStream {
         #fn_vis #fn_sig { #fn_block }
 
         #[automatically_derived]
+        #[expect(clippy::undocumented_unsafe_blocks)]
         extern "C" fn #callback_name(args: *mut jsg::v8::ffi::FunctionCallbackInfo) {
             let mut lock = unsafe { jsg::Lock::from_args(args) };
             let mut args = unsafe { jsg::v8::FunctionCallbackInfo::from_ffi(args) };

src/rust/jsg-test/lib.rs

@@ -78,12 +78,14 @@ impl EvalContext<'_> {
     where
         T: jsg::FromJS<ResultType = T>,
     {
+        // SAFETY: self.inner is a valid EvalContext from C++; code is a valid str.
         let result = unsafe { self.inner.eval(code) };
         let opt_local: Option<v8::ffi::Local> = result.value.into();
 
         if result.success {
             match opt_local {
                 Some(local) => {
+                    // SAFETY: self.isolate is valid and local is from a successful eval result.
                     let local = unsafe { v8::Local::from_ffi(self.isolate, local) };
                     match T::from_js(lock, local.clone()) {
                         Err(e) => Err(EvalError::UncoercibleResult {
@@ -98,6 +100,7 @@ impl EvalContext<'_> {
         } else {
             match opt_local {
                 Some(local) => {
+                    // SAFETY: self.isolate is valid and local is from an eval exception.
                     let value = unsafe { v8::Local::from_ffi(self.isolate, local) };
                     Err(EvalError::Exception(value))
                 }
@@ -110,17 +113,20 @@ impl EvalContext<'_> {
     ///
     /// Useful for obtaining handles (e.g. functions) that aren't `FromJS` types.
     pub fn eval_raw(&self, code: &str) -> Result<v8::Local<'_, v8::Value>, EvalError<'_>> {
+        // SAFETY: self.inner is a valid EvalContext from C++; code is a valid str.
         let result = unsafe { self.inner.eval(code) };
         let opt_local: Option<v8::ffi::Local> = result.value.into();
 
         if result.success {
             match opt_local {
+                // SAFETY: self.isolate is valid and local is from a successful eval result.
                 Some(local) => Ok(unsafe { v8::Local::from_ffi(self.isolate, local) }),
                 None => unreachable!(),
             }
         } else {
             match opt_local {
                 Some(local) => {
+                    // SAFETY: self.isolate is valid and local is from an eval exception.
                     let value = unsafe { v8::Local::from_ffi(self.isolate, local) };
                     Err(EvalError::Exception(value))
                 }
@@ -130,12 +136,14 @@ impl EvalContext<'_> {
     }
 
     pub fn set_global(&self, name: &str, value: v8::Local<v8::Value>) {
+        // SAFETY: self.inner is a valid EvalContext and value is a valid Local handle.
         unsafe { self.inner.set_global(name, value.into_ffi()) }
     }
 }
 
 impl Harness {
     pub fn new() -> Self {
+        // SAFETY: create_test_harness initializes the V8 platform and returns a valid harness.
         Self(unsafe { ffi::create_test_harness() })
     }
 
@@ -159,13 +167,16 @@ impl Harness {
         ) where
             F: FnOnce(&mut jsg::Lock, &mut EvalContext) -> Result<(), jsg::Error>,
         {
+            // SAFETY: data was cast from &raw mut Option<F> in run_in_context below.
             let cb = unsafe { &mut *(data as *mut Option<F>) };
             if let Some(callback) = cb.take() {
+                // SAFETY: isolate is a valid pointer provided by the C++ test harness.
                 let isolate_ptr = unsafe { v8::IsolatePtr::from_ffi(isolate) };
                 let mut eval_context = EvalContext {
                     inner: &context,
                     isolate: isolate_ptr,
                 };
+                // SAFETY: isolate is a valid pointer provided by the C++ test harness.
                 let mut lock = unsafe { jsg::Lock::from_isolate_ptr(isolate) };
                 if let Err(e) = callback(&mut lock, &mut eval_context) {
                     panic!("Test failed: {}: {}", e.name, e.message);
@@ -174,6 +185,7 @@ impl Harness {
         }
 
         let mut callback = Some(callback);
+        // SAFETY: callback pointer is valid for the duration of run_in_context.
         unsafe {
             self.0
                 .run_in_context(&raw mut callback as usize, trampoline::<F>);

src/rust/jsg-test/tests/arrays.rs

@@ -162,6 +162,7 @@ fn resource_accepts_array_parameter() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -184,6 +185,7 @@ fn resource_accepts_slice_parameter() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -207,6 +209,7 @@ fn resource_returns_array() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -234,6 +237,7 @@ fn resource_accepts_typed_array_parameter() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -258,6 +262,7 @@ fn resource_returns_typed_array() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -280,6 +285,7 @@ fn resource_accepts_typed_array_slice_parameter() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -313,6 +319,7 @@ fn typed_array_slice_rejects_wrong_type() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -346,6 +353,7 @@ fn resource_accepts_and_returns_struct_array() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -597,6 +605,7 @@ fn typed_array_iter_uint8() {
     harness.run_in_context(|lock, _ctx| {
         let data: Vec<u8> = vec![10, 20, 30];
         let js_val = data.to_js(lock);
+        // SAFETY: The isolate is locked and the FFI handle is from a valid eval result.
         let typed: jsg::v8::Local<'_, jsg::v8::Uint8Array> =
             unsafe { jsg::v8::Local::from_ffi(lock.isolate(), js_val.into_ffi()) };
 
@@ -623,6 +632,7 @@ fn typed_array_into_iter_uint8() {
     harness.run_in_context(|lock, _ctx| {
         let data: Vec<u8> = vec![1, 2, 3, 4, 5];
         let js_val = data.to_js(lock);
+        // SAFETY: The isolate is locked and the FFI handle is from a valid eval result.
         let typed: jsg::v8::Local<'_, jsg::v8::Uint8Array> =
             unsafe { jsg::v8::Local::from_ffi(lock.isolate(), js_val.into_ffi()) };
 
@@ -639,6 +649,7 @@ fn typed_array_iter_int32() {
     harness.run_in_context(|lock, _ctx| {
         let data: Vec<i32> = vec![-100, 0, 100];
         let js_val = data.to_js(lock);
+        // SAFETY: The isolate is locked and the FFI handle is from a valid eval result.
         let typed: jsg::v8::Local<'_, jsg::v8::Int32Array> =
             unsafe { jsg::v8::Local::from_ffi(lock.isolate(), js_val.into_ffi()) };
 
@@ -660,6 +671,7 @@ fn typed_array_iter_reverse() {
     harness.run_in_context(|lock, _ctx| {
         let data: Vec<u8> = vec![1, 2, 3, 4];
         let js_val = data.to_js(lock);
+        // SAFETY: The isolate is locked and the FFI handle is from a valid eval result.
         let typed: jsg::v8::Local<'_, jsg::v8::Uint8Array> =
             unsafe { jsg::v8::Local::from_ffi(lock.isolate(), js_val.into_ffi()) };
 
@@ -676,6 +688,7 @@ fn typed_array_empty() {
     harness.run_in_context(|lock, _ctx| {
         let data: Vec<u8> = vec![];
         let js_val = data.to_js(lock);
+        // SAFETY: The isolate is locked and the FFI handle is from a valid eval result.
         let typed: jsg::v8::Local<'_, jsg::v8::Uint8Array> =
             unsafe { jsg::v8::Local::from_ffi(lock.isolate(), js_val.into_ffi()) };
 
@@ -699,6 +712,7 @@ fn float32_array_parameter_and_return() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -734,6 +748,7 @@ fn float32_array_slice_parameter() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -761,6 +776,7 @@ fn float32_array_rejects_wrong_type() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -829,6 +845,7 @@ fn float64_array_parameter_and_return() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -864,6 +881,7 @@ fn float64_array_slice_parameter() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -891,6 +909,7 @@ fn float64_array_rejects_wrong_type() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -946,6 +965,7 @@ fn bigint64_array_parameter_and_return() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -987,6 +1007,7 @@ fn bigint64_array_slice_parameter() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -1014,6 +1035,7 @@ fn bigint64_array_rejects_wrong_type() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -1068,6 +1090,7 @@ fn biguint64_array_parameter_and_return() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -1103,6 +1126,7 @@ fn biguint64_array_slice_parameter() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);
 
@@ -1130,6 +1154,7 @@ fn biguint64_array_rejects_wrong_type() {
             _state: ResourceState::default(),
         });
         let mut template = ArrayResourceTemplate::new(lock);
+        // SAFETY: Lock is valid, resource is a valid Ref, and template holds a valid FunctionTemplate.
         let wrapped = unsafe { jsg::wrap_resource(lock, resource, &mut template) };
         ctx.set_global("arr", wrapped);