shrink util implementation by using upstream functions, avoid util in configs

[ranjithshegde]

Functions to deprecate

• path
• search_ancestors
• root_pattern
• find_git_ancestors
• find_mercurial_ancestors
• find_node_modules_ancestors
• find_package_json_ancestors

All of the above can be replaced with relevant functions from vim.fs module.

• parse_user_command_options
• create_module_commands

Both of these functions can be made private in the next releases seeing as user_commands part has already been signaled for eventual deprecation.

Proposal

1. Soft-deprecate after release of nvim 0.8 + stabilization period.
2. Internally redirect the calls to vim.fs.
3. Replace the functions inside configs/[server].lua wherever relevant
4. CI lints usage of banned/deprecated util functions. ci: check for deprecated util functions #3462
5. Provide examples/docs that use the recommended functions

[justinmk]

We don't necessarily need to remove lspconfig.util.root_pattern, we can start by updating their implementations to use vim.fs and minimizing code where possible. And where possible, show a deprecation message that explains how to use vim.fs directly instead of e.g. lspconfig.util.root_pattern. And update all of the examples in lspconfig docs to use vim.fs directly, instead of lspconfig.util.

[justinmk]

Plan

what general strategy is for removing lspconfig functions

we should keep the util stuff around to avoid breaking downstream. meanwhile, in this repo:

• CI should fail if any config uses a deprecated util function ci: check for deprecated util functions #3462
• we should remove all docs that suggest a deprecated util function

once we have isolated most of util in that way, then we can think about when/how to fully remove them.

[dundargoc]

Since we don't actually test/run any of the lua configs in CI to my knowledge, the best we can hope for is to make a string search of changed lines on PRs and complain if the deprecated function is used.

[justinmk]

The "Proposal" in the main description of this issue is updated and correct. To be clear:

• we don't need to remove anything on util, its interface can stay frozen for back-compat.
• internally, the implementation of util should shrink as we update it to use stdlib features of the minimum-supported Nvim version.
• as we bump the minimum required Nvim, we will add lint rules to the lspconfig CI which prevent configs from using util functions if the minimum supported Nvim (currently 0.9) has alternatives.

For example, search_ancestors currently doesn't use vim.fs.parents even though it has been available since Nvim 0.8

nvim-lspconfig/lua/lspconfig/util.lua

Line 248 in 39f31e1

function M.search_ancestors(startpath, func)

[justinmk]

Status

related work:

• refactor: use vim.lsp.start instead of vim.lsp.start_client #3450

[dundargoc]

I'm not really a fan of the suggested deprecation strategy. It creates extra CI work and makes the process harder because we now need to have bookkeeping of which util functions we use ourselves and which util functions are there for public use. What was so wrong about the previous deprecation strategy that we need to invent a new one on the spot?

[justinmk]

There was no previous deprecation strategy for nvim-lspconfig. The goal is to shrink nvim-lspconfig in order to make progress towards upstreaming it, while minimizing damage to user trust.

Which part of the above plan, specifically, do you want to change, and how, specifically? (Other than the CI work, which is merely a way to reduce the code review burden of nvim-lspconfig, and something that will materialize depending on the whims of the people maintaining nvim-lspconfig: #3462 .)

[justinmk]

Ok I see what you mean from the discussion in #3460

[justinmk]

#3460 is a good example of why removing util.dirname gains almost nothing. Keeping the skeleton around is low cost. Because we'll just freeze it once everything is upstreamed. And meanwhile, shrinking the actual implementation gives us a clear picture of which logic still doesn't have an upstream equivalent. The mere presence of util.dirname doesn't harm anything, though, yes, we will need to update something like .github/ci/run_sanitizer.sh to prevent future uses of util.dirname in configs.

As we hollow-out the guts of util, it will become almost empty. We can later decide whether we want to delete it entirely, and that can be done as one big "breaking" step instead of many incremental "breaking" steps.

[dundargoc]

Which part of the above plan, specifically, do you want to change, and how, specifically?

The proposed strategy is alright I guess, it's just more safe/careful than my personal preference. And I like having one deprecation strategy over all our repos if possible to reduce the mental overhead of keeping track of different strategies.

But I can work with the strategy you outlined, it's not a dealbreaker and we can make progress like you said.