GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — repl: do not cause side effects in tab completion · nodejs/node@0340fe9

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

lib
- repl.js
test/parallel

Expand file treeCollapse file tree

Open diff view settings

Collapse file

`‎lib/repl.js‎`

Copy file name to clipboardExpand all lines: lib/repl.js

+16-1Lines changed: 16 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -1754,10 +1754,25 @@ function findExpressionCompleteTarget(code) {`
`1754`	`1754`	`return findExpressionCompleteTarget(argumentCode);`
`1755`	`1755`	`}`
`1756`	`1756`
	`1757`	`+ // Walk the AST for the current block of code, and check whether it contains any`
	`1758`	`+ // statement or expression type that would potentially have side effects if evaluated.`
	`1759`	`+ let isAllowed = true;`
	`1760`	`+ const disallow = () => isAllowed = false;`
	`1761`	`+ acornWalk.simple(lastBodyStatement, {`
	`1762`	`+ ForInStatement: disallow,`
	`1763`	`+ ForOfStatement: disallow,`
	`1764`	`+ CallExpression: disallow,`
	`1765`	`+ AssignmentExpression: disallow,`
	`1766`	`+ UpdateExpression: disallow,`
	`1767`	`+ });`
	`1768`	`+ if (!isAllowed) {`
	`1769`	`+ return null;`
	`1770`	`+ }`
	`1771`	`+`
`1757`	`1772`	`// If any of the above early returns haven't activated then it means that`
`1758`	`1773`	`// the potential complete target is the full code (e.g. the code represents`
`1759`	`1774`	`// a simple partial identifier, a member expression, etc...)`
`1760`		`- return code;`
	`1775`	`+ return code.slice(lastBodyStatement.start, lastBodyStatement.end);`
`1761`	`1776`	`}`
`1762`	`1777`
`1763`	`1778`	`/**`

Collapse file

`‎test/parallel/test-repl-completion-on-getters-disabled.js‎`

Copy file name to clipboardExpand all lines: test/parallel/test-repl-completion-on-getters-disabled.js

-5Lines changed: 0 additions & 5 deletions

Original file line number	Diff line number	Diff line change
`@@ -61,10 +61,6 @@ describe('REPL completion in relation of getters', () => {`
`61`	`61`	test(`completions are generated for properties that don't trigger getters`, () => {
`62`	`62`	`runCompletionTests(`
`63`	`63`	`
`64`		`- function getFooKey() {`
`65`		`- return "foo";`
`66`		`- }`
`67`		`-`
`68`	`64`	`const fooKey = "foo";`
`69`	`65`
`70`	`66`	`const keys = {`
`@@ -90,7 +86,6 @@ describe('REPL completion in relation of getters', () => {`
`90`	`86`	`["objWithGetters[keys['foo key']].b", ["objWithGetters[keys['foo key']].bar"]],`
`91`	`87`	`['objWithGetters[fooKey].b', ['objWithGetters[fooKey].bar']],`
`92`	`88`	`["objWithGetters['f' + 'oo'].b", ["objWithGetters['f' + 'oo'].bar"]],`
`93`		`- ['objWithGetters[getFooKey()].b', ['objWithGetters[getFooKey()].bar']],`
`94`	`89`	`]);`
`95`	`90`	`});`
`96`	`91`

Collapse file

`‎test/parallel/test-repl-tab-complete-getter-error.js‎`

Copy file name to clipboardExpand all lines: test/parallel/test-repl-tab-complete-getter-error.js

+1-2Lines changed: 1 addition & 2 deletions

Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,6 @@ async function runTest() {`
`27`	`27`
`28`	`28`	`await new Promise((resolve, reject) => {`
`29`	`29`	replServer.eval(`
`30`		`- const getNameText = () => "name";`
`31`	`30`	`const foo = { get name() { throw new Error(); } };`
`32`	`31`	`, replServer.context, '', (err) => {
`33`	`32`	`if (err) {`
`@@ -38,7 +37,7 @@ async function runTest() {`
`38`	`37`	`});`
`39`	`38`	`});`
`40`	`39`
`41`		`- ['foo.name.', 'foo["name"].', 'foo[getNameText()].'].forEach((test) => {`
	`40`	`+ ['foo.name.', 'foo["name"].'].forEach((test) => {`
`42`	`41`	`replServer.complete(`
`43`	`42`	`test,`
`44`	`43`	`common.mustCall((error, data) => {`

Collapse file

`‎test/parallel/test-repl-tab-complete-nosideeffects.js‎`

Copy file name to clipboard

+56Lines changed: 56 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,56 @@`
	`1`	`+'use strict';`
	`2`	`+`
	`3`	`+const common = require('../common');`
	`4`	`+const ArrayStream = require('../common/arraystream');`
	`5`	`+const { describe, it } = require('node:test');`
	`6`	`+const assert = require('assert');`
	`7`	`+`
	`8`	`+const repl = require('repl');`
	`9`	`+`
	`10`	`+function prepareREPL() {`
	`11`	`+ const input = new ArrayStream();`
	`12`	`+ const replServer = repl.start({`
	`13`	`+ prompt: '',`
	`14`	`+ input,`
	`15`	`+ output: process.stdout,`
	`16`	`+ allowBlockingCompletions: true,`
	`17`	`+ });`
	`18`	`+`
	`19`	`+ // Some errors are passed to the domain, but do not callback`
	`20`	`+ replServer._domain.on('error', assert.ifError);`
	`21`	`+`
	`22`	`+ return { replServer, input };`
	`23`	`+}`
	`24`	`+`
	`25`	`+function getNoResultsFunction() {`
	`26`	`+ return common.mustSucceed((data) => {`
	`27`	`+ assert.deepStrictEqual(data[0], []);`
	`28`	`+ });`
	`29`	`+}`
	`30`	`+`
	`31`	`+describe('REPL tab completion without side effects', () => {`
	`32`	`+ const setup = [`
	`33`	`+ 'globalThis.counter = 0;',`
	`34`	`+ 'function incCounter() { return counter++; }',`
	`35`	`+ 'const arr = [{ bar: "baz" }];',`
	`36`	`+ ];`
	`37`	+ // None of these expressions should affect the value of `counter`
	`38`	`+ for (const code of [`
	`39`	`+ 'incCounter().',`
	`40`	`+ 'a=(counter+=1).foo.',`
	`41`	`+ 'a=(counter++).foo.',`
	`42`	`+ 'for((counter)of[1])foo.',`
	`43`	`+ 'for((counter)in{1:1})foo.',`
	`44`	`+ 'arr[incCounter()].b',`
	`45`	`+ ]) {`
	`46`	+ it(`does not evaluate with side effects (${code})`, async () => {
	`47`	`+ const { replServer, input } = prepareREPL();`
	`48`	`+ input.run(setup);`
	`49`	`+`
	`50`	`+ replServer.complete(code, getNoResultsFunction());`
	`51`	`+`
	`52`	`+ assert.strictEqual(replServer.context.counter, 0);`
	`53`	`+ replServer.close();`
	`54`	`+ });`
	`55`	`+ }`
	`56`	`+});`

0 commit comments

Comments

0 (0)

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.