From 99aa91957062096dc35e24e251da34eccf12a5f7 Mon Sep 17 00:00:00 2001 From: re-engineer <68780902+re-engineer@users.noreply.github.com> Date: Sat, 25 Jul 2020 10:55:49 -0400 Subject: [PATCH] Make RefactorSession.prototype.replace only count changed nodes Currently, the following code causes `shift-refactor` to crash (infinite recursion): [```js const {parseScript}=require("shift-parser"); const RefactorSession=require("shift-refactor"); const script=` window['example string with spaces']; `; const session=new RefactorSession(parseScript(script)); session.convertComputedToStatic(); ```](https://runkit.com/embed/5t8xd3wbw1am) It happens because `RefactorSession.prototype.convertComputedToStatic` uses `RefactorSession.prototype.replaceRecursive`, and its replacement program returns the original node in certain cases. When the `expression.value` of a `ComputedMemberExpression` or any of its variants contains a space, the function will return the original node (signifying that there should be no replacement). When `RefactorSession.prototype.replace` is run in `RefactorSession.prototype.replaceRecursive`, the nodes that were not replaced are still counted in the number of replacements. This means that even though there are no more nodes to replace, `RefactorSession.prototype.replaceRecursive` runs again. This PR solves the issue by changing `RefactorSession.prototype.replace` to count only changed nodes. --- src/index.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/index.js b/src/index.js index 96b73a3..ddc10a6 100644 --- a/src/index.js +++ b/src/index.js @@ -73,11 +73,12 @@ class RefactorSession { const replacement = typeof program === "string" ? parseScript(program) : null; - nodes.forEach(node => { + const replacedNodes=nodes.filter(node => { if (isFunction(program)) { const rv = program(node); + if(rv===node) return false; if (isShiftNode(rv)) { - this._queueReplacement(node, program(node)); + this._queueReplacement(node, rv); } else if (isString(rv)) { const returnedTree = parseScript(rv); if (isStatement(node)) { @@ -98,10 +99,11 @@ class RefactorSession { copy(replacement.statements[0].expression) ); } + return true; }); if (this.autoCleanup) this.cleanup(); - return nodes.length; + return replacedNodes.length; } replaceRecursive(selector, program) { const nodesReplaced = this.replace(selector, program);