Skip to content

Commit

Permalink
repl: do not include legacy getter/setter methods in completion
Browse files Browse the repository at this point in the history
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: #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>
  • Loading branch information
addaleax authored and targos committed Sep 4, 2021
1 parent 3482bca commit 9264cae
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 2 deletions.
24 changes: 22 additions & 2 deletions lib/repl.js
Expand Up @@ -1213,11 +1213,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) {
Expand Down
12 changes: 12 additions & 0 deletions test/parallel/test-repl-tab-complete.js
Expand Up @@ -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']);
Expand Down

0 comments on commit 9264cae

Please sign in to comment.