fix replacement logic in `collapse_vars` (#2475)

1 files changed, 114 insertions, 102 deletions

diff --git a/lib/compress.js b/lib/compress.js
index 0635c41d..a50dd52b 100644
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -839,6 +839,113 @@ merge(Compressor.prototype, {
             var args;
             var candidates = [];
             var stat_index = statements.length;
+            var scanner = new TreeTransformer(function(node, descend) {
+                if (abort) return node;
+                // Skip nodes before `candidate` as quickly as possible
+                if (!hit) {
+                    if (node === candidate) {
+                        hit = true;
+                        return node;
+                    }
+                    return;
+                }
+                // Stop immediately if these node types are encountered
+                var parent = scanner.parent();
+                if (node instanceof AST_Assign && node.operator != "=" && lhs.equivalent_to(node.left)
+                    || node instanceof AST_Call && lhs instanceof AST_PropAccess && lhs.equivalent_to(node.expression)
+                    || node instanceof AST_Debugger
+                    || node instanceof AST_IterationStatement && !(node instanceof AST_For)
+                    || node instanceof AST_SymbolRef && !node.is_declared(compressor)
+                    || node instanceof AST_Try
+                    || node instanceof AST_With
+                    || parent instanceof AST_For && node !== parent.init) {
+                    abort = true;
+                    return node;
+                }
+                // Replace variable with assignment when found
+                if (can_replace
+                    && !(node instanceof AST_SymbolDeclaration)
+                    && lhs.equivalent_to(node)) {
+                    if (is_lhs(node, parent)) {
+                        if (candidate.multiple) replaced++;
+                        return node;
+                    }
+                    CHANGED = abort = true;
+                    replaced++;
+                    compressor.info("Collapsing {name} [{file}:{line},{col}]", {
+                        name: node.print_to_string(),
+                        file: node.start.file,
+                        line: node.start.line,
+                        col: node.start.col
+                    });
+                    if (candidate instanceof AST_UnaryPostfix) {
+                        return make_node(AST_UnaryPrefix, candidate, candidate);
+                    }
+                    if (candidate instanceof AST_VarDef) {
+                        if (candidate.multiple) {
+                            abort = false;
+                            return node;
+                        }
+                        var def = candidate.name.definition();
+                        if (def.references.length - def.replaced == 1 && !compressor.exposed(def)) {
+                            def.replaced++;
+                            return maintain_this_binding(parent, node, candidate.value);
+                        }
+                        return make_node(AST_Assign, candidate, {
+                            operator: "=",
+                            left: make_node(AST_SymbolRef, candidate.name, candidate.name),
+                            right: candidate.value
+                        });
+                    }
+                    candidate.write_only = false;
+                    return candidate;
+                }
+                // These node types have child nodes that execute sequentially,
+                // but are otherwise not safe to scan into or beyond them.
+                var sym;
+                if (node instanceof AST_Call
+                    || node instanceof AST_Exit
+                    || node instanceof AST_PropAccess
+                        && (side_effects || node.expression.may_throw_on_access(compressor))
+                    || node instanceof AST_SymbolRef
+                        && (lvalues[node.name]
+                            || side_effects && !references_in_scope(node.definition()))
+                    || (sym = lhs_or_def(node))
+                        && (sym instanceof AST_PropAccess || sym.name in lvalues)
+                    || (side_effects || !replace_all)
+                        && (parent instanceof AST_Binary && lazy_op(parent.operator)
+                            || parent instanceof AST_Case
+                            || parent instanceof AST_Conditional
+                            || parent instanceof AST_If)) {
+                    if (!(node instanceof AST_Scope)) descend(node, scanner);
+                    abort = true;
+                    return node;
+                }
+                // Skip (non-executed) functions and (leading) default case in switch statements
+                if (node instanceof AST_Default || node instanceof AST_Scope) return node;
+            });
+            var multi_replacer = new TreeTransformer(function(node) {
+                if (abort) return node;
+                // Skip nodes before `candidate` as quickly as possible
+                if (!hit) {
+                    if (node === candidate) {
+                        hit = true;
+                        return node;
+                    }
+                    return;
+                }
+                // Replace variable when found
+                if (node instanceof AST_SymbolRef
+                    && node.name == def.name) {
+                    if (!--replaced) abort = true;
+                    if (is_lhs(node, multi_replacer.parent())) return node;
+                    def.replaced++;
+                    value_def.replaced--;
+                    return candidate.value;
+                }
+                // Skip (non-executed) functions and (leading) default case in switch statements
+                if (node instanceof AST_Default || node instanceof AST_Scope) return node;
+            });
             while (--stat_index >= 0) {
                 // Treat parameters as collapsible in IIFE, i.e.
                 //   function(a, b){ ... }(x());
@@ -862,121 +969,24 @@ merge(Compressor.prototype, {
                     var side_effects = value_has_side_effects(candidate);
                     var hit = candidate.name instanceof AST_SymbolFunarg;
                     var abort = false, replaced = 0, can_replace = !args || !hit;
-                    var tt = new TreeTransformer(function(node, descend) {
-                        if (abort) return node;
-                        // Skip nodes before `candidate` as quickly as possible
-                        if (!hit) {
-                            if (node === candidate) {
-                                hit = true;
-                                return node;
-                            }
-                            return;
-                        }
-                        // Stop immediately if these node types are encountered
-                        var parent = tt.parent();
-                        if (node instanceof AST_Assign && node.operator != "=" && lhs.equivalent_to(node.left)
-                            || node instanceof AST_Call && lhs instanceof AST_PropAccess && lhs.equivalent_to(node.expression)
-                            || node instanceof AST_Debugger
-                            || node instanceof AST_IterationStatement && !(node instanceof AST_For)
-                            || node instanceof AST_SymbolRef && !node.is_declared(compressor)
-                            || node instanceof AST_Try
-                            || node instanceof AST_With
-                            || parent instanceof AST_For && node !== parent.init) {
-                            abort = true;
-                            return node;
-                        }
-                        // Replace variable with assignment when found
-                        if (can_replace
-                            && !(node instanceof AST_SymbolDeclaration)
-                            && !is_lhs(node, parent)
-                            && lhs.equivalent_to(node)) {
-                            CHANGED = abort = true;
-                            replaced++;
-                            compressor.info("Collapsing {name} [{file}:{line},{col}]", {
-                                name: node.print_to_string(),
-                                file: node.start.file,
-                                line: node.start.line,
-                                col: node.start.col
-                            });
-                            if (candidate instanceof AST_UnaryPostfix) {
-                                return make_node(AST_UnaryPrefix, candidate, candidate);
-                            }
-                            if (candidate instanceof AST_VarDef) {
-                                if (candidate.multiple) {
-                                    abort = false;
-                                    return node;
-                                }
-                                var def = candidate.name.definition();
-                                if (def.references.length - def.replaced == 1 && !compressor.exposed(def)) {
-                                    def.replaced++;
-                                    return maintain_this_binding(parent, node, candidate.value);
-                                }
-                                return make_node(AST_Assign, candidate, {
-                                    operator: "=",
-                                    left: make_node(AST_SymbolRef, candidate.name, candidate.name),
-                                    right: candidate.value
-                                });
-                            }
-                            candidate.write_only = false;
-                            return candidate;
-                        }
-                        // These node types have child nodes that execute sequentially,
-                        // but are otherwise not safe to scan into or beyond them.
-                        var sym;
-                        if (node instanceof AST_Call
-                            || node instanceof AST_Exit
-                            || node instanceof AST_PropAccess
-                                && (side_effects || node.expression.may_throw_on_access(compressor))
-                            || node instanceof AST_SymbolRef
-                               && (lvalues[node.name]
-                                   || side_effects && !references_in_scope(node.definition()))
-                            || (sym = lhs_or_def(node))
-                                && (sym instanceof AST_PropAccess || sym.name in lvalues)
-                            || (side_effects || !replace_all)
-                                && (parent instanceof AST_Binary && lazy_op(parent.operator)
-                                    || parent instanceof AST_Case
-                                    || parent instanceof AST_Conditional
-                                    || parent instanceof AST_If)) {
-                            if (!(node instanceof AST_Scope)) descend(node, tt);
-                            abort = true;
-                            return node;
-                        }
-                        // Skip (non-executed) functions and (leading) default case in switch statements
-                        if (node instanceof AST_Default || node instanceof AST_Scope) return node;
-                    });
                     if (!can_replace) {
                         for (var j = compressor.self().argnames.lastIndexOf(candidate.name) + 1; !abort && j < args.length; j++) {
-                            args[j].transform(tt);
+                            args[j].transform(scanner);
                         }
                         can_replace = true;
                     }
                     for (var i = stat_index; !abort && i < statements.length; i++) {
-                        statements[i].transform(tt);
+                        statements[i].transform(scanner);
                     }
                     if (candidate.multiple) {
                         var def = candidate.name.definition();
-                        if (abort && def.references.length > replaced) replaced = false;
+                        if (abort && def.references.length - def.replaced > replaced) replaced = false;
                         else {
                             abort = false;
                             hit = candidate.name instanceof AST_SymbolFunarg;
                             var value_def = candidate.value.definition();
                             for (var i = stat_index; !abort && i < statements.length; i++) {
-                                statements[i].transform(new TreeTransformer(function(node) {
-                                    if (abort) return node;
-                                    if (!hit) {
-                                        if (node === candidate) {
-                                            hit = true;
-                                            return node;
-                                        }
-                                        return;
-                                    }
-                                    if (node instanceof AST_SymbolRef && node.name == def.name) {
-                                        def.replaced++;
-                                        value_def.replaced--;
-                                        if (!--replaced) abort = true;
-                                        return candidate.value;
-                                    }
-                                }));
+                                statements[i].transform(multi_replacer);
                             }
                         }
                     }
@@ -2549,12 +2559,14 @@ merge(Compressor.prototype, {
                             var def = tail.pop();
                             compressor.warn("Converting duplicated definition of variable {name} to assignment [{file}:{line},{col}]", template(def.name));
                             remove(var_defs, def);
-                            drop_decl(def.name.definition());
                             side_effects.unshift(make_node(AST_Assign, def, {
                                 operator: "=",
                                 left: make_node(AST_SymbolRef, def.name, def.name),
                                 right: def.value
                             }));
+                            def = def.name.definition();
+                            drop_decl(def);
+                            def.replaced--;
                         }
                     }
                     if (head.length > 0 || tail.length > 0) {