Fix a crash when setter is nullpta

In the current implementation, if we want to make a readonly property,
we should pass empty setter that is actual function but has no body
as follows:
  void HelloSetter(const Npai::CallbackInfo& info,
                   const Npai::Value& value) {
    // This is an empty setter to make a readonly property.
    // Do nothing
  }

  InstanceAccessor("hello_property", &HelloGetter, &HelloSetter);

If we allows taking `nullptr` as the argument, we can simplify user's
code more as follows:
  InstanceAccessor("hello_property", &HelloGetter, nullptr);

Currently, above code makes a crash. So, this patch makes
InstanceAccessor() take `nullptr` as the argument and then add some
tests for them.

PR-URL: #159
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Jason Ginchereau <[email protected]>

Author: romandev

napi-inl.h

@@ -2562,8 +2562,8 @@ inline ClassPropertyDescriptor<T> ObjectWrap<T>::InstanceAccessor(
 
   napi_property_descriptor desc = {};
   desc.utf8name = utf8name;
-  desc.getter = T::InstanceGetterCallbackWrapper;
-  desc.setter = T::InstanceSetterCallbackWrapper;
+  desc.getter = getter != nullptr ? T::InstanceGetterCallbackWrapper : nullptr;
+  desc.setter = setter != nullptr ? T::InstanceSetterCallbackWrapper : nullptr;
   desc.data = callbackData;
   desc.attributes = attributes;
   return desc;

test/objectwrap.cc

@@ -27,23 +27,34 @@ class Test : public Napi::ObjectWrap<Test> {
     Napi::ObjectWrap<Test>(info) {
   }
 
-  void Set(const Napi::CallbackInfo& info) {
+  void SetMethod(const Napi::CallbackInfo& info) {
     value = info[0].As<Napi::Number>();
   }
 
-  Napi::Value Get(const Napi::CallbackInfo& info) {
+  Napi::Value GetMethod(const Napi::CallbackInfo& info) {
     return Napi::Number::New(info.Env(), value);
   }
 
   Napi::Value Iter(const Napi::CallbackInfo& info) {
     return TestIter::Constructor.New({});
   }
 
+  void Setter(const Napi::CallbackInfo& info, const Napi::Value& new_value) {
+    value = new_value.As<Napi::Number>();
+  }
+
+  Napi::Value Getter(const Napi::CallbackInfo& info) {
+    return Napi::Number::New(info.Env(), value);
+  }
+
   static void Initialize(Napi::Env env, Napi::Object exports) {
     exports.Set("Test", DefineClass(env, "Test", {
-      InstanceMethod("test_set", &Test::Set),
-      InstanceMethod("test_get", &Test::Get),
-      InstanceMethod(Napi::Symbol::WellKnown(env, "iterator"), &Test::Iter)
+      InstanceMethod("test_set_method", &Test::SetMethod),
+      InstanceMethod("test_get_method", &Test::GetMethod),
+      InstanceMethod(Napi::Symbol::WellKnown(env, "iterator"), &Test::Iter),
+      InstanceAccessor("test_getter_only", &Test::Getter, nullptr),
+      InstanceAccessor("test_setter_only", nullptr, &Test::Setter),
+      InstanceAccessor("test_getter_setter", &Test::Getter, &Test::Setter),
     }));
   }
 

test/objectwrap.js

@@ -8,19 +8,51 @@ test(require(`./build/${buildType}/binding_noexcept.node`));
 function test(binding) {
   var Test = binding.objectwrap.Test;
 
-  function testSetGet(obj) {
-    obj.test_set(90);
-    assert.strictEqual(obj.test_get(), 90);
+  function testSetGetMethod(obj) {
+    obj.test_set_method(90);
+    assert.strictEqual(obj.test_get_method(), 90);
   }
 
   function testIter(obj) {
     for (const value of obj) {
     }
   }
 
+  function testGetterOnly(obj) {
+    obj.test_set_method(91);
+    assert.strictEqual(obj.test_getter_only, 91);
+
+    let error;
+    try {
+      // Can not assign to read only property.
+      obj.test_getter_only = 92;
+    } catch(e) {
+      error = e;
+    } finally {
+      assert.strictEqual(error.name, 'TypeError');
+    }
+  }
+
+  function testSetterOnly(obj) {
+    obj.test_setter_only = 93;
+    assert.strictEqual(obj.test_setter_only, undefined);
+    assert.strictEqual(obj.test_getter_only, 93);
+  }
+
+  function testGetterSetter(obj) {
+    obj.test_getter_setter = 94;
+    assert.strictEqual(obj.test_getter_setter, 94);
+
+    obj.test_getter_setter = 95;
+    assert.strictEqual(obj.test_getter_setter, 95);
+  }
+
   function testObj(obj) {
-    testSetGet(obj);
+    testSetGetMethod(obj);
     testIter(obj);
+    testGetterOnly(obj);
+    testSetterOnly(obj);
+    testGetterSetter(obj);
   }
 
   testObj(new Test());