diff --git a/.clang-format b/.clang-format new file mode 100644 index 000000000..4aad29c32 --- /dev/null +++ b/.clang-format @@ -0,0 +1,111 @@ +--- +Language: Cpp +# BasedOnStyle: Google +AccessModifierOffset: -1 +AlignAfterOpenBracket: Align +AlignConsecutiveAssignments: false +AlignConsecutiveDeclarations: false +AlignEscapedNewlines: Right +AlignOperands: true +AlignTrailingComments: true +AllowAllParametersOfDeclarationOnNextLine: true +AllowShortBlocksOnASingleLine: false +AllowShortCaseLabelsOnASingleLine: false +AllowShortFunctionsOnASingleLine: Inline +AllowShortIfStatementsOnASingleLine: true +AllowShortLoopsOnASingleLine: true +AlwaysBreakAfterDefinitionReturnType: None +AlwaysBreakAfterReturnType: None +AlwaysBreakBeforeMultilineStrings: false +AlwaysBreakTemplateDeclarations: true +BinPackArguments: false +BinPackParameters: false +BraceWrapping: + AfterClass: false + AfterControlStatement: false + AfterEnum: false + AfterFunction: false + AfterNamespace: false + AfterObjCDeclaration: false + AfterStruct: false + AfterUnion: false + AfterExternBlock: false + BeforeCatch: false + BeforeElse: false + IndentBraces: false + SplitEmptyFunction: true + SplitEmptyRecord: true + SplitEmptyNamespace: true +BreakBeforeBinaryOperators: None +BreakBeforeBraces: Attach +BreakBeforeInheritanceComma: false +BreakBeforeTernaryOperators: true +BreakConstructorInitializersBeforeComma: false +BreakConstructorInitializers: BeforeColon +BreakAfterJavaFieldAnnotations: false +BreakStringLiterals: true +ColumnLimit: 80 +CommentPragmas: '^ IWYU pragma:' +CompactNamespaces: false +ConstructorInitializerAllOnOneLineOrOnePerLine: true +ConstructorInitializerIndentWidth: 4 +ContinuationIndentWidth: 4 +Cpp11BracedListStyle: true +DerivePointerAlignment: false +DisableFormat: false +ExperimentalAutoDetectBinPacking: false +FixNamespaceComments: true +ForEachMacros: + - foreach + - Q_FOREACH + - BOOST_FOREACH +IncludeBlocks: Preserve +IncludeCategories: + - Regex: '^' + Priority: 2 + - Regex: '^<.*\.h>' + Priority: 1 + - Regex: '^<.*' + Priority: 2 + - Regex: '.*' + Priority: 3 +IncludeIsMainRegex: '([-_](test|unittest))?$' +IndentCaseLabels: true +IndentPPDirectives: None +IndentWidth: 2 +IndentWrappedFunctionNames: false +JavaScriptQuotes: Leave +JavaScriptWrapImports: true +KeepEmptyLinesAtTheStartOfBlocks: false +MacroBlockBegin: '' +MacroBlockEnd: '' +MaxEmptyLinesToKeep: 1 +NamespaceIndentation: None +ObjCBlockIndentWidth: 2 +ObjCSpaceAfterProperty: false +ObjCSpaceBeforeProtocolList: false +PenaltyBreakAssignment: 2 +PenaltyBreakBeforeFirstCallParameter: 1 +PenaltyBreakComment: 300 +PenaltyBreakFirstLessLess: 120 +PenaltyBreakString: 1000 +PenaltyExcessCharacter: 1000000 +PenaltyReturnTypeOnItsOwnLine: 200 +PointerAlignment: Left +ReflowComments: true +SortIncludes: true +SortUsingDeclarations: true +SpaceAfterCStyleCast: false +SpaceAfterTemplateKeyword: true +SpaceBeforeAssignmentOperators: true +SpaceBeforeParens: ControlStatements +SpaceInEmptyParentheses: false +SpacesBeforeTrailingComments: 2 +SpacesInAngles: false +SpacesInContainerLiterals: true +SpacesInCStyleCastParentheses: false +SpacesInParentheses: false +SpacesInSquareBrackets: false +Standard: Auto +TabWidth: 8 +UseTab: Never diff --git a/.github/workflows/linter.yml b/.github/workflows/linter.yml new file mode 100644 index 000000000..3cbf00634 --- /dev/null +++ b/.github/workflows/linter.yml @@ -0,0 +1,23 @@ +name: Style Checks + +on: [push, pull_request] + +jobs: + lint: + strategy: + matrix: + node-version: [14.x] + os: [ubuntu-latest] + + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v2 + with: + fetch-depth: 0 + - run: git branch -a + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v1 + with: + node-version: ${{ matrix.node-version }} + - run: npm install + - run: CLANG_FORMAT_START=refs/remotes/origin/master npm run lint diff --git a/README.md b/README.md index 6b0df4deb..c7e40bd92 100644 --- a/README.md +++ b/README.md @@ -37,8 +37,8 @@ APIs exposed by node-addon-api are generally used to create and manipulate JavaScript values. Concepts and operations generally map to ideas specified in the **ECMA262 Language Specification**. -The [N-API Resource](http://nodejs.github.io/node-addon-examples/) offers an -excellent orientation and tips for developers just getting started with N-API +The [N-API Resource](https://nodejs.github.io/node-addon-examples/) offers an +excellent orientation and tips for developers just getting started with N-API and node-addon-api. - **[Setup](#setup)** diff --git a/doc/array_buffer.md b/doc/array_buffer.md index 988a839a4..346fe6ace 100644 --- a/doc/array_buffer.md +++ b/doc/array_buffer.md @@ -130,4 +130,20 @@ void* Napi::ArrayBuffer::Data() const; Returns a pointer the wrapped data. +### Detach + +```cpp +void Napi::ArrayBuffer::Detach(); +``` + +Invokes the `ArrayBuffer` detach operation on a detachable `ArrayBuffer`. + +### IsDetached + +```cpp +bool Napi::ArrayBuffer::IsDetached() const; +``` + +Returns `true` if this `ArrayBuffer` has been detached. + [`Napi::Object`]: ./object.md diff --git a/doc/async_worker_variants.md b/doc/async_worker_variants.md index 8b892eaee..54007762c 100644 --- a/doc/async_worker_variants.md +++ b/doc/async_worker_variants.md @@ -243,7 +243,9 @@ be safely released. Note that `Napi::AsyncProgressWorker::ExecutionProcess::Send` merely guarantees **eventual** invocation of `Napi::AsyncProgressWorker::OnProgress`, which means multiple send might be coalesced into single invocation of `Napi::AsyncProgressWorker::OnProgress` -with latest data. +with latest data. If you would like to guarantee that there is one invocation of +`OnProgress` for every `Send` call, you should use the `Napi::AsyncProgressQueueWorker` +class instead which is documented further down this page. ```cpp void Napi::AsyncProgressWorker::ExecutionProcess::Send(const T* data, size_t count) const; @@ -269,7 +271,8 @@ function runs in the background out of the **event loop** thread and at the end the `Napi::AsyncProgressWorker::OnOK` or `Napi::AsyncProgressWorker::OnError` function will be called and are executed as part of the event loop. -The code below shows a basic example of the `Napi::AsyncProgressWorker` implementation: +The code below shows a basic example of the `Napi::AsyncProgressWorker` implementation along with an +example of how the counterpart in Javascript would appear: ```cpp #include @@ -281,28 +284,38 @@ using namespace Napi; class EchoWorker : public AsyncProgressWorker { public: - EchoWorker(Function& callback, std::string& echo) - : AsyncProgressWorker(callback), echo(echo) {} + EchoWorker(Function& okCallback, std::string& echo) + : AsyncProgressWorker(okCallback), echo(echo) {} ~EchoWorker() {} - // This code will be executed on the worker thread - void Execute(const ExecutionProgress& progress) { - // Need to simulate cpu heavy task - for (uint32_t i = 0; i < 100; ++i) { - progress.Send(&i, 1) - std::this_thread::sleep_for(std::chrono::seconds(1)); + + // This code will be executed on the worker thread + void Execute(const ExecutionProgress& progress) { + // Need to simulate cpu heavy task + // Note: This Send() call is not guaranteed to trigger an equal + // number of OnProgress calls (read documentation above for more info) + for (uint32_t i = 0; i < 100; ++i) { + progress.Send(&i, 1) + } + } + + void OnError(const Error &e) { + HandleScope scope(Env()); + // Pass error onto JS, no data for other parameters + Callback().Call({String::New(Env(), e.Message())}); } - } - void OnOK() { - HandleScope scope(Env()); - Callback().Call({Env().Null(), String::New(Env(), echo)}); - } + void OnOK() { + HandleScope scope(Env()); + // Pass no error, give back original data + Callback().Call({Env().Null(), String::New(Env(), echo)}); + } - void OnProgress(const uint32_t* data, size_t /* count */) { - HandleScope scope(Env()); - Callback().Call({Env().Null(), Env().Null(), Number::New(Env(), *data)}); - } + void OnProgress(const uint32_t* data, size_t /* count */) { + HandleScope scope(Env()); + // Pass no error, no echo data, but do pass on the progress data + Callback().Call({Env().Null(), Env().Null(), Number::New(Env(), *data)}); + } private: std::string echo; @@ -327,12 +340,23 @@ using namespace Napi; Value Echo(const CallbackInfo& info) { // We need to validate the arguments here - Function cb = info[1].As(); std::string in = info[0].As(); + Function cb = info[1].As(); EchoWorker* wk = new EchoWorker(cb, in); wk->Queue(); return info.Env().Undefined(); } + +// Register the native method for JS to access +Object Init(Env env, Object exports) +{ + exports.Set(String::New(env, "echo"), Function::New(env, Echo)); + + return exports; +} + +// Register our native addon +NODE_API_MODULE(nativeAddon, Init) ``` The implementation of a `Napi::AsyncProgressWorker` can be used by creating a @@ -341,6 +365,20 @@ asynchronous task ends and other data needed for the computation. Once created, the only other action needed is to call the `Napi::AsyncProgressWorker::Queue` method that will queue the created worker for execution. +Lastly, the following Javascript (ES6+) code would be associated the above example: + +```js +const { nativeAddon } = require('binding.node'); + +const exampleCallback = (errorResponse, okResponse, progressData) => { + // Use the data accordingly + // ... +}; + +// Call our native addon with the paramters of a string and a function +nativeAddon.echo("example", exampleCallback); +``` + # AsyncProgressQueueWorker `Napi::AsyncProgressQueueWorker` acts exactly like `Napi::AsyncProgressWorker` @@ -379,7 +417,9 @@ void Napi::AsyncProgressQueueWorker::ExecutionProcess::Send(const T* data, size_ ## Example -The code below shows a basic example of the `Napi::AsyncProgressQueueWorker` implementation: +The code below show an example of the `Napi::AsyncProgressQueueWorker` implementation, but +also demonsrates how to use multiple `Napi::Function`'s if you wish to provide multiple +callback functions for more object oriented code: ```cpp #include @@ -391,31 +431,55 @@ using namespace Napi; class EchoWorker : public AsyncProgressQueueWorker { public: - EchoWorker(Function& callback, std::string& echo) - : AsyncProgressQueueWorker(callback), echo(echo) {} + EchoWorker(Function& okCallback, Function& errorCallback, Function& progressCallback, std::string& echo) + : AsyncProgressQueueWorker(okCallback), echo(echo) { + // Set our function references to use them below + this->errorCallback.Reset(errorCallback, 1); + this->progressCallback.Reset(progressCallback, 1); + } ~EchoWorker() {} - // This code will be executed on the worker thread - void Execute(const ExecutionProgress& progress) { - // Need to simulate cpu heavy task - for (uint32_t i = 0; i < 100; ++i) { - progress.Send(&i, 1); - std::this_thread::sleep_for(std::chrono::seconds(1)); + + // This code will be executed on the worker thread + void Execute(const ExecutionProgress& progress) { + // Need to simulate cpu heavy task to demonstrate that + // every call to Send() will trigger an OnProgress function call + for (uint32_t i = 0; i < 100; ++i) { + progress.Send(&i, 1); + } } - } - void OnOK() { - HandleScope scope(Env()); - Callback().Call({Env().Null(), String::New(Env(), echo)}); - } + void OnOK() { + HandleScope scope(Env()); + // Call our onOkCallback in javascript with the data we were given originally + Callback().Call({String::New(Env(), echo)}); + } + + void OnError(const Error &e) { + HandleScope scope(Env()); + + // We call our callback provided in the constructor with 2 parameters + if (!this->errorCallback.IsEmpty()) { + // Call our onErrorCallback in javascript with the error message + this->errorCallback.Call(Receiver().Value(), {String::New(Env(), e.Message())}); + } + } - void OnProgress(const uint32_t* data, size_t /* count */) { - HandleScope scope(Env()); - Callback().Call({Env().Null(), Env().Null(), Number::New(Env(), *data)}); - } + void OnProgress(const uint32_t* data, size_t /* count */) { + HandleScope scope(Env()); + + if (!this->progressCallback.IsEmpty()) { + // Call our onProgressCallback in javascript with each integer from 0 to 99 (inclusive) + // as this function is triggered from the above Send() calls + this->progressCallback.Call(Receiver().Value(), {Number::New(Env(), *data)}); + } + } private: std::string echo; + FunctionReference progressCallback; + FunctionReference errorCallback; + }; ``` @@ -439,12 +503,25 @@ using namespace Napi; Value Echo(const CallbackInfo& info) { // We need to validate the arguments here. - Function cb = info[1].As(); std::string in = info[0].As(); - EchoWorker* wk = new EchoWorker(cb, in); + Function errorCb = info[1].As(); + Function okCb = info[2].As(); + Function progressCb = info[3].As(); + EchoWorker* wk = new EchoWorker(okCb, errorCb, progressCb, in); wk->Queue(); return info.Env().Undefined(); } + +// Register the native method for JS to access +Object Init(Env env, Object exports) +{ + exports.Set(String::New(env, "echo"), Function::New(env, Echo)); + + return exports; +} + +// Register our native addon +NODE_API_MODULE(nativeAddon, Init) ``` The implementation of a `Napi::AsyncProgressQueueWorker` can be used by creating a @@ -453,4 +530,28 @@ asynchronous task ends and other data needed for the computation. Once created, the only other action needed is to call the `Napi::AsyncProgressQueueWorker::Queue` method that will queue the created worker for execution. +Lastly, the following Javascript (ES6+) code would be associated the above example: + +```js +const { nativeAddon } = require('binding.node'); + +const onErrorCallback = (msg) => { + // Use the data accordingly + // ... +}; + +const onOkCallback = (echo) => { + // Use the data accordingly + // ... +}; + +const onProgressCallback = (num) => { + // Use the data accordingly + // ... +}; + +// Call our native addon with the paramters of a string and three callback functions +nativeAddon.echo("example", onErrorCallback, onOkCallback, onProgressCallback); +``` + [`Napi::AsyncWorker`]: ./async_worker.md diff --git a/doc/error.md b/doc/error.md index 7530262d7..08c156dd0 100644 --- a/doc/error.md +++ b/doc/error.md @@ -117,4 +117,4 @@ Returns a pointer to a null-terminated string that is used to identify the exception. This method can be used only if the exception mechanism is enabled. [`Napi::ObjectReference`]: ./object_reference.md -[`std::exception`]: http://cplusplus.com/reference/exception/exception/ +[`std::exception`]: https://cplusplus.com/reference/exception/exception/ diff --git a/doc/hierarchy.md b/doc/hierarchy.md index e40a65ff9..921f94a99 100644 --- a/doc/hierarchy.md +++ b/doc/hierarchy.md @@ -88,4 +88,4 @@ [`Napi::Uint8Array`]: ./typed_array_of.md [`Napi::Value`]: ./value.md [`Napi::VersionManagement`]: ./version_management.md -[`std::exception`]: http://cplusplus.com/reference/exception/exception/ +[`std::exception`]: https://cplusplus.com/reference/exception/exception/ diff --git a/napi-inl.h b/napi-inl.h index 55273b662..396b8aeae 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -1568,6 +1568,20 @@ inline size_t ArrayBuffer::ByteLength() { return length; } +#if NAPI_VERSION >= 7 +inline bool ArrayBuffer::IsDetached() const { + bool detached; + napi_status status = napi_is_detached_arraybuffer(_env, _value, &detached); + NAPI_THROW_IF_FAILED(_env, status, false); + return detached; +} + +inline void ArrayBuffer::Detach() { + napi_status status = napi_detach_arraybuffer(_env, _value); + NAPI_THROW_IF_FAILED_VOID(_env, status); +} +#endif // NAPI_VERSION >= 7 + //////////////////////////////////////////////////////////////////////////////// // DataView class //////////////////////////////////////////////////////////////////////////////// @@ -4001,6 +4015,7 @@ inline napi_value ObjectWrap::StaticSetterCallbackWrapper( template inline void ObjectWrap::FinalizeCallback(napi_env env, void* data, void* /*hint*/) { + HandleScope scope(env); T* instance = static_cast(data); instance->Finalize(Napi::Env(env)); delete instance; diff --git a/napi.h b/napi.h index b42b4b707..9d813778b 100644 --- a/napi.h +++ b/napi.h @@ -822,6 +822,11 @@ namespace Napi { void* Data(); ///< Gets a pointer to the data buffer. size_t ByteLength(); ///< Gets the length of the array buffer in bytes. + +#if NAPI_VERSION >= 7 + bool IsDetached() const; + void Detach(); +#endif // NAPI_VERSION >= 7 }; /// A JavaScript typed-array value with unknown array type. diff --git a/package.json b/package.json index 93f3fd81c..85d6ea2d1 100644 --- a/package.json +++ b/package.json @@ -264,8 +264,10 @@ "description": "Node.js API (N-API)", "devDependencies": { "benchmark": "^2.1.4", - "fs-extra": "^9.0.1", "bindings": "^1.5.0", + "clang-format": "^1.4.0", + "fs-extra": "^9.0.1", + "pre-commit": "^1.2.2", "safe-buffer": "^5.1.1" }, "directories": {}, @@ -300,7 +302,10 @@ "dev": "node test", "predev:incremental": "node-gyp configure build -C test --debug", "dev:incremental": "node test", - "doc": "doxygen doc/Doxyfile" + "doc": "doxygen doc/Doxyfile", + "lint": "node tools/clang-format.js", + "lint:fix": "git-clang-format" }, + "pre-commit": "lint", "version": "3.0.2" } diff --git a/test/addon_build/index.js b/test/addon_build/index.js index 0d0c6ea2f..c410bf3a8 100644 --- a/test/addon_build/index.js +++ b/test/addon_build/index.js @@ -1,7 +1,7 @@ 'use strict'; const { promisify } = require('util'); -const exec = promisify(require('child_process').exec); +const exec = promisify(require('child_process').exec); const { copy, remove } = require('fs-extra'); const path = require('path'); const assert = require('assert') @@ -26,8 +26,8 @@ async function test(addon) { const { stderr, stdout } = await exec('npm install', { cwd: path.join(ADDONS_FOLDER, addon) }) - console.log(` >Runting test for: '${addon}'`); - // Disabled the checks on stderr and stdout because of this issuue on npm: + console.log(` >Running test for: '${addon}'`); + // Disabled the checks on stderr and stdout because of this issue on npm: // Stop using process.umask(): https://github.com/npm/cli/issues/1103 // We should enable the following checks again after the resolution of // the reported issue. @@ -41,7 +41,6 @@ async function test(addon) { assert.strictEqual(binding.noexcept.echo(103), 103); } - module.exports = (async function() { await beforeAll(addons); for (const addon of addons) { diff --git a/test/arraybuffer.cc b/test/arraybuffer.cc index cc4f1f51c..4a1b2a34e 100644 --- a/test/arraybuffer.cc +++ b/test/arraybuffer.cc @@ -157,19 +157,43 @@ void CheckDetachUpdatesData(const CallbackInfo& info) { return; } - if (!info[1].IsFunction()) { - Error::New(info.Env(), "A function was expected.").ThrowAsJavaScriptException(); - return; - } - ArrayBuffer buffer = info[0].As(); - Function detach = info[1].As(); // This potentially causes the buffer to cache its data pointer and length. buffer.Data(); buffer.ByteLength(); - detach.Call({}); +#if NAPI_VERSION >= 7 + if (buffer.IsDetached()) { + Error::New(info.Env(), "Buffer should not be detached.").ThrowAsJavaScriptException(); + return; + } +#endif + + if (info.Length() == 2) { + // Detach externally (in JavaScript). + if (!info[1].IsFunction()) { + Error::New(info.Env(), "A function was expected.").ThrowAsJavaScriptException(); + return; + } + + Function detach = info[1].As(); + detach.Call({}); + } else { +#if NAPI_VERSION >= 7 + // Detach directly. + buffer.Detach(); +#else + return; +#endif + } + +#if NAPI_VERSION >= 7 + if (!buffer.IsDetached()) { + Error::New(info.Env(), "Buffer should be detached.").ThrowAsJavaScriptException(); + return; + } +#endif if (buffer.Data() != nullptr) { Error::New(info.Env(), "Incorrect data pointer.").ThrowAsJavaScriptException(); diff --git a/test/arraybuffer.js b/test/arraybuffer.js index 38d35d1e7..363de17d9 100644 --- a/test/arraybuffer.js +++ b/test/arraybuffer.js @@ -58,8 +58,13 @@ function test(binding) { 'ArrayBuffer updates data pointer and length when detached', () => { + // Detach the ArrayBuffer in JavaScript. const mem = new WebAssembly.Memory({ initial: 1 }); binding.arraybuffer.checkDetachUpdatesData(mem.buffer, () => mem.grow(1)); + + // Let C++ detach the ArrayBuffer. + const extBuffer = binding.arraybuffer.createExternalBuffer(); + binding.arraybuffer.checkDetachUpdatesData(extBuffer); }, ]); } diff --git a/test/objectwrap_worker_thread.js b/test/objectwrap_worker_thread.js index 348309976..5e5e50b7e 100644 --- a/test/objectwrap_worker_thread.js +++ b/test/objectwrap_worker_thread.js @@ -1,14 +1,15 @@ 'use strict'; -const buildType = process.config.target_defaults.default_configuration; -const { Worker, isMainThread } = require('worker_threads'); +const { Worker, isMainThread, workerData } = require('worker_threads'); if (isMainThread) { - new Worker(__filename); + const buildType = process.config.target_defaults.default_configuration; + new Worker(__filename, { workerData: buildType }); } else { const test = binding => { new binding.objectwrap.Test(); }; + const buildType = workerData; test(require(`./build/${buildType}/binding.node`)); test(require(`./build/${buildType}/binding_noexcept.node`)); } diff --git a/tools/README.md b/tools/README.md index 8d110609c..a1024d586 100644 --- a/tools/README.md +++ b/tools/README.md @@ -1,4 +1,10 @@ -# Migration Script +# Tools + +## clang-format + +The clang-format checking tools is designed to check changed lines of code compared to given git-refs. + +## Migration Script The migration tool is designed to reduce repetitive work in the migration process. However, the script is not aiming to convert every thing for you. There are usually some small fixes and major reconstruction required. diff --git a/tools/clang-format.js b/tools/clang-format.js new file mode 100644 index 000000000..3a2ba4a2f --- /dev/null +++ b/tools/clang-format.js @@ -0,0 +1,43 @@ +#!/usr/bin/env node + +const spawn = require('child_process').spawnSync; +const path = require('path'); + +const filesToCheck = ['*.h', '*.cc']; +const CLANG_FORMAT_START = process.env.CLANG_FORMAT_START || 'master'; + +function main(args) { + let clangFormatPath = path.dirname(require.resolve('clang-format')); + const options = ['--binary=node_modules/.bin/clang-format', '--style=file']; + + const gitClangFormatPath = path.join(clangFormatPath, + 'bin/git-clang-format'); + const result = spawn('python', [ + gitClangFormatPath, + ...options, + '--diff', + CLANG_FORMAT_START, + 'HEAD', + ...filesToCheck + ], { encoding: 'utf-8' }); + + if (result.error) { + console.error('Error running git-clang-format:', result.error); + return 2; + } + + const clangFormatOutput = result.stdout.trim(); + if (clangFormatOutput !== '' && + clangFormatOutput !== ('no modified files to format') && + clangFormatOutput !== ('clang-format did not modify any files')) { + console.error(clangFormatOutput); + const fixCmd = '"npm run lint:fix"'; + console.error(` + ERROR: please run ${fixCmd} to format changes in your commit`); + return 1; + } +} + +if (require.main === module) { + process.exitCode = main(process.argv.slice(2)); +}