Skip to content

Feature request: Improve usability of standalone diagnosis report #2830

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

Closed
kaklakariada opened this issue Aug 28, 2024 · 6 comments
Closed

Comments

@kaklakariada
Copy link

Thanks a lot for LuaLS, this is a really great project!

We are running LuaLS during the CI build to check type annotations using the --check command line option.

I would like to propose two usability improvements for the --check mode:

Exit code

The program should exit with a status code other than 0 when it finds problems.

  • Currently it always exits with status 0 even if it finds problems. This would simplify checking build success or failure.

Log problems

The program should log each problem to stdout or stderr

  • Currently users need to download check.json which is hard to read
  • When the program logs problems, users could immediately see problems without downloading a file
@tomlau10
Copy link
Contributor

Someone already did this, in a tool called llscheck
Maybe you would like to check it out 😄
(I first heard about it from this reply #2749 (comment))

@tomlau10
Copy link
Contributor

I would like to share my ci step as well, which only uses jq:

      - name: Run LuaLS
        run: |
          ${LUALS_PATH}/bin/lua-language-server --check=. --num_threads=2 --checklevel=Error

          # annotate result
          LUALS_RESULT_FILE=${LUALS_PATH}/log/check.json
          jq -r '
              to_entries[] |
                (.key | sub("^.*?\\./"; "")) as $file | 
              .value[] |
                .code as $title |
                (.range.start.line + 1) as $line |
                (.range.start.character + 1) as $col |
                .message as $message |
              "\($file):\($line):\($col)::\($message)\n" +
              "::error file=\($file),line=\($line),col=\($col),title=\($title)::\($message)"
          ' ${LUALS_RESULT_FILE}

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

@kaklakariada
Copy link
Author

@tomlau10 wow, thanks a lot! Both solutions look very good, that's exactly what I was looking for :-)
I already thought about using jq, but was afraid it would be very complicated. Looks like I was right 😅

@tomlau10
Copy link
Contributor

Yes jq looks complicated at first, I was afraid as well when I write my ci step at that time. But after I get a grasp on it, the expression is actually easy enough to understand or modify. 😄 @kaklakariada

  • First, the check.json has the following hierarchy { "filename": results[], ... }:
{
  "filename1": [
    <result1>,
    <result2>,
    ...
  ],
  ...
}
  • the first to_entries convert it into an array of kv entries, and we use [] to iterate it:
[
  {
    "key": "filename1",
    "value": [ <result>, ...]
  },
  ...
]
  • Any middle expression (xxx | yyy) as $zzz means store the value as variable for later reference, and will not alter the structure.
    So you can see I transformed .key (which is a filename) with a string substitution and store as $file
  • And then we need to deal with the .value array, we use [] to iterate it => .value[]
    • for each entry in the .value[] array, I extracted $title / $line / $col / $message from it
    • and finally print them to stdout using string template. To reference variable using the ($var), we need a \ escape before the (

In pseudo code, it can be interpreted as:

for _, entry in ipairs(to_entries(data)) do  -- to_entries[]
  local file = entry.key
  for _, v in ipairs(entry.value) do  -- .value[]
    local title, line, col, message = v.code, v.line, v.col, v.message
    print(file, title, line, col, message)
  end
end

There is a jq playground to test the query as well 👀
https://jqplay.org/

@itaranto
Copy link

@kaklakariada Could you re-open the issue?

Even if it has workarounds, it would be nice if LuaLS provided a "linter mode" ready to be used in CI/CD pipelines.

@itaranto
Copy link

itaranto commented Sep 26, 2024

I would like to share my ci step as well, which only uses jq:

      - name: Run LuaLS
        run: |
          ${LUALS_PATH}/bin/lua-language-server --check=. --num_threads=2 --checklevel=Error

          # annotate result
          LUALS_RESULT_FILE=${LUALS_PATH}/log/check.json
          jq -r '
              to_entries[] |
                (.key | sub("^.*?\\./"; "")) as $file | 
              .value[] |
                .code as $title |
                (.range.start.line + 1) as $line |
                (.range.start.character + 1) as $col |
                .message as $message |
              "\($file):\($line):\($col)::\($message)\n" +
              "::error file=\($file),line=\($line),col=\($col),title=\($title)::\($message)"
          ' ${LUALS_RESULT_FILE}

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

* then print it in **stdout** together with **github actions annotation syntax**
  https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/workflow-commands-for-github-actions#example-creating-an-annotation-for-an-error

* finally check array length of the file, if `> 0` then there is an error

If you only care about the exit code, this could be simplified to:

lua-language-server \
    --configpath .luarc.json \
    --logpath . \
    --loglevel error \
    --checklevel Hint \
    --check .

test "$(jq 'to_entries|length' check.json)" -gt 0

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

3 participants