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. PR-URL: #22408 Fixes: #21446 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
nodejs · Sep 3, 2018 · 0b340ab · 0b340ab
1 parent acfb29c
commit 0b340ab
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 54 deletions.
diff --git a/lib/internal/util.js b/lib/internal/util.js
@@ -364,7 +364,6 @@ function isInsideNodeModules() {
   return false;
 }
 
-
 module.exports = {
   assertCrypto,
   cachedResult,

diff --git a/lib/internal/util/comparisons.js b/lib/internal/util/comparisons.js
@@ -4,7 +4,12 @@ const { compare } = process.binding('buffer');
 const { isArrayBufferView } = require('internal/util/types');
 const { internalBinding } = require('internal/bootstrap/loaders');
 const { isDate, isMap, isRegExp, isSet } = internalBinding('types');
-const { getOwnNonIndexProperties } = process.binding('util');
+const {
+  getOwnNonIndexProperties,
+  propertyFilter: {
+    ONLY_ENUMERABLE
+  }
+} = process.binding('util');
 
 const ReflectApply = Reflect.apply;
 
@@ -107,8 +112,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);
@@ -139,8 +145,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
@@ -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,13 @@ const {
 const { sendInspectorCommand } = require('internal/util/inspector');
 const { experimentalREPLAwait } = process.binding('config');
 const { isRecoverableError } = require('internal/repl/recoverable');
+const {
+  getOwnNonIndexProperties,
+  propertyFilter: {
+    ALL_PROPERTIES,
+    SKIP_SYMBOLS
+  }
+} = process.binding('util');
 
 // Lazy-loaded.
 let processTopLevelAwait;
@@ -927,34 +933,10 @@ 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 filter = ALL_PROPERTIES | SKIP_SYMBOLS;
+  return getOwnNonIndexProperties(obj, filter).filter(isIdentifier);
 }
 
 function getGlobalLexicalScopeNames(contextId) {

diff --git a/src/node_util.cc b/src/node_util.cc
@@ -4,34 +4,43 @@
 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;
 
 static void GetOwnNonIndexProperties(
     const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   Local<Context> context = env->context();
 
-  if (!args[0]->IsObject())
-    return;
+  CHECK(args[0]->IsObject());
+  CHECK(args[1]->IsUint32());
+
+  Local<Object> object = args[0].As<Object>();
 
-  v8::Local<v8::Object> object = args[0].As<v8::Object>();
+  Local<Array> properties;
 
-  // Return only non-enumerable properties by default.
-  v8::Local<v8::Array> properties;
+  v8::PropertyFilter filter =
+    static_cast<v8::PropertyFilter>(args[1].As<Uint32>()->Value());
 
   if (!object->GetPropertyNames(
         context, v8::KeyCollectionMode::kOwnOnly,
-        v8::ONLY_ENUMERABLE,
+        filter,
         v8::IndexFilter::kSkipIndices)
           .ToLocal(&properties)) {
     return;
@@ -209,6 +218,17 @@ void Initialize(Local<Object> target,
                              WatchdogHasPendingSigint);
 
   env->SetMethod(target, "safeGetenv", SafeGetenv);
+
+  Local<Object> 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

diff --git 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);
   }));
 });