Repository: nodejs/node
Pull request: repl: improve tab completion on computed properties

improve the tab completion capabilities around computed properties by replacing the use of brittle and error prone Regex checks with more robust AST based analysis

___

Followup from https://github.com/nodejs/node/pull/58709


<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: lib/repl.js
patch: @@ -1225,8 +1225,6 @@ REPLServer.prototype.setPrompt = function setPrompt(prompt) {
 const importRE = /\bimport\s*\(\s*['"`](([\w@./:-]+\/)?(?:[\w@./:-]*))(?![^'"`])$/;
 const requireRE = /\brequire\s*\(\s*['"`](([\w@./:-]+\/)?(?:[\w@./:-]*))(?![^'"`])$/;
 const fsAutoCompleteRE = /fs(?:\.promises)?\.\s*[a-z][a-zA-Z]+\(\s*["'](.*)/;
-const simpleExpressionRE =
-    /(?:[\w$'"`[{(](?:(\w| |\t)*?['"`]|\$|['"`\]})])*\??(?:\.|])?)*?(?:[a-zA-Z_$])?(?:\w|\$)*\??\.?$/;
 const versionedFileNamesRe = /-\d+\.\d+/;
 
 function isIdentifier(str) {
@@ -1480,29 +1478,20 @@ function complete(line, callback) {
   } else if ((match = RegExpPrototypeExec(fsAutoCompleteRE, line)) !== null &&
              this.allowBlockingCompletions) {
     ({ 0: completionGroups, 1: completeOn } = completeFSFunctions(match));
-  // Handle variable member lookup.
-  // We support simple chained expressions like the following (no function
-  // calls, etc.). That is for simplicity and also because we *eval* that
-  // leading expression so for safety (see WARNING above) don't want to
-  // eval function calls.
-  //
-  //   foo.bar<|>     # completions for 'foo' with filter 'bar'
-  //   spam.eggs.<|>  # completions for 'spam.eggs' with filter ''
-  //   foo<|>         # all scope vars with filter 'foo'
-  //   foo.<|>        # completions for 'foo' with filter ''
   } else if (line.length === 0 ||
              RegExpPrototypeExec(/\w|\.|\$/, line[line.length - 1]) !== null) {
-    const { 0: match } = RegExpPrototypeExec(simpleExpressionRE, line) || [''];
-    if (line.length !== 0 && !match) {
+    const completeTarget = line.length === 0 ? line : findExpressionCompleteTarget(line);
+
+    if (line.length !== 0 && !completeTarget) {
       completionGroupsLoaded();
       return;
     }
     let expr = '';
-    completeOn = match;
+    completeOn = completeTarget;
     if (StringPrototypeEndsWith(line, '.')) {
-      expr = StringPrototypeSlice(match, 0, -1);
+      expr = StringPrototypeSlice(completeTarget, 0, -1);
     } else if (line.length !== 0) {
-      const bits = StringPrototypeSplit(match, '.');
+      const bits = StringPrototypeSplit(completeTarget, '.');
       filter = ArrayPrototypePop(bits);
       expr = ArrayPrototypeJoin(bits, '.');
     }
@@ -1531,7 +1520,7 @@ function complete(line, callback) {
     }
 
     return includesProxiesOrGetters(
-      StringPrototypeSplit(match, '.'),
+      StringPrototypeSplit(completeTarget, '.'),
       this.eval,
       this.context,
       (includes) => {
@@ -1642,6 +1631,100 @@ function complete(line, callback) {
   }
 }
 
+/**
+ * This function tries to extract a target for tab completion from code representing an expression.
+ *
+ * Such target is basically the last piece of the expression that can be evaluated for the potential
+ * tab completion.
+ *
+ * Some examples:
+ * - The complete target for `const a = obj.b` is `obj.b`
+ *   (because tab completion will evaluate and check the `obj.b` object)
+ * - The complete target for `tru` is `tru`
+ *   (since we'd ideally want to complete that to `true`)
+ * - The complete target for `{ a: tru` is `tru`
+ *   (like the last example, we'd ideally want that to complete to true)
+ * - There is no complete target for `{ a: true }`
+ *   (there is nothing to complete)
+ * @param {string} code the code representing the expression to analyze
+ * @returns {string|null} a substring of the code representing the complete target is there was one, `null` otherwise
+ */
+function findExpressionCompleteTarget(code) {
+  if (!code) {
+    return null;
+  }
+
+  if (code.at(-1) === '.') {
+    if (code.at(-2) === '?') {
+      // The code ends with the optional chaining operator (`?.`),
+      // such code can't generate a valid AST so we need to strip
+      // the suffix, run this function's logic and add back the
+      // optional chaining operator to the result if present
+      const result = findExpressionCompleteTarget(code.slice(0, -2));
+      return !result ? result : `${result}?.`;
+    }
+
+    // The code ends with a dot, such code can't generate a valid AST
+    // so we need to strip the suffix, run this function's logic and
+    // add back the dot to the result if present
+    const result = findExpressionCompleteTarget(code.slice(0, -1));
+    return !result ? result : `${result}.`;
+  }
+
+  let ast;
+  try {
+    ast = acornParse(code, { __proto__: null, sourceType: 'module', ecmaVersion: 'latest' });
+  } catch {
+    const keywords = code.split(' ');
+
+    if (keywords.length > 1) {
+      // Something went wrong with the parsing, however this can be due to incomplete code
+      // (that is for example missing a closing bracket, as for example `{ a: obj.te`), in
+      // this case we take the last code keyword and try again
+      // TODO(dario-piotrowicz): make this more robust, right now we only split by spaces
+      //                         but that's not always enough, for example it doesn't handle
+      //                         this code: `{ a: obj['hello world'].te`
+      return findExpressionCompleteTarget(keywords.at(-1));
+    }
+
+    // The ast parsing has legitimately failed so we return null
+    return null;
+  }
+
+  const lastBodyStatement = ast.body[ast.body.length - 1];
+
+  if (!lastBodyStatement) {
+    return null;
+  }
+
+  // If the last statement is a block we know there is not going to be a potential
+  // completion target (e.g. in `{ a: true }` there is no completion to be done)
+  if (lastBodyStatement.type === 'BlockStatement') {
+    return null;
+  }
+
+  // If the last statement is an expression and it has a right side, that's what we
+  // want to potentially complete on, so let's re-run the function's logic on that
+  if (lastBodyStatement.type === 'ExpressionStatement' && lastBodyStatement.expression.right) {
+    const exprRight = lastBodyStatement.expression.right;
+    const exprRightCode = code.slice(exprRight.start, exprRight.end);
+    return findExpressionCompleteTarget(exprRightCode);
+  }
+
+  // If the last statement is a variable declaration statement the last declaration is
+  // what we can potentially complete on, so let's re-run the function's logic on that
+  if (lastBodyStatement.type === 'VariableDeclaration') {
+    const lastDeclarationInit = lastBodyStatement.declarations.at(-1).init;
+    const lastDeclarationInitCode = code.slice(lastDeclarationInit.start, lastDeclarationInit.end);
+    return findExpressionCompleteTarget(lastDeclarationInitCode);
+  }
+
+  // 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;
+}
+
 function includesProxiesOrGetters(exprSegments, evalFn, context, callback, currentExpr = '', idx = 0) {
   const currentSegment = exprSegments[idx];
   currentExpr += `${currentExpr.length === 0 ? '' : '.'}${currentSegment}`;

filename: test/parallel/test-repl-tab-complete-computed-props.js
patch: @@ -109,6 +109,11 @@ describe('REPL tab object completion on computed properties', () => {
             [oneStr]: 1,
             ['Hello World']: 'hello world!',
           };
+
+          const lookupObj = {
+            stringLookup: helloWorldStr,
+            ['number lookup']: oneStr,
+          };
         `,
       ]);
     });
@@ -126,5 +131,29 @@ describe('REPL tab object completion on computed properties', () => {
       input: 'obj[helloWorldStr].tolocaleup',
       expectedCompletions: ['obj[helloWorldStr].toLocaleUpperCase'],
     }));
+
+    it('works with a simple inlined computed property', () => testCompletion(replServer, {
+      input: 'obj["Hello " + "World"].tolocaleup',
+      expectedCompletions: ['obj["Hello " + "World"].toLocaleUpperCase'],
+    }));
+
+    it('works with a ternary inlined computed property', () => testCompletion(replServer, {
+      input: 'obj[(1 + 2 > 5) ? oneStr : "Hello " + "World"].toLocaleUpperCase',
+      expectedCompletions: ['obj[(1 + 2 > 5) ? oneStr : "Hello " + "World"].toLocaleUpperCase'],
+    }));
+
+    it('works with an inlined computed property with a nested property lookup', () =>
+      testCompletion(replServer, {
+        input: 'obj[lookupObj.stringLookup].tolocaleupp',
+        expectedCompletions: ['obj[lookupObj.stringLookup].toLocaleUpperCase'],
+      })
+    );
+
+    it('works with an inlined computed property with a nested inlined computer property lookup', () =>
+      testCompletion(replServer, {
+        input: 'obj[lookupObj["number" + " lookup"]].toFi',
+        expectedCompletions: ['obj[lookupObj["number" + " lookup"]].toFixed'],
+      })
+    );
   });
 });