Incorrect activation `root_dir` documented for NeoVim

mosheavni commented 1 year ago

Server Version

I tested every version from the latest to 0.26.0, and I saw that there are no completions from language server from 0.29.0, so the last version that has completion is 0.28.1

Terraform Version

Terraform v1.3.4
on darwin_amd64

Client Version

NVIM v0.9.0-dev-552+g4d4e697ef

Terraform Configuration Files

Log Output

here are 1000 log lines, worth noting that with 0.28.1 there are many log lines but not as much as 0.29.0 where in few seconds there are 25,000 log lines (compared to some thousands in 0.28.1) https://gist.github.com/mosheavni/90571308a9373a629e65547d5bb905c6

Expected Behavior

Completion would work

Actual Behavior

There's no completion, and sometimes there's a diagnostic message that I'm positive I saw before, in a previous issue I opened here:

Argument or block definition required: An argument or block definition is required here. To set an argument, use the equals sign "=" to introduce the

Steps to Reproduce

download this file: https://gist.github.com/mosheavni/8bc1d65d5d7aa6aeada35657353b38f8
change line 110 to the real path of the language server
write a dummy main file:

data "aws_security_group" "default" {
  name   = "default"
  vpc_id = data.terraform_remote_state.vpc.outputs.vpc.vpc_id
}

open a terraform file using this configuration:

nvim -nu init.lua main.tf

start editing, try writing re, hit <ctrl><space> to trigger autocompletion, there's no completion.

do the same with 0.28.1, and you'll see completion.

I added a video:

https://user-images.githubusercontent.com/17252601/209698157-852f6f04-2ef5-4c0a-9160-65799cba3ee4.mp4

Thanks.

radeksimko commented 1 year ago

Hi @mosheavni I did not manage to reproduce the exact behaviour you are describing.

However, I did notice, that the default settings for the language server, inherited from here contain the following line

root_dir = util.root_pattern('.terraform', '.git'),

The most appropriate way of activating the language server would be on *.tf or *.tfvars files. The .git directory seems entirely irrelevant to Terraform and .terraform only indicates whether terraform init ran in that directory, which itself is no longer a pre-requisite for functioning of the language server.

This may not necessarily be related to the problem you described however.

With the above in mind, and my limited knowledge of Lua/Vim, I managed to make your simple example work with the latest version. See the attached video. I managed to reproduce it with both latest dev build of NeoVim (v0.9.0-dev-604+g5b22b32e5-dirty as installed via brew install --HEAD neovim) and latest stable build (v0.8.2 as installed via brew install neovim). I'm running this on the Apple M1 Pro (darwin/arm64), w/ macOS Monterey 12.6.2.

Also this is how I installed the language server itself:

brew install hashicorp/tap/terraform-ls

It's not clear from your report whether you use the official build from HashiCorp or the one compiled by Homebrew. I did not manage to reproduce the exact problem you described with the latest Homebrew build either though.

https://user-images.githubusercontent.com/287584/210377641-143554ce-dda2-48f9-abc6-6284f06ef7f5.mov

Here's my full list of commands in text:

ls -la
curl -f https://gist.githubusercontent.com/mosheavni/8bc1d65d5d7aa6aeada35657353b38f8/raw/b343b10d658d8cb11d00f11b8b5bbf8f224a8107/init.lua > init.lua
sed 's+/path/to/terraform-ls-0.29.0+/opt/homebrew/bin/terraform-ls+' init.lua
grep 'terraform-ls' init.lua
cat main.tf
mkdir .terraform
nvim -nu init.lua main.tf

mosheavni commented 1 year ago

Hi, just commenting to say it will take me a bit to test. Thanks.

mosheavni commented 1 year ago

I also can't reproduce anymore. Regarding the root dir, what would you think is a better default?

radeksimko commented 1 year ago

Regarding the root dir, what would you think is a better default?

The most appropriate way of activating the language server would be on any directory which has *.tf or *.tfvars files, as we also document under https://github.com/hashicorp/terraform-ls/blob/main/docs/language-clients.md

I can send a patch changing the line in question to this, which should cover the common case

root_dir = util.root_pattern('*.tf'),

I think we'd want that to be

root_dir = util.root_pattern('*.tf', '*.tfvars'),

but then also we'd need to make sure that *.tfvars files are sent with the appropriate language ID (terraform-vars, not terraform).

It looks like NeoVim does have some ways of achieving this https://github.com/neovim/nvim-lspconfig/blob/e69978a39e4d3262b09ce6a316beff384f443e3b/lua/lspconfig/server_configurations/ltex.lua#L3-L30

Perhaps the ideal way is detecting file types (just like ltex does) and mapping those to language IDs, but I have no idea where do these come from, whether that has to be built into NeoVim, or anything like that.

mosheavni commented 1 year ago

opened https://github.com/neovim/nvim-lspconfig/pull/2379

hashicorp / terraform-ls