From 3fa20866813f634d97d0958208b0bbf16977ae4d Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Tue, 27 Sep 2022 06:52:58 +0100 Subject: [PATCH] improve usability of `mangle.properties` (#5683) fixes #5682 --- lib/propmangle.js | 38 ++++---- test/compress/classes.js | 58 +++++++++++- test/compress/issue-1770.js | 2 +- test/compress/issue-747.js | 4 +- test/compress/properties.js | 170 ++++++++++++++++++++++++++++++++++-- test/mocha/let.js | 19 ++-- test/mocha/minify.js | 10 +-- 7 files changed, 260 insertions(+), 41 deletions(-) diff --git a/lib/propmangle.js b/lib/propmangle.js index 33f6dfa2a6b..960b9992c17 100644 --- a/lib/propmangle.js +++ b/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.start && node.start.quote) add(node.key); + if (node instanceof AST_ClassProperty || node instanceof AST_ObjectProperty) { + 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.operator == "in") addStrings(node.left, add); - } else if (node.TYPE == "Call") { + 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) { - if (typeof node.key == "string") add(node.key); + } else if (node instanceof AST_ClassProperty || node instanceof AST_ObjectProperty) { + if (node.key instanceof AST_Node) { + addStrings(node.key, add); + } else { + add(node.key); + } } else if (node instanceof AST_Dot) { - add(node.property); - } else if (node instanceof AST_ObjectProperty) { - if (typeof node.key == "string") add(node.key); + if (is_lhs(node, this.parent())) add(node.property); } 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) { - if (typeof node.key == "string") node.key = mangle(node.key); + } else if (node instanceof AST_ClassProperty || node instanceof AST_ObjectProperty) { + 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); } diff --git a/test/compress/classes.js b/test/compress/classes.js index f2e6e1860b6..4f418ab09f0 100644 --- a/test/compress/classes.js +++ b/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" +} diff --git a/test/compress/issue-1770.js b/test/compress/issue-1770.js index ad8b22a2603..cf17be5a826 100644 --- a/test/compress/issue-1770.js +++ b/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", ] } diff --git a/test/compress/issue-747.js b/test/compress/issue-747.js index 8d5289bba00..b61557edeb1 100644 --- a/test/compress/issue-747.js +++ b/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", ] } diff --git a/test/compress/properties.js b/test/compress/properties.js index 587fb8de293..9a5ed8ea1a1 100644 --- a/test/compress/properties.js +++ b/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['r']({b: "blue", a: "baz"}); + a.run(x.o, a.a); + 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" +} diff --git a/test/mocha/let.js b/test/mocha/let.js index 1efa6fb4a9d..8755208b8b3 100644 --- a/test/mocha/let.js +++ b/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.indexOf('v["' + name + '"]'), -1); + assert.notStrictEqual(result.code.indexOf('v["' + name + '"]'), -1); }); }); it("Should parse `let` as name correctly", function() { diff --git a/test/mocha/minify.js b/test/mocha/minify.js index 4340ede6738..2e0eb3355f6 100644 --- a/test/mocha/minify.js +++ b/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;"); }); });