Comparing any value to any non-RegExp literal or undefined using
strictEqual (or notStrictEqual) passes if and only if deepStrictEqual
(or notDeepStrictEqual, respectively) passes.
Unnecessarily using deep comparisons adds confusion.
This patch adds an ESLint rule that forbids the use of deepStrictEqual
and notDeepStrictEqual when the expected value (i.e., the second
argument) is a non-RegExp literal or undefined.
For reference, an ESTree literal is defined as follows.
extend interface Literal <: Expression {
type: "Literal";
value: string | boolean | null | number | RegExp | bigint;
}
The value `undefined` is an `Identifier` with `name: 'undefined'`.
PR-URL: https://github.com/nodejs/node/pull/40634
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
Currently the are two of following compiler warnings is
generated:
In file included from ../test_bigint.c:5:
../test_bigint.c: In function ‘CreateTooBigBigInt’:
../test_bigint.c:120:22: warning:
‘words’ may be used uninitialized [-Wmaybe-uninitialized]
This commit initialized the words array to avoid these warnings.
PR-URL: https://github.com/nodejs/node/pull/40253
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
All (or at least most) of the tests uses lambdas (or arrow functions
if you will) to call these functions internally inside of directly
calling them, this should also use this technique for consistency.
PR-URL: https://github.com/nodejs/node/pull/38560
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
`napi_instanceof()` is insufficient for reliably establishing the data
type to which a pointer stored with `napi_wrap()` or
`napi_create_external()` inside a JavaScript object points. Thus, we
need a way to "mark" an object with a value that, when later retrieved,
can unambiguously tell us whether it is safe to cast the pointer stored
inside it to a certain structure.
Such a check must survive loading/unloading/multiple instances of an
addon, so we use UUIDs chosen *a priori*.
Fixes: https://github.com/nodejs/node/issues/28164
Co-authored-by: Anna Henningsen <github@addaleax.net>
PR-URL: https://github.com/nodejs/node/pull/28237
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com>
Throwing an exception from a finalizer can cause the following fatal
error:
Error: async hook stack has become corrupted (actual: 2, expected: 0)
1: 0x970b5a node::InternalCallbackScope::~InternalCallbackScope()
[./node]
2: 0x99dda0 node::Environment::RunTimers(uv_timer_s*) [./node]
3: 0x13d8b22 [./node]
4: 0x13dbe42 uv_run [./node]
5: 0xa57974 node::NodeMainInstance::Run() [./node]
6: 0x9dbc17 node::Start(int, char**) [./node]
7: 0x7f4965417f43 __libc_start_main [/lib64/libc.so.6]
8: 0x96f4ae _start [./node]
By https://github.com/nodejs/node/issues/34341#issuecomment-658426281,
calling into JS from a finalizer and/or throwing exceptions from there
is not advised, because the stack may or may not be set up for JS
execution. The best solution is to run the user's finalizer from a
`SetImmediate()` callback.
Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com>
Fixes: https://github.com/nodejs/node/issues/34341
PR-URL: https://github.com/nodejs/node/pull/34386
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Add assertions that conversion and coercion N-APIs return appropriate
error statuses when given `NULL`s for parameters they expect to not be
`NULL`.
For `napi_get_value_string_*` this also checks that it returns
`napi_string_expected` when passed a `napi_value` not containing a
string.
PR-URL: https://github.com/nodejs/node/pull/34142
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Add test coverage for passing `NULL` to each parameter of
`napi.*(propert|element)` and `napi_set_prototype`. In the case of
`napi_define_properties` also test setting various initializer fields
to `NULL`.
PR-URL: https://github.com/nodejs/node/pull/31488
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
This completely refactors the `expectsError` behavior: so far it's
almost identical to `assert.throws(fn, object)` in case it was used
with a function as first argument. It had a magical property check
that allowed to verify a functions `type` in case `type` was passed
used in the validation object. This pattern is now completely removed
and `assert.throws()` should be used instead.
The main intent for `common.expectsError()` is to verify error cases
for callback based APIs. This is now more flexible by accepting all
validation possibilites that `assert.throws()` accepts as well. No
magical properties exist anymore. This reduces surprising behavior
for developers who are not used to the Node.js core code base.
This has the side effect that `common` is used significantly less
frequent.
PR-URL: https://github.com/nodejs/node/pull/31092
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Since worker threads are complete Node.js environments, including the
ability to load native addons, and since those native addons can
allocate resources to be freed when objects go out of scope, and since,
upon worker thread exit, the engine does not invoke the weak callbacks
responsible for freeing resources which still have references, this
modification introduces tracking for weak references such that a list
of outstanding weak references is maintained. This list is traversed
during environment teardown. The callbacks for the remaining weak
references are called.
This change is also relevant for Node.js embedder scenarios, because in
those cases the process also outlives the `node::Environment` and
therefore weak callbacks should also be rendered as environment cleanup
hooks to ensure proper cleanup after native addons. This changes
introduces the means by which this can be accomplished.
A benchmark is included which measures the time it takes to execute the
weak reference callback for a given number of weak references.
Re: https://github.com/tc39/proposal-weakrefs/issues/125#issuecomment-535832130
PR-URL: https://github.com/nodejs/node/pull/28428
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
napi_define_class is tested by passing NULL to all parameters that are
pointers, one at a time. Moreover, two bugs were corrected. One was
utf8name and the second was the property descriptor pointer. These
pointers were assumed to be non-NULL and now we have NULL checks.
PR-URL: https://github.com/nodejs/node/pull/27945
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
For napi_set_property(), each one of the following arguments is checked:
napi_env env,
napi_value object,
napi_value key,
napi_value* value.
For napi_has_property(), each one of the following arguments is checked:
napi_env env,
napi_value object,
napi_value key,
bool* result
For napi_get_property, each one of the following arguments is checked:
napi_env env,
napi_value object,
napi_value key,
napi_value* result
PR-URL: https://github.com/nodejs/node/pull/27628
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
