diff --git a/lib/repl.js b/lib/repl.js index b95beb45643eb2..1a436cafff6b74 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -1754,10 +1754,25 @@ function findExpressionCompleteTarget(code) { return findExpressionCompleteTarget(argumentCode); } + // Walk the AST for the current block of code, and check whether it contains any + // statement or expression type that would potentially have side effects if evaluated. + let isAllowed = true; + const disallow = () => isAllowed = false; + acornWalk.simple(lastBodyStatement, { + ForInStatement: disallow, + ForOfStatement: disallow, + CallExpression: disallow, + AssignmentExpression: disallow, + UpdateExpression: disallow, + }); + if (!isAllowed) { + return null; + } + // If any of the above early returns haven't activated then it means that // the potential complete target is the full code (e.g. the code represents // a simple partial identifier, a member expression, etc...) - return code; + return code.slice(lastBodyStatement.start, lastBodyStatement.end); } /** diff --git a/test/parallel/test-repl-completion-on-getters-disabled.js b/test/parallel/test-repl-completion-on-getters-disabled.js index 05cdd630d5a298..2b80f717f685d4 100644 --- a/test/parallel/test-repl-completion-on-getters-disabled.js +++ b/test/parallel/test-repl-completion-on-getters-disabled.js @@ -61,10 +61,6 @@ describe('REPL completion in relation of getters', () => { test(`completions are generated for properties that don't trigger getters`, () => { runCompletionTests( ` - function getFooKey() { - return "foo"; - } - const fooKey = "foo"; const keys = { @@ -90,7 +86,6 @@ describe('REPL completion in relation of getters', () => { ["objWithGetters[keys['foo key']].b", ["objWithGetters[keys['foo key']].bar"]], ['objWithGetters[fooKey].b', ['objWithGetters[fooKey].bar']], ["objWithGetters['f' + 'oo'].b", ["objWithGetters['f' + 'oo'].bar"]], - ['objWithGetters[getFooKey()].b', ['objWithGetters[getFooKey()].bar']], ]); }); diff --git a/test/parallel/test-repl-tab-complete-getter-error.js b/test/parallel/test-repl-tab-complete-getter-error.js index e2e36b85c58baa..cebe008247c3a8 100644 --- a/test/parallel/test-repl-tab-complete-getter-error.js +++ b/test/parallel/test-repl-tab-complete-getter-error.js @@ -27,7 +27,6 @@ async function runTest() { await new Promise((resolve, reject) => { replServer.eval(` - const getNameText = () => "name"; const foo = { get name() { throw new Error(); } }; `, replServer.context, '', (err) => { if (err) { @@ -38,7 +37,7 @@ async function runTest() { }); }); - ['foo.name.', 'foo["name"].', 'foo[getNameText()].'].forEach((test) => { + ['foo.name.', 'foo["name"].'].forEach((test) => { replServer.complete( test, common.mustCall((error, data) => { diff --git a/test/parallel/test-repl-tab-complete-nosideeffects.js b/test/parallel/test-repl-tab-complete-nosideeffects.js new file mode 100644 index 00000000000000..93cd9752925ced --- /dev/null +++ b/test/parallel/test-repl-tab-complete-nosideeffects.js @@ -0,0 +1,56 @@ +'use strict'; + +const common = require('../common'); +const ArrayStream = require('../common/arraystream'); +const { describe, it } = require('node:test'); +const assert = require('assert'); + +const repl = require('repl'); + +function prepareREPL() { + const input = new ArrayStream(); + const replServer = repl.start({ + prompt: '', + input, + output: process.stdout, + allowBlockingCompletions: true, + }); + + // Some errors are passed to the domain, but do not callback + replServer._domain.on('error', assert.ifError); + + return { replServer, input }; +} + +function getNoResultsFunction() { + return common.mustSucceed((data) => { + assert.deepStrictEqual(data[0], []); + }); +} + +describe('REPL tab completion without side effects', () => { + const setup = [ + 'globalThis.counter = 0;', + 'function incCounter() { return counter++; }', + 'const arr = [{ bar: "baz" }];', + ]; + // None of these expressions should affect the value of `counter` + for (const code of [ + 'incCounter().', + 'a=(counter+=1).foo.', + 'a=(counter++).foo.', + 'for((counter)of[1])foo.', + 'for((counter)in{1:1})foo.', + 'arr[incCounter()].b', + ]) { + it(`does not evaluate with side effects (${code})`, async () => { + const { replServer, input } = prepareREPL(); + input.run(setup); + + replServer.complete(code, getNoResultsFunction()); + + assert.strictEqual(replServer.context.counter, 0); + replServer.close(); + }); + } +});