Uglifyjs: [ES6] Error when parenthesis string is envolved by parenthesis

[viclm]

('(')

Unexpected token eof «undefined», expected arrow «=>»

[kzc]

$ uglifyjs -V
uglify-js 2.6.2

$ echo "foo( ('(') );" | uglifyjs -c -m
foo("(");

Are you using the uglify harmony branch?

[viclm]

@kzc yep

[kzc]

Could you put [ES6] in the issue title?

[viclm]

https://github.com/viclm/UglifyJS2/commit/0976a7ca2c04bf705acd91987c7f2c90bd7104ca

[Martii]

Someone beat me to this. :) Cc: @fabiosantoscode

Dummy Userscript extraction that doesn't really do anything other than produce the minification issue reported here

(function () {

// ==UserScript==
// @name          RFC 2606§3 - UglifyJS2 (Harmony) Unit Test
// @namespace     https://openuserjs.org/users/Marti
// @decription    Tests harmony minification
// @copyright     2016+, Marti Martz (https://openuserjs.org/users/Marti)
// @license       (CC); http://creativecommons.org/licenses/by-nc-sa/3.0/
// @license       GPL version 3 or any later version; http://www.gnu.org/copyleft/gpl.html
// @version       2016.03.09
// @icon          https://www.gravatar.com/avatar/7ff58eb098c23feafa72e0b4cd13f396?r=G&s=48&default=identicon

// @include   http://www.example.com/*
// @include   http://www.example.net/*
// @include   http://www.example.org/*

// @grant       none
// ==/UserScript==

  function (col) {
    return col.slice(0, col.indexOf('('));
  }

})();

MINIFICATION WARNING (harmony):
  message: Unexpected token: punc (()
  installName: Marti/RFC_2606§3_-_UglifyJS2_(Harmony)_Unit_Test.user.js
  line: 20 col: 11 pos: 742

We see _a lot_ of these...

  function UserscriptExtraction(col) {
    return col.slice(0, col.indexOf('('));
  }

MINIFICATION WARNING (harmony):
  message: Unexpected token punc «)», expected arrow «=>»
  installName: Marti/RFC_2606§3_-_UglifyJS2_(Harmony)_Unit_Test.user.js
  line: 21 col: 40 pos: 830

... modified to a named function from above to produce the arrow message.

---

Actual log excerpts:

2016-03-09 10:23:47.724 +00:00: MINIFICATION WARNING (harmony):
  message: Unexpected token punc «)», expected arrow «=>»
  installName: vBm/Omerta_Beyond.user.js
  line: 970 col: 40 pos: 34036

... taken from https://openuserjs.org/scripts/vBm/Omerta_Beyond/source and our logs. _(NOTE: Line numbers could change/disappear if published/updated date is newer)_

---

  function UserscriptExtraction() {
    var strFecha = getElementsByClass("start")[0].innerHTML;
    strFecha = strFecha.substring(strFecha.indexOf('(')+1, strFecha.indexOf(')'));
  }

2016-03-06 20:57:08.094 +00:00: MINIFICATION WARNING (harmony):
  message: Unexpected token operator «+», expected arrow «=>»
  installName: Amynka/CCcompact.user.js
  line: 528 col: 55 pos: 16404

... taken from https://openuserjs.org/scripts/Amynka/CCcompact/source and our logs. _(NOTE: Line numbers could change/disappear if published/updated date is newer)_

---

2016-03-07 02:05:33.281 +00:00: MINIFICATION WARNING (harmony):
  message: Unexpected token punc «[», expected arrow «=>»
  installName: open2chExtender/open2chYouVideo.user.js
  line: 132 col: 483 pos: 38238

... taken from https://openuserjs.org/scripts/open2chExtender/open2chYouVideo/source and our logs. _(NOTE: Line numbers could change/disappear if published/updated date is newer... this script is partially pre-minified too so it's difficult to extract the snippet)_

---

e.g. the issue seems to show up as more than just parens too... sometimes after return statements... sometimes not... but always the expected arrow message in our server logs.

[Martii]

@mishoo and @fabiosantoscode
I've set up a Warning header as close to the specification as I can _(there are some node and express issues with headers)_ so that if there's an issue with minification it will be possible to automate it just by dropping the ....min.... link in here. See OpenUserJs/OpenUserJS.org#923 if interested. It is Q encoded since the error messages generated here in this project may not be guaranteed to be ISO-8859-1... so a little bit of decoding will need to be done and/or memorizing what Unicode characters mean what in Q encoding for this project. _(I would suggest, if possible, converting some of those characters (e.g. in «)» and «=>» so they show up as clear ASCII like => instead of =?utf-8?Q?=C2=AB=3D=3E=C2=BB?=) to back ticks as those should be ASCII and won't need as much Q de/en-coding... but that's up to this project how much they want to memorize Unicode to Q conversions... NOTE I've done that with the double left/right pointing angle quotations now since I couldn't find them in the source with a search... may alter this again though ;)_

Example:

• https://openuserjs.org/src/scripts/vBm/Omerta_Beyond.min.user.js# _(view this with a request observer such as Live HTTP headers, or equivalent)_

https://openuserjs.org/src/scripts/vBm/Omerta_Beyond.min.user.js#

GET /src/scripts/vBm/Omerta_Beyond.min.user.js HTTP/1.1
Host: openuserjs.org
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:42.0) Gecko/20100101 SeaMonkey/2.39
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
DNT: 1
Referer: https://openuserjs.org/scripts/vBm/Omerta_Beyond/source
Cookie: *clipped*
Connection: keep-alive

HTTP/1.1 200 OK
X-Powered-By: Express
Strict-Transport-Security: max-age=31536000000; includeSubDomains
Content-Type: text/javascript; charset=UTF-8
Warning: 199 openuserjs.org MINIFICATION WARNING (harmony):   Unexpected token punc =?utf-8?Q?=C2=AB=29=C2=BB=2C?= expected arrow =?utf-8?Q?=C2=AB=3D=3E=C2=BB?=   line: 970 col: 40 pos: 34036
Vary: Accept-Encoding
Content-Encoding: gzip
Date: Wed, 09 Mar 2016 23:13:38 GMT
Connection: keep-alive
Transfer-Encoding: chunked

I've also added a Minified Source button dropdown to script homepages for more ease of checking

Example:

• https://openuserjs.org/scripts/vBm/Omerta_Beyond/source _(see currently "Raw Source", blue button with dropdown that says "Minified Source")_

[Martii]

@viclm
Ran your patch against some of our data... seems to fix the paren issue but not the square brace one _(the third example above named open2chYouVideo)_... nice job though on the parenthesis find. :)

[rvanvelzen]

@Martii:

(function () {
// ...
  function (col) {
    return col.slice(0, col.indexOf('('));
  }

})();

This is not a valid program, so it's not surprising that it fails to parse. (The inner function is in statement position, thus requiring a name)

[Martii]

@rvanvelzen

Dummy Userscript extraction that doesn't really do anything other than produce the minification issue reported here

===:

This is not a valid program

... Just because it's not a real program doesn't negate the parsing error in _UglifyJS2_... it is syntactically correct just doesn't do anything... it's an example if you would please reread what was posted and requoted ... technically it's dead code without the name but still a 100% valid function expression ... just think if it didn't have my typical IIFE around it... e.g. ignore that as it's just a layer of abstraction... however...

so it's not surprising that it fails to parse.

The parsing failure is denoted by @viclm and called out with a fix... I'm just here to confirm that he's right that there is definitely an issue. I can bombard this projects issue tracker with the dozens, if not hundreds, of parsing errors this project has _(we have ~300+ so far this month unsorted and this is a conservative count)_ but I thought I might be more constructive by actually isolating as much as I can for this project in order to improve it... filtering and condensing logs when needed including common mistakes that may generate a parsing error, etc. We have a growing, large, community over at our service so it would be nice to be able to offer suggestions, confirm accuracy, etc. to our dependency friends out there.

There are multiple errors with parsing in this project that I haven't posted yet including { instances and of course the [ that I already did post above... all end up containing the "expected arrow" message with valid Userscripts.

Point of the matter is that somewhere in the ES6 code it's interfering with with ES5. Cc: @fabiosantoscode

Our first logged instance of this issue occurred on 2016-02-18 02:14:39.067 +00:00: MINIFICATION WARNING (harmony):... so I would suspect somewhere around slightly before that date with one of the commits in this project is when the bug was introduced.

[kzc]

[email protected] agrees with @rvanvelzen's assessment:

$ acorn --ecma6 <<EOF
  (function () {
    // ...
    function (col) {
      return col.slice(0, col.indexOf('('));
    } 
  })();
EOF
Unexpected token (3:13)

[Martii]

@kzc and @rvanvelzen

You are both missing the point of the snippet... while it may be called out as an exception that doesn't negate @viclm's report... this project has a major flaw right now with defaulting to "expected arrow" messages. I always test my reduced case snippets and the error should be something along the lines of:

/*
Exception: SyntaxError: function statement requires a name
@Scratchpad/1:12
*/

not ...

