From 0dfe28812251748f42b7e191f87034e3f44f6f7b Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Thu, 17 Apr 2025 12:47:26 +0200 Subject: [PATCH 1/8] lib: disable REPL completion on proxies and getters the REPL completion logic evaluates code, this is generally ok but it can be problematic when there are objects with nested properties since the code evaluation would trigger their potential getters (e.g. the `obj.foo.b` line would trigger the getter of `foo`), the changes here disable the completion logic when proxies and getters are involved thus making sure that code evaluation does not take place when it can potentially (and surprisingly to the user) trigger side effectful behaviors --- lib/repl.js | 45 +- ...est-repl-completion-on-getters-disabled.js | 119 +++ test/parallel/test-repl-save-load.js | 277 ++--- test/parallel/test-repl-tab-complete.js | 991 +++++++++--------- 4 files changed, 814 insertions(+), 618 deletions(-) create mode 100644 test/parallel/test-repl-completion-on-getters-disabled.js diff --git a/lib/repl.js b/lib/repl.js index 6dfba9000e0790..103356ff01a68f 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -1328,9 +1328,11 @@ function completeFSFunctions(match) { // -> [['util.print', 'util.debug', 'util.log', 'util.inspect'], // 'util.' ] // -// Warning: This eval's code like "foo.bar.baz", so it will run property -// getter code. -function complete(line, callback) { +// Warning: This eval's code like "foo.bar.baz", so it could run property +// getter code. To avoid potential side-effecful getters the completion +// logic is skipped when getters or proxies are involved in the expression. +// (see: https://github.com/nodejs/node/issues/57829). +async function complete(line, callback) { // List of completion lists, one for each inheritance "level" let completionGroups = []; let completeOn, group; @@ -1525,6 +1527,12 @@ function complete(line, callback) { return; } + if (await includesProxiesOrGetters(match, this.eval, this.context)) { + // The expression involves proxies or getters, meaning that it + // can trigger side-effectful behaviors, so bail out + return completionGroupsLoaded(); + } + let chaining = '.'; if (StringPrototypeEndsWith(expr, '?')) { expr = StringPrototypeSlice(expr, 0, -1); @@ -1626,6 +1634,37 @@ function complete(line, callback) { } } +async function includesProxiesOrGetters(fullExpr, evalFn, context) { + const bits = StringPrototypeSplit(fullExpr, '.'); + + let currentExpr = ''; + for (let i = 0; i < bits.length - 1; i++) { + currentExpr += `${i === 0 ? '' : '.'}${bits[i]}`; + const currentExprIsObject = await evalPromisified(`try { ${currentExpr} !== null && typeof ${currentExpr} === 'object' } catch { false }`); + if (!currentExprIsObject) { + return false; + } + + const currentExprIsProxy = await evalPromisified(`require("node:util/types").isProxy(${currentExpr})`); + if (currentExprIsProxy) { + return true; + } + + const typeOfNextBitGet = await evalPromisified(`typeof Object.getOwnPropertyDescriptor(${currentExpr}, '${bits[i + 1]}')?.get`); + const nextBitHasGetter = typeOfNextBitGet === 'function'; + if (nextBitHasGetter) { + return true; + } + } + + function evalPromisified(evalExpr) { + return new Promise((resolve, reject) => + evalFn(evalExpr, context, getREPLResourceName(), (_, res) => { + resolve(res); + })); + } +} + REPLServer.prototype.completeOnEditorMode = (callback) => (err, results) => { if (err) return callback(err); diff --git a/test/parallel/test-repl-completion-on-getters-disabled.js b/test/parallel/test-repl-completion-on-getters-disabled.js new file mode 100644 index 00000000000000..17821bccd761b5 --- /dev/null +++ b/test/parallel/test-repl-completion-on-getters-disabled.js @@ -0,0 +1,119 @@ +'use strict'; + +const common = require('../common'); +const assert = require('node:assert'); +const { describe, test } = require('node:test'); + +const ArrayStream = require('../common/arraystream'); + +const repl = require('node:repl'); + +function runCompletionTests(replInit, tests) { + const stream = new ArrayStream(); + const testRepl = repl.start({ stream }); + + // Some errors are passed to the domain + testRepl._domain.on('error', assert.ifError); + + testRepl.write(replInit); + testRepl.write('\n'); + + tests.forEach(([query, expectedCompletions]) => { + testRepl.complete(query, common.mustCall((error, data) => { + const actualCompletions = data[0]; + if (expectedCompletions.length === 0) { + assert.deepStrictEqual(actualCompletions, []); + } else { + expectedCompletions.forEach((expectedCompletion) => + assert(actualCompletions.includes(expectedCompletion), `completion '${expectedCompletion}' not found`) + ); + } + })); + }); +} + +describe('REPL completion in relation of getters', () => { + describe('standard behavior without proxies/getters', () => { + test('completion of nested properties of an undeclared objects', () => { + runCompletionTests('', [ + ['nonExisting.', []], + ['nonExisting.f', []], + ['nonExisting.foo', []], + ['nonExisting.foo.', []], + ['nonExisting.foo.bar.b', []], + ]); + }); + + test('completion of nested properties on plain objects', () => { + runCompletionTests('const plainObj = { foo: { bar: { baz: {} } } };', [ + ['plainObj.', ['plainObj.foo']], + ['plainObj.f', ['plainObj.foo']], + ['plainObj.foo', ['plainObj.foo']], + ['plainObj.foo.', ['plainObj.foo.bar']], + ['plainObj.foo.bar.b', ['plainObj.foo.bar.baz']], + ['plainObj.fooBar.', []], + ['plainObj.fooBar.baz', []], + ]); + }); + }); + + describe('completions on an object with getters', () => { + test(`completions are generated for properties that don't trigger getters`, () => { + runCompletionTests( + ` + const objWithGetters = { + foo: { bar: { baz: {} }, get gBar() { return { baz: {} } } }, + get gFoo() { return { bar: { baz: {} } }; } + }; + `, [ + ['objWithGetters.', ['objWithGetters.foo']], + ['objWithGetters.f', ['objWithGetters.foo']], + ['objWithGetters.foo', ['objWithGetters.foo']], + ['objWithGetters.foo.', ['objWithGetters.foo.bar']], + ['objWithGetters.foo.bar.b', ['objWithGetters.foo.bar.baz']], + ['objWithGetters.gFo', ['objWithGetters.gFoo']], + ['objWithGetters.foo.gB', ['objWithGetters.foo.gBar']], + ]); + }); + + test('no completions are generated for properties that trigger getters', () => { + runCompletionTests( + ` + const objWithGetters = { + foo: { bar: { baz: {} }, get gBar() { return { baz: {} } } }, + get gFoo() { return { bar: { baz: {} } }; } + }; + `, + [ + ['objWithGetters.gFoo.', []], + ['objWithGetters.gFoo.b', []], + ['objWithGetters.gFoo.bar.b', []], + ['objWithGetters.foo.gBar.', []], + ['objWithGetters.foo.gBar.b', []], + ]); + }); + }); + + describe('completions on proxies', () => { + test('no completions are generated for a proxy object', () => { + runCompletionTests('const proxyObj = new Proxy({ foo: { bar: { baz: {} } } }, {});', [ + ['proxyObj.', []], + ['proxyObj.f', []], + ['proxyObj.foo', []], + ['proxyObj.foo.', []], + ['proxyObj.foo.bar.b', []], + ]); + }); + + test('no completions are generated for a proxy present in a standard object', () => { + runCompletionTests( + 'const objWithProxy = { foo: { bar: new Proxy({ baz: {} }, {}) } };', [ + ['objWithProxy.', ['objWithProxy.foo']], + ['objWithProxy.foo', ['objWithProxy.foo']], + ['objWithProxy.foo.', ['objWithProxy.foo.bar']], + ['objWithProxy.foo.b', ['objWithProxy.foo.bar']], + ['objWithProxy.foo.bar.', []], + ]); + }); + }); +}); diff --git a/test/parallel/test-repl-save-load.js b/test/parallel/test-repl-save-load.js index bb5130d1d71bbf..dd4eaccefff705 100644 --- a/test/parallel/test-repl-save-load.js +++ b/test/parallel/test-repl-save-load.js @@ -27,138 +27,155 @@ const fs = require('fs'); const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); -const repl = require('repl'); - -const works = [['inner.one'], 'inner.o']; - -const putIn = new ArrayStream(); -const testMe = repl.start('', putIn); - -// Some errors might be passed to the domain. -testMe._domain.on('error', function(reason) { - const err = new Error('Test failed'); - err.reason = reason; - throw err; -}); - -const testFile = [ - 'let inner = (function() {', - ' return {one:1};', - '})()', -]; -const saveFileName = tmpdir.resolve('test.save.js'); - -// Add some data. -putIn.run(testFile); - -// Save it to a file. -putIn.run([`.save ${saveFileName}`]); - -// The file should have what I wrote. -assert.strictEqual(fs.readFileSync(saveFileName, 'utf8'), - testFile.join('\n')); - -// Make sure that the REPL data is "correct". -testMe.complete('inner.o', common.mustSucceed((data) => { - assert.deepStrictEqual(data, works); -})); - -// Clear the REPL. -putIn.run(['.clear']); - -testMe._sawKeyPress = true; -// Load the file back in. -putIn.run([`.load ${saveFileName}`]); - -// Make sure loading doesn't insert extra indentation -// https://github.com/nodejs/node/issues/47673 -assert.strictEqual(testMe.line, ''); - -// Make sure that the REPL data is "correct". -testMe.complete('inner.o', common.mustSucceed((data) => { - assert.deepStrictEqual(data, works); -})); - -// Clear the REPL. -putIn.run(['.clear']); - -let loadFile = tmpdir.resolve('file.does.not.exist'); - -// Should not break. -putIn.write = common.mustCall(function(data) { - // Make sure I get a failed to load message and not some crazy error. - assert.strictEqual(data, `Failed to load: ${loadFile}\n`); - // Eat me to avoid work. - putIn.write = () => {}; -}); -putIn.run([`.load ${loadFile}`]); - -// Throw error on loading directory. -loadFile = tmpdir.path; -putIn.write = common.mustCall(function(data) { - assert.strictEqual(data, `Failed to load: ${loadFile} is not a valid file\n`); - putIn.write = () => {}; -}); -putIn.run([`.load ${loadFile}`]); - -// Clear the REPL. -putIn.run(['.clear']); - -// NUL (\0) is disallowed in filenames in UNIX-like operating systems and -// Windows so we can use that to test failed saves. -const invalidFileName = tmpdir.resolve('\0\0\0\0\0'); - -// Should not break. -putIn.write = common.mustCall(function(data) { - // Make sure I get a failed to save message and not some other error. - assert.strictEqual(data, `Failed to save: ${invalidFileName}\n`); - // Reset to no-op. - putIn.write = () => {}; -}); - -// Save it to a file. -putIn.run([`.save ${invalidFileName}`]); - -{ - // Save .editor mode code. - const cmds = [ - 'function testSave() {', - 'return "saved";', - '}', - ]; - const putIn = new ArrayStream(); - const replServer = repl.start({ terminal: true, stream: putIn }); - - putIn.run(['.editor']); - putIn.run(cmds); - replServer.write('', { ctrl: true, name: 'd' }); - - putIn.run([`.save ${saveFileName}`]); - replServer.close(); - assert.strictEqual(fs.readFileSync(saveFileName, 'utf8'), - `${cmds.join('\n')}\n`); -} +// TODO: the following async IIFE and the completePromise function are necessary because +// the reply tests are all run against the same repl instance (testMe) and thus coordination +// needs to be in place for the tests not to interfere with each other, this is really +// not ideal, the tests in this file should be refactored so that each use its own isolated +// repl instance, making sure that no special coordination needs to be in place for them +// and also allowing the tests to all be run in parallel +(async () => { + const repl = require('repl'); -// Check if the file is present when using save + const works = [['inner.one'], 'inner.o']; -// Clear the REPL. -putIn.run(['.clear']); - -// Error message when using save without a file -putIn.write = common.mustCall(function(data) { - assert.strictEqual(data, 'The "file" argument must be specified\n'); - putIn.write = () => {}; -}); -putIn.run(['.save']); + const putIn = new ArrayStream(); + const testMe = repl.start('', putIn); + + // Some errors might be passed to the domain. + testMe._domain.on('error', function(reason) { + const err = new Error('Test failed'); + err.reason = reason; + throw err; + }); + + async function completePromise(query, callback) { + return new Promise((resolve) => { + testMe.complete(query, (...args) => { + callback(...args); + resolve(); + }); + }); + } + + const testFile = [ + 'let inner = (function() {', + ' return {one:1};', + '})()', + ]; + const saveFileName = tmpdir.resolve('test.save.js'); -// Check if the file is present when using load + // Add some data. + putIn.run(testFile); -// Clear the REPL. -putIn.run(['.clear']); + // Save it to a file. + putIn.run([`.save ${saveFileName}`]); -// Error message when using load without a file -putIn.write = common.mustCall(function(data) { - assert.strictEqual(data, 'The "file" argument must be specified\n'); - putIn.write = () => {}; -}); -putIn.run(['.load']); + // The file should have what I wrote. + assert.strictEqual(fs.readFileSync(saveFileName, 'utf8'), + testFile.join('\n')); + + // Make sure that the REPL data is "correct". + await completePromise('inner.o', common.mustSucceed((data) => { + assert.deepStrictEqual(data, works); + })); + + // Clear the REPL. + putIn.run(['.clear']); + + testMe._sawKeyPress = true; + // Load the file back in. + putIn.run([`.load ${saveFileName}`]); + + // Make sure loading doesn't insert extra indentation + // https://github.com/nodejs/node/issues/47673 + assert.strictEqual(testMe.line, ''); + + // Make sure that the REPL data is "correct". + await completePromise('inner.o', common.mustSucceed((data) => { + assert.deepStrictEqual(data, works); + })); + + // Clear the REPL. + putIn.run(['.clear']); + + let loadFile = tmpdir.resolve('file.does.not.exist'); + + // Should not break. + putIn.write = common.mustCall(function(data) { + // Make sure I get a failed to load message and not some crazy error. + assert.strictEqual(data, `Failed to load: ${loadFile}\n`); + // Eat me to avoid work. + putIn.write = () => {}; + }); + putIn.run([`.load ${loadFile}`]); + + // Throw error on loading directory. + loadFile = tmpdir.path; + putIn.write = common.mustCall(function(data) { + assert.strictEqual(data, `Failed to load: ${loadFile} is not a valid file\n`); + putIn.write = () => {}; + }); + putIn.run([`.load ${loadFile}`]); + + // Clear the REPL. + putIn.run(['.clear']); + + // NUL (\0) is disallowed in filenames in UNIX-like operating systems and + // Windows so we can use that to test failed saves. + const invalidFileName = tmpdir.resolve('\0\0\0\0\0'); + + // Should not break. + putIn.write = common.mustCall(function(data) { + // Make sure I get a failed to save message and not some other error. + assert.strictEqual(data, `Failed to save: ${invalidFileName}\n`); + // Reset to no-op. + putIn.write = () => {}; + }); + + // Save it to a file. + putIn.run([`.save ${invalidFileName}`]); + + { + // Save .editor mode code. + const cmds = [ + 'function testSave() {', + 'return "saved";', + '}', + ]; + const putIn = new ArrayStream(); + const replServer = repl.start({ terminal: true, stream: putIn }); + + putIn.run(['.editor']); + putIn.run(cmds); + replServer.write('', { ctrl: true, name: 'd' }); + + putIn.run([`.save ${saveFileName}`]); + replServer.close(); + assert.strictEqual(fs.readFileSync(saveFileName, 'utf8'), + `${cmds.join('\n')}\n`); + } + + // Check if the file is present when using save + + // Clear the REPL. + putIn.run(['.clear']); + + // Error message when using save without a file + putIn.write = common.mustCall(function(data) { + assert.strictEqual(data, 'The "file" argument must be specified\n'); + putIn.write = () => {}; + }); + putIn.run(['.save']); + + // Check if the file is present when using load + + // Clear the REPL. + putIn.run(['.clear']); + + // Error message when using load without a file + putIn.write = common.mustCall(function(data) { + assert.strictEqual(data, 'The "file" argument must be specified\n'); + putIn.write = () => {}; + }); + putIn.run(['.load']); +})().then(common.mustCall()); diff --git a/test/parallel/test-repl-tab-complete.js b/test/parallel/test-repl-tab-complete.js index c79162129bd69b..cbc82e5bf0296e 100644 --- a/test/parallel/test-repl-tab-complete.js +++ b/test/parallel/test-repl-tab-complete.js @@ -53,568 +53,589 @@ function getNoResultsFunction() { }); } -const works = [['inner.one'], 'inner.o']; -const putIn = new ArrayStream(); -const testMe = repl.start({ - prompt: '', - input: putIn, - output: process.stdout, - allowBlockingCompletions: true -}); +// TODO: the following async IIFE and the completePromise function are necessary because +// the reply tests are all run against the same repl instance (testMe) and thus coordination +// needs to be in place for the tests not to interfere with each other, this is really +// not ideal, the tests in this file should be refactored so that each use its own isolated +// repl instance, making sure that no special coordination needs to be in place for them +// and also allowing the tests to all be run in parallel +(async () => { + const works = [['inner.one'], 'inner.o']; + const putIn = new ArrayStream(); + const testMe = repl.start({ + prompt: '', + input: putIn, + output: process.stdout, + allowBlockingCompletions: true + }); -// Some errors are passed to the domain, but do not callback -testMe._domain.on('error', assert.ifError); + async function completePromise(query, callback) { + return new Promise((resolve) => { + testMe.complete(query, (...args) => { + callback(...args); + resolve(); + }); + }); + } -// Tab Complete will not break in an object literal -putIn.run([ - 'var inner = {', - 'one:1', -]); -testMe.complete('inner.o', getNoResultsFunction()); + // Some errors are passed to the domain, but do not callback + testMe._domain.on('error', assert.ifError); -testMe.complete('console.lo', common.mustCall(function(error, data) { - assert.deepStrictEqual(data, [['console.log'], 'console.lo']); -})); + // Tab Complete will not break in an object literal + putIn.run([ + 'var inner = {', + 'one:1', + ]); + await completePromise('inner.o', getNoResultsFunction()); -testMe.complete('console?.lo', common.mustCall((error, data) => { - assert.deepStrictEqual(data, [['console?.log'], 'console?.lo']); -})); + await completePromise('console.lo', common.mustCall(function(error, data) { + assert.deepStrictEqual(data, [['console.log'], 'console.lo']); + })); -testMe.complete('console?.zzz', common.mustCall((error, data) => { - assert.deepStrictEqual(data, [[], 'console?.zzz']); -})); + await completePromise('console?.lo', common.mustCall((error, data) => { + assert.deepStrictEqual(data, [['console?.log'], 'console?.lo']); + })); -testMe.complete('console?.', common.mustCall((error, data) => { - assert(data[0].includes('console?.log')); - assert.strictEqual(data[1], 'console?.'); -})); + await completePromise('console?.zzz', common.mustCall((error, data) => { + assert.deepStrictEqual(data, [[], 'console?.zzz']); + })); -// Tab Complete will return globally scoped variables -putIn.run(['};']); -testMe.complete('inner.o', common.mustCall(function(error, data) { - assert.deepStrictEqual(data, works); -})); + await completePromise('console?.', common.mustCall((error, data) => { + assert(data[0].includes('console?.log')); + assert.strictEqual(data[1], 'console?.'); + })); -putIn.run(['.clear']); - -// Tab Complete will not break in an ternary operator with () -putIn.run([ - 'var inner = ( true ', - '?', - '{one: 1} : ', -]); -testMe.complete('inner.o', getNoResultsFunction()); - -putIn.run(['.clear']); - -// Tab Complete will return a simple local variable -putIn.run([ - 'var top = function() {', - 'var inner = {one:1};', -]); -testMe.complete('inner.o', getNoResultsFunction()); - -// When you close the function scope tab complete will not return the -// locally scoped variable -putIn.run(['};']); -testMe.complete('inner.o', getNoResultsFunction()); - -putIn.run(['.clear']); - -// Tab Complete will return a complex local variable -putIn.run([ - 'var top = function() {', - 'var inner = {', - ' one:1', - '};', -]); -testMe.complete('inner.o', getNoResultsFunction()); - -putIn.run(['.clear']); - -// Tab Complete will return a complex local variable even if the function -// has parameters -putIn.run([ - 'var top = function(one, two) {', - 'var inner = {', - ' one:1', - '};', -]); -testMe.complete('inner.o', getNoResultsFunction()); - -putIn.run(['.clear']); - -// Tab Complete will return a complex local variable even if the -// scope is nested inside an immediately executed function -putIn.run([ - 'var top = function() {', - '(function test () {', - 'var inner = {', - ' one:1', - '};', -]); -testMe.complete('inner.o', getNoResultsFunction()); - -putIn.run(['.clear']); - -// The definition has the params and { on a separate line. -putIn.run([ - 'var top = function() {', - 'r = function test (', - ' one, two) {', - 'var inner = {', - ' one:1', - '};', -]); -testMe.complete('inner.o', getNoResultsFunction()); - -putIn.run(['.clear']); - -// Currently does not work, but should not break, not the { -putIn.run([ - 'var top = function() {', - 'r = function test ()', - '{', - 'var inner = {', - ' one:1', - '};', -]); -testMe.complete('inner.o', getNoResultsFunction()); - -putIn.run(['.clear']); - -// Currently does not work, but should not break -putIn.run([ - 'var top = function() {', - 'r = function test (', - ')', - '{', - 'var inner = {', - ' one:1', - '};', -]); -testMe.complete('inner.o', getNoResultsFunction()); - -putIn.run(['.clear']); - -// Make sure tab completion works on non-Objects -putIn.run([ - 'var str = "test";', -]); -testMe.complete('str.len', common.mustCall(function(error, data) { - assert.deepStrictEqual(data, [['str.length'], 'str.len']); -})); + // Tab Complete will return globally scoped variables + putIn.run(['};']); + await completePromise('inner.o', common.mustCall(function(error, data) { + assert.deepStrictEqual(data, works); + })); -putIn.run(['.clear']); - -// Tab completion should be case-insensitive if member part is lower-case -putIn.run([ - 'var foo = { barBar: 1, BARbuz: 2, barBLA: 3 };', -]); -testMe.complete( - 'foo.b', - common.mustCall(function(error, data) { - assert.deepStrictEqual(data, [ - ['foo.BARbuz', 'foo.barBLA', 'foo.barBar'], - 'foo.b', - ]); - }) -); - -putIn.run(['.clear']); - -// Tab completion should be case-insensitive if member part is upper-case -putIn.run([ - 'var foo = { barBar: 1, BARbuz: 2, barBLA: 3 };', -]); -testMe.complete( - 'foo.B', - common.mustCall(function(error, data) { - assert.deepStrictEqual(data, [ - ['foo.BARbuz', 'foo.barBLA', 'foo.barBar'], - 'foo.B', - ]); - }) -); - -putIn.run(['.clear']); - -// Tab completion should not break on spaces -const spaceTimeout = setTimeout(function() { - throw new Error('timeout'); -}, 1000); - -testMe.complete(' ', common.mustSucceed((data) => { - assert.strictEqual(data[1], ''); - assert.ok(data[0].includes('globalThis')); - clearTimeout(spaceTimeout); -})); + putIn.run(['.clear']); -// Tab completion should pick up the global "toString" object, and -// any other properties up the "global" object's prototype chain -testMe.complete('toSt', common.mustCall(function(error, data) { - assert.deepStrictEqual(data, [['toString'], 'toSt']); -})); + // Tab Complete will not break in an ternary operator with () + putIn.run([ + 'var inner = ( true ', + '?', + '{one: 1} : ', + ]); + await completePromise('inner.o', getNoResultsFunction()); -// Own properties should shadow properties on the prototype -putIn.run(['.clear']); -putIn.run([ - 'var x = Object.create(null);', - 'x.a = 1;', - 'x.b = 2;', - 'var y = Object.create(x);', - 'y.a = 3;', - 'y.c = 4;', -]); -testMe.complete('y.', common.mustCall(function(error, data) { - assert.deepStrictEqual(data, [['y.b', '', 'y.a', 'y.c'], 'y.']); -})); + putIn.run(['.clear']); -// Tab complete provides built in libs for require() -putIn.run(['.clear']); + // Tab Complete will return a simple local variable + putIn.run([ + 'var top = function() {', + 'var inner = {one:1};', + ]); + await completePromise('inner.o', getNoResultsFunction()); -testMe.complete('require(\'', common.mustCall(function(error, data) { - assert.strictEqual(error, null); - publicModules.forEach((lib) => { - assert( - data[0].includes(lib) && (lib.startsWith('node:') || data[0].includes(`node:${lib}`)), - `${lib} not found` - ); - }); - const newModule = 'foobar'; - assert(!builtinModules.includes(newModule)); - repl.builtinModules.push(newModule); - testMe.complete('require(\'', common.mustCall((_, [modules]) => { - assert.strictEqual(data[0].length + 1, modules.length); - assert(modules.includes(newModule)); + // When you close the function scope tab complete will not return the + // locally scoped variable + putIn.run(['};']); + await completePromise('inner.o', getNoResultsFunction()); + + putIn.run(['.clear']); + + // Tab Complete will return a complex local variable + putIn.run([ + 'var top = function() {', + 'var inner = {', + ' one:1', + '};', + ]); + await completePromise('inner.o', getNoResultsFunction()); + + putIn.run(['.clear']); + + // Tab Complete will return a complex local variable even if the function + // has parameters + putIn.run([ + 'var top = function(one, two) {', + 'var inner = {', + ' one:1', + '};', + ]); + await completePromise('inner.o', getNoResultsFunction()); + + putIn.run(['.clear']); + + // Tab Complete will return a complex local variable even if the + // scope is nested inside an immediately executed function + putIn.run([ + 'var top = function() {', + '(function test () {', + 'var inner = {', + ' one:1', + '};', + ]); + await completePromise('inner.o', getNoResultsFunction()); + + putIn.run(['.clear']); + + // The definition has the params and { on a separate line. + putIn.run([ + 'var top = function() {', + 'r = function test (', + ' one, two) {', + 'var inner = {', + ' one:1', + '};', + ]); + await completePromise('inner.o', getNoResultsFunction()); + + putIn.run(['.clear']); + + // Currently does not work, but should not break, not the { + putIn.run([ + 'var top = function() {', + 'r = function test ()', + '{', + 'var inner = {', + ' one:1', + '};', + ]); + await completePromise('inner.o', getNoResultsFunction()); + + putIn.run(['.clear']); + + // Currently does not work, but should not break + putIn.run([ + 'var top = function() {', + 'r = function test (', + ')', + '{', + 'var inner = {', + ' one:1', + '};', + ]); + await completePromise('inner.o', getNoResultsFunction()); + + putIn.run(['.clear']); + + // Make sure tab completion works on non-Objects + putIn.run([ + 'var str = "test";', + ]); + // TODO + await completePromise('str.len', common.mustCall(function(error, data) { + assert.deepStrictEqual(data, [['str.length'], 'str.len']); })); -})); -testMe.complete("require\t( 'n", common.mustCall(function(error, data) { - assert.strictEqual(error, null); - assert.strictEqual(data.length, 2); - assert.strictEqual(data[1], 'n'); - // require(...) completions include `node:`-prefixed modules: - let lastIndex = -1; + putIn.run(['.clear']); - publicModules.filter((lib) => !lib.startsWith('node:')).forEach((lib, index) => { - lastIndex = data[0].indexOf(`node:${lib}`); - assert.notStrictEqual(lastIndex, -1); - }); - assert.strictEqual(data[0][lastIndex + 1], ''); - // There is only one Node.js module that starts with n: - assert.strictEqual(data[0][lastIndex + 2], 'net'); - assert.strictEqual(data[0][lastIndex + 3], ''); - // It's possible to pick up non-core modules too - data[0].slice(lastIndex + 4).forEach((completion) => { - assert.match(completion, /^n/); - }); -})); + // Tab completion should be case-insensitive if member part is lower-case + putIn.run([ + 'var foo = { barBar: 1, BARbuz: 2, barBLA: 3 };', + ]); + await completePromise( + 'foo.b', + common.mustCall(function(error, data) { + assert.deepStrictEqual(data, [ + ['foo.BARbuz', 'foo.barBLA', 'foo.barBar'], + 'foo.b', + ]); + }) + ); -{ - const expected = ['@nodejsscope', '@nodejsscope/']; - // Require calls should handle all types of quotation marks. - for (const quotationMark of ["'", '"', '`']) { - putIn.run(['.clear']); - testMe.complete('require(`@nodejs', common.mustCall((err, data) => { - assert.strictEqual(err, null); - assert.deepStrictEqual(data, [expected, '@nodejs']); - })); + putIn.run(['.clear']); - putIn.run(['.clear']); - // Completions should not be greedy in case the quotation ends. - const input = `require(${quotationMark}@nodejsscope${quotationMark}`; - testMe.complete(input, common.mustCall((err, data) => { - assert.strictEqual(err, null); - assert.deepStrictEqual(data, [[], undefined]); - })); - } -} + // Tab completion should be case-insensitive if member part is upper-case + putIn.run([ + 'var foo = { barBar: 1, BARbuz: 2, barBLA: 3 };', + ]); + await completePromise( + 'foo.B', + common.mustCall(function(error, data) { + assert.deepStrictEqual(data, [ + ['foo.BARbuz', 'foo.barBLA', 'foo.barBar'], + 'foo.B', + ]); + }) + ); -{ putIn.run(['.clear']); - // Completions should find modules and handle whitespace after the opening - // bracket. - testMe.complete('require \t("no_ind', common.mustCall((err, data) => { - assert.strictEqual(err, null); - assert.deepStrictEqual(data, [['no_index', 'no_index/'], 'no_ind']); + + // Tab completion should not break on spaces + const spaceTimeout = setTimeout(function() { + throw new Error('timeout'); + }, 1000); + + await completePromise(' ', common.mustSucceed((data) => { + assert.strictEqual(data[1], ''); + assert.ok(data[0].includes('globalThis')); + clearTimeout(spaceTimeout); })); -} -// Test tab completion for require() relative to the current directory -{ + // Tab completion should pick up the global "toString" object, and + // any other properties up the "global" object's prototype chain + await completePromise('toSt', common.mustCall(function(error, data) { + assert.deepStrictEqual(data, [['toString'], 'toSt']); + })); + + // Own properties should shadow properties on the prototype putIn.run(['.clear']); + putIn.run([ + 'var x = Object.create(null);', + 'x.a = 1;', + 'x.b = 2;', + 'var y = Object.create(x);', + 'y.a = 3;', + 'y.c = 4;', + ]); + await completePromise('y.', common.mustCall(function(error, data) { + assert.deepStrictEqual(data, [['y.b', '', 'y.a', 'y.c'], 'y.']); + })); - const cwd = process.cwd(); - process.chdir(__dirname); + // Tab complete provides built in libs for require() + putIn.run(['.clear']); - ['require(\'.', 'require(".'].forEach((input) => { - testMe.complete(input, common.mustCall((err, data) => { - assert.strictEqual(err, null); - assert.strictEqual(data.length, 2); - assert.strictEqual(data[1], '.'); - assert.strictEqual(data[0].length, 2); - assert.ok(data[0].includes('./')); - assert.ok(data[0].includes('../')); + await completePromise('require(\'', common.mustCall(async function(error, data) { + assert.strictEqual(error, null); + publicModules.forEach((lib) => { + assert( + data[0].includes(lib) && (lib.startsWith('node:') || data[0].includes(`node:${lib}`)), + `${lib} not found` + ); + }); + const newModule = 'foobar'; + assert(!builtinModules.includes(newModule)); + repl.builtinModules.push(newModule); + await completePromise('require(\'', common.mustCall((_, [modules]) => { + assert.strictEqual(data[0].length + 1, modules.length); + assert(modules.includes(newModule)); })); - }); + })); - ['require(\'..', 'require("..'].forEach((input) => { - testMe.complete(input, common.mustCall((err, data) => { + await completePromise("require\t( 'n", common.mustCall(function(error, data) { + assert.strictEqual(error, null); + assert.strictEqual(data.length, 2); + assert.strictEqual(data[1], 'n'); + // require(...) completions include `node:`-prefixed modules: + let lastIndex = -1; + + publicModules.filter((lib) => !lib.startsWith('node:')).forEach((lib, index) => { + lastIndex = data[0].indexOf(`node:${lib}`); + assert.notStrictEqual(lastIndex, -1); + }); + assert.strictEqual(data[0][lastIndex + 1], ''); + // There is only one Node.js module that starts with n: + assert.strictEqual(data[0][lastIndex + 2], 'net'); + assert.strictEqual(data[0][lastIndex + 3], ''); + // It's possible to pick up non-core modules too + data[0].slice(lastIndex + 4).forEach((completion) => { + assert.match(completion, /^n/); + }); + })); + + { + const expected = ['@nodejsscope', '@nodejsscope/']; + // Require calls should handle all types of quotation marks. + for (const quotationMark of ["'", '"', '`']) { + putIn.run(['.clear']); + await completePromise('require(`@nodejs', common.mustCall((err, data) => { + assert.strictEqual(err, null); + assert.deepStrictEqual(data, [expected, '@nodejs']); + })); + + putIn.run(['.clear']); + // Completions should not be greedy in case the quotation ends. + const input = `require(${quotationMark}@nodejsscope${quotationMark}`; + testMe.complete(input, common.mustCall((err, data) => { + assert.strictEqual(err, null); + assert.deepStrictEqual(data, [[], undefined]); + })); + } + } + + { + putIn.run(['.clear']); + // Completions should find modules and handle whitespace after the opening + // bracket. + testMe.complete('require \t("no_ind', common.mustCall((err, data) => { assert.strictEqual(err, null); - assert.deepStrictEqual(data, [['../'], '..']); + assert.deepStrictEqual(data, [['no_index', 'no_index/'], 'no_ind']); })); - }); + } + + // Test tab completion for require() relative to the current directory + { + putIn.run(['.clear']); + + const cwd = process.cwd(); + process.chdir(__dirname); - ['./', './test-'].forEach((path) => { - [`require('${path}`, `require("${path}`].forEach((input) => { + ['require(\'.', 'require(".'].forEach((input) => { testMe.complete(input, common.mustCall((err, data) => { assert.strictEqual(err, null); assert.strictEqual(data.length, 2); - assert.strictEqual(data[1], path); - assert.ok(data[0].includes('./test-repl-tab-complete')); + assert.strictEqual(data[1], '.'); + assert.strictEqual(data[0].length, 2); + assert.ok(data[0].includes('./')); + assert.ok(data[0].includes('../')); })); }); - }); - ['../parallel/', '../parallel/test-'].forEach((path) => { - [`require('${path}`, `require("${path}`].forEach((input) => { + ['require(\'..', 'require("..'].forEach((input) => { testMe.complete(input, common.mustCall((err, data) => { assert.strictEqual(err, null); - assert.strictEqual(data.length, 2); - assert.strictEqual(data[1], path); - assert.ok(data[0].includes('../parallel/test-repl-tab-complete')); + assert.deepStrictEqual(data, [['../'], '..']); })); }); - }); - - { - const path = '../fixtures/repl-folder-extensions/f'; - testMe.complete(`require('${path}`, common.mustSucceed((data) => { - assert.strictEqual(data.length, 2); - assert.strictEqual(data[1], path); - assert.ok(data[0].includes('../fixtures/repl-folder-extensions/foo.js')); - })); - } - process.chdir(cwd); -} - -// Make sure tab completion works on context properties -putIn.run(['.clear']); + ['./', './test-'].forEach((path) => { + [`require('${path}`, `require("${path}`].forEach((input) => { + testMe.complete(input, common.mustCall((err, data) => { + assert.strictEqual(err, null); + assert.strictEqual(data.length, 2); + assert.strictEqual(data[1], path); + assert.ok(data[0].includes('./test-repl-tab-complete')); + })); + }); + }); -putIn.run([ - 'var custom = "test";', -]); -testMe.complete('cus', common.mustCall(function(error, data) { - assert.deepStrictEqual(data, [['CustomEvent', 'custom'], 'cus']); -})); + ['../parallel/', '../parallel/test-'].forEach((path) => { + [`require('${path}`, `require("${path}`].forEach((input) => { + testMe.complete(input, common.mustCall((err, data) => { + assert.strictEqual(err, null); + assert.strictEqual(data.length, 2); + assert.strictEqual(data[1], path); + assert.ok(data[0].includes('../parallel/test-repl-tab-complete')); + })); + }); + }); -// Make sure tab completion doesn't crash REPL with half-baked proxy objects. -// See: https://github.com/nodejs/node/issues/2119 -putIn.run(['.clear']); + { + const path = '../fixtures/repl-folder-extensions/f'; + testMe.complete(`require('${path}`, common.mustSucceed((data) => { + assert.strictEqual(data.length, 2); + assert.strictEqual(data[1], path); + assert.ok(data[0].includes('../fixtures/repl-folder-extensions/foo.js')); + })); + } -putIn.run([ - 'var proxy = new Proxy({}, {ownKeys: () => { throw new Error(); }});', -]); + process.chdir(cwd); + } -testMe.complete('proxy.', common.mustCall(function(error, data) { - assert.strictEqual(error, null); - assert(Array.isArray(data)); -})); + // Make sure tab completion works on context properties + putIn.run(['.clear']); -// Make sure tab completion does not include integer members of an Array -putIn.run(['.clear']); + putIn.run([ + 'var custom = "test";', + ]); + await completePromise('cus', common.mustCall(function(error, data) { + assert.deepStrictEqual(data, [['CustomEvent', 'custom'], 'cus']); + })); -putIn.run(['var ary = [1,2,3];']); -testMe.complete('ary.', common.mustCall(function(error, data) { - assert.strictEqual(data[0].includes('ary.0'), false); - assert.strictEqual(data[0].includes('ary.1'), false); - assert.strictEqual(data[0].includes('ary.2'), false); -})); + // Make sure tab completion doesn't crash REPL with half-baked proxy objects. + // See: https://github.com/nodejs/node/issues/2119 + putIn.run(['.clear']); -// Make sure tab completion does not include integer keys in an object -putIn.run(['.clear']); -putIn.run(['var obj = {1:"a","1a":"b",a:"b"};']); + putIn.run([ + 'var proxy = new Proxy({}, {ownKeys: () => { throw new Error(); }});', + ]); -testMe.complete('obj.', common.mustCall(function(error, data) { - assert.strictEqual(data[0].includes('obj.1'), false); - assert.strictEqual(data[0].includes('obj.1a'), false); - assert(data[0].includes('obj.a')); -})); + await completePromise('proxy.', common.mustCall(function(error, data) { + assert.strictEqual(error, null); + assert(Array.isArray(data)); + })); -// Don't try to complete results of non-simple expressions -putIn.run(['.clear']); -putIn.run(['function a() {}']); + // Make sure tab completion does not include integer members of an Array + putIn.run(['.clear']); -testMe.complete('a().b.', getNoResultsFunction()); + putIn.run(['var ary = [1,2,3];']); + await completePromise('ary.', common.mustCall(function(error, data) { + assert.strictEqual(data[0].includes('ary.0'), false); + assert.strictEqual(data[0].includes('ary.1'), false); + assert.strictEqual(data[0].includes('ary.2'), false); + })); -// Works when prefixed with spaces -putIn.run(['.clear']); -putIn.run(['var obj = {1:"a","1a":"b",a:"b"};']); + // Make sure tab completion does not include integer keys in an object + putIn.run(['.clear']); + putIn.run(['var obj = {1:"a","1a":"b",a:"b"};']); -testMe.complete(' obj.', common.mustCall((error, data) => { - assert.strictEqual(data[0].includes('obj.1'), false); - assert.strictEqual(data[0].includes('obj.1a'), false); - assert(data[0].includes('obj.a')); -})); + await completePromise('obj.', common.mustCall(function(error, data) { + assert.strictEqual(data[0].includes('obj.1'), false); + assert.strictEqual(data[0].includes('obj.1a'), false); + assert(data[0].includes('obj.a')); + })); -// Works inside assignments -putIn.run(['.clear']); + // Don't try to complete results of non-simple expressions + putIn.run(['.clear']); + putIn.run(['function a() {}']); -testMe.complete('var log = console.lo', common.mustCall((error, data) => { - assert.deepStrictEqual(data, [['console.log'], 'console.lo']); -})); + await completePromise('a().b.', getNoResultsFunction()); -// Tab completion for defined commands -putIn.run(['.clear']); + // Works when prefixed with spaces + putIn.run(['.clear']); + putIn.run(['var obj = {1:"a","1a":"b",a:"b"};']); -testMe.complete('.b', common.mustCall((error, data) => { - assert.deepStrictEqual(data, [['break'], 'b']); -})); -putIn.run(['.clear']); -putIn.run(['var obj = {"hello, world!": "some string", "key": 123}']); -testMe.complete('obj.', common.mustCall((error, data) => { - assert.strictEqual(data[0].includes('obj.hello, world!'), false); - assert(data[0].includes('obj.key')); -})); + await completePromise(' obj.', common.mustCall((error, data) => { + assert.strictEqual(data[0].includes('obj.1'), false); + assert.strictEqual(data[0].includes('obj.1a'), false); + assert(data[0].includes('obj.a')); + })); -// Make sure tab completion does not include __defineSetter__ and friends. -putIn.run(['.clear']); + // Works inside assignments + 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); -})); + await completePromise('var log = console.lo', common.mustCall((error, data) => { + assert.deepStrictEqual(data, [['console.log'], 'console.lo']); + })); -// Tab completion for files/directories -{ + // Tab completion for defined commands putIn.run(['.clear']); - process.chdir(__dirname); - const readFileSyncs = ['fs.readFileSync("', 'fs.promises.readFileSync("']; - if (!common.isWindows) { - readFileSyncs.forEach((readFileSync) => { - const fixturePath = `${readFileSync}../fixtures/test-repl-tab-completion`; - testMe.complete(fixturePath, common.mustCall((err, data) => { - assert.strictEqual(err, null); - assert.ok(data[0][0].includes('.hiddenfiles')); - assert.ok(data[0][1].includes('hellorandom.txt')); - assert.ok(data[0][2].includes('helloworld.js')); - })); + await completePromise('.b', common.mustCall((error, data) => { + assert.deepStrictEqual(data, [['break'], 'b']); + })); + putIn.run(['.clear']); + putIn.run(['var obj = {"hello, world!": "some string", "key": 123}']); + await completePromise('obj.', common.mustCall((error, data) => { + assert.strictEqual(data[0].includes('obj.hello, world!'), false); + assert(data[0].includes('obj.key')); + })); - testMe.complete(`${fixturePath}/hello`, - common.mustCall((err, data) => { - assert.strictEqual(err, null); - assert.ok(data[0][0].includes('hellorandom.txt')); - assert.ok(data[0][1].includes('helloworld.js')); - }) - ); + // Make sure tab completion does not include __defineSetter__ and friends. + putIn.run(['.clear']); - testMe.complete(`${fixturePath}/.h`, - common.mustCall((err, data) => { - assert.strictEqual(err, null); - assert.ok(data[0][0].includes('.hiddenfiles')); - }) - ); + putIn.run(['var obj = {};']); + await completePromise('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); + })); - testMe.complete(`${readFileSync}./xxxRandom/random`, - common.mustCall((err, data) => { - assert.strictEqual(err, null); - assert.strictEqual(data[0].length, 0); - }) - ); + // Tab completion for files/directories + { + putIn.run(['.clear']); + process.chdir(__dirname); + + const readFileSyncs = ['fs.readFileSync("', 'fs.promises.readFileSync("']; + if (!common.isWindows) { + readFileSyncs.forEach((readFileSync) => { + const fixturePath = `${readFileSync}../fixtures/test-repl-tab-completion`; + testMe.complete(fixturePath, common.mustCall((err, data) => { + assert.strictEqual(err, null); + assert.ok(data[0][0].includes('.hiddenfiles')); + assert.ok(data[0][1].includes('hellorandom.txt')); + assert.ok(data[0][2].includes('helloworld.js')); + })); + + testMe.complete(`${fixturePath}/hello`, + common.mustCall((err, data) => { + assert.strictEqual(err, null); + assert.ok(data[0][0].includes('hellorandom.txt')); + assert.ok(data[0][1].includes('helloworld.js')); + }) + ); - const testPath = fixturePath.slice(0, -1); - testMe.complete(testPath, common.mustCall((err, data) => { - assert.strictEqual(err, null); - assert.ok(data[0][0].includes('test-repl-tab-completion')); - assert.strictEqual( - data[1], - path.basename(testPath) + testMe.complete(`${fixturePath}/.h`, + common.mustCall((err, data) => { + assert.strictEqual(err, null); + assert.ok(data[0][0].includes('.hiddenfiles')); + }) ); - })); - }); - } -} -[ - Array, - Buffer, - - Uint8Array, - Uint16Array, - Uint32Array, - - Uint8ClampedArray, - Int8Array, - Int16Array, - Int32Array, - Float32Array, - Float64Array, -].forEach((type) => { - putIn.run(['.clear']); + testMe.complete(`${readFileSync}./xxxRandom/random`, + common.mustCall((err, data) => { + assert.strictEqual(err, null); + assert.strictEqual(data[0].length, 0); + }) + ); - if (type === Array) { - putIn.run([ - 'var ele = [];', - 'for (let i = 0; i < 1e6 + 1; i++) ele[i] = 0;', - 'ele.biu = 1;', - ]); - } else if (type === Buffer) { - putIn.run(['var ele = Buffer.alloc(1e6 + 1); ele.biu = 1;']); - } else { - putIn.run([`var ele = new ${type.name}(1e6 + 1); ele.biu = 1;`]); + const testPath = fixturePath.slice(0, -1); + testMe.complete(testPath, common.mustCall((err, data) => { + assert.strictEqual(err, null); + assert.ok(data[0][0].includes('test-repl-tab-completion')); + assert.strictEqual( + data[1], + path.basename(testPath) + ); + })); + }); + } } - hijackStderr(common.mustNotCall()); - testMe.complete('ele.', common.mustCall((err, data) => { - restoreStderr(); - assert.ifError(err); + for (const type of [ + Array, + Buffer, + + Uint8Array, + Uint16Array, + Uint32Array, + + Uint8ClampedArray, + Int8Array, + Int16Array, + Int32Array, + Float32Array, + Float64Array, + ]) { + putIn.run(['.clear']); - const ele = (type === Array) ? - [] : - (type === Buffer ? - Buffer.alloc(0) : - new type(0)); + if (type === Array) { + putIn.run([ + 'var ele = [];', + 'for (let i = 0; i < 1e6 + 1; i++) ele[i] = 0;', + 'ele.biu = 1;', + ]); + } else if (type === Buffer) { + putIn.run(['var ele = Buffer.alloc(1e6 + 1); ele.biu = 1;']); + } else { + putIn.run([`var ele = new ${type.name}(1e6 + 1); ele.biu = 1;`]); + } + + hijackStderr(common.mustNotCall()); + await completePromise('ele.', common.mustCall((err, data) => { + restoreStderr(); + assert.ifError(err); + + const ele = (type === Array) ? + [] : + (type === Buffer ? + Buffer.alloc(0) : + new type(0)); + + assert.strictEqual(data[0].includes('ele.biu'), true); + + data[0].forEach((key) => { + if (!key || key === 'ele.biu') return; + assert.notStrictEqual(ele[key.slice(4)], undefined); + }); + })); + }; - assert.strictEqual(data[0].includes('ele.biu'), true); + // check Buffer.prototype.length not crashing. + // Refs: https://github.com/nodejs/node/pull/11961 + putIn.run(['.clear']); + await completePromise('Buffer.prototype.', common.mustCall()); - data[0].forEach((key) => { - if (!key || key === 'ele.biu') return; - assert.notStrictEqual(ele[key.slice(4)], undefined); - }); + // Make sure repl gives correct autocomplete on literals + await completePromise('``.a', common.mustCall((err, data) => { + assert.strictEqual(data[0].includes('``.at'), true); + })); + await completePromise('\'\'.a', common.mustCall((err, data) => { + assert.strictEqual(data[0].includes('\'\'.at'), true); + })); + await completePromise('"".a', common.mustCall((err, data) => { + assert.strictEqual(data[0].includes('"".at'), true); + })); + await completePromise('("").a', common.mustCall((err, data) => { + assert.strictEqual(data[0].includes('("").at'), true); + })); + await completePromise('[].a', common.mustCall((err, data) => { + assert.strictEqual(data[0].includes('[].at'), true); + })); + await completePromise('{}.a', common.mustCall((err, data) => { + assert.deepStrictEqual(data[0], []); })); -}); -// check Buffer.prototype.length not crashing. -// Refs: https://github.com/nodejs/node/pull/11961 -putIn.run(['.clear']); -testMe.complete('Buffer.prototype.', common.mustCall()); +})().then(common.mustCall()); -// Make sure repl gives correct autocomplete on literals -testMe.complete('``.a', common.mustCall((err, data) => { - assert.strictEqual(data[0].includes('``.at'), true); -})); -testMe.complete('\'\'.a', common.mustCall((err, data) => { - assert.strictEqual(data[0].includes('\'\'.at'), true); -})); -testMe.complete('"".a', common.mustCall((err, data) => { - assert.strictEqual(data[0].includes('"".at'), true); -})); -testMe.complete('("").a', common.mustCall((err, data) => { - assert.strictEqual(data[0].includes('("").at'), true); -})); -testMe.complete('[].a', common.mustCall((err, data) => { - assert.strictEqual(data[0].includes('[].at'), true); -})); -testMe.complete('{}.a', common.mustCall((err, data) => { - assert.deepStrictEqual(data[0], []); -})); +const putIn = new ArrayStream(); const testNonGlobal = repl.start({ input: putIn, From 63125aae92056e31559d6f2bf358e02b27d7c22a Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Fri, 18 Apr 2025 10:26:04 +0200 Subject: [PATCH 2/8] Apply suggestions from code review Co-authored-by: James M Snell --- lib/repl.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 103356ff01a68f..64fa80ce205000 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -1328,8 +1328,8 @@ function completeFSFunctions(match) { // -> [['util.print', 'util.debug', 'util.log', 'util.inspect'], // 'util.' ] // -// Warning: This eval's code like "foo.bar.baz", so it could run property -// getter code. To avoid potential side-effecful getters the completion +// Warning: This evals code like "foo.bar.baz", so it could run property +// getter code. To avoid potential triggering side-effects with getters the completion // logic is skipped when getters or proxies are involved in the expression. // (see: https://github.com/nodejs/node/issues/57829). async function complete(line, callback) { From 46e0d263e84f9118a09d893e9f6c7c7c95edaa12 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Fri, 18 Apr 2025 13:33:45 +0200 Subject: [PATCH 3/8] fixup! lib: disable REPL completion on proxies and getters improve solution and use ObjectGetOwnPropertyDescriptor primordial --- lib/repl.js | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 64fa80ce205000..685681b5d967a5 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -96,6 +96,10 @@ const { globalThis, } = primordials; +const { + isProxy, +} = require('internal/util/types'); + const { BuiltinModule } = require('internal/bootstrap/realm'); const { makeRequireFunction, @@ -1640,29 +1644,28 @@ async function includesProxiesOrGetters(fullExpr, evalFn, context) { let currentExpr = ''; for (let i = 0; i < bits.length - 1; i++) { currentExpr += `${i === 0 ? '' : '.'}${bits[i]}`; - const currentExprIsObject = await evalPromisified(`try { ${currentExpr} !== null && typeof ${currentExpr} === 'object' } catch { false }`); - if (!currentExprIsObject) { - return false; - } + const currentResult = await new Promise((resolve) => + evalFn(`try { ${currentExpr} } catch { }`, context, getREPLResourceName(), (_, currentObj) => { + if (typeof currentObj !== 'object' || currentObj === null) { + return resolve(false); + } - const currentExprIsProxy = await evalPromisified(`require("node:util/types").isProxy(${currentExpr})`); - if (currentExprIsProxy) { - return true; - } + if (isProxy(currentObj)) { + return resolve(true); + } - const typeOfNextBitGet = await evalPromisified(`typeof Object.getOwnPropertyDescriptor(${currentExpr}, '${bits[i + 1]}')?.get`); - const nextBitHasGetter = typeOfNextBitGet === 'function'; - if (nextBitHasGetter) { - return true; - } - } + const nextBitHasGetter = typeof ObjectGetOwnPropertyDescriptor(currentObj, bits[i + 1])?.get === 'function'; + if (nextBitHasGetter) { + return resolve(true); + } - function evalPromisified(evalExpr) { - return new Promise((resolve, reject) => - evalFn(evalExpr, context, getREPLResourceName(), (_, res) => { - resolve(res); + return resolve(); })); + if (currentResult !== undefined) { + return currentResult; + } } + return false; } REPLServer.prototype.completeOnEditorMode = (callback) => (err, results) => { From 8dbef4a531aa37a7492395247be19bb4f4631a40 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Sat, 31 May 2025 16:30:40 +0100 Subject: [PATCH 4/8] avoid promises based solution --- lib/repl.js | 142 ++++++++++++++++++++++++++-------------------------- 1 file changed, 72 insertions(+), 70 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 685681b5d967a5..6c51e31a570593 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -1531,56 +1531,61 @@ async function complete(line, callback) { return; } - if (await includesProxiesOrGetters(match, this.eval, this.context)) { - // The expression involves proxies or getters, meaning that it - // can trigger side-effectful behaviors, so bail out - return completionGroupsLoaded(); - } - - let chaining = '.'; - if (StringPrototypeEndsWith(expr, '?')) { - expr = StringPrototypeSlice(expr, 0, -1); - chaining = '?.'; - } - - const memberGroups = []; - const evalExpr = `try { ${expr} } catch {}`; - this.eval(evalExpr, this.context, getREPLResourceName(), (e, obj) => { - try { - let p; - if ((typeof obj === 'object' && obj !== null) || - typeof obj === 'function') { - ArrayPrototypePush(memberGroups, filteredOwnPropertyNames(obj)); - p = ObjectGetPrototypeOf(obj); - } else { - p = obj.constructor ? obj.constructor.prototype : null; + return includesProxiesOrGetters( + StringPrototypeSplit(match, '.'), + this.eval, + this.context, + (includes) => { + if (includes) { + // The expression involves proxies or getters, meaning that it + // can trigger side-effectful behaviors, so bail out + return completionGroupsLoaded(); } - // Circular refs possible? Let's guard against that. - let sentinel = 5; - while (p !== null && sentinel-- !== 0) { - ArrayPrototypePush(memberGroups, filteredOwnPropertyNames(p)); - p = ObjectGetPrototypeOf(p); + + let chaining = '.'; + if (StringPrototypeEndsWith(expr, '?')) { + expr = StringPrototypeSlice(expr, 0, -1); + chaining = '?.'; } - } catch { - // Maybe a Proxy object without `getOwnPropertyNames` trap. - // We simply ignore it here, as we don't want to break the - // autocompletion. Fixes the bug - // https://github.com/nodejs/node/issues/2119 - } - if (memberGroups.length) { - expr += chaining; - ArrayPrototypeForEach(memberGroups, (group) => { - ArrayPrototypePush(completionGroups, - ArrayPrototypeMap(group, - (member) => `${expr}${member}`)); - }); - filter &&= `${expr}${filter}`; - } + const memberGroups = []; + const evalExpr = `try { ${expr} } catch {}`; + this.eval(evalExpr, this.context, getREPLResourceName(), (e, obj) => { + try { + let p; + if ((typeof obj === 'object' && obj !== null) || + typeof obj === 'function') { + ArrayPrototypePush(memberGroups, filteredOwnPropertyNames(obj)); + p = ObjectGetPrototypeOf(obj); + } else { + p = obj.constructor ? obj.constructor.prototype : null; + } + // Circular refs possible? Let's guard against that. + let sentinel = 5; + while (p !== null && sentinel-- !== 0) { + ArrayPrototypePush(memberGroups, filteredOwnPropertyNames(p)); + p = ObjectGetPrototypeOf(p); + } + } catch { + // Maybe a Proxy object without `getOwnPropertyNames` trap. + // We simply ignore it here, as we don't want to break the + // autocompletion. Fixes the bug + // https://github.com/nodejs/node/issues/2119 + } - completionGroupsLoaded(); - }); - return; + if (memberGroups.length) { + expr += chaining; + ArrayPrototypeForEach(memberGroups, (group) => { + ArrayPrototypePush(completionGroups, + ArrayPrototypeMap(group, + (member) => `${expr}${member}`)); + }); + filter &&= `${expr}${filter}`; + } + + completionGroupsLoaded(); + }); + }); } return completionGroupsLoaded(); @@ -1638,34 +1643,31 @@ async function complete(line, callback) { } } -async function includesProxiesOrGetters(fullExpr, evalFn, context) { - const bits = StringPrototypeSplit(fullExpr, '.'); - - let currentExpr = ''; - for (let i = 0; i < bits.length - 1; i++) { - currentExpr += `${i === 0 ? '' : '.'}${bits[i]}`; - const currentResult = await new Promise((resolve) => - evalFn(`try { ${currentExpr} } catch { }`, context, getREPLResourceName(), (_, currentObj) => { - if (typeof currentObj !== 'object' || currentObj === null) { - return resolve(false); - } +function includesProxiesOrGetters(exprSegments, evalFn, context, callback, currentExpr = '') { + if (exprSegments.length === 0) { + return callback(false); + } - if (isProxy(currentObj)) { - return resolve(true); - } + const currentSegment = exprSegments[0]; + const remainingSegments = ArrayPrototypeSlice(exprSegments, 1); + currentExpr += `${currentExpr.length === 0 ? '' : '.'}${currentSegment}`; + evalFn(`try { ${currentExpr} } catch { }`, context, getREPLResourceName(), (_, currentObj) => { + if (typeof currentObj !== 'object' || currentObj === null) { + return callback(false); + } - const nextBitHasGetter = typeof ObjectGetOwnPropertyDescriptor(currentObj, bits[i + 1])?.get === 'function'; - if (nextBitHasGetter) { - return resolve(true); - } + if (isProxy(currentObj)) { + return callback(true); + } - return resolve(); - })); - if (currentResult !== undefined) { - return currentResult; + const nextSegmentProp = ObjectGetOwnPropertyDescriptor(currentObj, remainingSegments[0]); + const nextSegmentPropHasGetter = typeof nextSegmentProp?.get === 'function'; + if (nextSegmentPropHasGetter) { + return callback(true); } - } - return false; + + return includesProxiesOrGetters(remainingSegments, evalFn, context, callback, currentExpr); + }); } REPLServer.prototype.completeOnEditorMode = (callback) => (err, results) => { From ddfd8b6dbb7829c225caae8fdd999815f9f04466 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Mon, 2 Jun 2025 23:25:36 +0100 Subject: [PATCH 5/8] Update lib/repl.js Co-authored-by: Ruben Bridgewater --- lib/repl.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 6c51e31a570593..a842872afc9a5b 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -1644,10 +1644,6 @@ async function complete(line, callback) { } function includesProxiesOrGetters(exprSegments, evalFn, context, callback, currentExpr = '') { - if (exprSegments.length === 0) { - return callback(false); - } - const currentSegment = exprSegments[0]; const remainingSegments = ArrayPrototypeSlice(exprSegments, 1); currentExpr += `${currentExpr.length === 0 ? '' : '.'}${currentSegment}`; From 85b201befd1666f20b86fe3e6c48b4441d03d42c Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Mon, 2 Jun 2025 23:28:50 +0100 Subject: [PATCH 6/8] add remaining segments check --- lib/repl.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/repl.js b/lib/repl.js index a842872afc9a5b..0160922be011da 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -1656,6 +1656,10 @@ function includesProxiesOrGetters(exprSegments, evalFn, context, callback, curre return callback(true); } + if (remainingSegments.length === 0) { + return callback(false); + } + const nextSegmentProp = ObjectGetOwnPropertyDescriptor(currentObj, remainingSegments[0]); const nextSegmentPropHasGetter = typeof nextSegmentProp?.get === 'function'; if (nextSegmentPropHasGetter) { From b146e7d094f2c802460899bb144c6c9b6a5807e6 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Mon, 2 Jun 2025 23:29:12 +0100 Subject: [PATCH 7/8] remove unnecessary async --- lib/repl.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/repl.js b/lib/repl.js index 0160922be011da..4d443a0509bcf6 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -1336,7 +1336,7 @@ function completeFSFunctions(match) { // getter code. To avoid potential triggering side-effects with getters the completion // logic is skipped when getters or proxies are involved in the expression. // (see: https://github.com/nodejs/node/issues/57829). -async function complete(line, callback) { +function complete(line, callback) { // List of completion lists, one for each inheritance "level" let completionGroups = []; let completeOn, group; From 0f1f1e488c684a6f29f9681c5df1883391764f2c Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Mon, 2 Jun 2025 23:42:29 +0100 Subject: [PATCH 8/8] avoid unnecessary array operations --- lib/repl.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 4d443a0509bcf6..d500181736b7ac 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -1643,9 +1643,8 @@ function complete(line, callback) { } } -function includesProxiesOrGetters(exprSegments, evalFn, context, callback, currentExpr = '') { - const currentSegment = exprSegments[0]; - const remainingSegments = ArrayPrototypeSlice(exprSegments, 1); +function includesProxiesOrGetters(exprSegments, evalFn, context, callback, currentExpr = '', idx = 0) { + const currentSegment = exprSegments[idx]; currentExpr += `${currentExpr.length === 0 ? '' : '.'}${currentSegment}`; evalFn(`try { ${currentExpr} } catch { }`, context, getREPLResourceName(), (_, currentObj) => { if (typeof currentObj !== 'object' || currentObj === null) { @@ -1656,17 +1655,19 @@ function includesProxiesOrGetters(exprSegments, evalFn, context, callback, curre return callback(true); } - if (remainingSegments.length === 0) { + const nextIdx = idx + 1; + + if (nextIdx >= exprSegments.length) { return callback(false); } - const nextSegmentProp = ObjectGetOwnPropertyDescriptor(currentObj, remainingSegments[0]); + const nextSegmentProp = ObjectGetOwnPropertyDescriptor(currentObj, exprSegments[nextIdx]); const nextSegmentPropHasGetter = typeof nextSegmentProp?.get === 'function'; if (nextSegmentPropHasGetter) { return callback(true); } - return includesProxiesOrGetters(remainingSegments, evalFn, context, callback, currentExpr); + return includesProxiesOrGetters(exprSegments, evalFn, context, callback, currentExpr, nextIdx); }); }