From e6312f42eacdc4cf05f8f86d680a66de041eda38 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Sat, 4 Sep 2021 21:06:28 -0400 Subject: [PATCH] Fix private getter/setter mangling (#1060) * Mangle private properties by default when manglin * Fix sourcemap test for private fields * Fix property mangling private getter/setter * tmp * Improve sourcemap output for private properties * remove test grep * Don't print regular string literals --- lib/ast.js | 2 +- lib/minify.js | 2 ++ lib/output.js | 13 ++++--- lib/propmangle.js | 60 +++++++++++++++++++++---------- test/compress.js | 2 ++ test/compress/class-properties.js | 10 ++++-- test/mocha/sourcemaps.js | 30 +++++++++++++--- 7 files changed, 88 insertions(+), 31 deletions(-) diff --git a/lib/ast.js b/lib/ast.js index 3aca3dd77..c0a50d437 100644 --- a/lib/ast.js +++ b/lib/ast.js @@ -1330,7 +1330,7 @@ var AST_ClassProperty = DEFNODE("ClassProperty", "static quote", { } }, AST_ObjectProperty); -var AST_ClassPrivateProperty = DEFNODE("ClassProperty", "", { +var AST_ClassPrivateProperty = DEFNODE("ClassPrivateProperty", "", { $documentation: "A class property for a private property", }, AST_ClassProperty); diff --git a/lib/minify.js b/lib/minify.js index 8ba1848a6..d3c501fee 100644 --- a/lib/minify.js +++ b/lib/minify.js @@ -14,6 +14,7 @@ import { Compressor } from "./compress/index.js"; import { SourceMap } from "./sourcemap.js"; import { mangle_properties, + mangle_private_properties, reserve_quoted_keys, } from "./propmangle.js"; @@ -204,6 +205,7 @@ async function minify(files, options) { if (options.mangle) { toplevel.compute_char_frequency(options.mangle); toplevel.mangle_names(options.mangle); + toplevel = mangle_private_properties(toplevel, options.mangle); } if (timings) timings.properties = Date.now(); if (options.mangle && options.mangle.properties) { diff --git a/lib/output.js b/lib/output.js index 382810b77..c85b61b46 100644 --- a/lib/output.js +++ b/lib/output.js @@ -349,9 +349,11 @@ function OutputStream(options) { var do_add_mapping = mappings ? function() { mappings.forEach(function(mapping) { try { - let name = !mapping.name && mapping.token.type == "name" ? mapping.token.value : mapping.name; - if (name instanceof AST_Symbol) { - name = name.name; + let { name, token } = mapping; + if (token.type == "name" || token.type === "privatename") { + name = token.value; + } else if (name instanceof AST_Symbol) { + name = token.type === "string" ? token.value : name.name; } options.source_map.add( mapping.token.file, @@ -1824,6 +1826,7 @@ function OutputStream(options) { if (self.optional) output.print("?"); output.print(".#"); + output.add_mapping(self.end); output.print_name(prop); }); DEFPRINT(AST_Sub, function(self, output) { @@ -2277,8 +2280,10 @@ function OutputStream(options) { DEFMAP([ AST_ObjectGetter, AST_ObjectSetter, + AST_PrivateGetter, + AST_PrivateSetter, ], function(output) { - output.add_mapping(this.start, this.key.name); + output.add_mapping(this.key.end, this.key.name); }); DEFMAP([ AST_ObjectProperty ], function(output) { diff --git a/lib/propmangle.js b/lib/propmangle.js index 374c045ab..6f1a55a47 100644 --- a/lib/propmangle.js +++ b/lib/propmangle.js @@ -59,6 +59,8 @@ import { AST_ObjectKeyVal, AST_ObjectProperty, AST_PrivateMethod, + AST_PrivateGetter, + AST_PrivateSetter, AST_Sequence, AST_String, AST_Sub, @@ -140,6 +142,36 @@ function addStrings(node, add) { })); } +function mangle_private_properties(ast, options) { + var cprivate = -1; + var private_cache = new Map(); + var nth_identifier = options.nth_identifier || base54; + + ast = ast.transform(new TreeTransformer(function(node) { + if ( + node instanceof AST_ClassPrivateProperty + || node instanceof AST_PrivateMethod + || node instanceof AST_PrivateGetter + || node instanceof AST_PrivateSetter + ) { + node.key.name = mangle_private(node.key.name); + } else if (node instanceof AST_DotHash) { + node.property = mangle_private(node.property); + } + })); + return ast; + + function mangle_private(name) { + let mangled = private_cache.get(name); + if (!mangled) { + mangled = nth_identifier.get(++cprivate); + private_cache.set(name, mangled); + } + + return mangled; + } +} + function mangle_properties(ast, options) { options = defaults(options, { builtins: false, @@ -161,10 +193,8 @@ function mangle_properties(ast, options) { if (!options.builtins) find_builtins(reserved); var cname = -1; - var cprivate = -1; var cache; - var private_cache = new Map(); if (options.cache) { cache = options.cache.props; cache.forEach(function(mangled_name) { @@ -187,7 +217,6 @@ function mangle_properties(ast, options) { var names_to_mangle = new Set(); var unmangleable = new Set(); - var private_properties = new Set(); var keep_quoted_strict = options.keep_quoted === "strict"; @@ -196,10 +225,11 @@ function mangle_properties(ast, options) { if ( node instanceof AST_ClassPrivateProperty || node instanceof AST_PrivateMethod + || node instanceof AST_PrivateGetter + || node instanceof AST_PrivateSetter + || node instanceof AST_DotHash ) { - private_properties.add(node.key.name); - } else if (node instanceof AST_DotHash) { - private_properties.add(node.property); + // handled by mangle_private_properties } else if (node instanceof AST_ObjectKeyVal) { if (typeof node.key == "string" && (!keep_quoted_strict || !node.quote)) { @@ -240,10 +270,11 @@ function mangle_properties(ast, options) { if ( node instanceof AST_ClassPrivateProperty || node instanceof AST_PrivateMethod + || node instanceof AST_PrivateGetter + || node instanceof AST_PrivateSetter + || node instanceof AST_DotHash ) { - node.key.name = mangle_private(node.key.name); - } else if (node instanceof AST_DotHash) { - node.property = mangle_private(node.property); + // handled by mangle_private_properties } else if (node instanceof AST_ObjectKeyVal) { if (typeof node.key == "string" && (!keep_quoted_strict || !node.quote)) { @@ -324,16 +355,6 @@ function mangle_properties(ast, options) { return mangled; } - function mangle_private(name) { - let mangled = private_cache.get(name); - if (!mangled) { - mangled = nth_identifier.get(++cprivate); - private_cache.set(name, mangled); - } - - return mangled; - } - function mangleStrings(node) { return node.transform(new TreeTransformer(function(node) { if (node instanceof AST_Sequence) { @@ -353,4 +374,5 @@ function mangle_properties(ast, options) { export { reserve_quoted_keys, mangle_properties, + mangle_private_properties, }; diff --git a/test/compress.js b/test/compress.js index f2bf315f3..8c17c219f 100644 --- a/test/compress.js +++ b/test/compress.js @@ -14,6 +14,7 @@ import { Compressor } from "../lib/compress/index.js"; import { reserve_quoted_keys, mangle_properties, + mangle_private_properties, } from "../lib/propmangle.js"; import { base54 } from "../lib/scope.js"; import { string_template, defaults } from "../lib/utils/index.js"; @@ -227,6 +228,7 @@ async function run_compress_tests() { } })(test.mangle.cache); output.mangle_names(test.mangle); + mangle_private_properties(output, test.mangle); if (test.mangle.properties) { output = mangle_properties(output, test.mangle.properties); } diff --git a/test/compress/class-properties.js b/test/compress/class-properties.js index 5393690d3..290aef881 100644 --- a/test/compress/class-properties.js +++ b/test/compress/class-properties.js @@ -364,8 +364,10 @@ private_properties_can_be_mangled: { #bbbbbb() { return "SS" } + get #cccccc() {} + set #dddddd(v) {} log() { - console.log(this.aaaaaa + this.#aaaaaa + this.#bbbbbb()) + console.log(this.aaaaaa + this.#aaaaaa + this.#bbbbbb() + this.#cccccc + this.#dddddd) } } @@ -375,11 +377,13 @@ private_properties_can_be_mangled: { class X { t = "P" #a = "A" - #b() { + #s() { return "SS" } + get #c() {} + set #t(a) {} log() { - console.log(this.t + this.#a + this.#b()) + console.log(this.t + this.#a + this.#s() + this.#c + this.#t) } } diff --git a/test/mocha/sourcemaps.js b/test/mocha/sourcemaps.js index c1035c1d7..cb8f31215 100644 --- a/test/mocha/sourcemaps.js +++ b/test/mocha/sourcemaps.js @@ -137,23 +137,45 @@ describe("sourcemaps", function() { it("Should grab names from methods and properties correctly", async () => { const code = `class Foo { property = 6 - #private = 4 method () {} 404() {} "quoted method name" () {} get getter(){} set setter(){} + #private = 4 + #private_method() {} + get #private_getter() {} + set #private_setter() {} + + test() { + this.property; + this.method; + this[404]; + this["quoted method name"]; + this.getter; + this.setter; + this.#private; + this.#private_method; + this.#private_getter; + this.#private_setter; + } }`; const result = await minify(code, { - sourceMap: {asObject: true}, + sourceMap: { asObject: true }, + mangle: { properties: true }, }); assert.deepStrictEqual(result.map.names, [ "Foo", "property", - "private", "method", "getter", - "setter" + "setter", + "private", + "private_method", + "private_getter", + "private_setter", + "test", + "this", ]); });