ansible / ansible-language-server

🚧 Ansible Language Server codebase is now included in vscode-ansible repository
https://github.com/ansible/vscode-ansible
MIT License
248 stars 91 forks source link

Language server fails with exception when ansible.ansibleLint.arguments is not set #139

Closed strayer closed 2 years ago

strayer commented 2 years ago

Summary

I'm using ansiblels installed via https://github.com/williamboman/nvim-lsp-installer with https://github.com/neovim/nvim-lspconfig/ in neovim 0.6 (HEAD / nightly).

When I'm opening an ansible file, linting with ansible-lint is not working. The log file (see log output below) pointed me to this code: https://github.com/ansible/ansible-language-server/blob/30a3a89/src/services/ansibleLint.ts#L64-L69

The default configuration of nvim-lspconfig does not set ansible.ansibleLint.arguments which results in this code calling match on undefined in line 67. When I set ansible.ansibleLint.arguments to an empty string, everything works as expected.

coc-ansible seems to default this to an empty string. I'm not sure if this is a bug in ansible-language-server or the default configuration in nvim-lspconfig. If it is the latter I'll gladly open a PR there to fix this.

Extension version

latest nvim-lspconfig

VS Code version

using nvim-lspconfig with neovim 0.6 HEAD

Ansible Version

ansible [core 2.11.6] 
  config file = /Users/strayer/dev/valheim-dedicated-server/ansible.cfg
  configured module search path = ['/Users/strayer/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/Cellar/ansible/4.8.0/libexec/lib/python3.10/site-packages/ansible
  ansible collection location = /Users/strayer/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/local/bin/ansible
  python version = 3.10.0 (default, Oct 13 2021, 06:45:00) [Clang 13.0.0 (clang-1300.0.29.3)]
  jinja version = 3.0.2
  libyaml = True

OS / Environment

macOS Monterey 12.1 Beta (21C5039b), ansible-lint 5.2.1

Relevant log output

[START][2021-11-23 12:47:01] LSP logging initiated
[WARN][2021-11-23 12:47:01] ...lsp/handlers.lua:108 "The language server ansiblels triggers a registerCapability handler despite dynamicRegistration set to false. Report upstream, this warning is harmless"
[WARN][2021-11-23 12:47:01] ...lsp/handlers.lua:108 "The language server ansiblels triggers a registerCapability handler despite dynamicRegistration set to false. Report upstream, this warning is harmless"
[WARN][2021-11-23 12:47:01] ...lsp/handlers.lua:108 "The language server ansiblels triggers a registerCapability handler despite dynamicRegistration set to false. Report upstream, this warning is harmless"
[ERROR][2021-11-23 12:47:01] ...lsp/handlers.lua:403    "An error occurred in 'onDidOpen' handler: [TypeError] Cannot read properties of undefined (reading 'match')\nTypeError: Cannot read properties of undefined (reading 'match')\n    at AnsibleLint.<anonymous> (/Users/strayer/.local/share/nvim/lsp_servers/ansiblels/out/server/src/services/ansibleLint.js:59:73)\n    at Generator.next (<anonymous>)\n    at fulfilled (/Users/strayer/.local/share/nvim/lsp_servers/ansiblels/out/server/src/services/ansibleLint.js:5:58)"
ssbarnea commented 2 years ago

I think it would be up to @yaegassy to review this one but at the same time it should be easy to make a pull request that ensures that we fallback to empty string if the configured value is undefined or not already a string, so we avoid a crash. Feel free to make a PR and you can even add a comment to it explaining that this is a fail-safe mechanism for potentially unexpected configuration.

yaegassy commented 2 years ago

@strayer Adjust the settings on the nvim-lspconfig side. https://github.com/neovim/nvim-lspconfig/blob/master/lua/lspconfig/ansiblels.lua#L8-L24

ssbarnea commented 2 years ago

@strayer I made a PR that should address your problem but it would be really useful if you can describe exactly how can someone use our ALS with neovim, maybe we can add one additional GHA for testing integration with it?

strayer commented 2 years ago

@ssbarnea Thank you! I planned on doing exactly the same change later this evening, but you beat me to it :)

In which way should I describe the setup? It is rather complicated, as pretty much every customized neovim setup, but the basics are relatively simple. This problem should happen as soon as one is using the current neovim stable release with https://github.com/neovim/nvim-lspconfig.

With ansible-language-server and nvim-lspconfig installed the init.vim should really only need to have this to get the integration running:

lua << EOF
require'lspconfig'.ansiblels.setup{}
EOF

I'm not sure how to automate this using a Github action though. My knowledge about neovim internals is rather limited. As far as my experience with neovim goes there should be some way to test the language server with lua code.

Edit:

With #142 merged I'd also not open a PR at nvim-lspconfig to avoid causing too much trouble there, since the defaults should work fine in future. Do you agree?

ssbarnea commented 2 years ago

You don't need GHA knowledge, we can help he. As long you can come-up with a script or step by step guide with few test steps, we can take care of the rest. Stuff like `install editor, install plugin, execute this command".

Yep defaults should work better but I am sure that w/o some testing we will soon find something else breaking. The only way to ensure we keep it working is to have a level of tests on our CI.

strayer commented 2 years ago

Github Actions wouldn't be an issue. I'm really not competent enough with lua internals of neovim to get something like this going, sorry :( As far as I can see some kind of lua script would need to launch neovim with an example ansible playbook and expect specific linter messages via internal lua LSP APIs (at least to cover this issue). In case I find the time to get into the thick of this I'll gladly contribute, but I'd rather make clear that I probably won't be able to help with this.