Skip to content

Commit

Permalink
unit tests and logic cleanup for RefCell borrowing errors
Browse files Browse the repository at this point in the history
  • Loading branch information
dherman committed Feb 2, 2025
1 parent 56a1e27 commit 837cbfe
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 8 deletions.
4 changes: 2 additions & 2 deletions crates/neon/src/types_impl/extract/container.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ impl<'cx, T: 'static> TryFromJs<'cx> for Ref<'cx, T> {
match v.downcast::<JsBox<RefCell<T>>, _>(cx) {
Ok(v) => {
let cell = JsBox::deref(&v);
Ok(cell.try_borrow().map_err(|_| RefCellError::Borrowed))
Ok(cell.try_borrow().map_err(|_| RefCellError::MutablyBorrowed))
}
Err(_) => Ok(Err(RefCellError::WrongType)),
}
Expand All @@ -81,7 +81,7 @@ impl<'cx, T: 'static> TryFromJs<'cx> for RefMut<'cx, T> {
let cell = JsBox::deref(&v);
Ok(cell
.try_borrow_mut()
.map_err(|_| RefCellError::MutablyBorrowed))
.map_err(|_| RefCellError::Borrowed))
}
Err(_) => Ok(Err(RefCellError::WrongType)),
}
Expand Down
4 changes: 2 additions & 2 deletions crates/neon/src/types_impl/extract/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ impl fmt::Display for RefCellError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
RefCellError::WrongType => write!(f, "expected {}", RefCell::<()>::container_name()),
RefCellError::MutablyBorrowed => write!(f, "std::cell::RefCell is mutably borrowed"),
RefCellError::Borrowed => write!(f, "std::cell::RefCell is borrowed"),
RefCellError::MutablyBorrowed => write!(f, "RefCell is mutably borrowed"),
RefCellError::Borrowed => write!(f, "RefCell is borrowed"),
}
}
}
Expand Down
38 changes: 35 additions & 3 deletions test/napi/lib/container.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,26 @@ const addon = require("..");
const { expect } = require("chai");
const assert = require("chai").assert;

describe("container", function () {
describe("Container type extractors", function () {
it("can produce and consume a RefCell", function () {
const cell = addon.createStringRefCell("my sekret mesij");
const s = addon.readStringRefCell(cell);
assert.strictEqual(s, "my sekret mesij");
});

it("concatenates a RefCell<String> with a String", function () {
it("can produce and modify a RefCell", function () {
const cell = addon.createStringRefCell("new");
addon.writeStringRefCell(cell, "modified");
assert.strictEqual(addon.readStringRefCell(cell), "modified");
})

it("can concatenate a RefCell<String> with a String", function () {
const cell = addon.createStringRefCell("hello");
const s = addon.stringRefCellConcat(cell, " world");
assert.strictEqual(s, "hello world");
});

it("fails with a type error when not given a RefCell", function () {
it("fail with a type error when not given a RefCell", function () {
try {
addon.stringRefCellConcat("hello", " world");
assert.fail("should have thrown");
Expand All @@ -24,4 +30,30 @@ describe("container", function () {
assert.strictEqual(err.message, "expected std::cell::RefCell");
}
});

it("dynamically fail when borrowing a mutably borrowed RefCell", function () {
const cell = addon.createStringRefCell("hello");
try {
addon.borrowMutAndThen(cell, () => {
addon.stringRefConcat(cell, " world");
});
assert.fail("should have thrown");
} catch (err) {
assert.instanceOf(err, Error);
assert.strictEqual(err.message, "RefCell is mutably borrowed");
}
});

it("dynamically fail when modifying a borrowed RefCell", function () {
const cell = addon.createStringRefCell("hello");
try {
addon.borrowAndThen(cell, () => {
addon.writeStringRef(cell, "world");
});
assert.fail("should have thrown");
} catch (err) {
assert.instanceOf(err, Error);
assert.strictEqual(err.message, "RefCell is borrowed");
}
});
});
36 changes: 35 additions & 1 deletion test/napi/src/js/container.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use std::cell::RefCell;
use neon::{prelude::*, types::extract::Error};

use std::cell::{Ref, RefCell, RefMut};

#[neon::export]
fn create_string_ref_cell(s: String) -> RefCell<String> {
Expand All @@ -10,7 +12,39 @@ fn read_string_ref_cell(s: &RefCell<String>) -> String {
s.borrow().clone()
}

#[neon::export]
fn write_string_ref_cell(s: &RefCell<String>, value: String) {
*s.borrow_mut() = value;
}

#[neon::export]
fn string_ref_cell_concat(lhs: &RefCell<String>, rhs: String) -> String {
lhs.borrow().clone() + &rhs
}

#[neon::export]
fn string_ref_concat(lhs: Ref<String>, rhs: String) -> String {
lhs.clone() + &rhs
}

#[neon::export]
fn write_string_ref(mut s: RefMut<String>, value: String) {
*s = value;
}

#[neon::export]
fn borrow_and_then<'cx>(cx: &mut Cx<'cx>, cell: &RefCell<String>, f: Handle<JsFunction>)
-> JsResult<'cx, JsString> {
let s = cell.borrow();
f.bind(cx).exec()?;
Ok(cx.string(s.clone()))
}

#[neon::export]
fn borrow_mut_and_then<'cx>(cx: &mut Cx<'cx>, cell: &RefCell<String>, f: Handle<JsFunction>)
-> JsResult<'cx, JsString> {
let mut s = cell.borrow_mut();
f.bind(cx).exec()?;
*s = "overwritten".to_string();
Ok(cx.string(s.clone()))
}

0 comments on commit 837cbfe

Please sign in to comment.