[feature] Support pre-commit hook

[Freed-Wu]

Can it support pre-commit like https://github.com/Koihik/LuaFormatter? TIA!

[carsakiller]

You may be able to write your own precommit script that utilizes --check to check for errors. I am not sure of any built-in way to support this.

[Freed-Wu]

$ lua-language-server --check XXX.lua
Diagnosis complete, 1 problems found, see /home/wzy/.cache/lua-language-server/log/check.json
$ echo $?
0

Can it return 1 when problems found? It can be used by pre-commit.

[sumneko]

AFAIK, someone has created a Github Action for this automated check, but I forgot the name of the project.

[sumneko]

https://github.com/mrcjkb/lua-typecheck-action

[Freed-Wu]

Can it return 1 when problems found

Hard, because script/cli/check.lua use multi process, main process cannot know if sub process find error.

[tomlau10]

Can it return 1 when problems found Hard, because script/cli/check.lua use multi process, main process cannot know if sub process find error.

I guess main process can know the exit code of sub process. So if sub process uses exit 1 when finding >= 1 problems, the main process can then also exit 1 at the end 🤔 @Freed-Wu

For example:

• cli/check_worker.lua will return true if check passed, otherwise false if there are problems
• then in cli/init.lua, we can check the return value

if _G['CHECK_WORKER'] then
    local checkPassed = require 'cli.check_worker'
    os.exit(checkPassed and 0 or 1, true)
end

• finally in cli/check.lua, I believe that the process:wait() will return the exit code, then we can catch it

local checkPassed = true
for _, process in ipairs(procs) do
    checkPassed = process:wait() == 0 and checkPassed
end
...
return checkPassed

• again in cli/init.lua, we can check the return value

if _G['CHECK'] then
    local checkPassed = require 'cli.check'
    os.exit(checkPassed and 0 or 1, true)
end

[fidgetingbits]

you can see a lua-ls script in these nix pre-commit hooks https://github.com/cachix/git-hooks.nix/blob/master/modules/hooks.nix that handles checking for errors by using a a wrapper and checking check.json file contents after the run.

I'm writing a similar shell script wrapper and ran into some other problems. for example --check doesn't seem to support checking a single file(?), only a directory. but that is very slow if you only want to check changed files. if you filter the pre-commit rule for only .lua files, it will pass each lua file as an argument to the script, and also possibly make multiple invocations, so you have to cope with that to avoid constantly rerunning --check across the whole directory multiple times.

Definitely seems it would be useful if there was a --check-files option or something

[tomlau10]

using a a wrapper and checking check.json file contents after the run.

Isn't that check.json will always be created no matter if there are problems or not? When no problems found, the file content will be an empty json array [].

            text = ''
              set -e
              export logpath="$(mktemp -d)"
              lua-language-server --check $(realpath .) \
                --checklevel="${hooks.lua-ls.settings.checklevel}" \
                --configpath="${luarc-dir}/.luarc.json" \
                --logpath="$logpath"
              if [[ -f $logpath/check.json ]]; then
                echo "+++++++++++++++ lua-language-server diagnostics +++++++++++++++"
                cat $logpath/check.json
                exit 1
              fi
            '';

In my github action lint workflow, I just use jq to check the length of that json array 👀

          # exit error if any
          if [ $(jq -r 'length' ${LUALS_RESULT_FILE}) -gt 0 ]; then
            exit 1
          fi

---

--check doesn't seem to support checking a single file(?)

I also thought about this when I integrate LuaLS into my project's gha lint workflow. I have been using luacheck before and I cached the result when the files are identical to speed up the process. However there are no similar things for LuaLS. But on a second thought, I think it will just not work for LuaLS to check a single file, because many diagnosis are cross files based 🤔.

Let me give an example. In a project you have a meta.lua which defines some API, and your other files depends on it. One day you updated 1 of the APIs in this meta.lua, say by changing the type of a function param. But then you didn't change any other files. If LuaLS only checks for files that are modified, it will only check this meta.lua but not others => this will create false negative check result as all other files are actually using an outdated API 💥

This kind of problem doesn't exist in luacheck because all checking of luacheck are file based. 😕

---

but that is very slow if you only want to check changed files

Btw did you try the --num_threads=* flag? LuaLS recently supports multi-threaded check in #2638 and it can speed up the whole process up to around 60% time.

I used this in my gha workflow, but unfortunately I can only use --num_threads=2 as the free gha ubuntu vm only has 2 cores... At least it speeds up the overall time from 40s => 30s for me 😂