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

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

@@ -2414,6 +2414,9 @@ describe('CliRepl', function () {
 
       it('completes use <db>', async function () {
         if (!hasDatabaseNames) return;
+        input.write('db.getMongo()._listDatabases()\n'); // populate database cache
+        await waitEval(cliRepl.bus);
+
         input.write('use adm');
         await tab();
         await waitCompletion(cliRepl.bus);
@@ -2429,12 +2432,13 @@ describe('CliRepl', function () {
 
       it('completes properties of shell API result types', async function () {
         if (!hasCollectionNames) return;
+
         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;
+
+        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 (