docs(napi): clarify unsafe function invariants by sapphi-red · Pull Request #3267 · napi-rs/napi-rs

sapphi-red · 2026-04-27T16:37:07Z

Clarified unsafe function invariants in the safety comments.

Summary by CodeRabbit

Documentation
- Clarified and expanded safety docs for unsafe N-API conversions, validations, and external-data constructors. Updated requirements for environment and value pointer validity, memory lifetime and length guarantees, and finalize callbacks across string, buffer/array-buffer, and generic value APIs to make caller preconditions and safety contracts explicit.

coderabbitai · 2026-04-27T16:37:22Z

📝 Walkthrough

Walkthrough

Safety documentation across the N-API crate was made more explicit: unsafe functions now list concrete caller preconditions for environment validity, napi_value/pointer validity, data lifetime and size, and finalize callback responsibilities. No executable logic or public signatures were changed.

Changes

Cohort / File(s)	Summary
Trait Method Documentation `crates/napi/src/bindgen_runtime/js_values.rs`	Updated safety doc-comments for `ToNapiValue::to_napi_value`, `FromNapiValue::from_napi_value`, `FromNapiRef::from_napi_ref`, `FromNapiMutRef::from_napi_mut_ref`, and `ValidateNapiValue::validate` to state explicit `env` and `napi_value` validity requirements and conversion/validation relationships.
External Data Constructor Documentation `crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs`, `crates/napi/src/bindgen_runtime/js_values/buffer.rs`	Clarified `from_external` safety: caller must keep raw data pointer valid for buffer lifetime, ensure it covers at least the documented length, and provide a finalize callback that correctly frees/cleans up data.
String Constructor Documentation `crates/napi/src/js_values/string/latin1.rs`, `crates/napi/src/js_values/string/utf16.rs`	Clarified `from_external` safety for Latin1 and UTF-16 strings: data pointer must remain valid for the JS string lifetime and reference at least `len` code units/bytes.
Environment & Type Conversion Documentation `crates/napi/src/env.rs`, `crates/napi/src/js_values/unknown.rs`	Refined `Env::create_string_from_c_char` safety to require valid pointer/length combos (including `NAPI_AUTO_LENGTH`) and updated `Unknown::cast` / `Unknown::from_raw_unchecked` docs to require valid `env`, valid `value` pointer, and that target type matches runtime type.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

fix(napi): validate status before copying data in env arraybuffer fallback #3077: Overlaps with ArrayBuffer/Buffer external-creation docpaths and prior discussion/changes to from_external / from_data APIs.

Poem

🐰 I hopped through docs with care and cheer,

Pinned lifetimes, pointers, now crystal clear,
Finalize callbacks told where to go,
Env and values checked before the show,
A little rabbit signs off — safe and slow.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'docs(napi): clarify unsafe function invariants' accurately summarizes the main change—updating documentation for unsafe functions to clarify their safety invariants across multiple files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs`:
- Around line 1037-1041: The doc comment incorrectly states `data_len` as a byte
count; update the safety docs in arraybuffer.rs to clarify that `data_len` is an
element count and that the backing storage must be at least `data_len *
size_of::<$rust_type>()` bytes (for typed-array slices like i16/i32/f32/f64),
and keep the existing note about the finalize callback; target the doc block
around the `data_len` and `finalize` description in this file to replace "at
least `len` bytes" with the element-count/size_of formulation and a short note
that `data_len` represents number of elements.

In `@crates/napi/src/js_values/string/utf16.rs`:
- Around line 114-119: Update the safety doc for the UTF-16 string to clarify
that `len` counts UTF-16 code units rather than bytes: change the phrase "at
least `len` bytes" to "at least `len` UTF-16 code units (i.e. at least `len *
size_of::<u16>()` bytes)". Ensure the same clarification is applied to the
bullet mentioning the finalize callback and any nearby references to `len` so
callers understand the storage requirement for the backing data and the meaning
of `len`.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 59a95c4c-e62b-423e-a3e1-26cf64e1f4f5

📥 Commits

Reviewing files that changed from the base of the PR and between 9c6f720 and f3c0689.

📒 Files selected for processing (7)

crates/napi/src/bindgen_runtime/js_values.rs
crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs
crates/napi/src/bindgen_runtime/js_values/buffer.rs
crates/napi/src/env.rs
crates/napi/src/js_values/string/latin1.rs
crates/napi/src/js_values/string/utf16.rs
crates/napi/src/js_values/unknown.rs

coderabbitai

♻️ Duplicate comments (1)

crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs (1)

1040-1040: ⚠️ Potential issue | 🟡 Minor

Doc references the wrong parameter name.

The function signature uses data_len (line 1054), but the safety bullet still says “at least len elements”. Please rename to match the actual parameter so readers can map the requirement to the API.

📝 Proposed fix

-      #[doc = "- The data pointer is valid for the lifetime of the buffer and points to a memory region of at least `len` elements"]
+      #[doc = "- The data pointer is valid for the lifetime of the buffer and points to a memory region of at least `data_len` elements"]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs` at line 1040,
Update the doc comment that currently says "at least `len` elements" to use the
actual parameter name `data_len` so it matches the function signature; locate
the safety bullet in the ArrayBuffer-related docs in arraybuffer.rs (the doc for
the function that accepts the parameter `data_len`) and replace the reference
`len` with `data_len`, and scan nearby documentation for any other mismatched
occurrences to keep parameter names consistent.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs`:
- Line 1040: Update the doc comment that currently says "at least `len`
elements" to use the actual parameter name `data_len` so it matches the function
signature; locate the safety bullet in the ArrayBuffer-related docs in
arraybuffer.rs (the doc for the function that accepts the parameter `data_len`)
and replace the reference `len` with `data_len`, and scan nearby documentation
for any other mismatched occurrences to keep parameter names consistent.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d69d1e09-8aa8-43b7-896c-c4bc3405914d

📥 Commits

Reviewing files that changed from the base of the PR and between f3c0689 and 22c01c7.

📒 Files selected for processing (2)

crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs
crates/napi/src/js_values/string/utf16.rs

✅ Files skipped from review due to trivial changes (1)

crates/napi/src/js_values/string/utf16.rs

docs(napi): clarify unsafe function invariants

f3c0689

coderabbitai Bot reviewed Apr 27, 2026

View reviewed changes

Comment thread crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs

Comment thread crates/napi/src/js_values/string/utf16.rs

docs: review

22c01c7

coderabbitai Bot reviewed Apr 27, 2026

View reviewed changes

sapphi-red mentioned this pull request Apr 27, 2026

perf(napi): skip duplicate validation #3268

Merged

Brooooooklyn merged commit 837d122 into napi-rs:main Apr 28, 2026
73 checks passed

github-actions Bot mentioned this pull request Apr 28, 2026

chore: release #3259

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

docs(napi): clarify unsafe function invariants#3267

docs(napi): clarify unsafe function invariants#3267
Brooooooklyn merged 2 commits into
napi-rs:mainfrom
sapphi-red:docs/napi-clarify-unsafe-function-invariants

sapphi-red commented Apr 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 27, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

sapphi-red commented Apr 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sapphi-red commented Apr 27, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 27, 2026 •

edited

Loading