improve usability of mangle.properties (#5683)

fixes #5682

lib/propmangle.js

@@ -124,12 +124,14 @@ function get_builtins() {
 
 function reserve_quoted_keys(ast, reserved) {
     ast.walk(new TreeWalker(function(node) {
-        if (node instanceof AST_ClassProperty) {
+        if (node instanceof AST_ClassProperty || node instanceof AST_ObjectProperty) {
-            if (node.start && node.start.quote) add(node.key);
+            if (node.key instanceof AST_Node) {
+                addStrings(node.key, add);
+            } else if (node.start && node.start.quote) {
+                add(node.key);
+            }
         } else if (node instanceof AST_Dot) {
             if (node.quoted) add(node.property);
-        } else if (node instanceof AST_ObjectProperty) {
-            if (node.start && node.start.quote) add(node.key);
         } else if (node instanceof AST_Sub) {
             addStrings(node.property, add);
         }
@@ -191,9 +193,7 @@ function mangle_properties(ast, options) {
 
     // step 1: find candidates to mangle
     ast.walk(new TreeWalker(function(node) {
-        if (node instanceof AST_Binary) {
+        if (node.TYPE == "Call") {
-            if (node.operator == "in") addStrings(node.left, add);
-        } else if (node.TYPE == "Call") {
             var exp = node.expression;
             if (exp instanceof AST_Dot) switch (exp.property) {
               case "defineProperty":
@@ -210,14 +210,16 @@ function mangle_properties(ast, options) {
                 addStrings(node.args[0], add);
                 break;
             }
-        } else if (node instanceof AST_ClassProperty) {
+        } else if (node instanceof AST_ClassProperty || node instanceof AST_ObjectProperty) {
-            if (typeof node.key == "string") add(node.key);
+            if (node.key instanceof AST_Node) {
+                addStrings(node.key, add);
+            } else {
+                add(node.key);
+            }
         } else if (node instanceof AST_Dot) {
-            add(node.property);
+            if (is_lhs(node, this.parent())) add(node.property);
-        } else if (node instanceof AST_ObjectProperty) {
-            if (typeof node.key == "string") add(node.key);
         } else if (node instanceof AST_Sub) {
-            addStrings(node.property, add);
+            if (is_lhs(node, this.parent())) addStrings(node.property, add);
         }
     }));
 
@@ -242,12 +244,14 @@ function mangle_properties(ast, options) {
                 mangleStrings(node.args[0]);
                 break;
             }
-        } else if (node instanceof AST_ClassProperty) {
+        } else if (node instanceof AST_ClassProperty || node instanceof AST_ObjectProperty) {
-            if (typeof node.key == "string") node.key = mangle(node.key);
+            if (node.key instanceof AST_Node) {
+                mangleStrings(node.key);
+            } else {
+                node.key = mangle(node.key);
+            }
         } else if (node instanceof AST_Dot) {
             node.property = mangle(node.property);
-        } else if (node instanceof AST_ObjectProperty) {
-            if (typeof node.key == "string") node.key = mangle(node.key);
         } else if (node instanceof AST_Sub) {
             if (!options.keep_quoted) mangleStrings(node.property);
         }

test/compress/classes.js

@@ -2202,11 +2202,11 @@ mangle_properties: {
     expect_stdout: "PASS 42"
     expect_warnings: [
         "INFO: Preserving reserved property q",
-        "INFO: Preserving reserved property log",
         "INFO: Mapping property #P to #t",
         "INFO: Mapping property Q to s",
         "INFO: Mapping property #p to #i",
         "INFO: Mapping property r to e",
+        "INFO: Preserving reserved property log",
     ]
     node_version: ">=14.6"
 }
@@ -3619,3 +3619,59 @@ issue_5662: {
     expect_stdout: "PASS"
     node_version: ">=6"
 }
+
+issue_5682_class_key: {
+    mangle = {
+        properties: true,
+    }
+    input: {
+        "use strict";
+        function f(a) {
+            return "foo" in a;
+        }
+        class A {
+            foo() {}
+        }
+        console.log(f(new A()) ? "PASS" : "FAIL");
+    }
+    expect: {
+        "use strict";
+        function f(o) {
+            return "o" in o;
+        }
+        class A {
+            o() {}
+        }
+        console.log(f(new A()) ? "PASS" : "FAIL");
+    }
+    expect_stdout: "PASS"
+    node_version: ">=4"
+}
+
+issue_5682_class_key_computed: {
+    mangle = {
+        properties: true,
+    }
+    input: {
+        "use strict";
+        function f(a) {
+            return "foo" in a;
+        }
+        class A {
+            ["foo"]() {}
+        }
+        console.log(f(new A()) ? "PASS" : "FAIL");
+    }
+    expect: {
+        "use strict";
+        function f(o) {
+            return "o" in o;
+        }
+        class A {
+            ["o"]() {}
+        }
+        console.log(f(new A()) ? "PASS" : "FAIL");
+    }
+    expect_stdout: "PASS"
+    node_version: ">=4"
+}

test/compress/issue-1770.js

@@ -115,9 +115,9 @@ numeric_literal: {
         "8 7 8",
     ]
     expect_warnings: [
-        "INFO: Preserving reserved property log",
         "INFO: Mapping property 0x25 to o",
         "INFO: Mapping property 1E42 to b",
+        "INFO: Preserving reserved property log",
     ]
 }
 

test/compress/issue-747.js

@@ -21,8 +21,8 @@ dont_reuse_prop: {
     expect_stdout: "123"
     expect_warnings: [
         "INFO: Preserving excluded property a",
-        "INFO: Preserving reserved property log",
         "INFO: Mapping property asd to b",
+        "INFO: Preserving reserved property log",
     ]
 }
 
@@ -49,7 +49,7 @@ unmangleable_props_should_always_be_reserved: {
     expect_stdout: "123"
     expect_warnings: [
         "INFO: Preserving excluded property a",
-        "INFO: Preserving reserved property log",
         "INFO: Mapping property asd to b",
+        "INFO: Preserving reserved property log",
     ]
 }

test/compress/properties.js

@@ -147,8 +147,8 @@ mangle_properties_1: {
         a["a"] = "bar";
         a.b = "red";
         x = {o: 10};
-        a.r(x.o, a.a);
+        a.run(x.o, a.a);
-        a['r']({b: "blue", a: "baz"});
+        a['run']({b: "blue", a: "baz"});
     }
 }
 
@@ -962,14 +962,14 @@ issue_2256: {
     }
     input: {
         ({ "keep": 42 });
-        global.keep = global.change;
+        global.keep = global.change = "PASS";
         console.log(keep);
     }
     expect: {
-        global.keep = global.l;
+        global.keep = global.l = "PASS";
         console.log(keep);
     }
-    expect_stdout: "undefined"
+    expect_stdout: "PASS"
 }
 
 lhs_prop_1: {
@@ -1645,3 +1645,163 @@ issue_5177: {
     expect_stdout: "PASS"
     node_version: ">=4"
 }
+
+issue_5682_in_1: {
+    mangle = {
+        properties: true,
+    }
+    input: {
+        function f(a) {
+            return "foo" in a;
+        }
+        var o = {};
+        var p = "foo";
+        o[p] = 42;
+        console.log(f(o) ? "PASS" : "FAIL");
+    }
+    expect: {
+        function f(o) {
+            return "foo" in o;
+        }
+        var o = {};
+        var p = "foo";
+        o[p] = 42;
+        console.log(f(o) ? "PASS" : "FAIL");
+    }
+    expect_stdout: "PASS"
+}
+
+issue_5682_in_2: {
+    mangle = {
+        properties: true,
+    }
+    input: {
+        function f(a) {
+            return "foo" in a;
+        }
+        var o = { foo: 42 };
+        console.log(f(o) ? "PASS" : "FAIL");
+    }
+    expect: {
+        function f(o) {
+            return "o" in o;
+        }
+        var o = { o: 42 };
+        console.log(f(o) ? "PASS" : "FAIL");
+    }
+    expect_stdout: "PASS"
+}
+
+issue_5682_dot_1: {
+    mangle = {
+        properties: true,
+    }
+    input: {
+        function f(a) {
+            return a.foo;
+        }
+        var o = {};
+        var p = "foo";
+        o[p] = "PASS";
+        console.log(f(o));
+    }
+    expect: {
+        function f(o) {
+            return o.foo;
+        }
+        var o = {};
+        var p = "foo";
+        o[p] = "PASS";
+        console.log(f(o));
+    }
+    expect_stdout: "PASS"
+}
+
+issue_5682_dot_2: {
+    mangle = {
+        properties: true,
+    }
+    input: {
+        function f(a) {
+            return a.foo;
+        }
+        var o = { foo: "PASS" };
+        console.log(f(o));
+    }
+    expect: {
+        function f(o) {
+            return o.o;
+        }
+        var o = { o: "PASS" };
+        console.log(f(o));
+    }
+    expect_stdout: "PASS"
+}
+
+issue_5682_dot_2_computed: {
+    mangle = {
+        properties: true,
+    }
+    input: {
+        function f(a) {
+            return a.foo;
+        }
+        var o = { ["foo"]: "PASS" };
+        console.log(f(o));
+    }
+    expect: {
+        function f(o) {
+            return o.o;
+        }
+        var o = { ["o"]: "PASS" };
+        console.log(f(o));
+    }
+    expect_stdout: "PASS"
+    node_version: ">=4"
+}
+
+issue_5682_sub_1: {
+    mangle = {
+        properties: true,
+    }
+    input: {
+        function f(a) {
+            return a["foo"];
+        }
+        var o = {};
+        var p = "foo";
+        o[p] = "PASS";
+        console.log(f(o));
+    }
+    expect: {
+        function f(o) {
+            return o["foo"];
+        }
+        var o = {};
+        var p = "foo";
+        o[p] = "PASS";
+        console.log(f(o));
+    }
+    expect_stdout: "PASS"
+}
+
+issue_5682_sub_2: {
+    mangle = {
+        properties: true,
+    }
+    input: {
+        function f(a) {
+            return a["foo"];
+        }
+        var o = { foo: "PASS" };
+        console.log(f(o));
+    }
+    expect: {
+        function f(o) {
+            return o["o"];
+        }
+        var o = { o: "PASS" };
+        console.log(f(o));
+    }
+    expect_stdout: "PASS"
+}

test/mocha/let.js

@@ -12,17 +12,16 @@ describe("let", function() {
         s += '}';
         var result = UglifyJS.minify(s, {
             compress: false,
-        }).code;
+        });
-
+        if (result.error) throw result.error;
         // Verify that select keywords and reserved keywords not produced
         [
             "do",
             "let",
             "var",
         ].forEach(function(name) {
-            assert.strictEqual(result.indexOf("var " + name + "="), -1);
+            assert.strictEqual(result.code.indexOf("var " + name + "="), -1);
         });
-
         // Verify that the variable names that appeared immediately before
         // and after the erroneously generated variable name still exist
         // to show the test generated enough symbols.
@@ -31,27 +30,27 @@ describe("let", function() {
             "eet", "fet",
             "rar", "oar",
         ].forEach(function(name) {
-            assert.notStrictEqual(result.indexOf("var " + name + "="), -1);
+            assert.notStrictEqual(result.code.indexOf("var " + name + "="), -1);
         });
     });
     it("Should quote mangled properties that are reserved keywords", function() {
         var s = '"rrrrrnnnnniiiiiaaaaa";';
         for (var i = 0; i < 18000; i++) {
-            s += "v.b" + i + ";";
+            s += "v.b" + i + "=v;";
         }
         var result = UglifyJS.minify(s, {
             compress: false,
             ie: true,
             mangle: {
                 properties: true,
-            }
+            },
-        }).code;
+        });
+        if (result.error) throw result.error;
         [
             "in",
             "var",
         ].forEach(function(name) {
-            assert.notStrictEqual(result.indexOf(name), -1);
+            assert.notStrictEqual(result.code.indexOf('v["' + name + '"]'), -1);
-            assert.notStrictEqual(result.indexOf('v["' + name + '"]'), -1);
         });
     });
     it("Should parse `let` as name correctly", function() {

test/mocha/minify.js

@@ -205,15 +205,15 @@ describe("minify", function() {
                     'a["foo"]="bar",a.a="red",x={"bar":10};');
         });
         it("Should not mangle quoted property within dead code", function() {
-            var result = UglifyJS.minify('({ "keep": 1 }); g.keep = g.change;', {
+            var result = UglifyJS.minify('({ "keep": 1 }); g.keep = g.change = 42;', {
                 mangle: {
                     properties: {
-                        keep_quoted: true
+                        keep_quoted: true,
-                    }
+                    },
-                }
+                },
             });
             if (result.error) throw result.error;
-            assert.strictEqual(result.code, "g.keep=g.g;");
+            assert.strictEqual(result.code, "g.keep=g.g=42;");
         });
     });