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

AP_Scripting: add lua-language-server checks and docs generation. #27001

Merged
merged 14 commits into from
May 20, 2024

Conversation

IamPete1
Copy link
Member

@IamPete1 IamPete1 commented May 7, 2024

This updates the docs and adds configuration and helpers to run lua-language-server. It is smarter than the current luacheck checks we run allowing type checking. https://luals.github.io/ This is the same checker that is used in by the recommended VSCode extension.

The new check is added as part of CI. Lots of stuff currently fails, rather than trying to fix (for the most part) this just adds overrides to ignore warnings in those files. The eventual goal would be to gradually resolve those warnings over time. Most of these failures are relatively harmless, but there are some quite nasty ones in there.

The tool checks the scripts against the lua documentation file, there are lots of updates to it to ensure that it is correct.

The check can be run locally (after installation) with python ./Tools/scripts/run_lua_language_check.py but the recommended way would be to use the VSCode extension which reports the same issues in any case (although we should add the VSCode config to point the extension at the new setup file).

This also enables automatic generation of the docs in a easier to read .md format. This can be generated with ./Tools/scripts/generate_lua_docs.sh. This is added to CI and the docs are uploaded as a artefact.

For example: ScriptingDocs.md

The eventual goal would be to convert this to a rst format for direct inclusion into the wiki.

Copy link
Contributor

@tridge tridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is in CI, and fails PRs if not passing, then alls devs would need to be able to run the checks locally, so will all devs need to install brew ?

.github/workflows/test_scripting.yml Outdated Show resolved Hide resolved
Tools/scripts/generate_lua_docs.sh Outdated Show resolved Hide resolved
@tridge tridge removed the DevCallEU label May 8, 2024
@tridge
Copy link
Contributor

tridge commented May 8, 2024

@IamPete1 maybe we could chat during the week and discuss the developer tooling implications?

@IamPete1
Copy link
Member Author

This is the install instructions: https://luals.github.io/#other-install

There are several ways, I used homebrew because I found a example of using it for the install in GitHub actions. I have not tried any of the others. (nor am I really qualified to know which would be best for most people)

@Ryanf55
Copy link
Collaborator

Ryanf55 commented May 13, 2024

Nice - looks like this is already part of the recommended VSCode extensions in AP.

tridge
tridge previously requested changes May 13, 2024
Copy link
Contributor

@tridge tridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need a script in Tools/environment_install/install_lua_checker.sh

@IamPete1
Copy link
Member Author

/home/peter/ardupilot/Tools/AP_Periph/Web/scripts/pppgw_webui.lua:617:17
        Code: need-check-nil
        Need check nil.
/home/peter/ardupilot/Tools/AP_Periph/Web/scripts/pppgw_webui.lua:647:20
        Code: need-check-nil
        Need check nil.
/home/peter/ardupilot/Tools/AP_Periph/Web/scripts/pppgw_webui.lua:627:13
        Code: need-check-nil
        Need check nil.
/home/peter/ardupilot/Tools/AP_Periph/Web/scripts/pppgw_webui.lua:79:28
        Code: param-type-mismatch
        Cannot assign `integer` to parameter `boolean`.
        - `integer` cannot match `boolean`
        - Type `integer` cannot match `boolean`
        - Type `number` cannot match `boolean`
/home/peter/ardupilot/Tools/AP_Periph/Web/scripts/pppgw_webui.lua:261:36
        Code: param-type-mismatch
        Cannot assign `number|nil` to parameter `integer`.
        - `nil` cannot match `integer`
        - Type `nil` cannot match `integer`
/home/peter/ardupilot/Tools/AP_Periph/Web/scripts/pppgw_webui.lua:671:28
        Code: param-type-mismatch
        Cannot assign `nil` to parameter `fun(...any):...unknown`.
        - `nil` cannot match `fun(...any):...unknown`
        - Type `nil` cannot match `fun(...any):...unknown`
/home/peter/ardupilot/Tools/AP_Periph/Web/scripts/pppgw_webui.lua:712:42
        Code: param-type-mismatch
        Cannot assign `string|nil` to parameter `string|number`.
        - `nil` cannot match `string|number`
        - `nil` cannot match any subtypes in `string|number`
        - Type `nil` cannot match `number`
        - Type `nil` cannot match `string`
/home/peter/ardupilot/Tools/AP_Periph/Web/scripts/pppgw_webui.lua:714:39
        Code: param-type-mismatch
        Cannot assign `string|nil` to parameter `string|number`.
        - `nil` cannot match `string|number`
        - `nil` cannot match any subtypes in `string|number`
        - Type `nil` cannot match `number`
        - Type `nil` cannot match `string`
/home/peter/ardupilot/Tools/AP_Periph/Web/scripts/pppgw_webui.lua:617:22
        Code: undefined-field
        Undefined field `read`.
/home/peter/ardupilot/Tools/AP_Periph/Web/scripts/pppgw_webui.lua:647:25
        Code: undefined-field
        Undefined field `read`.
/home/peter/ardupilot/Tools/AP_Periph/Web/scripts/pppgw_webui.lua:627:18
        Code: undefined-field
        Undefined field `seek`.
/home/peter/ardupilot/Tools/AP_Periph/Web/scripts/pppgw_webui.lua:913:22
        Code: redundant-parameter
        This function expects a maximum of 0 argument(s) but instead it is receiving 1.

@IamPete1 IamPete1 force-pushed the scripting_checks_2 branch from 9d916b6 to 0956628 Compare May 19, 2024 23:27
@IamPete1 IamPete1 requested a review from tridge May 19, 2024 23:28
@IamPete1 IamPete1 dismissed tridge’s stale review May 19, 2024 23:28

Now installs itself without brew

@IamPete1 IamPete1 force-pushed the scripting_checks_2 branch from 0956628 to 3005cb8 Compare May 19, 2024 23:31
@IamPete1 IamPete1 force-pushed the scripting_checks_2 branch from 3005cb8 to d60477d Compare May 19, 2024 23:35
@tridge tridge merged commit 91cdea1 into ArduPilot:master May 20, 2024
92 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants