[ALOY-273] uglifyjs improperly handles if/else without curly braces

GitHub Issue	n/a
Type	Bug
Priority	High
Status	Resolved
Resolution	Fixed
Resolution Date	2012-11-15T16:09:28.000+0000
Affected Version/s	2012 Sprint 19
Fix Version/s	Alloy 0.3.2, 2012 Sprint 23
Components	XML
Labels	n/a
Reporter	Tony Lukasavage
Assignee	Unknown
Created	2012-09-17T06:21:35.000+0000
Updated	2018-03-07T22:26:10.000+0000

Description

The following code ends in error:

if(Ti.Platform.osname != 'android') 
    activityWindow.show();
else
    activityIndicator.show();

here's the error:

[INFO] Copying builtin /usr/local/lib/node_modules/alloy/Alloy/builtins/string.js to /Users/tlukasavage/Development/alloy/test/projects/Harness/Resources/alloy/string.js
node.js:201
        throw e; // process.nextTick error, or 'error' event on first tick
              ^
TypeError: Cannot call method 'apply' of undefined
    at walk (/usr/local/lib/node_modules/alloy/Alloy/uglify-js/lib/process.js:226:36)
    at doit (/usr/local/lib/node_modules/alloy/Alloy/uglify-js/lib/process.js:2039:37)
    at /usr/local/lib/node_modules/alloy/Alloy/uglify-js/lib/process.js:2056:72
    at redo_if (/usr/local/lib/node_modules/alloy/Alloy/uglify-js/lib/process.js:827:30)
    at Array.<anonymous> (/usr/local/lib/node_modules/alloy/Alloy/uglify-js/lib/process.js:869:43)
    at walk (/usr/local/lib/node_modules/alloy/Alloy/uglify-js/lib/process.js:221:47)
    at /usr/local/lib/node_modules/alloy/Alloy/uglify-js/lib/process.js:880:24
    at Object.with_walkers (/usr/local/lib/node_modules/alloy/Alloy/uglify-js/lib/process.js:249:27)
    at prepare_ifs (/usr/local/lib/node_modules/alloy/Alloy/uglify-js/lib/process.js:863:18)
    at /usr/local/lib/node_modules/alloy/Alloy/uglify-js/lib/process.js:1407:31
    at Object.with_walkers (/usr/local/lib/node_modules/alloy/Alloy/uglify-js/lib/process.js:249:27)
    at Object.ast_squeeze (/usr/local/lib/node_modules/alloy/Alloy/uglify-js/lib/process.js:1306:18)
    at Object.formatAST (/usr/local/lib/node_modules/alloy/Alloy/commands/compile/compilerUtils.js:736:12)
    at Object.makeRequiresRelative (/usr/local/lib/node_modules/alloy/Alloy/commands/compile/requires.js:132:12)
    at /usr/local/lib/node_modules/alloy/Alloy/commands/compile/index.js:489:26
    at Array.forEach (native)
    at Function.<anonymous> (/usr/local/lib/node_modules/alloy/Alloy/lib/alloy/underscore.js:11:39)
    at optimizeCompiledCode (/usr/local/lib/node_modules/alloy/Alloy/commands/compile/index.js:483:4)
    at /usr/local/lib/node_modules/alloy/Alloy/commands/compile/index.js:176:2
    at Object.<anonymous> (/usr/local/lib/node_modules/alloy/Alloy/alloy.js:101:44)
    at Module._compile (module.js:441:26)
    at Object..js (module.js:459:10)
    at Module.load (module.js:348:31)
    at Function._load (module.js:308:12)
    at Module.require (module.js:354:17)
    at require (module.js:370:17)
    at Object.<anonymous> (/usr/local/lib/node_modules/alloy/bin/alloy:3:1)
    at Module._compile (module.js:441:26)
    at Object..js (module.js:459:10)
    at Module.load (module.js:348:31)
    at Function._load (module.js:308:12)
    at Array.0 (module.js:479:10)
    at EventEmitter._tickCallback (node.js:192:40)
[ERROR] Alloy compile failed
[ERROR] Error: Traceback (most recent call last):
  File "/Users/tlukasavage/Library/Application Support/Titanium/mobilesdk/osx/2.1.1.GA/iphone/builder.py", line 1115, in main
    p.compile(compiler_config)
  File "/Users/tlukasavage/Development/alloy/test/projects/Harness/plugins/ti.alloy/plugin.py", line 62, in compile
    sys.exit(retcode)
SystemExit: 1

The issue us resolved simply by adding curly braces to the code:

if(Ti.Platform.osname != 'android') {
    activityWindow.show();
} else {
    activityIndicator.show();
}

We need to dig in and figure out why uglifyjs doesn't handle this properly.

Comments

Tony Lukasavage 2012-09-17

It appears as though the optimizer.js code is failing on trying to optimize platform comparisons when encountering this syntax. It is not an error with uglifyjs, but with how the AST is being manipulated. Added a failing test to the optimizer test suite. Will see if I can make it pass.
Tony Lukasavage 2012-09-17

Finding a number of other invalida cases with the optimizer. I'm going to spend some time with it, but I may need to disable the "if" and "var" optimizing part of the AST optimizing if this takes too long. Compiling out runtime platform checks is probably not worth the potential instability.
Tony Lukasavage 2012-09-17

The initial developer reported issue has been resolved by disabling the optimizer. ALOY-274 has been created to discuss how we can potentially re-enable the optimizer.js code, and if this is even desired.
Jeff Haynie 2012-10-27

This is not fixed and must be reopened and fixed
Tony Lukasavage 2012-10-27

I was using ALOY-274 as a broad stroke to cover tightening up the optimizer.js code, which included this issue. I'll reopen this one and make it a dependency of it.
Tony Lukasavage 2012-11-15

optimizer.js code now handles conditionals with no curly braces. All tests pass, including new ones that test this use case. This is all a result of a major simplification of the optimizer.js code in combination with allowing uglifyjs to remove dead code based on string to string comparisons. In the end we have a more stable optimizer that covers more cases and handles all known edge cases. The commit(s) can be seen here: * refactored code: ** https://github.com/appcelerator/alloy/commit/bcaffef93fada754103f4e1b8927133bb41f250a ** https://github.com/appcelerator/alloy/commit/a2e3294d6ab695b9a2f44d229f969a7fd9933453

JSON Source