fix corner cases in collapse_vars & unused (#4807)

fixes #4806
mishoo · Mar 20, 2021 · 24619da · 24619da
1 parent b89cc84
commit 24619da
Show file tree

Hide file tree

Showing 3 changed files with 121 additions and 13 deletions.
diff --git a/lib/compress.js b/lib/compress.js
@@ -2701,8 +2701,8 @@ merge(Compressor.prototype, {
 
             function patch_sequence(node) {
                 if (node instanceof AST_Sequence) switch (node.expressions.length) {
-                    case 0: return null;
-                    case 1: return node.expressions[0];
+                  case 0: return null;
+                  case 1: return maintain_this_binding(compressor, this.parent(), node, node.expressions[0]);
                 }
             }
 
@@ -6386,14 +6386,13 @@ merge(Compressor.prototype, {
                 scope = save_scope;
             }
         }, function(node, in_list) {
-            if (node instanceof AST_BlockStatement) {
-                return trim_block(node, tt.parent(), in_list);
-            } else if (node instanceof AST_For) {
-                // Certain combination of unused name + side effect leads to invalid AST:
-                //    https://github.com/mishoo/UglifyJS/issues/44
-                //    https://github.com/mishoo/UglifyJS/issues/1838
-                //    https://github.com/mishoo/UglifyJS/issues/3371
-                // We fix it at this stage by moving the `var` outside the `for`.
+            if (node instanceof AST_BlockStatement) return trim_block(node, tt.parent(), in_list);
+            // Certain combination of unused name + side effect leads to invalid AST:
+            //    https://github.com/mishoo/UglifyJS/issues/44
+            //    https://github.com/mishoo/UglifyJS/issues/1838
+            //    https://github.com/mishoo/UglifyJS/issues/3371
+            // We fix it at this stage by moving the `var` outside the `for`.
+            if (node instanceof AST_For) {
                 var block;
                 if (node.init instanceof AST_BlockStatement) {
                     block = node.init;
@@ -6414,7 +6413,8 @@ merge(Compressor.prototype, {
                     node.init = null;
                 }
                 return !block ? node : in_list ? List.splice(block.body) : block;
-            } else if (node instanceof AST_ForIn) {
+            }
+            if (node instanceof AST_ForIn) {
                 if (!drop_vars || !compressor.option("loops")) return;
                 if (!is_empty(node.body)) return;
                 var sym = get_init_symbol(node);
@@ -6435,8 +6435,10 @@ merge(Compressor.prototype, {
                     body.push(node.init);
                 }
                 return insert_statements(body, node, in_list);
-            } else if (node instanceof AST_Sequence) {
-                if (node.expressions.length == 1) return node.expressions[0];
+            }
+            if (node instanceof AST_Sequence) {
+                if (node.expressions.length > 1) return;
+                return maintain_this_binding(compressor, tt.parent(), node, node.expressions[0]);
             }
         });
         tt.push(compressor.parent());

diff --git a/test/compress/collapse_vars.js b/test/compress/collapse_vars.js
@@ -8857,3 +8857,26 @@ dot_non_local: {
     }
     expect_stdout: "42"
 }
+
+issue_4806: {
+    options = {
+        collapse_vars: true,
+    }
+    input: {
+        var a, o = {
+            f: function() {
+                console.log(this === o ? "FAIL" : "PASS");
+            },
+        };
+        (a = 42, o.f)(42);
+    }
+    expect: {
+        var a, o = {
+            f: function() {
+                console.log(this === o ? "FAIL" : "PASS");
+            },
+        };
+        (0, o.f)(a = 42);
+    }
+    expect_stdout: "PASS"
+}
diff --git a/test/compress/drop-unused.js b/test/compress/drop-unused.js
@@ -3274,3 +3274,86 @@ issue_4662: {
     }
     expect_stdout: "1 1"
 }
+
+issue_4806_1: {
+    options = {
+        evaluate: true,
+        reduce_vars: true,
+        toplevel: true,
+        unused: true,
+    }
+    input: {
+        O = {
+            f: function() {
+                console.log(this === O ? "FAIL" : "PASS");
+            },
+        };
+        var a;
+        (a = 42, O.f)();
+        a;
+    }
+    expect: {
+        O = {
+            f: function() {
+                console.log(this === O ? "FAIL" : "PASS");
+            },
+        };
+        (0, O.f)();
+        42;
+    }
+    expect_stdout: "PASS"
+}
+
+issue_4806_2: {
+    options = {
+        sequences: true,
+        toplevel: true,
+        unused: true,
+    }
+    input: {
+        O = {
+            f: function() {
+                console.log(this === O ? "FAIL" : "PASS");
+            },
+        };
+        var a;
+        (a = 42, O.f)();
+        a;
+    }
+    expect: {
+        O = {
+            f: function() {
+                console.log(this === O ? "FAIL" : "PASS");
+            },
+        },
+        (0, O.f)();
+    }
+    expect_stdout: "PASS"
+}
+
+issue_4806_3: {
+    options = {
+        side_effects: true,
+        toplevel: true,
+        unused: true,
+    }
+    input: {
+        O = {
+            f: function() {
+                console.log(this === O ? "FAIL" : "PASS");
+            },
+        };
+        var a;
+        (a = 42, O.f)();
+        a;
+    }
+    expect: {
+        O = {
+            f: function() {
+                console.log(this === O ? "FAIL" : "PASS");
+            },
+        };
+        (0, O.f)();
+    }
+    expect_stdout: "PASS"
+}