fix corner cases in `collapse_vars`, `unused` & `varify` (#4292)

fixes #4290

4 files changed, 141 insertions, 34 deletions

diff --git a/lib/compress.js b/lib/compress.js
index c86bb5f0..9adcec8c 100644
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -368,7 +368,7 @@ merge(Compressor.prototype, {
         } while (sym = sym.parent_scope);
     }
 
-    function can_drop_symbol(tw, ref, keep_lambda) {
+    function can_drop_symbol(ref, keep_lambda) {
         var orig = ref.definition().orig;
         if (ref.in_arg && (orig[0] instanceof AST_SymbolFunarg || orig[1] instanceof AST_SymbolFunarg)) return false;
         return all(orig, function(sym) {
@@ -678,7 +678,7 @@ merge(Compressor.prototype, {
                     var d = sym.definition();
                     d.assignments++;
                     if (!is_modified(compressor, tw, node, node.right, 0)
-                        && can_drop_symbol(tw, sym) && safe_to_assign(tw, d)) {
+                        && can_drop_symbol(sym) && safe_to_assign(tw, d)) {
                         push_ref(d, sym);
                         mark(tw, d);
                         tw.loop_ids[d.id] = tw.in_loop;
@@ -1706,9 +1706,11 @@ merge(Compressor.prototype, {
                 if (node instanceof AST_DWLoop) return true;
                 if (node instanceof AST_LoopControl) return true;
                 if (node instanceof AST_SymbolRef) {
-                    if (node.is_declared(compressor) ? node.fixed_value() || all(node.definition().orig, function(sym) {
-                        return !(sym instanceof AST_SymbolConst || sym instanceof AST_SymbolLet);
-                    }) : parent instanceof AST_Assign && parent.operator == "=" && parent.left === node) return false;
+                    if (node.is_declared(compressor)) {
+                        if (node.fixed_value() || can_drop_symbol(node)) return false;
+                    } else if (parent instanceof AST_Assign && parent.operator == "=" && parent.left === node) {
+                        return false;
+                    }
                     if (!replace_all) return true;
                     scan_rhs = false;
                     return false;
@@ -1779,14 +1781,12 @@ merge(Compressor.prototype, {
                     }
                     if (side_effects && may_modify(node)) return true;
                     var def = node.definition();
-                    return (in_try || def.scope.resolve() !== scope) && !all(def.orig, function(sym) {
-                        return !(sym instanceof AST_SymbolConst || sym instanceof AST_SymbolLet);
-                    });
+                    return (in_try || def.scope.resolve() !== scope) && !can_drop_symbol(node);
                 }
                 if (node instanceof AST_This) return symbol_in_lvalues(node, parent);
                 if (node instanceof AST_VarDef) {
                     if ((in_try || !lhs_local) && node.name instanceof AST_Destructured) return true;
-                    return node.value && node.name.match_symbol(function(node) {
+                    return (node.value || parent instanceof AST_Let) && node.name.match_symbol(function(node) {
                         return node instanceof AST_SymbolDeclaration
                             && (lvalues.has(node.name) || side_effects && may_modify(node));
                     });
@@ -4304,7 +4304,7 @@ merge(Compressor.prototype, {
         });
         def(AST_SymbolDeclaration, return_false);
         def(AST_SymbolRef, function(compressor) {
-            return !this.is_declared(compressor) || !can_drop_symbol(compressor, this);
+            return !this.is_declared(compressor) || !can_drop_symbol(this);
         });
         def(AST_This, return_false);
         def(AST_Try, function(compressor) {
@@ -4974,7 +4974,7 @@ merge(Compressor.prototype, {
         var self = this;
         var drop_funcs = !(self instanceof AST_Toplevel) || compressor.toplevel.funcs;
         var drop_vars = !(self instanceof AST_Toplevel) || compressor.toplevel.vars;
-        var assign_as_unused = /keep_assign/.test(compressor.option("unused")) ? return_false : function(tw, node, props) {
+        var assign_as_unused = /keep_assign/.test(compressor.option("unused")) ? return_false : function(node, props) {
             var sym, nested = false;
             if (node instanceof AST_Assign) {
                 if (node.write_only || node.operator == "=") sym = node.left;
@@ -4990,7 +4990,7 @@ merge(Compressor.prototype, {
             }
             if (!(sym instanceof AST_SymbolRef)) return;
             if (compressor.exposed(sym.definition())) return;
-            if (!can_drop_symbol(tw, sym, nested)) return;
+            if (!can_drop_symbol(sym, nested)) return;
             return sym;
         };
         var assign_in_use = Object.create(null);
@@ -5046,8 +5046,9 @@ merge(Compressor.prototype, {
                                 var redef = def.redefined();
                                 if (redef) var_defs_by_id.add(redef.id, defn);
                             }
-                            if ((!drop_vars || (node instanceof AST_Const ? def.redefined() : def.const_redefs))
-                                && !(def.id in in_use_ids)) {
+                            if (!(def.id in in_use_ids) && (!drop_vars
+                                || (node instanceof AST_Const ? def.redefined() : def.const_redefs)
+                                || !(node instanceof AST_Var || is_safe_lexical(def)))) {
                                 in_use_ids[def.id] = true;
                                 in_use.push(def);
                             }
@@ -5128,7 +5129,7 @@ merge(Compressor.prototype, {
         var tt = new TreeTransformer(function(node, descend, in_list) {
             var parent = tt.parent();
             if (drop_vars) {
-                var props = [], sym = assign_as_unused(tt, node, props);
+                var props = [], sym = assign_as_unused(node, props);
                 if (sym) {
                     var def = sym.definition();
                     var in_use = def.id in in_use_ids;
@@ -5245,6 +5246,7 @@ merge(Compressor.prototype, {
                 // into the next one, or next statement.
                 var side_effects = [];
                 var duplicated = 0;
+                var is_var = node instanceof AST_Var;
                 node.definitions.forEach(function(def) {
                     if (def.value) def.value = def.value.transform(tt);
                     if (def.name instanceof AST_Destructured) {
@@ -5313,7 +5315,11 @@ merge(Compressor.prototype, {
                                 return node;
                             }
                             if (node instanceof AST_SymbolDeclaration) {
-                                return !drop_vars || node.definition().id in in_use_ids || is_catch(node) ? node : null;
+                                if (!drop_vars) return node;
+                                if (node.definition().id in in_use_ids) return node;
+                                if (is_catch(node)) return node;
+                                if (is_var && !can_drop_symbol(node)) return node;
+                                return null;
                             }
                         });
                         var name = def.name.transform(trimmer);
@@ -5326,7 +5332,8 @@ merge(Compressor.prototype, {
                         return;
                     }
                     var sym = def.name.definition();
-                    if (!drop_vars || sym.id in in_use_ids) {
+                    var drop_sym = !is_var || can_drop_symbol(def.name);
+                    if (!drop_sym || !drop_vars || sym.id in in_use_ids) {
                         if (def.value && indexOf_assign(sym, def) < 0) {
                             var write_only = def.value.write_only;
                             var value = def.value.drop_side_effect_free(compressor);
@@ -5343,7 +5350,7 @@ merge(Compressor.prototype, {
                         }
                         var old_def, var_defs = var_defs_by_id.get(sym.id);
                         if (!def.value && !(node instanceof AST_Let)) {
-                            if (var_defs.length > 1) {
+                            if (drop_sym && var_defs.length > 1) {
                                 AST_Node.info("Dropping declaration of variable {name} [{file}:{line},{col}]", template(def.name));
                                 remove(var_defs, def);
                                 sym.eliminated++;
@@ -5376,7 +5383,7 @@ merge(Compressor.prototype, {
                             });
                             body.push(defun);
                         } else {
-                            if (var_defs.length > 1 && sym.orig.indexOf(def.name) > sym.eliminated) {
+                            if (drop_sym && var_defs.length > 1 && sym.orig.indexOf(def.name) > sym.eliminated) {
                                 remove(var_defs, def);
                                 duplicated++;
                             }
@@ -5661,7 +5668,7 @@ merge(Compressor.prototype, {
                 var def = node.expression.definition();
                 if (def.scope === self) assignments.add(def.id, node);
             }
-            var node_def, props = [], sym = assign_as_unused(tw, node, props);
+            var node_def, props = [], sym = assign_as_unused(node, props);
             if (sym && self.variables.get(sym.name) === (node_def = sym.definition())) {
                 props.forEach(function(prop) {
                     prop.walk(tw);
@@ -6079,9 +6086,7 @@ merge(Compressor.prototype, {
                 && all(def.references, function(ref) {
                     return ref.fixed_value() === right;
                 })
-                && all(def.orig, function(sym) {
-                    return !(sym instanceof AST_SymbolConst || sym instanceof AST_SymbolLet);
-                });
+                && can_drop_symbol(sym);
         }
     });
 
@@ -6299,7 +6304,7 @@ merge(Compressor.prototype, {
             return make_sequence(this, [ expression, property ]);
         });
         def(AST_SymbolRef, function(compressor) {
-            return this.is_declared(compressor) && can_drop_symbol(compressor, this) ? null : this;
+            return this.is_declared(compressor) && can_drop_symbol(this) ? null : this;
         });
         def(AST_This, return_null);
         def(AST_Unary, function(compressor, first_in_statement) {
@@ -6308,9 +6313,7 @@ merge(Compressor.prototype, {
                 this.write_only = !exp.has_side_effects(compressor);
                 return this;
             }
-            if (this.operator == "typeof" && exp instanceof AST_SymbolRef && can_drop_symbol(compressor, exp)) {
-                return null;
-            }
+            if (this.operator == "typeof" && exp instanceof AST_SymbolRef && can_drop_symbol(exp)) return null;
             var node = exp.drop_side_effect_free(compressor, first_in_statement);
             if (first_in_statement && node && is_iife_call(node)) {
                 if (node === exp && this.operator == "!") return this;
@@ -7080,6 +7083,10 @@ merge(Compressor.prototype, {
         return make_sequence(this, assignments);
     });
 
+    function is_safe_lexical(def) {
+        return def.orig.length < (def.orig[0] instanceof AST_SymbolLambda ? 3 : 2);
+    }
+
     function may_overlap(compressor, def) {
         if (compressor.exposed(def)) return true;
         var scope = def.scope.resolve();
@@ -7109,7 +7116,8 @@ merge(Compressor.prototype, {
         return compressor.option("varify") && all(self.definitions, function(defn) {
             return !defn.name.match_symbol(function(node) {
                 if (node instanceof AST_SymbolDeclaration) {
-                    return !node.fixed_value() || may_overlap(compressor, node.definition());
+                    var def = node.definition();
+                    return !node.fixed_value() || !is_safe_lexical(def) || may_overlap(compressor, def);
                 }
             }, true);
         }) ? to_var(self) : self;
@@ -8015,12 +8023,9 @@ merge(Compressor.prototype, {
                 // typeof always returns a non-empty string, thus it's
                 // always true in booleans
                 AST_Node.warn("Boolean expression always true [{file}:{line},{col}]", self.start);
-                return (exp instanceof AST_SymbolRef && all(exp.definition().orig, function(sym) {
-                    return !(sym instanceof AST_SymbolConst || sym instanceof AST_SymbolLet);
-                }) ? make_node(AST_True, self) : make_sequence(self, [
-                    exp,
-                    make_node(AST_True, self)
-                ])).optimize(compressor);
+                var exprs = [ make_node(AST_True, self) ];
+                if (!(exp instanceof AST_SymbolRef && can_drop_symbol(exp))) exprs.unshift(exp);
+                return make_sequence(self, exprs).optimize(compressor);
             }
         }
         if (op == "-" && exp instanceof AST_Infinity) exp = exp.transform(compressor);
diff --git a/test/compress/const.js b/test/compress/const.js
index 333171b4..0f5e3fdd 100644
--- a/test/compress/const.js
+++ b/test/compress/const.js
@@ -1225,3 +1225,18 @@ issue_4274_2: {
     }
     expect_stdout: true
 }
+
+issue_4290_1: {
+    options = {
+        unused: true,
+    }
+    input: {
+        const a = 0;
+        var a;
+    }
+    expect: {
+        const a = 0;
+        var a;
+    }
+    expect_stdout: true
+}
diff --git a/test/compress/let.js b/test/compress/let.js
index 20bec4d3..0476c431 100644
--- a/test/compress/let.js
+++ b/test/compress/let.js
@@ -1064,3 +1064,53 @@ issue_4276_2: {
     expect_stdout: "PASS"
     node_version: ">=4"
 }
+
+issue_4290_1: {
+    options = {
+        unused: true,
+    }
+    input: {
+        "use strict";
+        let a;
+        var a;
+    }
+    expect: {
+        "use strict";
+        let a;
+        var a;
+    }
+    expect_stdout: true
+    node_version: ">=4"
+}
+
+issue_4290_2: {
+    options = {
+        collapse_vars: true,
+    }
+    input: {
+        "use strict";
+        try {
+            console.log(function(a) {
+                a = c;
+                let c;
+                return a;
+            }());
+        } catch (e) {
+            console.log("PASS");
+        }
+    }
+    expect: {
+        "use strict";
+        try {
+            console.log(function(a) {
+                a = c;
+                let c;
+                return a;
+            }());
+        } catch (e) {
+            console.log("PASS");
+        }
+    }
+    expect_stdout: "PASS"
+    node_version: ">=4"
+}
diff --git a/test/compress/varify.js b/test/compress/varify.js
index d4894d2d..95acd032 100644
--- a/test/compress/varify.js
+++ b/test/compress/varify.js
@@ -353,3 +353,40 @@ forin_let_2: {
     ]
     node_version: ">=6"
 }
+
+issue_4290_1_const: {
+    options = {
+        reduce_vars: true,
+        toplevel: true,
+        varify: true,
+    }
+    input: {
+        const a = 0;
+        var a;
+    }
+    expect: {
+        const a = 0;
+        var a;
+    }
+    expect_stdout: true
+}
+
+issue_4290_1_let: {
+    options = {
+        reduce_vars: true,
+        toplevel: true,
+        varify: true,
+    }
+    input: {
+        "use strict";
+        let a = 0;
+        var a;
+    }
+    expect: {
+        "use strict";
+        let a = 0;
+        var a;
+    }
+    expect_stdout: true
+    node_version: ">=4"
+}