Skip to content

Commit 00cd4d8

Browse files
committed
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. Fixes: #33238 Signed-off-by: Ruben Bridgewater <[email protected]>
1 parent c24445d commit 00cd4d8

File tree

4 files changed

+70
-55
lines changed

4 files changed

+70
-55
lines changed

lib/repl.js

+37-48
Original file line numberDiff line numberDiff line change
@@ -1047,7 +1047,7 @@ REPLServer.prototype.turnOffEditorMode = deprecate(
10471047
'REPLServer.turnOffEditorMode() is deprecated',
10481048
'DEP0078');
10491049

1050-
const requireRE = /\brequire\s*\(['"](([\w@./-]+\/)?(?:[\w@./-]*))/;
1050+
const requireRE = /\brequire\s*\(\s*['"`](([\w@./-]+\/)?(?:[\w@./-]*))(?![^'"`])$/;
10511051
const fsAutoCompleteRE = /fs(?:\.promises)?\.\s*[a-z][a-zA-Z]+\(\s*["'](.*)/;
10521052
const simpleExpressionRE =
10531053
/(?:[a-zA-Z_$](?:\w|\$)*\.)*[a-zA-Z_$](?:\w|\$)*\.?$/;
@@ -1095,8 +1095,13 @@ REPLServer.prototype.complete = function() {
10951095
this.completer.apply(this, arguments);
10961096
};
10971097

1098-
// TODO: Native module names should be auto-resolved.
1099-
// That improves the auto completion.
1098+
function gracefulOperation(fn, args, alternative) {
1099+
try {
1100+
return fn(...args);
1101+
} catch {
1102+
return alternative;
1103+
}
1104+
}
11001105

11011106
// Provide a list of completions for the given leading text. This is
11021107
// given to the readline interface for handling tab completion.
@@ -1118,26 +1123,25 @@ function complete(line, callback) {
11181123

11191124
// REPL commands (e.g. ".break").
11201125
let filter;
1121-
let match = line.match(/^\s*\.(\w*)$/);
1122-
if (match) {
1126+
if (/^\s*\.(\w*)$/.test(line)) {
11231127
completionGroups.push(ObjectKeys(this.commands));
1124-
completeOn = match[1];
1125-
if (match[1].length) {
1126-
filter = match[1];
1128+
completeOn = line.match(/^\s*\.(\w*)$/)[1];
1129+
if (completeOn.length) {
1130+
filter = completeOn;
11271131
}
11281132

11291133
completionGroupsLoaded();
1130-
} else if (match = line.match(requireRE)) {
1134+
} else if (requireRE.test(line)) {
11311135
// require('...<Tab>')
1132-
const exts = ObjectKeys(this.context.require.extensions);
1133-
const indexRe = new RegExp('^index(?:' + exts.map(regexpEscape).join('|') +
1134-
')$');
1136+
const extensions = ObjectKeys(this.context.require.extensions);
1137+
const indexes = extensions.map((extension) => `index${extension}`);
1138+
indexes.push('package.json', 'index');
11351139
const versionedFileNamesRe = /-\d+\.\d+/;
11361140

1141+
const match = line.match(requireRE);
11371142
completeOn = match[1];
11381143
const subdir = match[2] || '';
1139-
filter = match[1];
1140-
let dir, files, subfiles, isDirectory;
1144+
filter = completeOn;
11411145
group = [];
11421146
let paths = [];
11431147

@@ -1151,41 +1155,31 @@ function complete(line, callback) {
11511155
paths = module.paths.concat(CJSModule.globalPaths);
11521156
}
11531157

1154-
for (let i = 0; i < paths.length; i++) {
1155-
dir = path.resolve(paths[i], subdir);
1156-
try {
1157-
files = fs.readdirSync(dir);
1158-
} catch {
1159-
continue;
1160-
}
1161-
for (let f = 0; f < files.length; f++) {
1162-
const name = files[f];
1158+
for (let dir of paths) {
1159+
dir = path.resolve(dir, subdir);
1160+
const files = gracefulOperation(fs.readdirSync, [dir], []);
1161+
for (const name of files) {
11631162
const ext = path.extname(name);
11641163
const base = name.slice(0, -ext.length);
11651164
if (versionedFileNamesRe.test(base) || name === '.npm') {
11661165
// Exclude versioned names that 'npm' installs.
11671166
continue;
11681167
}
11691168
const abs = path.resolve(dir, name);
1170-
try {
1171-
isDirectory = fs.statSync(abs).isDirectory();
1172-
} catch {
1169+
const stats = gracefulOperation(fs.statSync, [abs]);
1170+
if (!stats || !stats.isDirectory()) {
1171+
if (extensions.includes(ext) && (!subdir || base !== 'index')) {
1172+
group.push(`${subdir}${base}`);
1173+
}
11731174
continue;
11741175
}
1175-
if (isDirectory) {
1176-
group.push(subdir + name + '/');
1177-
try {
1178-
subfiles = fs.readdirSync(abs);
1179-
} catch {
1180-
continue;
1181-
}
1182-
for (let s = 0; s < subfiles.length; s++) {
1183-
if (indexRe.test(subfiles[s])) {
1184-
group.push(subdir + name);
1185-
}
1176+
group.push(`${subdir}${name}/`);
1177+
const subfiles = gracefulOperation(fs.readdirSync, [abs], []);
1178+
for (const subfile of subfiles) {
1179+
if (indexes.includes(subfile)) {
1180+
group.push(`${subdir}${name}`);
1181+
break;
11861182
}
1187-
} else if (exts.includes(ext) && (!subdir || base !== 'index')) {
1188-
group.push(subdir + base);
11891183
}
11901184
}
11911185
}
@@ -1198,11 +1192,10 @@ function complete(line, callback) {
11981192
}
11991193

12001194
completionGroupsLoaded();
1201-
} else if (match = line.match(fsAutoCompleteRE)) {
1202-
1203-
let filePath = match[1];
1204-
let fileList;
1195+
} else if (fsAutoCompleteRE.test(line)) {
12051196
filter = '';
1197+
let filePath = line.match(fsAutoCompleteRE)[1];
1198+
let fileList;
12061199

12071200
try {
12081201
fileList = fs.readdirSync(filePath, { withFileTypes: true });
@@ -1233,7 +1226,7 @@ function complete(line, callback) {
12331226
// foo<|> # all scope vars with filter 'foo'
12341227
// foo.<|> # completions for 'foo' with filter ''
12351228
} else if (line.length === 0 || /\w|\.|\$/.test(line[line.length - 1])) {
1236-
match = simpleExpressionRE.exec(line);
1229+
const match = simpleExpressionRE.exec(line);
12371230
if (line.length !== 0 && !match) {
12381231
completionGroupsLoaded();
12391232
return;
@@ -1583,10 +1576,6 @@ function defineDefaultCommands(repl) {
15831576
}
15841577
}
15851578

1586-
function regexpEscape(s) {
1587-
return s.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, '\\$&');
1588-
}
1589-
15901579
function Recoverable(err) {
15911580
this.err = err;
15921581
}

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

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/fixtures/node_modules/no_index/package.json

+3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/parallel/test-repl-tab-complete.js

+29-7
Original file line numberDiff line numberDiff line change
@@ -229,24 +229,46 @@ testMe.complete('require(\'', common.mustCall(function(error, data) {
229229
});
230230
}));
231231

232-
testMe.complete('require(\'n', common.mustCall(function(error, data) {
232+
testMe.complete("require\t( 'n", common.mustCall(function(error, data) {
233233
assert.strictEqual(error, null);
234234
assert.strictEqual(data.length, 2);
235235
assert.strictEqual(data[1], 'n');
236-
assert(data[0].includes('net'));
236+
// There is only one Node.js module that starts with n:
237+
assert.strictEqual(data[0][0], 'net');
238+
assert.strictEqual(data[0][1], '');
237239
// It's possible to pick up non-core modules too
238-
data[0].forEach(function(completion) {
239-
if (completion)
240-
assert(/^n/.test(completion));
240+
data[0].slice(2).forEach((completion) => {
241+
assert.match(completion, /^n/);
241242
});
242243
}));
243244

244245
{
245246
const expected = ['@nodejsscope', '@nodejsscope/'];
247+
// Require calls should handle all types of quotation marks.
248+
for (const quotationMark of ["'", '"', '`']) {
249+
putIn.run(['.clear']);
250+
testMe.complete('require(`@nodejs', common.mustCall((err, data) => {
251+
assert.strictEqual(err, null);
252+
assert.deepStrictEqual(data, [expected, '@nodejs']);
253+
}));
254+
255+
putIn.run(['.clear']);
256+
// Completions should not be greedy in case the quotation ends.
257+
const input = `require(${quotationMark}@nodejsscope${quotationMark}`;
258+
testMe.complete(input, common.mustCall((err, data) => {
259+
assert.strictEqual(err, null);
260+
assert.deepStrictEqual(data, [[], undefined]);
261+
}));
262+
}
263+
}
264+
265+
{
246266
putIn.run(['.clear']);
247-
testMe.complete('require(\'@nodejs', common.mustCall((err, data) => {
267+
// Completions should find modules and handle whitespace after the opening
268+
// bracket.
269+
testMe.complete('require \t("no_ind', common.mustCall((err, data) => {
248270
assert.strictEqual(err, null);
249-
assert.deepStrictEqual(data, [expected, '@nodejs']);
271+
assert.deepStrictEqual(data, [['no_index', 'no_index/'], 'no_ind']);
250272
}));
251273
}
252274

0 commit comments

Comments
 (0)