Uglifyjs: uglify-es es6+ const duplicate mangled variable name collision / shadowing
Bug report or feature request? Bug report
For the JS input provided below the output contains a variable shadowing bug. In the input the const funcB is clearly the argument passed to console.log, however in the output the mangled variable name is now shadowed.
ES5 or ES6+ input? ES6+
Uglify version (uglifyjs -V) uglify-es 3.3.5 (using Node 8.9.1, 8.6.0 and 8.5.0)
JavaScript input
(() => {
if (someTrueValue) { // this if is needed
const funcA = () => {
const foo = bar();
console.log(foo);
console.log(funcA);
console.log(funcB);
};
const funcB = () => { // funcB must be defined AFTER funcA
funcA();
};
funcA();
}
})();
JavaScript output or error produced.
(() => {
if (someTrueValue) {
const o = () => {
const l = bar();
console.log(l);
console.log(o);
console.log(l);
};
const l = () => {
o();
};
o();
}
})();
Notice that variable l is shadowed within the function o so when the console logs it will log the result of bar() and not the definition of l, this does not match our input.
Workarounds / things I have tried
- Remove the
ifstatement fixes it - Changing the
funcBandfoovariables fromconsttovarfixes it (now they are hoisted) - Defining
funcBor declaringfuncBbeforefuncAfixes it
johnemau
All 9 comments
Thanks for the report. Don't really need an explanation - only code that outputs a different result once uglified...
$ cat t2762.js
function bar() {
return 123;
}
var someTrueValue = 1;
(() => {
if (someTrueValue) {
const funcA = () => {
const foo = bar();
console.log(foo);
console.log(funcA);
console.log(funcB);
};
const funcB = () => {
funcA();
};
funcA();
}
})();
```
$ cat t2762.js | node
123
[Function: funcA]
[Function: funcB]
$ cat t2762.js | bin/uglifyjs -bm | node
123
[Function: o]
123
```js
$ cat t2762.js | bin/uglifyjs -bm
function bar() {
return 123;
}
var someTrueValue = 1;
(() => {
if (someTrueValue) {
const o = () => {
const e = bar();
console.log(e);
console.log(o);
console.log(e);
};
const e = () => {
o();
};
o();
}
})();
kzc
on 11 Jan 2018
Reduced test case:
$ cat t2762b.js
var bar = 1, T = true;
(function() {
if (T) {
const a = function() {
var foo = bar;
console.log(foo, a.prop, b.prop);
};
a.prop = 2;
const b = { prop: 3 };
a();
}
})();
```
$ cat t2762b.js | node
1 2 3
$ cat t2762b.js | bin/uglifyjs -bm | node
1 2 undefined
```js
$ cat t2762b.js | bin/uglifyjs -bm
var bar = 1, T = true;
(function() {
if (T) {
const o = function() {
var r = bar;
console.log(r, o.prop, r.prop);
};
o.prop = 2;
const r = {
prop: 3
};
o();
}
})();
Using #2778 to dump the AST to see more detail on the mangle collision of foo and b:
$ cat t2762b.js | bin/uglifyjs -m -o ast
{
"_class": "AST_Toplevel",
"globals": [
"1000008 console"
],
"variables": [
"1000001 bar",
"1000002 T"
],
"enclosed": [
"1000002 T",
"1000001 bar",
"1000008 console"
],
"body": [
{
"_class": "AST_Var",
"definitions": [
{
"_class": "AST_VarDef",
"name": {
"_class": "AST_SymbolVar",
"name": "bar",
"thedef": "1000001 bar"
},
"value": {
"_class": "AST_Number",
"value": 1
}
},
{
"_class": "AST_VarDef",
"name": {
"_class": "AST_SymbolVar",
"name": "T",
"thedef": "1000002 T"
},
"value": {
"_class": "AST_True"
}
}
]
},
{
"_class": "AST_SimpleStatement",
"body": {
"_class": "AST_Call",
"expression": {
"_class": "AST_Function",
"name": null,
"argnames": [],
"uses_arguments": false,
"is_generator": false,
"async": false,
"variables": [
"1000003 arguments"
],
"enclosed": [
"1000002 T",
"1000001 bar",
"1000008 console"
],
"body": [
{
"_class": "AST_If",
"condition": {
"_class": "AST_SymbolRef",
"name": "T",
"thedef": "1000002 T"
},
"alternative": null,
"body": {
"_class": "AST_BlockStatement",
"body": [
{
"_class": "AST_Const",
"definitions": [
{
"_class": "AST_VarDef",
"name": {
"_class": "AST_SymbolConst",
"name": "a",
"thedef": "1000004 a o"
},
"value": {
"_class": "AST_Function",
"name": null,
"argnames": [],
"uses_arguments": false,
"is_generator": false,
"async": false,
"variables": [
"1000005 arguments",
"1000006 foo r"
],
"enclosed": [
"1000001 bar",
"1000006 foo r",
"1000004 a o",
"1000007 b r",
"1000008 console"
],
"body": [
{
"_class": "AST_Var",
"definitions": [
{
"_class": "AST_VarDef",
"name": {
"_class": "AST_SymbolVar",
"name": "foo",
"thedef": "1000006 foo r"
},
"value": {
"_class": "AST_SymbolRef",
"name": "bar",
"thedef": "1000001 bar"
}
}
]
},
{
"_class": "AST_SimpleStatement",
"body": {
"_class": "AST_Call",
"expression": {
"_class": "AST_Dot",
"expression": {
"_class": "AST_SymbolRef",
"name": "console",
"thedef": "1000008 console"
},
"property": "log"
},
"args": [
{
"_class": "AST_SymbolRef",
"name": "foo",
"thedef": "1000006 foo r"
},
{
"_class": "AST_Dot",
"expression": {
"_class": "AST_SymbolRef",
"name": "a",
"thedef": "1000004 a o"
},
"property": "prop"
},
{
"_class": "AST_Dot",
"expression": {
"_class": "AST_SymbolRef",
"name": "b",
"thedef": "1000007 b r"
},
"property": "prop"
}
]
}
}
]
}
}
]
},
{
"_class": "AST_SimpleStatement",
"body": {
"_class": "AST_Assign",
"operator": "=",
"left": {
"_class": "AST_Dot",
"expression": {
"_class": "AST_SymbolRef",
"name": "a",
"thedef": "1000004 a o"
},
"property": "prop"
},
"right": {
"_class": "AST_Number",
"value": 2
}
}
},
{
"_class": "AST_Const",
"definitions": [
{
"_class": "AST_VarDef",
"name": {
"_class": "AST_SymbolConst",
"name": "b",
"thedef": "1000007 b r"
},
"value": {
"_class": "AST_Object",
"properties": [
{
"_class": "AST_ObjectKeyVal",
"key": "prop",
"value": {
"_class": "AST_Number",
"value": 3
}
}
]
}
}
]
},
{
"_class": "AST_SimpleStatement",
"body": {
"_class": "AST_Call",
"expression": {
"_class": "AST_SymbolRef",
"name": "a",
"thedef": "1000004 a o"
},
"args": []
}
}
]
}
}
]
},
"args": []
}
}
]
}
@alexlamsl The mangle collision can be seen in enclosed above:
"enclosed": [
"1000001 bar",
"1000006 foo r",
"1000004 a o",
"1000007 b r",
"1000008 console"
],
next_mangled is aware of scope.enclosed - but something is obviously going wrong with respect to const b and var foo.
kzc
on 13 Jan 2018
My bet is simply that enclosed isn't getting populated properly due to block scope - see if removing the if block in https://github.com/mishoo/UglifyJS2/issues/2762#issuecomment-356793454 would result in correct mangling for const.
alexlamsl
on 13 Jan 2018
The issue could lie within:
https://github.com/mishoo/UglifyJS2/blob/aa1786dedfb21680f049bc37be521b3a877cd5ca/lib/scope.js#L559-L565
which does not respect scope ordering for non-block-scoped variables:
https://github.com/mishoo/UglifyJS2/blob/aa1786dedfb21680f049bc37be521b3a877cd5ca/lib/scope.js#L549-L552
alexlamsl
on 13 Jan 2018
Possible fix (+ minimising diff with master):
--- a/lib/scope.js
+++ b/lib/scope.js
@@ -550,24 +550,23 @@ AST_Toplevel.DEFMETHOD("mangle_names", function(options){
node.variables.each(collect);
return;
}
+ if (node.is_block_scope()) {
+ node.block_scope.variables.each(collect);
+ return;
+ }
if (node instanceof AST_Label) {
var name;
do name = base54(++lname); while (!is_identifier(name));
node.mangled_name = name;
return true;
}
- var mangle_with_block_scope =
- (!options.ie8 && node instanceof AST_SymbolCatch) ||
- node instanceof AST_SymbolBlockDeclaration;
- if (mangle_with_block_scope && options.reserved.indexOf(node.name) < 0) {
+ if (!options.ie8 && node instanceof AST_SymbolCatch) {
to_mangle.push(node.definition());
return;
}
});
this.walk(tw);
- to_mangle.forEach(function(def){
- def.mangle(options);
- });
+ to_mangle.forEach(function(def){ def.mangle(options) });
function collect(symbol) {
if (!member(symbol.name, options.reserved)) {
It appears to work. In hindsight that makes sense.
Are you okay with putting a PR together?
I now see that the block_scope property created by figure_out_scope is not dumped by -o ast. I'll look into that.
kzc
on 13 Jan 2018
This patch will add support for block_scope in -o ast in harmony once #2778 is merged:
--- a/bin/uglifyjs
+++ b/bin/uglifyjs
@@ -268,6 +268,13 @@ function run() {
var result = {
_class: "AST_" + value.TYPE
};
+ if (value.block_scope) {
+ result.block_scope = {
+ variables: value.block_scope.variables,
+ functions: value.block_scope.functions,
+ enclosed: value.block_scope.enclosed,
+ };
+ }
value.CTOR.PROPS.forEach(function(prop) {
result[prop] = value[prop];
});
Individual properties are picked out from value.block_scope to avoid duplicating the body property.
kzc
on 13 Jan 2018
Related issues
alexlamsl
路
4Comments
GrosSacASac
路
3Comments
JoeUX
路
3Comments
neverfox
路
4Comments
alexlamsl
路
4Comments