repl: improve REPL disabling completion on proxies and getters · nodejs/node@3a3eb68
@@ -1523,8 +1523,24 @@ function complete(line, callback) {
15231523return;
15241524}
152515251526+// If the target ends with a dot (e.g. `obj.foo.`) such code won't be valid for AST parsing
1527+// so in order to make it correct we add an identifier to its end (e.g. `obj.foo.x`)
1528+const parsableCompleteTarget = completeTarget.endsWith('.') ? `${completeTarget}x` : completeTarget;
1529+1530+let completeTargetAst;
1531+try {
1532+completeTargetAst = acornParse(
1533+parsableCompleteTarget, { __proto__: null, sourceType: 'module', ecmaVersion: 'latest' },
1534+);
1535+} catch { /* No need to specifically handle parse errors */ }
1536+1537+if (!completeTargetAst) {
1538+return completionGroupsLoaded();
1539+}
1540+15261541return includesProxiesOrGetters(
1527-StringPrototypeSplit(completeTarget, '.'),
1542+completeTargetAst.body[0].expression,
1543+parsableCompleteTarget,
15281544this.eval,
15291545this.context,
15301546(includes) => {
@@ -1729,32 +1745,156 @@ function findExpressionCompleteTarget(code) {
17291745return code;
17301746}
173117471732-function includesProxiesOrGetters(exprSegments, evalFn, context, callback, currentExpr = '', idx = 0) {
1733-const currentSegment = exprSegments[idx];
1734-currentExpr += `${currentExpr.length === 0 ? '' : '.'}${currentSegment}`;
1735-evalFn(`try { ${currentExpr} } catch { }`, context, getREPLResourceName(), (_, currentObj) => {
1736-if (typeof currentObj !== 'object' || currentObj === null) {
1737-return callback(false);
1738-}
1748+/**
1749+ * Utility used to determine if an expression includes object getters or proxies.
1750+ *
1751+ * Example: given `obj.foo`, the function lets you know if `foo` has a getter function
1752+ * associated to it, or if `obj` is a proxy
1753+ * @param {any} expr The expression, in AST format to analyze
1754+ * @param {string} exprStr The string representation of the expression
1755+ * @param {(str: string, ctx: any, resourceName: string, cb: (error, evaled) => void) => void} evalFn
1756+ * Eval function to use
1757+ * @param {any} ctx The context to use for any code evaluation
1758+ * @param {(includes: boolean) => void} callback Callback that will be called with the result of the operation
1759+ * @returns {void}
1760+ */
1761+function includesProxiesOrGetters(expr, exprStr, evalFn, ctx, callback) {
1762+if (expr?.type !== 'MemberExpression') {
1763+// If the expression is not a member one for obvious reasons no getters are involved
1764+return callback(false);
1765+}
173917661740-if (isProxy(currentObj)) {
1741-return callback(true);
1767+if (expr.object.type === 'MemberExpression') {
1768+// The object itself is a member expression, so we need to recurse (e.g. the expression is `obj.foo.bar`)
1769+return includesProxiesOrGetters(
1770+expr.object,
1771+exprStr.slice(0, expr.object.end),
1772+evalFn,
1773+ctx,
1774+(includes, lastEvaledObj) => {
1775+if (includes) {
1776+// If the recurred call found a getter we can also terminate
1777+return callback(includes);
1778+}
1779+1780+if (isProxy(lastEvaledObj)) {
1781+return callback(true);
1782+}
1783+1784+// If a getter/proxy hasn't been found by the recursion call we need to check if maybe a getter/proxy
1785+// is present here (e.g. in `obj.foo.bar` we found that `obj.foo` doesn't involve any getters so we now
1786+// need to check if `bar` on `obj.foo` (i.e. `lastEvaledObj`) has a getter or if `obj.foo.bar` is a proxy)
1787+return hasGetterOrIsProxy(lastEvaledObj, expr.property, (doesHaveGetterOrIsProxy) => {
1788+return callback(doesHaveGetterOrIsProxy);
1789+});
1790+},
1791+);
1792+}
1793+1794+// This is the base of the recursion we have an identifier for the object and an identifier or literal
1795+// for the property (e.g. we have `obj.foo` or `obj['foo']`, `obj` is the object identifier and `foo`
1796+// is the property identifier/literal)
1797+if (expr.object.type === 'Identifier') {
1798+return evalFn(`try { ${expr.object.name} } catch {}`, ctx, getREPLResourceName(), (err, obj) => {
1799+if (err) {
1800+return callback(false);
1801+}
1802+1803+if (isProxy(obj)) {
1804+return callback(true);
1805+}
1806+1807+return hasGetterOrIsProxy(obj, expr.property, (doesHaveGetterOrIsProxy) => {
1808+if (doesHaveGetterOrIsProxy) {
1809+return callback(true);
1810+}
1811+1812+return evalFn(
1813+`try { ${exprStr} } catch {} `, ctx, getREPLResourceName(), (err, obj) => {
1814+if (err) {
1815+return callback(false);
1816+}
1817+return callback(false, obj);
1818+});
1819+});
1820+});
1821+}
1822+1823+/**
1824+ * Utility to see if a property has a getter associated to it or if
1825+ * the property itself is a proxy object.
1826+ */
1827+function hasGetterOrIsProxy(obj, astProp, cb) {
1828+if (!obj || !astProp) {
1829+return cb(false);
17421830}
174318311744-const nextIdx = idx + 1;
1832+if (astProp.type === 'Literal') {
1833+// We have something like `obj['foo'].x` where `x` is the literal
174518341746-if (nextIdx >= exprSegments.length) {
1747-return callback(false);
1835+if (isProxy(obj[astProp.value])) {
1836+return cb(true);
1837+}
1838+1839+const propDescriptor = ObjectGetOwnPropertyDescriptor(
1840+obj,
1841+`${astProp.value}`,
1842+);
1843+const propHasGetter = typeof propDescriptor?.get === 'function';
1844+return cb(propHasGetter);
17481845}
174918461750-const nextSegmentProp = ObjectGetOwnPropertyDescriptor(currentObj, exprSegments[nextIdx]);
1751-const nextSegmentPropHasGetter = typeof nextSegmentProp?.get === 'function';
1752-if (nextSegmentPropHasGetter) {
1753-return callback(true);
1847+if (
1848+astProp.type === 'Identifier' &&
1849+exprStr.at(astProp.start - 1) === '.'
1850+) {
1851+// We have something like `obj.foo.x` where `foo` is the identifier
1852+1853+if (isProxy(obj[astProp.name])) {
1854+return cb(true);
1855+}
1856+1857+const propDescriptor = ObjectGetOwnPropertyDescriptor(
1858+obj,
1859+`${astProp.name}`,
1860+);
1861+const propHasGetter = typeof propDescriptor?.get === 'function';
1862+return cb(propHasGetter);
17541863}
175518641756-return includesProxiesOrGetters(exprSegments, evalFn, context, callback, currentExpr, nextIdx);
1757-});
1865+return evalFn(
1866+// Note: this eval runs the property expression, which might be side-effectful, for example
1867+// the user could be running `obj[getKey()].` where `getKey()` has some side effects.
1868+// Arguably this behavior should not be too surprising, but if it turns out that it is,
1869+// then we can revisit this behavior and add logic to analyze the property expression
1870+// and eval it only if we can confidently say that it can't have any side effects
1871+`try { ${exprStr.slice(astProp.start, astProp.end)} } catch {} `,
1872+ctx,
1873+getREPLResourceName(),
1874+(err, evaledProp) => {
1875+if (err) {
1876+return callback(false);
1877+}
1878+1879+if (typeof evaledProp === 'string') {
1880+if (isProxy(obj[evaledProp])) {
1881+return cb(true);
1882+}
1883+1884+const propDescriptor = ObjectGetOwnPropertyDescriptor(
1885+obj,
1886+evaledProp,
1887+);
1888+const propHasGetter = typeof propDescriptor?.get === 'function';
1889+return cb(propHasGetter);
1890+}
1891+1892+return callback(false);
1893+},
1894+);
1895+}
1896+1897+return callback(false);
17581898}
1759189917601900REPLServer.prototype.completeOnEditorMode = (callback) => (err, results) => {