repl: do not cause side effects in tab completion

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);
 }
 
 /**

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']],
         ]);
     });
 

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) => {

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();
+    });
+  }
+});