Skip to content

repl: improve tab completion on computed properties #58775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 101 additions & 18 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 ''
Comment on lines -1483 to -1492
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I'm removing this comment since I think that it is inaccurate (and as a consequence misleading), since various type of lines pass through here, like for example { a: true }

I think/hope that the code structure makes it clear enough what this else if block is for (I also have half a mind to do some refactoring to also make things clearer later on 🤔)

} 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, '.');
}
Expand Down Expand Up @@ -1531,7 +1520,7 @@ function complete(line, callback) {
}

return includesProxiesOrGetters(
StringPrototypeSplit(match, '.'),
StringPrototypeSplit(completeTarget, '.'),
this.eval,
this.context,
(includes) => {
Expand Down Expand Up @@ -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}`;
Expand Down
29 changes: 29 additions & 0 deletions test/parallel/test-repl-tab-complete-computed-props.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
`,
]);
});
Expand All @@ -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'],
})
);
});
});
Loading