repl: improve repl autocompletion for require calls

This improves the autocompletion for require calls. It had multiple
small issues so far. Most important: it won't suggest completions for
require statements that are fully written out. Second, it'll detect
require calls that have whitespace behind the opening bracket. Third,
it makes sure node modules are detected as such instead of only
suggesting them as folders. Last, it adds suggestions for input that
starts with backticks.

Signed-off-by: Ruben Bridgewater <[email protected]>

PR-URL: #33282
Fixes: #33238
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>

Author: BridgeAR

lib/repl.js

@@ -1046,7 +1046,7 @@ REPLServer.prototype.turnOffEditorMode = deprecate(
   'REPLServer.turnOffEditorMode() is deprecated',
   'DEP0078');
 
-const requireRE = /\brequire\s*\(['"](([\w@./-]+\/)?(?:[\w@./-]*))/;
+const requireRE = /\brequire\s*\(\s*['"`](([\w@./-]+\/)?(?:[\w@./-]*))(?![^'"`])$/;
 const fsAutoCompleteRE = /fs(?:\.promises)?\.\s*[a-z][a-zA-Z]+\(\s*["'](.*)/;
 const simpleExpressionRE =
     /(?:[a-zA-Z_$](?:\w|\$)*\.)*[a-zA-Z_$](?:\w|\$)*\.?$/;
@@ -1094,8 +1094,13 @@ REPLServer.prototype.complete = function() {
   this.completer.apply(this, arguments);
 };
 
-// TODO: Native module names should be auto-resolved.
-// That improves the auto completion.
+function gracefulOperation(fn, args, alternative) {
+  try {
+    return fn(...args);
+  } catch {
+    return alternative;
+  }
+}
 
 // Provide a list of completions for the given leading text. This is
 // given to the readline interface for handling tab completion.
@@ -1117,26 +1122,25 @@ function complete(line, callback) {
 
   // REPL commands (e.g. ".break").
   let filter;
-  let match = line.match(/^\s*\.(\w*)$/);
-  if (match) {
+  if (/^\s*\.(\w*)$/.test(line)) {
     completionGroups.push(ObjectKeys(this.commands));
-    completeOn = match[1];
-    if (match[1].length) {
-      filter = match[1];
+    completeOn = line.match(/^\s*\.(\w*)$/)[1];
+    if (completeOn.length) {
+      filter = completeOn;
     }
 
     completionGroupsLoaded();
-  } else if (match = line.match(requireRE)) {
+  } else if (requireRE.test(line)) {
     // require('...<Tab>')
-    const exts = ObjectKeys(this.context.require.extensions);
-    const indexRe = new RegExp('^index(?:' + exts.map(regexpEscape).join('|') +
-                             ')$');
+    const extensions = ObjectKeys(this.context.require.extensions);
+    const indexes = extensions.map((extension) => `index${extension}`);
+    indexes.push('package.json', 'index');
     const versionedFileNamesRe = /-\d+\.\d+/;
 
+    const match = line.match(requireRE);
     completeOn = match[1];
     const subdir = match[2] || '';
-    filter = match[1];
-    let dir, files, subfiles, isDirectory;
+    filter = completeOn;
     group = [];
     let paths = [];
 
@@ -1150,41 +1154,34 @@ function complete(line, callback) {
       paths = module.paths.concat(CJSModule.globalPaths);
     }
 
-    for (let i = 0; i < paths.length; i++) {
-      dir = path.resolve(paths[i], subdir);
-      try {
-        files = fs.readdirSync(dir);
-      } catch {
-        continue;
-      }
-      for (let f = 0; f < files.length; f++) {
-        const name = files[f];
-        const ext = path.extname(name);
-        const base = name.slice(0, -ext.length);
-        if (versionedFileNamesRe.test(base) || name === '.npm') {
+    for (let dir of paths) {
+      dir = path.resolve(dir, subdir);
+      const dirents = gracefulOperation(
+        fs.readdirSync,
+        [dir, { withFileTypes: true }],
+        []
+      );
+      for (const dirent of dirents) {
+        if (versionedFileNamesRe.test(dirent.name) || dirent.name === '.npm') {
           // Exclude versioned names that 'npm' installs.
           continue;
         }
-        const abs = path.resolve(dir, name);
-        try {
-          isDirectory = fs.statSync(abs).isDirectory();
-        } catch {
+        const extension = path.extname(dirent.name);
+        const base = dirent.name.slice(0, -extension.length);
+        if (!dirent.isDirectory()) {
+          if (extensions.includes(extension) && (!subdir || base !== 'index')) {
+            group.push(`${subdir}${base}`);
+          }
           continue;
         }
-        if (isDirectory) {
-          group.push(subdir + name + '/');
-          try {
-            subfiles = fs.readdirSync(abs);
-          } catch {
-            continue;
+        group.push(`${subdir}${dirent.name}/`);
+        const absolute = path.resolve(dir, dirent.name);
+        const subfiles = gracefulOperation(fs.readdirSync, [absolute], []);
+        for (const subfile of subfiles) {
+          if (indexes.includes(subfile)) {
+            group.push(`${subdir}${dirent.name}`);
+            break;
           }
-          for (let s = 0; s < subfiles.length; s++) {
-            if (indexRe.test(subfiles[s])) {
-              group.push(subdir + name);
-            }
-          }
-        } else if (exts.includes(ext) && (!subdir || base !== 'index')) {
-          group.push(subdir + base);
         }
       }
     }
@@ -1197,11 +1194,10 @@ function complete(line, callback) {
     }
 
     completionGroupsLoaded();
-  } else if (match = line.match(fsAutoCompleteRE)) {
-
-    let filePath = match[1];
-    let fileList;
+  } else if (fsAutoCompleteRE.test(line)) {
     filter = '';
+    let filePath = line.match(fsAutoCompleteRE)[1];
+    let fileList;
 
     try {
       fileList = fs.readdirSync(filePath, { withFileTypes: true });
@@ -1232,7 +1228,7 @@ function complete(line, callback) {
   //   foo<|>         # all scope vars with filter 'foo'
   //   foo.<|>        # completions for 'foo' with filter ''
   } else if (line.length === 0 || /\w|\.|\$/.test(line[line.length - 1])) {
-    match = simpleExpressionRE.exec(line);
+    const match = simpleExpressionRE.exec(line);
     if (line.length !== 0 && !match) {
       completionGroupsLoaded();
       return;
@@ -1582,10 +1578,6 @@ function defineDefaultCommands(repl) {
   }
 }
 
-function regexpEscape(s) {
-  return s.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, '\\$&');
-}
-
 function Recoverable(err) {
   this.err = err;
 }

test/fixtures/node_modules/no_index/lib/index.js

@@ -229,24 +229,46 @@ testMe.complete('require(\'', common.mustCall(function(error, data) {
   });
 }));
 
-testMe.complete('require(\'n', common.mustCall(function(error, data) {
+testMe.complete("require\t( 'n", common.mustCall(function(error, data) {
   assert.strictEqual(error, null);
   assert.strictEqual(data.length, 2);
   assert.strictEqual(data[1], 'n');
-  assert(data[0].includes('net'));
+  // There is only one Node.js module that starts with n:
+  assert.strictEqual(data[0][0], 'net');
+  assert.strictEqual(data[0][1], '');
   // It's possible to pick up non-core modules too
-  data[0].forEach(function(completion) {
-    if (completion)
-      assert(/^n/.test(completion));
+  data[0].slice(2).forEach((completion) => {
+    assert.match(completion, /^n/);
   });
 }));
 
 {
   const expected = ['@nodejsscope', '@nodejsscope/'];
+  // Require calls should handle all types of quotation marks.
+  for (const quotationMark of ["'", '"', '`']) {
+    putIn.run(['.clear']);
+    testMe.complete('require(`@nodejs', common.mustCall((err, data) => {
+      assert.strictEqual(err, null);
+      assert.deepStrictEqual(data, [expected, '@nodejs']);
+    }));
+
+    putIn.run(['.clear']);
+    // Completions should not be greedy in case the quotation ends.
+    const input = `require(${quotationMark}@nodejsscope${quotationMark}`;
+    testMe.complete(input, common.mustCall((err, data) => {
+      assert.strictEqual(err, null);
+      assert.deepStrictEqual(data, [[], undefined]);
+    }));
+  }
+}
+
+{
   putIn.run(['.clear']);
-  testMe.complete('require(\'@nodejs', common.mustCall((err, data) => {
+  // Completions should find modules and handle whitespace after the opening
+  // bracket.
+  testMe.complete('require \t("no_ind', common.mustCall((err, data) => {
     assert.strictEqual(err, null);
-    assert.deepStrictEqual(data, [expected, '@nodejs']);
+    assert.deepStrictEqual(data, [['no_index', 'no_index/'], 'no_ind']);
   }));
 }