Skip to content

Commit

Permalink
src: enable type cast checks in tests (#1581)
Browse files Browse the repository at this point in the history
  • Loading branch information
legendecas authored Oct 3, 2024
1 parent 48a43de commit d8523a7
Show file tree
Hide file tree
Showing 22 changed files with 146 additions and 104 deletions.
13 changes: 13 additions & 0 deletions doc/value.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,19 @@ In order to enforce expected type, use `Napi::Value::Is*()` methods to check
the type before calling `Napi::Value::As()`, or compile with definition
`NODE_ADDON_API_ENABLE_TYPE_CHECK_ON_AS` to enforce type checks.

### UnsafeAs

```cpp
template <typename T> T Napi::Value::UnsafeAs() const;
```

Casts to another type of `Napi::Value`, when the actual type is known or
assumed.

This conversion does not coerce the type. This does not check the type even if
`NODE_ADDON_API_ENABLE_TYPE_CHECK_ON_AS` is defined. This indicates intentional
unsafe type cast. Use `Napi::Value::As()` if possible.

### Env

```cpp
Expand Down
22 changes: 19 additions & 3 deletions napi-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -896,6 +896,16 @@ inline T Value::As() const {
return T(_env, _value);
}

template <typename T>
inline T Value::UnsafeAs() const {
return T(_env, _value);
}

// static
inline void Value::CheckCast(napi_env /* env */, napi_value value) {
NAPI_CHECK(value != nullptr, "Value::CheckCast", "empty value");
}

inline MaybeOrValue<Boolean> Value::ToBoolean() const {
napi_value result;
napi_status status = napi_coerce_to_bool(_env, _value, &result);
Expand Down Expand Up @@ -1303,12 +1313,15 @@ inline Symbol Symbol::New(napi_env env, napi_value description) {

inline MaybeOrValue<Symbol> Symbol::WellKnown(napi_env env,
const std::string& name) {
// No need to check if the return value is a symbol or undefined.
// Well known symbols are definite and it is an develop time error
// if the symbol does not exist.
#if defined(NODE_ADDON_API_ENABLE_MAYBE)
Value symbol_obj;
Value symbol_value;
if (Napi::Env(env).Global().Get("Symbol").UnwrapTo(&symbol_obj) &&
symbol_obj.As<Object>().Get(name).UnwrapTo(&symbol_value)) {
return Just<Symbol>(symbol_value.As<Symbol>());
return Just<Symbol>(symbol_value.UnsafeAs<Symbol>());
}
return Nothing<Symbol>();
#else
Expand All @@ -1317,7 +1330,7 @@ inline MaybeOrValue<Symbol> Symbol::WellKnown(napi_env env,
.Get("Symbol")
.As<Object>()
.Get(name)
.As<Symbol>();
.UnsafeAs<Symbol>();
#endif
}

Expand Down Expand Up @@ -1535,7 +1548,10 @@ inline void Object::CheckCast(napi_env env, napi_value value) {
napi_valuetype type;
napi_status status = napi_typeof(env, value, &type);
NAPI_CHECK(status == napi_ok, "Object::CheckCast", "napi_typeof failed");
NAPI_INTERNAL_CHECK_EQ(type, napi_object, "%d", "Object::CheckCast");
NAPI_INTERNAL_CHECK(type == napi_object || type == napi_function,
"Object::CheckCast",
"Expect napi_object or napi_function, but got %d.",
type);
}

inline Object::Object() : TypeTaggable() {}
Expand Down
6 changes: 6 additions & 0 deletions napi.h
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,8 @@ class Value {
template <typename T>
static Value From(napi_env env, const T& value);

static void CheckCast(napi_env env, napi_value value);

/// Converts to a Node-API value primitive.
///
/// If the instance is _empty_, this returns `nullptr`.
Expand Down Expand Up @@ -527,6 +529,10 @@ class Value {
template <typename T>
T As() const;

// Unsafe Value::As(), should be avoided.
template <typename T>
T UnsafeAs() const;

MaybeOrValue<Boolean> ToBoolean()
const; ///< Coerces a value to a JavaScript boolean.
MaybeOrValue<Number> ToNumber()
Expand Down
6 changes: 3 additions & 3 deletions test/async_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ class TestWorkerWithUserDefRecv : public AsyncWorker {
static void DoWorkWithAsyncRes(const CallbackInfo& info) {
Object recv = info[0].As<Object>();
Function cb = info[1].As<Function>();
Object resource = info[2].As<Object>();
Value resource = info[2];

TestWorkerWithUserDefRecv* worker = nullptr;
if (resource == info.Env().Null()) {
worker = new TestWorkerWithUserDefRecv(recv, cb, "TestResource");
} else {
worker =
new TestWorkerWithUserDefRecv(recv, cb, "TestResource", resource);
worker = new TestWorkerWithUserDefRecv(
recv, cb, "TestResource", resource.As<Object>());
}

worker->Queue();
Expand Down
3 changes: 2 additions & 1 deletion test/binding.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@
{
'target_name': 'binding',
'dependencies': ['../node_addon_api.gyp:node_addon_api_except'],
'sources': ['>@(build_sources)']
'sources': ['>@(build_sources)'],
'defines': ['NODE_ADDON_API_ENABLE_TYPE_CHECK_ON_AS']
},
{
'target_name': 'binding_noexcept',
Expand Down
2 changes: 1 addition & 1 deletion test/function.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ function test (binding) {
assert.deepStrictEqual(args, [7, 8, 9]);

assert.throws(() => {
binding.callWithInvalidReceiver();
binding.callWithInvalidReceiver(() => {});
}, /Invalid (pointer passed as )?argument/);

obj = binding.callConstructorWithArgs(testConstructor, 5, 6, 7);
Expand Down
8 changes: 4 additions & 4 deletions test/globalObject/global_object_delete_property.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,27 @@ using namespace Napi;

Value DeletePropertyWithCStyleStringAsKey(const CallbackInfo& info) {
Object globalObject = info.Env().Global();
String key = info[0].As<String>();
String key = info[0].UnsafeAs<String>();
return Boolean::New(
info.Env(), MaybeUnwrap(globalObject.Delete(key.Utf8Value().c_str())));
}

Value DeletePropertyWithCppStyleStringAsKey(const CallbackInfo& info) {
Object globalObject = info.Env().Global();
String key = info[0].As<String>();
String key = info[0].UnsafeAs<String>();
return Boolean::New(info.Env(),
MaybeUnwrap(globalObject.Delete(key.Utf8Value())));
}

Value DeletePropertyWithInt32AsKey(const CallbackInfo& info) {
Object globalObject = info.Env().Global();
Number key = info[0].As<Number>();
Number key = info[0].UnsafeAs<Number>();
return Boolean::New(info.Env(),
MaybeUnwrap(globalObject.Delete(key.Uint32Value())));
}

Value DeletePropertyWithNapiValueAsKey(const CallbackInfo& info) {
Object globalObject = info.Env().Global();
Name key = info[0].As<Name>();
Name key = info[0].UnsafeAs<Name>();
return Boolean::New(info.Env(), MaybeUnwrap(globalObject.Delete(key)));
}
8 changes: 4 additions & 4 deletions test/globalObject/global_object_get_property.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,25 @@ using namespace Napi;

Value GetPropertyWithNapiValueAsKey(const CallbackInfo& info) {
Object globalObject = info.Env().Global();
Name key = info[0].As<Name>();
Name key = info[0].UnsafeAs<Name>();
return MaybeUnwrap(globalObject.Get(key));
}

Value GetPropertyWithInt32AsKey(const CallbackInfo& info) {
Object globalObject = info.Env().Global();
Number key = info[0].As<Napi::Number>();
Number key = info[0].UnsafeAs<Napi::Number>();
return MaybeUnwrapOr(globalObject.Get(key.Uint32Value()), Value());
}

Value GetPropertyWithCStyleStringAsKey(const CallbackInfo& info) {
Object globalObject = info.Env().Global();
String cStrkey = info[0].As<String>();
String cStrkey = info[0].UnsafeAs<String>();
return MaybeUnwrapOr(globalObject.Get(cStrkey.Utf8Value().c_str()), Value());
}

Value GetPropertyWithCppStyleStringAsKey(const CallbackInfo& info) {
Object globalObject = info.Env().Global();
String cppStrKey = info[0].As<String>();
String cppStrKey = info[0].UnsafeAs<String>();
return MaybeUnwrapOr(globalObject.Get(cppStrKey.Utf8Value()), Value());
}

Expand Down
6 changes: 3 additions & 3 deletions test/globalObject/global_object_has_own_property.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ using namespace Napi;

Value HasPropertyWithCStyleStringAsKey(const CallbackInfo& info) {
Object globalObject = info.Env().Global();
String key = info[0].As<String>();
String key = info[0].UnsafeAs<String>();
return Boolean::New(
info.Env(),
MaybeUnwrapOr(globalObject.HasOwnProperty(key.Utf8Value().c_str()),
Expand All @@ -14,15 +14,15 @@ Value HasPropertyWithCStyleStringAsKey(const CallbackInfo& info) {

Value HasPropertyWithCppStyleStringAsKey(const CallbackInfo& info) {
Object globalObject = info.Env().Global();
String key = info[0].As<String>();
String key = info[0].UnsafeAs<String>();
return Boolean::New(
info.Env(),
MaybeUnwrapOr(globalObject.HasOwnProperty(key.Utf8Value()), false));
}

Value HasPropertyWithNapiValueAsKey(const CallbackInfo& info) {
Object globalObject = info.Env().Global();
Name key = info[0].As<Name>();
Name key = info[0].UnsafeAs<Name>();
return Boolean::New(info.Env(),
MaybeUnwrap(globalObject.HasOwnProperty(key)));
}
10 changes: 5 additions & 5 deletions test/globalObject/global_object_set_property.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,28 @@ using namespace Napi;

void SetPropertyWithCStyleStringAsKey(const CallbackInfo& info) {
Object globalObject = info.Env().Global();
String key = info[0].As<String>();
String key = info[0].UnsafeAs<String>();
Value value = info[1];
globalObject.Set(key.Utf8Value().c_str(), value);
}

void SetPropertyWithCppStyleStringAsKey(const CallbackInfo& info) {
Object globalObject = info.Env().Global();
String key = info[0].As<String>();
String key = info[0].UnsafeAs<String>();
Value value = info[1];
globalObject.Set(key.Utf8Value(), value);
}

void SetPropertyWithInt32AsKey(const CallbackInfo& info) {
Object globalObject = info.Env().Global();
Number key = info[0].As<Number>();
Number key = info[0].UnsafeAs<Number>();
Value value = info[1];
globalObject.Set(key.Uint32Value(), value);
}

void SetPropertyWithNapiValueAsKey(const CallbackInfo& info) {
Object globalObject = info.Env().Global();
Name key = info[0].As<Name>();
Name key = info[0].UnsafeAs<Name>();
Value value = info[1];
globalObject.Set(key, value);
}
}
18 changes: 10 additions & 8 deletions test/name.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,21 @@ Value EchoString(const CallbackInfo& info) {

Value CreateString(const CallbackInfo& info) {
String encoding = info[0].As<String>();
Number length = info[1].As<Number>();
Value length = info[1];

if (encoding.Utf8Value() == "utf8") {
if (length.IsUndefined()) {
return String::New(info.Env(), testValueUtf8);
} else {
return String::New(info.Env(), testValueUtf8, length.Uint32Value());
return String::New(
info.Env(), testValueUtf8, length.As<Number>().Uint32Value());
}
} else if (encoding.Utf8Value() == "utf16") {
if (length.IsUndefined()) {
return String::New(info.Env(), testValueUtf16);
} else {
return String::New(info.Env(), testValueUtf16, length.Uint32Value());
return String::New(
info.Env(), testValueUtf16, length.As<Number>().Uint32Value());
}
} else {
Error::New(info.Env(), "Invalid encoding.").ThrowAsJavaScriptException();
Expand All @@ -44,20 +46,20 @@ Value CreateString(const CallbackInfo& info) {
Value CheckString(const CallbackInfo& info) {
String value = info[0].As<String>();
String encoding = info[1].As<String>();
Number length = info[2].As<Number>();
Value length = info[2];

if (encoding.Utf8Value() == "utf8") {
std::string testValue = testValueUtf8;
if (!length.IsUndefined()) {
testValue = testValue.substr(0, length.Uint32Value());
testValue = testValue.substr(0, length.As<Number>().Uint32Value());
}

std::string stringValue = value;
return Boolean::New(info.Env(), stringValue == testValue);
} else if (encoding.Utf8Value() == "utf16") {
std::u16string testValue = testValueUtf16;
if (!length.IsUndefined()) {
testValue = testValue.substr(0, length.Uint32Value());
testValue = testValue.substr(0, length.As<Number>().Uint32Value());
}

std::u16string stringValue = value;
Expand All @@ -69,10 +71,10 @@ Value CheckString(const CallbackInfo& info) {
}

Value CreateSymbol(const CallbackInfo& info) {
String description = info[0].As<String>();
Value description = info[0];

if (!description.IsUndefined()) {
return Symbol::New(info.Env(), description);
return Symbol::New(info.Env(), description.As<String>());
} else {
return Symbol::New(info.Env());
}
Expand Down
10 changes: 5 additions & 5 deletions test/object/delete_property.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,34 @@
using namespace Napi;

Value DeletePropertyWithUint32(const CallbackInfo& info) {
Object obj = info[0].As<Object>();
Object obj = info[0].UnsafeAs<Object>();
Number key = info[1].As<Number>();
return Boolean::New(info.Env(), MaybeUnwrap(obj.Delete(key.Uint32Value())));
}

Value DeletePropertyWithNapiValue(const CallbackInfo& info) {
Object obj = info[0].As<Object>();
Object obj = info[0].UnsafeAs<Object>();
Name key = info[1].As<Name>();
return Boolean::New(
info.Env(),
MaybeUnwrapOr(obj.Delete(static_cast<napi_value>(key)), false));
}

Value DeletePropertyWithNapiWrapperValue(const CallbackInfo& info) {
Object obj = info[0].As<Object>();
Object obj = info[0].UnsafeAs<Object>();
Name key = info[1].As<Name>();
return Boolean::New(info.Env(), MaybeUnwrapOr(obj.Delete(key), false));
}

Value DeletePropertyWithCStyleString(const CallbackInfo& info) {
Object obj = info[0].As<Object>();
Object obj = info[0].UnsafeAs<Object>();
String jsKey = info[1].As<String>();
return Boolean::New(
info.Env(), MaybeUnwrapOr(obj.Delete(jsKey.Utf8Value().c_str()), false));
}

Value DeletePropertyWithCppStyleString(const CallbackInfo& info) {
Object obj = info[0].As<Object>();
Object obj = info[0].UnsafeAs<Object>();
String jsKey = info[1].As<String>();
return Boolean::New(info.Env(),
MaybeUnwrapOr(obj.Delete(jsKey.Utf8Value()), false));
Expand Down
10 changes: 5 additions & 5 deletions test/object/get_property.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,31 @@
using namespace Napi;

Value GetPropertyWithNapiValue(const CallbackInfo& info) {
Object obj = info[0].As<Object>();
Object obj = info[0].UnsafeAs<Object>();
Name key = info[1].As<Name>();
return MaybeUnwrapOr(obj.Get(static_cast<napi_value>(key)), Value());
}

Value GetPropertyWithNapiWrapperValue(const CallbackInfo& info) {
Object obj = info[0].As<Object>();
Object obj = info[0].UnsafeAs<Object>();
Name key = info[1].As<Name>();
return MaybeUnwrapOr(obj.Get(key), Value());
}

Value GetPropertyWithUint32(const CallbackInfo& info) {
Object obj = info[0].As<Object>();
Object obj = info[0].UnsafeAs<Object>();
Number key = info[1].As<Number>();
return MaybeUnwrap(obj.Get(key.Uint32Value()));
}

Value GetPropertyWithCStyleString(const CallbackInfo& info) {
Object obj = info[0].As<Object>();
Object obj = info[0].UnsafeAs<Object>();
String jsKey = info[1].As<String>();
return MaybeUnwrapOr(obj.Get(jsKey.Utf8Value().c_str()), Value());
}

Value GetPropertyWithCppStyleString(const CallbackInfo& info) {
Object obj = info[0].As<Object>();
Object obj = info[0].UnsafeAs<Object>();
String jsKey = info[1].As<String>();
return MaybeUnwrapOr(obj.Get(jsKey.Utf8Value()), Value());
}
Loading

0 comments on commit d8523a7

Please sign in to comment.