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

"Expected: :" error in ExtendScript #1144

Closed
fusepilot opened this issue Jun 19, 2016 · 8 comments · Fixed by #5648
Closed

"Expected: :" error in ExtendScript #1144

fusepilot opened this issue Jun 19, 2016 · 8 comments · Fixed by #5648

Comments

@fusepilot
Copy link

I'm trying to use Uglify to compress and obscure Adobe ExtendScript ( ES3 dialect of JS ) code. Uglify seems to strip parentheses in nested ternary expressions. ExtendScript can't understand this and throws an error: Expected: :.

Input code:

// input.js

function toInteger(value) {
  var result = parseFloat(value);
  var remainder = result % 1;

  return result === result ? (remainder ? result - remainder : result) : 0;
}

toInteger(2.1); // Result: 2

Output code from uglifyjs -b -c -o output.js -- input.js:

// output.js

function toInteger(value) {
    var result = parseFloat(value), remainder = result % 1;
    return result === result ? remainder ? result - remainder : result : 0; // Error: Expected: :
}

toInteger(2.1);

I'm aware how edge case this is, but it would be great if there was a compression option to keep the parentheses in for this scenario.

@kzc
Copy link
Contributor

kzc commented Jun 19, 2016

There's no such compression option. But if you want to add parens to the consequent of a conditional here's the UglifyJS2 patch:

--- a/lib/output.js
+++ b/lib/output.js
@@ -639,7 +639,7 @@ function OutputStream(options) {
         if (p instanceof AST_Call && p.expression === this)
             return true;
         // (a = foo) ? bar : baz
-        if (p instanceof AST_Conditional && p.condition === this)
+        if (p instanceof AST_Conditional && (p.condition === this || p.consequent === this))
             return true;
         // (a = foo)["prop"] —or— (a = foo).prop
         if (p instanceof AST_PropAccess && p.expression === this)

If you have the same issue with the ternary alternative you'd incorporate a similar change with p.alternative.

Without patch:

$ echo 'function foo(x,y,z) {return x ? y ? z-1 : x-z : z;}' | uglifyjs -c
function foo(x,y,z){return x?y?z-1:x-z:z}

With the patch:

$ echo 'function foo(x,y,z) {return x ? y ? z-1 : x-z : z;}' | bin/uglifyjs -c
function foo(x,y,z){return x?(y?z-1:x-z):z}

Since this patch makes the output larger it will not be incorporated into uglify. You should probably report this ExtendScript grammar bug to Adobe as it's not ECMAScript compliant.

@rvanvelzen
Copy link
Collaborator

UglifyJS targets ECMAScript compliant engines. I'm sorry, this is not going to happen in the upstream, not even with a flag.

Thanks for the report!

fusepilot added a commit to fusepilot/uglify-js-es-compat that referenced this issue Jun 19, 2016
Added patch from mishoo#1144 (comment). Fixes ````Expected: :```` error in ExtendScript.
@avdg
Copy link
Contributor

avdg commented Jun 25, 2016

I think this is a valid es 5.1 bug (at the side of the adobe interpreter), but I'll have to document the valid parser waterfall before looking for a fix (edit: which is fix their software).

Edit: done

ConditionalExpression: LogicExpression '?' AssignmentExpression ':' AssignmentExpression

Eventually, very deep in the parse tree, an assignementExpression accepts a primaryExpression, which accepts '(' expression ')', which acccepts another conditionalExpression.

@avdg
Copy link
Contributor

avdg commented Jun 25, 2016

Parentheses aren't a compress option, they are read once, converted to ast and inserted again at the output phase if needed (to avoid confusion). Compression is done in AST trees and since the relations there are clear, there is no need for semicolons in the trees themselves.

@kzc
Copy link
Contributor

kzc commented Jun 26, 2016

Major JS engines and browsers accept the code without parens. ExtendScript has a bug.

@avdg
Copy link
Contributor

avdg commented Jun 26, 2016

Yeah, they should fix their engines and meh I care too much about even that piece of closed software.

@rvanvelzen
Copy link
Collaborator

I would be willing to re-open this if ExtendScript were a major engine currently in use. I've never of it except for 2 reports here.

Does anyone know whether ExtendScript has significant (enough) market share to consider this?

@vespakoen
Copy link

vespakoen commented May 30, 2017

I use ExtendScript as well and ran into the same problem.
I have avoided ternary expressions all together, and am wondering if I can stop UglifyJS optimising if/else's into the ternary expressions all together, I have tried these already without success:

{
  if_return: false,
  conditionals: false,
  booleans: false,
  comparisons: false
}

I also suggest that we "fix" this by adding an option to UglifyJS.

Making Adobe change something in ExtendScript and waiting for everyone to update is going to take many, many years.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants