From 815fbec6f1cb4baaa1cb2cfa2531e90362833ba3 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Thu, 29 Jul 2021 16:52:06 +0200
Subject: [PATCH] repl: do not include legacy getter/setter methods in
 completion
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

For every object that inherits from `Object.prototype`, the REPL
includes the `Object.prototype` methods in its autocompletion.

This is already a little noisy, but in particular, this also
includes the legacy `__defineGetter__` family of methods;
since those are deprecated and not in practical use anymore,
it helps reduce noise a bit to remove them.

This commit does not remove `__proto__` as it is a little
more popular and, despite its downsides, a slightly more convenient
way to access the prototype of an object in the REPL than
`Object.getPrototypeOf(...)`.

PR-URL: https://github.com/nodejs/node/pull/39576
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
---
 lib/repl.js                             | 24 ++++++++++++++++++++++--
 test/parallel/test-repl-tab-complete.js | 12 ++++++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/lib/repl.js b/lib/repl.js
index fd626e1824f85b..19011841519351 100644
--- a/lib/repl.js
+++ b/lib/repl.js
@@ -1188,11 +1188,31 @@ function isIdentifier(str) {
   return true;
 }
 
+function isNotLegacyObjectPrototypeMethod(str) {
+  return isIdentifier(str) &&
+    str !== '__defineGetter__' &&
+    str !== '__defineSetter__' &&
+    str !== '__lookupGetter__' &&
+    str !== '__lookupSetter__';
+}
+
 function filteredOwnPropertyNames(obj) {
   if (!obj) return [];
+  // `Object.prototype` is the only non-contrived object that fulfills
+  // `Object.getPrototypeOf(X) === null &&
+  //  Object.getPrototypeOf(Object.getPrototypeOf(X.constructor)) === X`.
+  let isObjectPrototype = false;
+  if (ObjectGetPrototypeOf(obj) === null) {
+    const ctorDescriptor = ObjectGetOwnPropertyDescriptor(obj, 'constructor');
+    if (ctorDescriptor && ctorDescriptor.value) {
+      const ctorProto = ObjectGetPrototypeOf(ctorDescriptor.value);
+      isObjectPrototype = ctorProto && ObjectGetPrototypeOf(ctorProto) === obj;
+    }
+  }
   const filter = ALL_PROPERTIES | SKIP_SYMBOLS;
-  return ArrayPrototypeFilter(getOwnNonIndexProperties(obj, filter),
-                              isIdentifier);
+  return ArrayPrototypeFilter(
+    getOwnNonIndexProperties(obj, filter),
+    isObjectPrototype ? isNotLegacyObjectPrototypeMethod : isIdentifier);
 }
 
 function getGlobalLexicalScopeNames(contextId) {
diff --git a/test/parallel/test-repl-tab-complete.js b/test/parallel/test-repl-tab-complete.js
index da0ebfba9f965a..9597c2a3480f33 100644
--- a/test/parallel/test-repl-tab-complete.js
+++ b/test/parallel/test-repl-tab-complete.js
@@ -443,6 +443,18 @@ testMe.complete('obj.', common.mustCall((error, data) => {
   assert(data[0].includes('obj.key'));
 }));
 
+// Make sure tab completion does not include __defineSetter__ and friends.
+putIn.run(['.clear']);
+
+putIn.run(['var obj = {};']);
+testMe.complete('obj.', common.mustCall(function(error, data) {
+  assert.strictEqual(data[0].includes('obj.__defineGetter__'), false);
+  assert.strictEqual(data[0].includes('obj.__defineSetter__'), false);
+  assert.strictEqual(data[0].includes('obj.__lookupGetter__'), false);
+  assert.strictEqual(data[0].includes('obj.__lookupSetter__'), false);
+  assert.strictEqual(data[0].includes('obj.__proto__'), true);
+}));
+
 // Tab completion for files/directories
 {
   putIn.run(['.clear']);