MINIFICATION WARNING (harmony):
  message: Unexpected token: punc (()
  installName: Marti/RFC_2606§3_-_UglifyJS2_(Harmony)_Unit_Test.user.js
  line: 20 col: 11 pos: 742

... so presuming that _acorn_ is the deriving backend here... clearly it is incomplete on the error message for that particular code snippet that was thrown together as a reduced case scenario.... anyhow... this is the end of nit picking on this particular sample snippet as it produces no effective resolution for the base issue here... and why I named it _(you both are clearly demonstrating that you missed that in the initial post above too... see "We see a lot of these..." section)_... e.g. please don't assume that I'm stupider than you are please... this project has been notified of an issue and I would hope that one of the maintainers would be willing to address the subject of the parens and get the patch verified that @viclm did... it works here for the hundreds of errors that are in our logs from this project. I can understand if those here don't want to fix a known issue and I also understand that those here may not understand common issue tracking as I've been doing this for over three decades... quite possibly longer than most have been alive here but I won't presume that is true in all cases... so please try to concentrate on constructive issues rather than assert ones programming ego on a single code snippet. :)

Secondly the parens issue is 100% reproducible which is what this issue is about... and there is more than just parens.

It would be great not to have to switch to a different package that is willing to properly address issues... I am starting to realize how our other contributors don't like the idea of altering the code because at the very least it's buggy at times... this is putting it nicely... they didn't put it nicely. I would hope that this project wouldn't want to burn me or us... we represent a larger community group of users than all of _node_.

What's left for maintainers to do constructively:

1. Verify that patch from @viclm is indeed the fix.
2. Create a PR or ask him to do so. _(EDIT: done at #1002)_
3. Merge it.
4. Close this issue.

I'll reference it in another one/set with the [ and { and every other character that is showing up from this project in our logs... I don't have to do reduced test cases if you would like and the mess that ensues from this package can be sorted out by its collaborators instead of the generous time that I've dedicated to promoting this project as well as the data collection... Should I change my mind?

[kzc]

@Martii You're not helping your cause with such long-winded and insulting diatribes. We're all volunteering our time here. We're happy to receive succinct bug reports and pull requests. But if you want to take your javascript minification business elsewhere, that's up to you.

[Martii]

@kzc

We're all volunteering our time here

So are we... and we've bent over backwards to promote this project and suppress the negative connotations in the global community. e.g. change perceptions.

insulting diatribes

Change your perception please as it's incorrect as well as snarky _(flaming, trolling, misguided, etc.)_

long-winded

Prefer the term of completeness instead of being lazy like some want and expect. My colleagues in the office have just read this and they concur with the assessment. We are handing this data as a courtesy... perhaps some equal return courtesy would be nice... including reading everything and not falling back on the TLDR aspect of some.

We're happy to receive succinct bug reports and pull requests.

That's great... perhaps referencing back to the PR would have been prudent as I just did that above... that's why CONTRIBUTING.md exists on GH. :) I don't expect full a bureaucracy but to alleviate some miscommunications it's a wise idea to do.

But if you want to take your javascript minification business elsewhere, that's up to you.

It's actually up to this project and how it will be rated from a global standpoint. e.g. this projects reputation is on the line as the aphorism goes and so far at least one is burning one of the few advocates _(me)_ of it. This I will be blunt on... that's a stupid and arrogant move.

Anyhow... rather than me receiving a defensive posture and drama which seems to be this issues preferred responsed form of communication so far... how about confirming that is the correct patch for some constructive feedback? Is it? Do you know?

One more time in case it was missed for the third time... is that the correct patch? _(if no one decides to answer this simple question I won't be responding anymore to this issue as it's a confirmed issue on our end of several million users in the Userscript and node communities. :)_

---

@fabiosantoscode
If I'm gong to get this sort of response for confirming an issue from a user standpoint _(endpoint)_... it's unlikely that I will be isolating exact issues and let you and the others with the harmony branch figure things out... which is one of the reasons the HTTP header was created in the first place in case I'm not available. I've asked you on occasion to come visit somewhere and wanted to say thank you for you assistance and professionalism. You are truly an objective developer from what I've experienced.

[rvanvelzen]

I've merged @viclm's PR, which should resolve the original issue. For issues other than the eager parsing of arrow functions, please open a new issue.

[kzc]

@Martii Please stop hijacking threads with useless commentary.

[Martii]

Thank you @rvanvelzen for the confirmation.

---

All three reported Userscripts now show as being minified without errors. _(cross-section check of all March 2016 logged errors ... the 300ish report above... shows as successful now)_ :)

Awesome modified patch @viclm ... thank you for your patience and effort.

---

Closed by https://github.com/mishoo/UglifyJS2/commit/6780d0906c324102ec655d8e844754223876382a

[fabiosantoscode]

I completely missed the development of this conversation, and just wrote an unnecessary fix lol :) thanks @viclm for fixing the bug I introduced. Sorry about the oversight.

Not mentioning any names in specific:

https://xkcd.com/438/

Let's get along, everyone :heart: there's no need to be aggressive. I believe we're all working towards the same goal.

Have a beautiful day!