Add missing methods to Nan::Maybe<T> (#852)

* Add missing methods to Nan::Maybe<T> Nan::Maybe<T> was an alias for v8::Maybe<T> with most Node.js versions but that's problematic because v8::Maybe<T> has grown new methods over time that are missing in older versions. From now on we use our custom Maybe<T> implementation with all Node.js versions. This commit also adds the following missing methods: * Maybe<T>::Check() const * Maybe<T>::To(T*) const * Maybe<T>::ToChecked() const Fixes: #851 * Fix cpplint warnings. The code base is now `make lint`-clean again.
nodejs · May 16, 2019 · 4e96248 · 4e96248
1 parent 8b2b384
commit 4e96248
Show file tree

Hide file tree

Showing 13 changed files with 155 additions and 77 deletions.
diff --git a/Makefile b/Makefile
@@ -55,6 +55,7 @@ LINT_SOURCES = \
 	test/cpp/json-parse.cpp \
 	test/cpp/json-stringify.cpp \
 	test/cpp/makecallback.cpp \
+	test/cpp/maybe.cpp \
 	test/cpp/morenews.cpp \
 	test/cpp/multifile1.cpp \
 	test/cpp/multifile2.cpp \

diff --git a/nan.h b/nan.h
@@ -213,6 +213,70 @@ template<typename T, typename M = NonCopyablePersistentTraits<T> >
 class Persistent;
 #endif  // NODE_MODULE_VERSION
 
+template<typename T>
+class Maybe {
+ public:
+  inline bool IsNothing() const { return !has_value_; }
+  inline bool IsJust() const { return has_value_; }
+
+  inline T ToChecked() const { return FromJust(); }
+  inline void Check() const { FromJust(); }
+
+  inline bool To(T* out) const {
+    if (IsJust()) *out = value_;
+    return IsJust();
+  }
+
+  inline T FromJust() const {
+#if defined(V8_ENABLE_CHECKS)
+    assert(IsJust() && "FromJust is Nothing");
+#endif  // V8_ENABLE_CHECKS
+    return value_;
+  }
+
+  inline T FromMaybe(const T& default_value) const {
+    return has_value_ ? value_ : default_value;
+  }
+
+  inline bool operator==(const Maybe &other) const {
+    return (IsJust() == other.IsJust()) &&
+        (!IsJust() || FromJust() == other.FromJust());
+  }
+
+  inline bool operator!=(const Maybe &other) const {
+    return !operator==(other);
+  }
+
+#if defined(V8_MAJOR_VERSION) && (V8_MAJOR_VERSION > 4 ||                      \
+  (V8_MAJOR_VERSION == 4 && defined(V8_MINOR_VERSION) && V8_MINOR_VERSION >= 3))
+  // Allow implicit conversions from v8::Maybe<T> to Nan::Maybe<T>.
+  Maybe(const v8::Maybe<T>& that)  // NOLINT(runtime/explicit)
+    : has_value_(that.IsJust())
+    , value_(that.FromMaybe(T())) {}
+#endif
+
+ private:
+  Maybe() : has_value_(false) {}
+  explicit Maybe(const T& t) : has_value_(true), value_(t) {}
+  bool has_value_;
+  T value_;
+
+  template<typename U>
+  friend Maybe<U> Nothing();
+  template<typename U>
+  friend Maybe<U> Just(const U& u);
+};
+
+template<typename T>
+inline Maybe<T> Nothing() {
+  return Maybe<T>();
+}
+
+template<typename T>
+inline Maybe<T> Just(const T& t) {
+  return Maybe<T>(t);
+}
+
 #if defined(V8_MAJOR_VERSION) && (V8_MAJOR_VERSION > 4 ||                      \
   (V8_MAJOR_VERSION == 4 && defined(V8_MINOR_VERSION) && V8_MINOR_VERSION >= 3))
 # include "nan_maybe_43_inl.h"  // NOLINT(build/include)
@@ -1079,7 +1143,8 @@ class Utf8String {
         const int flags =
             v8::String::NO_NULL_TERMINATION | imp::kReplaceInvalidUtf8;
 #if NODE_MAJOR_VERSION >= 11
-        length_ = string->WriteUtf8(v8::Isolate::GetCurrent(), str_, static_cast<int>(len), 0, flags);
+        length_ = string->WriteUtf8(v8::Isolate::GetCurrent(), str_,
+                                    static_cast<int>(len), 0, flags);
 #else
         // See https://github.com/nodejs/nan/issues/832.
         // Disable the warning as there is no way around it.

diff --git a/nan_implementation_12_inl.h b/nan_implementation_12_inl.h
@@ -347,7 +347,8 @@ Factory<v8::StringObject>::return_t
 Factory<v8::StringObject>::New(v8::Local<v8::String> value) {
 // V8 > 7.0
 #if V8_MAJOR_VERSION > 7 || (V8_MAJOR_VERSION == 7 && V8_MINOR_VERSION > 0)
-  return v8::StringObject::New(v8::Isolate::GetCurrent(), value).As<v8::StringObject>();
+  return v8::StringObject::New(v8::Isolate::GetCurrent(), value)
+      .As<v8::StringObject>();
 #else
 #ifdef _MSC_VER
 #pragma warning(push)

diff --git a/nan_maybe_43_inl.h b/nan_maybe_43_inl.h
@@ -12,19 +12,6 @@
 template<typename T>
 using MaybeLocal = v8::MaybeLocal<T>;
 
-template<typename T>
-using Maybe = v8::Maybe<T>;
-
-template<typename T>
-inline Maybe<T> Nothing() {
-  return v8::Nothing<T>();
-}
-
-template<typename T>
-inline Maybe<T> Just(const T& t) {
-  return v8::Just<T>(t);
-}
-
 inline
 MaybeLocal<v8::String> ToDetailString(v8::Local<v8::Value> val) {
   v8::Isolate *isolate = v8::Isolate::GetCurrent();

diff --git a/nan_maybe_pre_43_inl.h b/nan_maybe_pre_43_inl.h
@@ -48,54 +48,6 @@ class MaybeLocal {
   v8::Local<T> val_;
 };
 
-template<typename T>
-class Maybe {
- public:
-  inline bool IsNothing() const { return !has_value_; }
-  inline bool IsJust() const { return has_value_; }
-
-  inline T FromJust() const {
-#if defined(V8_ENABLE_CHECKS)
-    assert(IsJust() && "FromJust is Nothing");
-#endif  // V8_ENABLE_CHECKS
-    return value_;
-  }
-
-  inline T FromMaybe(const T& default_value) const {
-    return has_value_ ? value_ : default_value;
-  }
-
-  inline bool operator==(const Maybe &other) const {
-    return (IsJust() == other.IsJust()) &&
-        (!IsJust() || FromJust() == other.FromJust());
-  }
-
-  inline bool operator!=(const Maybe &other) const {
-    return !operator==(other);
-  }
-
- private:
-  Maybe() : has_value_(false) {}
-  explicit Maybe(const T& t) : has_value_(true), value_(t) {}
-  bool has_value_;
-  T value_;
-
-  template<typename U>
-  friend Maybe<U> Nothing();
-  template<typename U>
-  friend Maybe<U> Just(const U& u);
-};
-
-template<typename T>
-inline Maybe<T> Nothing() {
-  return Maybe<T>();
-}
-
-template<typename T>
-inline Maybe<T> Just(const T& t) {
-  return Maybe<T>(t);
-}
-
 inline
 MaybeLocal<v8::String> ToDetailString(v8::Handle<v8::Value> val) {
   return MaybeLocal<v8::String>(val->ToDetailString());

diff --git a/test/binding.gyp b/test/binding.gyp
@@ -90,6 +90,10 @@
         "target_name" : "makecallback"
       , "sources"     : [ "cpp/makecallback.cpp" ]
     }
+    , {
+        "target_name" : "maybe"
+      , "sources"     : [ "cpp/maybe.cpp" ]
+    }
     , {
         "target_name" : "asyncresource"
       , "sources"     : [ "cpp/asyncresource.cpp" ]

diff --git a/test/cpp/maybe.cpp b/test/cpp/maybe.cpp
@@ -0,0 +1,39 @@
+/*********************************************************************
+ * NAN - Native Abstractions for Node.js
+ *
+ * Copyright (c) 2019 NAN contributors
+ *
+ * MIT License <https://github.com/nodejs/nan/blob/master/LICENSE.md>
+ ********************************************************************/
+
+#include <nan.h>
+
+using namespace Nan;  // NOLINT(build/namespaces)
+
+NAN_METHOD(Test) {
+  info.GetReturnValue().Set(false);
+  {
+    Maybe<int> mb = Nothing<int>();
+    if (mb.IsJust()) return;
+    if (!mb.IsNothing()) return;
+    if (mb.To(NULL)) return;
+  }
+  {
+    Maybe<int> mb = Just(42);
+    if (!mb.IsJust()) return;
+    if (mb.IsNothing()) return;
+    if (42 != mb.FromJust()) return;
+    if (42 != mb.ToChecked()) return;
+    mb.Check();
+    int v;
+    if (!mb.To(&v)) return;
+    if (42 != v) return;
+  }
+  info.GetReturnValue().Set(true);
+}
+
+NAN_MODULE_INIT(Init) {
+  SetMethod(target, "test", Test);
+}
+
+NODE_MODULE(maybe, Init)
diff --git a/test/cpp/morenews.cpp b/test/cpp/morenews.cpp
@@ -72,11 +72,13 @@ NAN_MODULE_INIT(Init) {
   );
   Set(target
     , New("newNegativeInteger").ToLocalChecked()
-    , GetFunction(New<v8::FunctionTemplate>(NewNegativeInteger)).ToLocalChecked()
+    , GetFunction(New<v8::FunctionTemplate>(NewNegativeInteger))
+        .ToLocalChecked()
   );
   Set(target
     , New("newPositiveInteger").ToLocalChecked()
-    , GetFunction(New<v8::FunctionTemplate>(NewPositiveInteger)).ToLocalChecked()
+    , GetFunction(New<v8::FunctionTemplate>(NewPositiveInteger))
+        .ToLocalChecked()
   );
   Set(target
     , New("newUtf8String").ToLocalChecked()
@@ -92,11 +94,13 @@ NAN_MODULE_INIT(Init) {
   );
   Set(target
     , New("newExternalStringResource").ToLocalChecked()
-    , GetFunction(New<v8::FunctionTemplate>(NewExternalStringResource)).ToLocalChecked()
+    , GetFunction(New<v8::FunctionTemplate>(NewExternalStringResource))
+        .ToLocalChecked()
   );
   Set(target
     , New("newExternalAsciiStringResource").ToLocalChecked()
-    , GetFunction(New<v8::FunctionTemplate>(NewExternalAsciiStringResource)).ToLocalChecked()
+    , GetFunction(New<v8::FunctionTemplate>(NewExternalAsciiStringResource))
+        .ToLocalChecked()
   );
 }
 

diff --git a/test/cpp/multifile2.cpp b/test/cpp/multifile2.cpp
@@ -7,7 +7,7 @@
  ********************************************************************/
 
 #include <nan.h>
-#include "multifile2.h"
+#include "multifile2.h"  // NOLINT(build/include)
 
 using namespace Nan;  // NOLINT(build/namespaces)
 

diff --git a/test/cpp/news.cpp b/test/cpp/news.cpp
@@ -169,31 +169,38 @@ NAN_MODULE_INIT(Init) {
   );
   Set(target
     , New<v8::String>("newNegativeInteger").ToLocalChecked()
-    , GetFunction(New<v8::FunctionTemplate>(NewNegativeInteger)).ToLocalChecked()
+    , GetFunction(New<v8::FunctionTemplate>(NewNegativeInteger))
+        .ToLocalChecked()
   );
   Set(target
     , New<v8::String>("newPositiveInteger").ToLocalChecked()
-    , GetFunction(New<v8::FunctionTemplate>(NewPositiveInteger)).ToLocalChecked()
+    , GetFunction(New<v8::FunctionTemplate>(NewPositiveInteger))
+        .ToLocalChecked()
   );
   Set(target
     , New<v8::String>("newUnsignedInteger").ToLocalChecked()
-    , GetFunction(New<v8::FunctionTemplate>(NewUnsignedInteger)).ToLocalChecked()
+    , GetFunction(New<v8::FunctionTemplate>(NewUnsignedInteger))
+        .ToLocalChecked()
   );
   Set(target
     , New<v8::String>("newInt32FromPositive").ToLocalChecked()
-    , GetFunction(New<v8::FunctionTemplate>(NewInt32FromPositive)).ToLocalChecked()
+    , GetFunction(New<v8::FunctionTemplate>(NewInt32FromPositive))
+        .ToLocalChecked()
   );
   Set(target
     , New<v8::String>("newInt32FromNegative").ToLocalChecked()
-    , GetFunction(New<v8::FunctionTemplate>(NewInt32FromNegative)).ToLocalChecked()
+    , GetFunction(New<v8::FunctionTemplate>(NewInt32FromNegative))
+        .ToLocalChecked()
   );
   Set(target
     , New<v8::String>("newUint32FromPositive").ToLocalChecked()
-    , GetFunction(New<v8::FunctionTemplate>(NewUint32FromPositive)).ToLocalChecked()
+    , GetFunction(New<v8::FunctionTemplate>(NewUint32FromPositive))
+        .ToLocalChecked()
   );
   Set(target
     , New<v8::String>("newUint32FromNegative").ToLocalChecked()
-    , GetFunction(New<v8::FunctionTemplate>(NewUint32FromNegative)).ToLocalChecked()
+    , GetFunction(New<v8::FunctionTemplate>(NewUint32FromNegative))
+        .ToLocalChecked()
   );
   Set(target
     , New<v8::String>("newUtf8String").ToLocalChecked()

diff --git a/test/cpp/setcallhandler.cpp b/test/cpp/setcallhandler.cpp
@@ -37,7 +37,8 @@ NAN_MODULE_INIT(Init) {
   );
   Set(target
     , New("b").ToLocalChecked()
-    , GetFunction(New<v8::FunctionTemplate>(CallAsFunctionHandlerSetter)).ToLocalChecked()
+    , GetFunction(New<v8::FunctionTemplate>(CallAsFunctionHandlerSetter))
+        .ToLocalChecked()
   );
 }
 

diff --git a/test/cpp/threadlocal.cpp b/test/cpp/threadlocal.cpp
@@ -63,7 +63,8 @@ NAN_METHOD(thread_local_storage) {
 NAN_MODULE_INIT(Init) {
   Set(target
     , New<v8::String>("thread_local_storage").ToLocalChecked()
-    , GetFunction(New<v8::FunctionTemplate>(thread_local_storage)).ToLocalChecked()
+    , GetFunction(New<v8::FunctionTemplate>(thread_local_storage))
+        .ToLocalChecked()
   );
 }
 

diff --git a/test/js/maybe-test.js b/test/js/maybe-test.js
@@ -0,0 +1,16 @@
+/*********************************************************************
+ * NAN - Native Abstractions for Node.js
+ *
+ * Copyright (c) 2019 NAN contributors
+ *
+ * MIT License <https://github.com/nodejs/nan/blob/master/LICENSE.md>
+ ********************************************************************/
+
+const test     = require('tap').test
+    , testRoot = require('path').resolve(__dirname, '..')
+    , bindings = require('bindings')({ module_root: testRoot, bindings: 'maybe' })
+
+test('maybe', function (t) {
+    t.plan(1);
+    t.ok(bindings.test());
+});