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

Using vim.emtpy_dict or empty table in combination with deep_extend is breaking user configs #3081

Open
asmodeus812 opened this issue Mar 18, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@asmodeus812
Copy link

asmodeus812 commented Mar 18, 2024

Description

This is quite an obscure problem but, but here is the issue. We have the following static default_config map which is initialized when the util file is sources like that

M.default_config = {
  log_level = lsp.protocol.MessageType.Warning,
  message_level = lsp.protocol.MessageType.Warning,
  settings = vim.empty_dict(),
  init_options = vim.empty_dict(),
  handlers = {},
  autostart = true,
  capabilities = lsp.protocol.make_client_capabilities(),
}

Then in several places we deep_extend from user's config like that

  local default_config = tbl_deep_extend('keep', config_def.default_config, util.default_config)

This seems like a good idea, until you realize that if the config_def.default_config config has no settings or init_options, the deep extend will simply copy by reference of the vim.empty_dict and not 'deep' copy it (same is applicable for {}). This means that the result of the deep extend will now carry not a copy of the default init_options or settings but the reference created from vim.emtpy_dict itself.

Now every subsequent deep_extend executed on the result of the above deep_extend (default_config) will extend the initial reference and not a copy / clone of it for settings or init_options.

This is something i noticed when i started passing around default configs without init_options, and having init_options be dynamically extending the current config after on_setup, where some configs would be coming in with properties from another unrelated server.

Here is a reproduction steps on what is going on

local default_config = {
  settings = vim.empty_dict(),
  init_options = vim.empty_dict(),
  handlers = {},
}

local emtpy1 = {
    root_dir = "e1",
    cmd = { "test1" }
}

local empty2 = {
    root_dir = "e2",
    cmd = { "test1" }
}

local extend1 = vim.tbl_deep_extend('keep', emtpy1, default_config)
extend1.init_options.from_extend_one = "extend1"
extend1.handlers.from_extend_one = "extend1"
vim.print(extend1)

local extend2 = vim.tbl_deep_extend('keep', empty2, default_config)
extend2.init_options.from_extend_two = "extend2"
extend2.handlers.from_extend_two = "extend2"
vim.print(extend2)

You would notice that extend2 has the property _extend_one on top of the newly attached _extend_two. Same goes for handlers table too. It would seem deep extend would not deep copy empty tables, which might cause unexpected side effects.

The solution i recommend is vim.fn.deepcopy or equivalent the default_config provided by nvim-lspconfig, to create a proper deep copy of all referenced tables.

lua local default_config = tbl_deep_extend('keep', config_def.default_config, vim.fn.deepcopy(util.default_config))

@asmodeus812 asmodeus812 added the bug Something isn't working label Mar 18, 2024
@asmodeus812 asmodeus812 changed the title Using vim.emtpy_dict in combination with deep_extend is breaking user configs Using vim.emtpy_dict or empty table in combination with deep_extend is breaking user configs Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant