JohnnyMorganz/StyLua

Lua 5.2 label syntax error when using binary built all features enabled

Feez opened this issue · 5 comments

Feez commented

I was experimenting with using StyLua on minified Lua, and found that StyLua fails to parse this valid Lua when minified.

source:

local function test()
    goto end_thing

    local x = 1

    ::end_thing::
end

-> minified:

local function a()goto b;local c=1::b::end

with error:

error: could not format file minified.lua: error parsing: unexpected token `end`. (starting from line 1, character 40 
and ending on line 1, character 43)
additional information: expected identifier after `::`

If I add a semicolon after local c = 1 in the minified Lua then it parses as expected.

I assume this is through downloading the binaries from GitHub releases? This is unfortunately because the binary is currently built with both Lua 5.2 and Luau parsing enabled, which causes this conflict. This was actually reported before too: #309 (comment)

The only option right now is to install the binary using cargo cargo install stylua --features lua52

Feez commented

Sure, I'll have to do some setup of cargo.

I believe the output of this should probably be a feature request in being able to enable/disable these features at runtime for the ones that are installed, with the addition of having this error message (and others if they exist) to ask the user to provide the parser feature they intend to use.

I definitely agree with having a runtime flag to do this, that is the ideal scenario.

Unfortunately though the issue is upstream in our parser dependency, so it needs to be solved there

Feez commented

Maybe in the meantime, could provide additional binaries in the GitHub releases with the respective features compiled into them individually, as well as the default one for all. There seems to be quite a lot that goes into installing Cargo on a system like Windows (in terms of dependencies) and I can imagine some developers would have already stopped trying at this point.

Edit:
Installing the lua52 parsing worked on the original issue :)

I also agree on your second point, having Github release binaries for all possible flags would be good (#103). But as I mentioned in #309 (comment), right now there is potentially an issue with toolchain managers where they don't know which binary to install.

I might test it out one release though, and publish all the binaries and see if anything breaks