From 3f26e1f071de56bc652e0020563a029ad5ff4556 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 20 Aug 2018 03:26:27 +0200 Subject: [PATCH 1/3] repl: tab auto complete big arrays Due to a new API it's possible to skip the indices. That allows to use auto completion with big (typed) arrays. Fixes: #21446 --- lib/internal/util.js | 9 +++++++ lib/internal/util/comparisons.js | 11 +++++---- lib/repl.js | 31 ++++--------------------- src/node_util.cc | 13 +++++++---- test/parallel/test-repl-tab-complete.js | 19 ++++----------- 5 files changed, 32 insertions(+), 51 deletions(-) diff --git a/lib/internal/util.js b/lib/internal/util.js index bf8f9c26cf6e9a..bf293268c6aa7e 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -366,6 +366,14 @@ function isInsideNodeModules() { return false; } +const kPropertyFilter = Object.freeze({ + ALL_PROPERTIES: 0, + ONLY_WRITABLE: 1, + ONLY_ENUMERABLE: 2, + ONLY_CONFIGURABLE: 4, + SKIP_STRINGS: 8, + SKIP_SYMBOLS: 16 +}); module.exports = { assertCrypto, @@ -386,6 +394,7 @@ module.exports = { promisify, spliceOne, removeColors, + kPropertyFilter, // Symbol used to customize promisify conversion customPromisifyArgs: kCustomPromisifyArgsSymbol, diff --git a/lib/internal/util/comparisons.js b/lib/internal/util/comparisons.js index 9844e9d59c0c5a..3ce4a612b0987c 100644 --- a/lib/internal/util/comparisons.js +++ b/lib/internal/util/comparisons.js @@ -11,6 +11,7 @@ const { isSet } = internalBinding('types'); const { getOwnNonIndexProperties } = process.binding('util'); +const { kPropertyFilter: { ONLY_ENUMERABLE } } = require('internal/util'); const ReflectApply = Reflect.apply; @@ -118,8 +119,9 @@ function strictDeepEqual(val1, val2, memos) { if (val1.length !== val2.length) { return false; } - const keys1 = getOwnNonIndexProperties(val1); - if (keys1.length !== getOwnNonIndexProperties(val2).length) { + const keys1 = getOwnNonIndexProperties(val1, ONLY_ENUMERABLE); + const keys2 = getOwnNonIndexProperties(val2, ONLY_ENUMERABLE); + if (keys1.length !== keys2.length) { return false; } return keyCheck(val1, val2, kStrict, memos, kIsArray, keys1); @@ -150,8 +152,9 @@ function strictDeepEqual(val1, val2, memos) { // Buffer.compare returns true, so val1.length === val2.length. If they both // only contain numeric keys, we don't need to exam further than checking // the symbols. - const keys1 = getOwnNonIndexProperties(val1); - if (keys1.length !== getOwnNonIndexProperties(val2).length) { + const keys1 = getOwnNonIndexProperties(val1, ONLY_ENUMERABLE); + const keys2 = getOwnNonIndexProperties(val2, ONLY_ENUMERABLE); + if (keys1.length !== keys2.length) { return false; } return keyCheck(val1, val2, kStrict, memos, kNoIterator, keys1); diff --git a/lib/repl.js b/lib/repl.js index 9530d57a347468..c9514ef83ec7e6 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -52,7 +52,6 @@ const { isIdentifierChar } = require('internal/deps/acorn/dist/acorn'); const internalUtil = require('internal/util'); -const { isTypedArray } = require('internal/util/types'); const util = require('util'); const utilBinding = process.binding('util'); const { inherits } = util; @@ -74,6 +73,7 @@ const { const { sendInspectorCommand } = require('internal/util/inspector'); const { experimentalREPLAwait } = process.binding('config'); const { isRecoverableError } = require('internal/repl/recoverable'); +const { getOwnNonIndexProperties } = process.binding('util'); // Lazy-loaded. let processTopLevelAwait; @@ -927,34 +927,11 @@ function isIdentifier(str) { return true; } -const ARRAY_LENGTH_THRESHOLD = 1e6; - -function mayBeLargeObject(obj) { - if (Array.isArray(obj)) { - return obj.length > ARRAY_LENGTH_THRESHOLD ? ['length'] : null; - } else if (isTypedArray(obj)) { - return obj.length > ARRAY_LENGTH_THRESHOLD ? [] : null; - } - - return null; -} - function filteredOwnPropertyNames(obj) { if (!obj) return []; - const fakeProperties = mayBeLargeObject(obj); - if (fakeProperties !== null) { - this.outputStream.write('\r\n'); - process.emitWarning( - 'The current array, Buffer or TypedArray has too many entries. ' + - 'Certain properties may be missing from completion output.', - 'REPLWarning', - undefined, - undefined, - true); - - return fakeProperties; - } - return Object.getOwnPropertyNames(obj).filter(isIdentifier); + const { kPropertyFilter } = internalUtil; + const filter = kPropertyFilter.ALL_PROPERTIES | kPropertyFilter.SKIP_SYMBOLS; + return getOwnNonIndexProperties(obj, filter).filter(isIdentifier); } function getGlobalLexicalScopeNames(contextId) { diff --git a/src/node_util.cc b/src/node_util.cc index 591f3e3b5eb91f..9002a82e248183 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -21,17 +21,20 @@ static void GetOwnNonIndexProperties( Environment* env = Environment::GetCurrent(args); Local context = env->context(); - if (!args[0]->IsObject()) + if (!args[0]->IsObject() || !args[1]->IsUint32()) return; - v8::Local object = args[0].As(); + Local object = args[0].As(); + + Local properties; - // Return only non-enumerable properties by default. - v8::Local properties; + v8::PropertyFilter filter = + static_cast( + args[1]->Uint32Value(env->context()).FromJust()); if (!object->GetPropertyNames( context, v8::KeyCollectionMode::kOwnOnly, - v8::ONLY_ENUMERABLE, + filter, v8::IndexFilter::kSkipIndices) .ToLocal(&properties)) { return; diff --git a/test/parallel/test-repl-tab-complete.js b/test/parallel/test-repl-tab-complete.js index 9d6ecf4e284811..ca7d2054758d21 100644 --- a/test/parallel/test-repl-tab-complete.js +++ b/test/parallel/test-repl-tab-complete.js @@ -393,12 +393,6 @@ testMe.complete('obj.', common.mustCall((error, data) => { assert(data[0].includes('obj.key')); })); -// tab completion for large buffer -const warningRegEx = new RegExp( - '\\(node:\\d+\\) REPLWarning: The current array, Buffer or TypedArray has ' + - 'too many entries\\. Certain properties may be missing from completion ' + - 'output\\.'); - [ Array, Buffer, @@ -428,11 +422,7 @@ const warningRegEx = new RegExp( putIn.run([`var ele = new ${type.name}(1e6 + 1); ele.biu = 1;`]); } - common.hijackStderr(common.mustCall((err) => { - process.nextTick(() => { - assert.ok(warningRegEx.test(err)); - }); - })); + common.hijackStderr(common.mustNotCall()); testMe.complete('ele.', common.mustCall((err, data) => { common.restoreStderr(); assert.ifError(err); @@ -443,13 +433,12 @@ const warningRegEx = new RegExp( Buffer.alloc(0) : new type(0)); + assert.strictEqual(data[0].includes('ele.biu'), true); + data[0].forEach((key) => { - if (!key) return; + if (!key || key === 'ele.biu') return; assert.notStrictEqual(ele[key.substr(4)], undefined); }); - - // no `biu` - assert.strictEqual(data.includes('ele.biu'), false); })); }); From 7be3845308eba83691e63f2331242eeae6550d43 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 20 Aug 2018 17:45:44 +0200 Subject: [PATCH 2/3] fixup: address comments --- lib/internal/util.js | 10 ---------- lib/internal/util/comparisons.js | 8 ++++++-- lib/repl.js | 11 ++++++++--- src/node_util.cc | 25 +++++++++++++++++++++---- 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/lib/internal/util.js b/lib/internal/util.js index bf293268c6aa7e..40afd9207e70f4 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -366,15 +366,6 @@ function isInsideNodeModules() { return false; } -const kPropertyFilter = Object.freeze({ - ALL_PROPERTIES: 0, - ONLY_WRITABLE: 1, - ONLY_ENUMERABLE: 2, - ONLY_CONFIGURABLE: 4, - SKIP_STRINGS: 8, - SKIP_SYMBOLS: 16 -}); - module.exports = { assertCrypto, cachedResult, @@ -394,7 +385,6 @@ module.exports = { promisify, spliceOne, removeColors, - kPropertyFilter, // Symbol used to customize promisify conversion customPromisifyArgs: kCustomPromisifyArgsSymbol, diff --git a/lib/internal/util/comparisons.js b/lib/internal/util/comparisons.js index 3ce4a612b0987c..1cde5502ca8778 100644 --- a/lib/internal/util/comparisons.js +++ b/lib/internal/util/comparisons.js @@ -10,8 +10,12 @@ const { isRegExp, isSet } = internalBinding('types'); -const { getOwnNonIndexProperties } = process.binding('util'); -const { kPropertyFilter: { ONLY_ENUMERABLE } } = require('internal/util'); +const { + getOwnNonIndexProperties, + propertyFilter: { + ONLY_ENUMERABLE + } +} = process.binding('util'); const ReflectApply = Reflect.apply; diff --git a/lib/repl.js b/lib/repl.js index c9514ef83ec7e6..a0cf2c1dd086ea 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -73,7 +73,13 @@ const { const { sendInspectorCommand } = require('internal/util/inspector'); const { experimentalREPLAwait } = process.binding('config'); const { isRecoverableError } = require('internal/repl/recoverable'); -const { getOwnNonIndexProperties } = process.binding('util'); +const { + getOwnNonIndexProperties, + propertyFilter: { + ALL_PROPERTIES, + SKIP_SYMBOLS + } +} = process.binding('util'); // Lazy-loaded. let processTopLevelAwait; @@ -929,8 +935,7 @@ function isIdentifier(str) { function filteredOwnPropertyNames(obj) { if (!obj) return []; - const { kPropertyFilter } = internalUtil; - const filter = kPropertyFilter.ALL_PROPERTIES | kPropertyFilter.SKIP_SYMBOLS; + const filter = ALL_PROPERTIES | SKIP_SYMBOLS; return getOwnNonIndexProperties(obj, filter).filter(isIdentifier); } diff --git a/src/node_util.cc b/src/node_util.cc index 9002a82e248183..0b02281996bd31 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -14,23 +14,29 @@ using v8::Private; using v8::Promise; using v8::Proxy; using v8::String; +using v8::Uint32; using v8::Value; +using v8::ALL_PROPERTIES; +using v8::ONLY_WRITABLE; +using v8::ONLY_ENUMERABLE; +using v8::ONLY_CONFIGURABLE; +using v8::SKIP_STRINGS; +using v8::SKIP_SYMBOLS; static void GetOwnNonIndexProperties( const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Local context = env->context(); - if (!args[0]->IsObject() || !args[1]->IsUint32()) - return; + CHECK(args[0]->IsObject()); + CHECK(args[1]->IsUint32()); Local object = args[0].As(); Local properties; v8::PropertyFilter filter = - static_cast( - args[1]->Uint32Value(env->context()).FromJust()); + static_cast(args[1].As()->Value()); if (!object->GetPropertyNames( context, v8::KeyCollectionMode::kOwnOnly, @@ -212,6 +218,17 @@ void Initialize(Local target, WatchdogHasPendingSigint); env->SetMethod(target, "safeGetenv", SafeGetenv); + + Local constants = Object::New(env->isolate()); + NODE_DEFINE_CONSTANT(constants, ALL_PROPERTIES); + NODE_DEFINE_CONSTANT(constants, ONLY_WRITABLE); + NODE_DEFINE_CONSTANT(constants, ONLY_ENUMERABLE); + NODE_DEFINE_CONSTANT(constants, ONLY_CONFIGURABLE); + NODE_DEFINE_CONSTANT(constants, SKIP_STRINGS); + NODE_DEFINE_CONSTANT(constants, SKIP_SYMBOLS); + target->Set(context, + FIXED_ONE_BYTE_STRING(env->isolate(), "propertyFilter"), + constants).FromJust(); } } // namespace util From 65993be9e0ae2ae58e9d12b87cf23c17179a4b2a Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 20 Aug 2018 17:50:34 +0200 Subject: [PATCH 3/3] fixup: address c++ linter --- src/node_util.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/node_util.cc b/src/node_util.cc index 0b02281996bd31..5adecf4d9753c3 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -4,24 +4,24 @@ namespace node { namespace util { +using v8::ALL_PROPERTIES; using v8::Array; using v8::Context; using v8::FunctionCallbackInfo; using v8::Integer; using v8::Local; using v8::Object; +using v8::ONLY_CONFIGURABLE; +using v8::ONLY_ENUMERABLE; +using v8::ONLY_WRITABLE; using v8::Private; using v8::Promise; using v8::Proxy; +using v8::SKIP_STRINGS; +using v8::SKIP_SYMBOLS; using v8::String; using v8::Uint32; using v8::Value; -using v8::ALL_PROPERTIES; -using v8::ONLY_WRITABLE; -using v8::ONLY_ENUMERABLE; -using v8::ONLY_CONFIGURABLE; -using v8::SKIP_STRINGS; -using v8::SKIP_SYMBOLS; static void GetOwnNonIndexProperties( const FunctionCallbackInfo& args) {