Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Enable mangling of global references #1326

Open
wants to merge 2 commits into
base: harmony
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
61 changes: 58 additions & 3 deletions lib/propmangle.js
Expand Up @@ -83,7 +83,8 @@ function mangle_properties(ast, options) {
cache : null,
only_cache : false,
regex : null,
ignore_quoted : false
ignore_quoted : false,
treat_as_global : []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a trailing comma to make smaller diffs for future options

});

var reserved = options.reserved;
Expand All @@ -104,7 +105,21 @@ function mangle_properties(ast, options) {
var names_to_mangle = [];
var unmangleable = [];
var ignored = {};


if (!cache.global_defs) cache.global_defs = {};

var treat_as_global = options.treat_as_global;
var mangle_globals = !!treat_as_global.length;

function is_global_alias(name)
{
return treat_as_global.indexOf(name) >= 0;
}

// TODO: don't know if this is necessary to get scope information?
//if (mangle_globals)
// ast.figure_out_scope();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary.


// step 1: find candidates to mangle
ast.walk(new TreeWalker(function(node){
if (node instanceof AST_ObjectKeyVal) {
Expand All @@ -116,13 +131,34 @@ function mangle_properties(ast, options) {
}
else if (node instanceof AST_Dot) {
add(node.property);

// if the left side of the dot is a global alias (e.g. window.foo), and the left side
// does not refer to a local variable, add it to a list of global definitions.
if (mangle_globals &&
node.expression instanceof AST_SymbolRef &&
node.expression.global() &&
is_global_alias(node.expression.name) &&
!is_global_alias(node.property))
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uglify code style puts the appends { to the previous line while leaving indenting as you have it. Yes it's unusual.

cache.global_defs[node.property] = true;
}
}
else if (node instanceof AST_Sub) {
addStrings(node.property, ignore_quoted);
}
else if (node instanceof AST_ConciseMethod) {
add(node.name.name);
}
else if (node instanceof AST_SymbolRef) {
// if this term stands alone (e.g. just 'foo' where 'foo' cannot be shown to
// be a local variable in scope), also treat it as a global definition.
if (mangle_globals &&
node.global() &&
!is_global_alias(node.name))
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code style

cache.global_defs[node.name] = true;
}
}
}));

// step 2: transform the tree, renaming properties
Expand All @@ -147,6 +183,18 @@ function mangle_properties(ast, options) {
node.name.name = mangle(node.name.name);
}
}
else if (node instanceof AST_SymbolRef) {
// if this is a standalone global reference which does not refer to a local variable in scope,
// and it's on our list of global definitions, then mangle it.
if (mangle_globals &&
node.global() &&
node.name in cache.global_defs)
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code style

var mangled_name = mangle(node.name);
node.name = mangled_name;
node.definition().mangled_name = mangled_name;
}
}
// else if (node instanceof AST_String) {
// if (should_mangle(node.value)) {
// AST_Node.warn(
Expand Down Expand Up @@ -178,7 +226,8 @@ function mangle_properties(ast, options) {
if (regex && !regex.test(name)) return false;
if (reserved.indexOf(name) >= 0) return false;
return cache.props.has(name)
|| names_to_mangle.indexOf(name) >= 0;
|| names_to_mangle.indexOf(name) >= 0
|| (mangle_globals && name in cache.global_defs);
}

function add(name, ignore) {
Expand All @@ -205,6 +254,12 @@ function mangle_properties(ast, options) {
do {
mangled = base54(++cache.cname);
} while (!can_mangle(mangled));

// HACK: to avoid mangled global references colliding with local variable names, use
// a prefix on all mangled global names to effectively move them to a different namespace.
if (mangle_globals && name in cache.global_defs)
mangled = "g_" + mangled;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not crazy about this hack. Collisions can be avoided in a similar fashion to the other PR you wrote.


cache.props.set(name, mangled);
}
return mangled;
Expand Down
46 changes: 46 additions & 0 deletions test/compress/properties.js
Expand Up @@ -143,6 +143,52 @@ mangle_unquoted_properties: {
}
}

mangle_global_properties: {
mangle_props = {
treat_as_global: ["window"],
reserved: ["console", "log"]
}
input: {
window.foo = {};
foo.bar = 1;
external.baz = 2;
console.log(external);
}
expect: {
window.g_a = {};
g_a.b = 1;
g_d.c = 2;
console.log(g_d);
}
}

mangle_global_properties_not_local: {
options = {
}
mangle_props = {
treat_as_global: ["self"],
reserved: ["console", "log"]
}
input: {
self.foo = {};
function test()
{
var self = this;
console.log(self.bar);
};
console.log(typeof bar);
}
expect: {
self.g_a = {};
function test()
{
var self = this;
console.log(self.b);
};
console.log(typeof g_b);
}
}

first_256_chars_as_properties: {
beautify = {
ascii_only: true,
Expand Down