chore(cli-repl): clean up autocomplete implementation and fix flaky test MONGOSH-1908 MONGOSH-1960 (#2317)

* chore(cli-repl): clean autocomplete implementation up a bit

Best viewed as a whitespace-diff-disabled change, doesn't include
any functional work.

* chore(cli-repl): fix flaky autocompletion test MONGOSH-1960

Our current autocomplete logic does not always wait for the collection
or database name caches to be filled, in case it takes longer than 200ms
to do so. By ensuring that the caches are filled before we run the test,
we get consistent results.

* chore(cli-repl,shell-api): filter autocompletions for CCs MONGOSH-1908

Collection and database names, as well as other completion items, can
mostly contain arbitrary characters with only a few exceptions.
Since some characters may mess with our REPL input, we should filter
those autocompletions out.

Author: addaleax

packages/cli-repl/src/cli-repl.spec.ts

@@ -2365,7 +2365,7 @@ describe('CliRepl', function () {
       });
 
       it('includes collection names', async function () {
-        if (!hasCollectionNames) return;
+        if (!hasCollectionNames) return this.skip();
         const collname = `testcollection${Date.now()}${
           (Math.random() * 1000) | 0
         }`;
@@ -2413,7 +2413,10 @@ describe('CliRepl', function () {
       });
 
       it('completes use <db>', async function () {
-        if (!hasDatabaseNames) return;
+        if (!hasDatabaseNames) return this.skip();
+        input.write('db.getMongo()._listDatabases()\n'); // populate database cache
+        await waitEval(cliRepl.bus);
+
         input.write('use adm');
         await tab();
         await waitCompletion(cliRepl.bus);
@@ -2428,13 +2431,14 @@ describe('CliRepl', function () {
       });
 
       it('completes properties of shell API result types', async function () {
-        if (!hasCollectionNames) return;
+        if (!hasCollectionNames) return this.skip();
+
         input.write(
           'res = db.autocompleteTestColl.deleteMany({ deletetestdummykey: 1 })\n'
         );
         await waitEval(cliRepl.bus);
 
-        // Consitency check: The result actually has a shell API type tag:
+        // Consistency check: The result actually has a shell API type tag:
         output = '';
         input.write('res[Symbol.for("@@mongosh.shellApiType")]\n');
         await waitEval(cliRepl.bus);
@@ -2445,6 +2449,24 @@ describe('CliRepl', function () {
         await waitCompletion(cliRepl.bus);
         expect(output).to.include('res.acknowledged');
       });
+
+      it('completes only collection names that do not include control characters', async function () {
+        if (!hasCollectionNames) return this.skip();
+
+        input.write(
+          'db["actestcoll1"].insertOne({}); db["actestcoll2\\x1bfooobar"].insertOne({})\n'
+        );
+        await waitEval(cliRepl.bus);
+        input.write('db._getCollectionNames()\n'); // populate collection name cache
+        await waitEval(cliRepl.bus);
+
+        output = '';
+        input.write('db.actestc');
+        await tabtab();
+        await waitCompletion(cliRepl.bus);
+        expect(output).to.include('db.actestcoll1');
+        expect(output).to.not.include('db.actestcoll2');
+      });
     });
   }
 

packages/cli-repl/src/mongosh-repl.ts

@@ -45,6 +45,9 @@ import { Script, createContext, runInContext } from 'vm';
 
 declare const __non_webpack_require__: any;
 
+// eslint-disable-next-line no-control-regex
+const CONTROL_CHAR_REGEXP = /[\x00-\x1F\x7F-\x9F]/g;
+
 /**
  * All CLI flags that are useful for {@link MongoshNodeRepl}.
  */
@@ -388,52 +391,64 @@ class MongoshNodeRepl implements EvaluationListener {
       null,
       instanceState.getAutocompleteParameters()
     );
-    (repl as Mutable<typeof repl>).completer = callbackify(
+    const innerCompleter = async (
+      text: string
+    ): Promise<[string[], string]> => {
+      // Merge the results from the repl completer and the mongosh completer.
+      const [
+        [replResults, replOrig],
+        [mongoshResults, , mongoshResultsExclusive],
+      ] = await Promise.all([
+        (async () => (await origReplCompleter(text)) || [[]])(),
+        (async () => await mongoshCompleter(text))(),
+      ]);
+      this.bus.emit('mongosh:autocompletion-complete'); // For testing.
+
+      // Sometimes the mongosh completion knows that what it is doing is right,
+      // and that autocompletion based on inspecting the actual objects that
+      // are being accessed will not be helpful, e.g. in `use a<tab>`, we know
+      // that we want *only* database names and not e.g. `assert`.
+      if (mongoshResultsExclusive) {
+        return [mongoshResults, text];
+      }
+
+      // The REPL completer may not complete the entire string; for example,
+      // when completing ".ed" to ".editor", it reports as having completed
+      // only the last piece ("ed"), or when completing "{ $g", it completes
+      // only "$g" and not the entire result.
+      // The mongosh completer always completes on the entire string.
+      // In order to align them, we always extend the REPL results to include
+      // the full string prefix.
+      const replResultPrefix = replOrig
+        ? text.substr(0, text.lastIndexOf(replOrig))
+        : '';
+      const longReplResults = replResults.map(
+        (result: string) => replResultPrefix + result
+      );
+
+      // Remove duplicates, because shell API methods might otherwise show
+      // up in both completions.
+      const deduped = [...new Set([...longReplResults, ...mongoshResults])];
+
+      return [deduped, text];
+    };
+
+    const nodeReplCompleter = callbackify(
       async (text: string): Promise<[string[], string]> => {
         this.insideAutoCompleteOrGetPrompt = true;
         try {
-          // Merge the results from the repl completer and the mongosh completer.
-          const [
-            [replResults, replOrig],
-            [mongoshResults, , mongoshResultsExclusive],
-          ] = await Promise.all([
-            (async () => (await origReplCompleter(text)) || [[]])(),
-            (async () => await mongoshCompleter(text))(),
-          ]);
-          this.bus.emit('mongosh:autocompletion-complete'); // For testing.
-
-          // Sometimes the mongosh completion knows that what it is doing is right,
-          // and that autocompletion based on inspecting the actual objects that
-          // are being accessed will not be helpful, e.g. in `use a<tab>`, we know
-          // that we want *only* database names and not e.g. `assert`.
-          if (mongoshResultsExclusive) {
-            return [mongoshResults, text];
-          }
-
-          // The REPL completer may not complete the entire string; for example,
-          // when completing ".ed" to ".editor", it reports as having completed
-          // only the last piece ("ed"), or when completing "{ $g", it completes
-          // only "$g" and not the entire result.
-          // The mongosh completer always completes on the entire string.
-          // In order to align them, we always extend the REPL results to include
-          // the full string prefix.
-          const replResultPrefix = replOrig
-            ? text.substr(0, text.lastIndexOf(replOrig))
-            : '';
-          const longReplResults = replResults.map(
-            (result: string) => replResultPrefix + result
+          // eslint-disable-next-line prefer-const
+          let [results, completeOn] = await innerCompleter(text);
+          results = results.filter(
+            (result) => !CONTROL_CHAR_REGEXP.test(result)
           );
-
-          // Remove duplicates, because shell API methods might otherwise show
-          // up in both completions.
-          const deduped = [...new Set([...longReplResults, ...mongoshResults])];
-
-          return [deduped, text];
+          return [results, completeOn];
         } finally {
           this.insideAutoCompleteOrGetPrompt = false;
         }
       }
     );
+    (repl as Mutable<typeof repl>).completer = nodeReplCompleter;
 
     // This is used below for multiline history manipulation.
     let originalHistory: string[] | null = null;

packages/shell-api/src/shell-instance-state.ts

@@ -126,6 +126,9 @@ export interface ShellPlugin {
   transformError?: (err: Error) => Error;
 }
 
+// eslint-disable-next-line no-control-regex
+const CONTROL_CHAR_REGEXP = /[\x00-\x1F\x7F-\x9F]/g;
+
 /**
  * Anything to do with the state of the shell API and API objects is stored here.
  *
@@ -452,8 +455,10 @@ export default class ShellInstanceState {
         try {
           const collectionNames =
             await this.currentDb._getCollectionNamesForCompletion();
-          return collectionNames.filter((name) =>
-            name.toLowerCase().startsWith(collName.toLowerCase())
+          return collectionNames.filter(
+            (name) =>
+              name.toLowerCase().startsWith(collName.toLowerCase()) &&
+              !CONTROL_CHAR_REGEXP.test(name)
           );
         } catch (err: any) {
           if (
@@ -469,8 +474,10 @@ export default class ShellInstanceState {
         try {
           const dbNames =
             await this.currentDb._mongo._getDatabaseNamesForCompletion();
-          return dbNames.filter((name) =>
-            name.toLowerCase().startsWith(dbName.toLowerCase())
+          return dbNames.filter(
+            (name) =>
+              name.toLowerCase().startsWith(dbName.toLowerCase()) &&
+              !CONTROL_CHAR_REGEXP.test(name)
           );
         } catch (err: any) {
           if (