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

Add flag --emit-lua-preserve-columns to benefit from Lua diagnostics tools #567

Open
bjornbm opened this issue Mar 22, 2023 · 2 comments
Open

Comments

@bjornbm
Copy link
Contributor

bjornbm commented Mar 22, 2023

Opening an issue based on discussion at: #277 (comment)

For “cheap” editor support, I am considering running --emit-lua and then, for example, run Luacheck on the .lua file(s) and use this to drive the editor (for example, [neo]vim quickfix functionality). @sumneko's Lua language server can be jury-rigged for Pallene in a similar fashion (roundtripping via --emit-lua).

The diagnostics (and editor highlighting/cursor positioning) could be more accurate if the columns were preserved in the Lua output. Basically the point of this issue is to add a flag to revert the change in #277. The proposed name is --emit-lua-preserve-columns. It is long, but better descriptive than short, I guess. Other suggestions are welcome.

@hugomg said “maybe it's useful if there are extra error messages you can get from luacheck but not from pallenec.” As a motivating example:

local m = {}

local function f(x : any) : float
   local y : integer
   local x : integer = y
end

return m

Luacheck output:

Checking lck.lua                                  6 warnings

    lck.lua:1:1: setting undefined field ln of global math
    lck.lua:3:16: unused function f
    lck.lua:3:18: unused argument x
    lck.lua:4:10: variable y is never set
    lck.lua:5:10: unused variable x
    lck.lua:5:10: variable x was previously defined as an argument on line 3

Total: 6 warnings / 0 errors in 1 file

In this simple case the column numbers were OK. The point was to demonstrate that Luacheck adds value on top of pallenec. (The first warning should to be ignored with Luacheck ignore pattern.)

A PR will follow (although feel free if anyone is less time constrained).

Note that I'm not using any of this in my editor yet. I might try to add it in @JLPLabs' pallene-vim.

@bjornbm
Copy link
Contributor Author

bjornbm commented Mar 22, 2023

I also realize now that pallenec --emit-lua does not perform full checking. In particular, those of src/pallene/uninitialized.lua. For example, if does not catch the use of unassigned y and the absence of a return in the above example. I think this warrants a new issue?

@hugomg
Copy link
Member

hugomg commented Apr 4, 2023

I agree that this deserves a separate issue. I also expect that it shouldn't be too difficult to implement. Add a couple of lines to the compile_pln_to_lua function in driver.lua

local function compile_pln_to_lua(input_ext, output_ext, input_file_name, base_name)
    assert(input_ext == "pln")

    local input, err = driver.load_input(input_file_name)
    if not input then
        return false, { err }
    end

    local prog_ast, errs = driver.compile_internal(input_file_name, input, "checker")
    if not prog_ast then
        return false, errs
    end

    -- HERE
    -- Add some code to continue the compilation up to step "uninitialized"

    local translation = translator.translate(input, prog_ast)

    assert(util.set_file_contents(base_name .. "." .. output_ext, translation))
    return true, {}
end

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

No branches or pull requests

2 participants