Fix multiple cases of bracket and semicolon mishandling#89
Fix multiple cases of bracket and semicolon mishandling#899382 wants to merge 3 commits intomathiasbynens:masterfrom
Conversation
Not sure why the test cases are configured to see 1() or true() as valid, as they aren't Based on prefixexp ::= var | functioncall | `(´ exp `)´ (lua 5.1 docs)
In lua, f(x) and (f(x)) are different; (f(x)) will always be truncated to one value, while f(x) won't be This implementation is imperfect in terms of minification, as the brackets will be kept in cases where its not needed For example, `local x, y = (f()), nil` would not need to keep the brackets, but we do anyways. Still correct code, just not perfectly written (Also oops, already broke a test case set up just a commit ago)
Guarantee a ; is present when required to avoid accidentally turning 2 statements into one larger statement, e.g. the difference between a=b;(c or d)(e) and a=b(c or d)(e)
Characters checked for are determined via what can constitute a valid functioncall (essentially if the ending of our first statement could be seen as a prefixexp, which would be any var (Name, a[]) or functioncall (a(), a"", a'', a[[]], a{}) or bracketed expression (a)), since a CallStatement is the only way to start a statement with a non-alphanumeric character
9382
changed the title
|
Wow. That looks great. I will try it out your new commits and let you know how it goes. I have luamin installed using node, fyi, and I invoke it from the command line in various build scripts that I use which then minify large numbers of files. Because of all the file opening, writing, and closing, the process it is quite slow and occupies 90% or more of my entire project build time. Is there any way to improve that invocation speed? |
|
I could certainly have a look to see if I could figure out any performance improvements, but I don't often mess with javascript or nodejs, so I can't guarantee I'll be able to improve anything |
|
Thanks. I tried out your version with the latest fixes and the ; and () problems seem to be fixed!! In the coming days, I'll test it on a big codebase and let you know if there are any problems. Thank you for your work on this! |
| return a + b; | ||
| } | ||
| if (separator == ';' && firstCharB == '(') { | ||
| if ( |
There was a problem hiding this comment.
Note: To expand further on the last 2 paragraphs in my initial message, this entire if actually ended up being a collection of every valid ending to a lua statement, and so will always return true. There are times where this optimsation is possible (E.g. t={};(a or b)(c) is 2 statements with or without a ;), but we can't be 100% sure from the last character alone it wasn't something like t=f{};(a or b)(c) instead (which would change its meaning with no ;), so we'd need to know the exact statement instead of just the 2 peices of text to be joined, and we just don't provide that right now
luamin stable incorrectly removes parentheses and semicolons in cases where their removal changes the behaviour of the code. Switch to the version from pull request #89, which fixes these issues. mathiasbynens/luamin#89
2 participants
Fix prefix expressions not being bracketed when required
Now code like
(nil)()or("")""will stop producing invalid syntaxNot sure why the test cases were configured to see
1()ortrue()as valid, as calling a literal like this is invalid syntaxBased on
prefixexp ::= var | functioncall | `(´ exp `)´(lua 5.1 docs)Fixes #87
Fixes #48
Fix truncation via parenthesis being removed when it shouldn't be
Fix
(f())being shortened tof(), even though these aren't actually the same (In lua, if a call expression is surrounded by brackets, its result will always be truncated to one value, which can be an important difference)Solves the first issue of #88
Fix semicolons not being written in potential ambiguous syntax cases
Fixes the case where
a = b; ({})(c)(and similar statement combinations) would be written without the semicolon when minified, causing it to become one large statementThe only way to start a statement with anything other than an alphanumeric (+ underscore) character is a call statement that has its base wrapped in brackets (see
stat ::=BNF), so if we see this occur we make sure the end of our first statement couldn't be construed as a prefixexp (which I've just considered might encompass every possible ending to a statement)Not perfect in terms of minification since it just considers what the last character is instead of its context (e.g.
do end;({})()is the same with or without the semicolon), but we only pass the resultant string, not the statements involved, so I don't think this can be easily improvedFully fixes #88