nvimtools / none-ls.nvim

null-ls.nvim reloaded / Use Neovim as a language server to inject LSP diagnostics, code actions, and more via Lua.
The Unlicense
2.43k stars 72 forks source link

Deprecating builtins with unmaintained upstream or native LSP replacement #58

Closed mochaaP closed 6 months ago

mochaaP commented 8 months ago

Related projects:


I'm thinking of cleaning up some builtins with the following criteria:


Planned to remove (will keep updating): edit history ā†’ https://github.com/nvimtools/none-ls.nvim/issues/58#issuecomment-1902758817

    deleted:    code_actions/eslint.lua        (use eslint-language-server / available in none-ls-extras.nvim)
    deleted:    code_actions/eslint_d.lua      (use eslint-language-server / available in none-ls-extras.nvim)
    deleted:    code_actions/ltrs.lua          (use ltex-ls)
    deleted:    code_actions/shellcheck.lua    (use bashls / available in gbprod/none-ls-shellcheck.nvim)
    deleted:    code_actions/xo.lua            (use eslint-language-server)
    deleted:    diagnostics/bandit.lua         (use ruff)
    deleted:    diagnostics/chktex.lua         (use texlab)
    deleted:    diagnostics/clang_check.lua    (use clangd)
    deleted:    diagnostics/cpplint.lua        (use clangd / available in none-ls-extras.nvim)
    deleted:    diagnostics/curlylint.lua
    deleted:    diagnostics/deno_lint.lua      (use deno lsp)
    deleted:    diagnostics/eslint.lua         (use eslint-language-server)
    deleted:    diagnostics/eslint_d.lua       (use eslint-language-server)
    deleted:    diagnostics/flake8.lua         (use ruff / available in none-ls-extras.nvim)
    deleted:    diagnostics/gospel.lua
    deleted:    diagnostics/jshint.lua         (use eslint-language-server)
    deleted:    diagnostics/jsonlint.lua       (use jsonls)
    deleted:    diagnostics/luacheck.lua       (use selene / available in gbprod/none-ls-luacheck.nvim)
    deleted:    diagnostics/misspell.lua
    deleted:    diagnostics/php.lua            (available in gbprod/none-ls-php.nvim)
    deleted:    diagnostics/protoc_gen_lint.lua (use buf)
    deleted:    diagnostics/puglint.lua
    deleted:    diagnostics/psalm.lua          (use psalm lsp / available in gbprod/none-ls-psalm.nvim)
    deleted:    diagnostics/puppet_lint.lua
    deleted:    diagnostics/pycodestyle.lua    (use ruff)
    deleted:    diagnostics/pydocstyle.lua     (use ruff)
    deleted:    diagnostics/pylama.lua         (use ruff)
    deleted:    diagnostics/pyproject_flake8.lua (use ruff)
    deleted:    diagnostics/ruff.lua           (use ruff lsp)
    deleted:    diagnostics/semistandardjs.lua (use eslint-language-server)
    deleted:    diagnostics/shellcheck.lua     (use bashls / available in gbprod/none-ls-shellcheck.nvim)
    deleted:    diagnostics/standardjs.lua     (use eslint-language-server)
    deleted:    diagnostics/standardrb.lua     (use standardrb lsp)
    deleted:    diagnostics/tsc.lua            (use tsserver)
    deleted:    diagnostics/typos.lua          (use typos-lsp)
    deleted:    diagnostics/vulture.lua        (use ruff)
    deleted:    diagnostics/xo.lua             (use eslint-language-server)
    deleted:    formatting/autoflake.lua       (use ruff)
    deleted:    formatting/autopep8.lua        (use ruff)
    deleted:    formatting/beautysh.lua        (use shfmt)
    deleted:    formatting/brittany.lua        (use haskell-language-server)
    deleted:    formatting/cabal_fmt.lua       (use haskell-language-server)
    deleted:    formatting/deno_fmt.lua        (use deno lsp)
    deleted:    formatting/docformatter.lua    (use ruff)
    deleted:    formatting/dprint.lua          (use dprint lsp)
    deleted:    formatting/dtsfmt.lua          (upstream is missing)
    deleted:    formatting/eslint.lua          (use eslint-language-server)
    deleted:    formatting/eslint_d.lua        (use eslint-language-server)
    deleted:    formatting/fixjson.lua         (use jsonls)
    deleted:    formatting/fourmolu.lua        (use haskell-language-server)
    deleted:    formatting/htmlbeautifier.lua  (use tidy)
    deleted:    formatting/jq.lua              (use jsonls)
    deleted:    formatting/json_tool.lua       (use jsonls)
    deleted:    formatting/jsonnetfmt.lua      (use jsonnet-language-server)
    deleted:    formatting/latexindent.lua     (use texlab / available in none-ls-extras.nvim)
    deleted:    formatting/lua_format.lua      (use stylua)
    deleted:    formatting/markdown_toc.lua    (use marksman)
    deleted:    formatting/perlimports.lua     (use PerlNavigator)
    deleted:    formatting/perltidy.lua        (use PerlNavigator)
    deleted:    formatting/puppet_lint.lua
    deleted:    formatting/pyflyby.lua         (use ruff)
    deleted:    formatting/reorder_python_imports.lua (use ruff, isort or usort)
    deleted:    formatting/ruff.lua            (use ruff lsp)
    deleted:    formatting/ruff_format.lua     (use ruff lsp)
    deleted:    formatting/rustfmt.lua         (use rust-analyzer)
    deleted:    formatting/semistandardjs.lua  (use eslint-language-server)
    deleted:    formatting/standardjs.lua      (use eslint-language-server)
    deleted:    formatting/standardrb.lua      (use standardrb lsp)
    deleted:    formatting/standardts.lua      (use eslint-language-server)
    deleted:    formatting/stylish_haskell.lua (use haskell-language-server)
    deleted:    formatting/taplo.lua           (use taplo lsp)
    deleted:    formatting/templ.lua           (use templ lsp)
    deleted:    formatting/terrafmt.lua
    deleted:    formatting/trim_newlines.lua   (use editorconfig)
    deleted:    formatting/trim_whitespace.lua (use editorconfig)
    deleted:    formatting/vfmt.lua            (use vls)
    deleted:    formatting/xmlformat.lua       (use lemminx)
    deleted:    formatting/xmllint.lua         (use lemminx)
    deleted:    formatting/xq.lua              (use lemminx)
    deleted:    formatting/yq.lua              (use yamlls)
    deleted:    formatting/zigfmt.lua          (use zls)
jakenvac commented 8 months ago

I support this. Maybe create a 'legacy' branch so people can easily use an old version of the plugin if they rely on something that is due to be removed?

mochaaP commented 8 months ago

This won't happen very soon without further notice. I guess we could add a notice on startup if users are using any of the deprecated builtins, and finally remove them, for likely 1 to 2 months later?

Also, users could just copy & paste the builtins they use into their config, so I don't feel the very need to maintain a separate branch.

mochaaP commented 8 months ago

Hi @towry @tiagovla could you give some feedback and reasons on the previous -1? It will really help the decision šŸ˜„

towry commented 8 months ago

. I guess we could add a notice on startup if users are using any of the deprecated builtins

This just reminds me of a post on reddit

https://github.com/mellow-theme/mellow.nvim/blob/716a0a70563b8b986b910e7a51f60c424fee01bc/lua/mellow/init.lua#L314

kmoschcau commented 8 months ago

Hi, for the unmaintained ones, what is the main motivation to remove them? Is it just out of caution, that they might break? I have contributed two diagnostics parsers (checkstyle and pmd), which I have last touched over two years ago, but they should still work. Outright removing them might not be the best idea.

mochaaP commented 8 months ago

@kmoschcau

for the unmaintained ones

It's just for the upstream abandoned projects. pmd and checkstyle are all fine, since upstream actively maintains them :)

kmoschcau commented 8 months ago

Ah OK gotcha. I first understood that whether or not the builtins themselves are maintained or not is the deciding factor. I simply misread. In that case this sounds like a sensible idea. Carry on. šŸ˜€

mochaaP commented 7 months ago

(moved to the issue body)

wookayin commented 7 months ago

Please don't remove yapf, pyink, flake8, black, isort, pep8, etc (anything related to python/ruff). These are all still popular and widely used --- IMHO Ruff is still not mature enough to replace all the formatters.

I also find that the list you suggest is quite aggressive. Not only for python but for other langs. The removal/deprecation should be done in more like a opt-in manner rather than opt-out manner.

Also I think it'd be great to main the current, up-to-date list of the plan in the body (i.e. at the very top) of this issue because comments in the middle will get easily out of attention?

mochaaP commented 7 months ago

@wookayin:

Please don't remove yapf, pyink, flake8, black, isort, pep8, etc (anything related to python/ruff). These are all still popular and widely used --- IMHO Ruff is still not mature enough to replace all the formatters.

See the FAQ here. I tried not to break things if there are a significant number of rules not implemented.

I also find that the list you suggest is quite aggressive. Not only for python for other langs. The removal/deprecation should be done in more like a opt-in manner rather than opt-out manner.

The change may start rolling out on a separate branch first. I look forward to merging that into master in 1ā€“2 years. The removals are mostly due to the overhead on executing binaries, especially on Windows with EDR activated that hooks into LaunchProcess. Furthermore, most recommendations are using the same underlying engine.

Also I think it'd be great to main the current, up-to-date list of the plan in the body (i.e. at the very top) of this issue because comments in the middle will get easily out of attention?

I will put a link to this comment (to preserve edit history).

wookayin commented 7 months ago

See the FAQ here. I tried not to break things if there are a significant number of rules not implemented.

That's ruff's goal, but at this point not really. Ruff is a good diagnostics tool so for diagnostics maybe yes, but as a formatter I don't think it's mature enough yet. Your list includes more than what Ruff is currently capable of. For flake8 I would say it's OK (although very controversial) to remove, but ruff is never going to be the 100% exactly the same with other formatters that are still widely used in many projects, it can't be a replacement. For instance: isort, black, yapf, etc. which are all clear NO. Ruff-formatter aims to be as similar as black, although it isn't ATM, and black is not the only formatter in the Python community.

https://docs.astral.sh/ruff/faq/#how-does-ruffs-import-sorting-compare-to-isort. https://docs.astral.sh/ruff/faq/#is-the-ruff-linter-compatible-with-black

mochaaP commented 7 months ago

Fair point, thanks. I've restored isort, black, yapf, pyink from the list.

sblask commented 7 months ago

I never used flake8 and have not looked into ruff, but removing a tool because there is an alternative tool can be problematic. You don't always have the choice of what tool to use. So if ruff doesn't do exactly the same thing and uses the same configuration files it's not a good replacement. Same might be the case for other tools.

wookayin commented 7 months ago

@mochaaP Thanks for considering the suggetsion!

but removing a tool because there is an alternative tool can be problematic.

+1, I second this point by @sblask. None-ls.nvim should be deprecating things only if the project is abandoned (or more precisely, users are also leaving and abandoning it) and good modern alternatives are available. There could be inactive, not-so-quite-updated tools that can still work great and be used in many projects. None-ls.nvim is a collection of tools that users can pick and start using out-of-box. If a tool that's already available in our curated list is abandoned, it's provided as-is, hence no maintenance cost added.

mochaaP commented 7 months ago

Since this project is a community effort, the most important factor to take into consideration is your feedback. Thank you all for caring about this project! ā™„ļø

g-plane commented 7 months ago

dprint also has its native language server: https://github.com/dprint/dprint/pull/803

mochaaP commented 7 months ago

@g-plane: updated, thanks.

pysan3 commented 7 months ago

Hi, do you plan to remove the tools listed above also from mason.nvim and mason-tool-installer? @williamboman @WhoIsSethDaniel

Or only for the recipes defined in none-ls?

WhoIsSethDaniel commented 7 months ago

Hi, do you plan to remove the tools listed above also from mason.nvim and mason-tool-installer? @williamboman @WhoIsSethDaniel

Or only for the recipes defined in none-ls?

There's no change to mason-tool-installer. It will continue to support whatever mason can download.

g-plane commented 7 months ago

rustfmt shouldn't be deprecated. rust-analyzer won't do anything about formatting, and the rust-analyzer config in lspconfig has nothing about rustfmt or formatting.

mochaaP commented 7 months ago

šŸ‘Œ

gbprod commented 7 months ago

Hi! I think it's a good thing to remove obsolete/unmaintained tools or tools with better alternative (LSP) !

Anyway, I still using the php builtin (mainly because I sometimes don't want to use a LSP server and because it's faster) and I've tried to create a none-ls plugin here : https://github.com/gbprod/none-ls-php.nvim. I think it could be a good alternative for some tools that none-ls's core team does'nt want to maintain, maybe it could be documented or liked somewhere ? I'll probably do the same with luacheck that I'm still using.

Thanks for your work !

mochaaP commented 7 months ago

Hi @gbprod, Your project was already featured here! ;)

altermo commented 7 months ago

Why is typos on the list? (It is actively maintained (or am I missing something))

mochaaP commented 7 months ago

@altermo: https://github.com/neovim/nvim-lspconfig/tree/master/lua/lspconfig/server_configurations/typos_lsp.lua typos-lsp is available from nvim-lspconfig ;)

Nargonath commented 7 months ago

rustfmt shouldn't be deprecated. rust-analyzer won't do anything about formatting, and the rust-analyzer config in lspconfig has nothing about rustfmt or formatting.

You can definitely install rustfmt through rustup but how do you link the rustup install with none-ls? For now I still install rustfmt through mason-null-ls.

mochaaP commented 7 months ago

@g-plane (https://github.com/nvimtools/none-ls.nvim/issues/58#issuecomment-1913483872):

https://github.com/rust-lang/rust-analyzer/blob/master/crates/rust-analyzer/src/caps.rs#L71 https://github.com/rust-lang/rust-analyzer/blob/master/crates/rust-analyzer/src/handlers/request.rs#L1915 I suppose rustfmt is actually included in rust-analyzer? Correct me if I'm wrong.

gbprod commented 7 months ago

FYI, I use psalm with none-ls but I know there is a LSP alternative, I don't know if we should deprecate this builtin or not :sweat_smile: but if you do, I can create another plugin ;)

g-plane commented 7 months ago

If I disable none-ls and enable rust-analyzer with lspconfig only, it won't format. Not sure whether my config is wrong or there's a bug in lspconfig.

g-plane commented 7 months ago

My config is wrong, sorry.

mochaaP commented 7 months ago

@gbprod:

Yeah, I missed that. You can go ahead and create another plugin! :)

Nargonath commented 7 months ago

My config is wrong, sorry.

Can you share what was wrong with your config then, please? I have rust-analyzer setup as well but it doesn't format if I don't setup rustfmt through mason-null-ls and none-ls. I looked into the settings and I saw rust-analyzer actually supports formatting but I couldn't make it work.

g-plane commented 7 months ago

@Nargonath You need to configure like this:

local function format_on_save(client, bufnr)
  vim.api.nvim_clear_autocmds({ group = augroup, buffer = bufnr })
  vim.api.nvim_create_autocmd("BufWritePre", {
    group = augroup,
    buffer = bufnr,
    callback = function()
      vim.lsp.buf.format({ bufnr = bufnr })
    end,
  })
end

local lspconfig = require("lspconfig")
lspconfig.rust_analyzer.setup({
  on_attach = format_on_save,
})

If you've already configured the on_attach with something else, for example, I'm using "lspstatus", you can configure like this:

local function format_on_save(client, bufnr)
  vim.api.nvim_clear_autocmds({ group = augroup, buffer = bufnr })
  vim.api.nvim_create_autocmd("BufWritePre", {
    group = augroup,
    buffer = bufnr,
    callback = function()
      vim.lsp.buf.format({ bufnr = bufnr })
    end,
  })
end

local lspconfig = require("lspconfig")
lspconfig.rust_analyzer.setup({
  on_attach = lspconfig.util.add_hook_after(format_on_save, require("lsp-status").on_attach),
})

Hope it helps.

g-plane commented 7 months ago

There's an official Ruff language server, so we can deprecate Ruff as well:

Nargonath commented 7 months ago

Thanks @g-plane for sharing your setup. It's definitely helpful as I do the same but in a different way and I think I understand why it's not working now. I use lazy.nvim for my plugins management. Here is what I have on my side:

-- CONFIG_ROUTE/lua/plugins/lspconfig.lua
local augroupFormat = vim.api.nvim_create_augroup("LspFormatting", {})

return {
  {
    "williamboman/mason.nvim",
    config = true,
  },
  {
    "williamboman/mason-lspconfig.nvim",
    opts = {
      ensure_installed = {
        "tsserver",
        "dockerls",
        "bashls",
        "terraformls",
        "tailwindcss",
        "rust_analyzer",
        "taplo",
        "graphql",
      },
      handlers = {
        function(server_name)
          local settings = {
            common = {
              capabilities = require("cmp_nvim_lsp").default_capabilities(),
              flags = {
                debounce_text_changes = 150,
              },
            },

            tsserver = {
              settings = {
                typescript = {
                  format = {
                    enable = false,
                  },
                },
                javascript = {
                  format = {
                    enable = false,
                  },
                },
              },
            },
          }

          require("lspconfig")[server_name].setup(
            vim.tbl_deep_extend("force", settings.common, settings[server_name] or {})
          )
        end,
      },
    },
  },
  {
    "nvimtools/none-ls.nvim",
    opts = {
      on_attach = function(client, bufnr)
        if client.supports_method("textDocument/formatting") then
          vim.api.nvim_clear_autocmds({ group = augroupFormat, buffer = bufnr })
          vim.api.nvim_create_autocmd("BufWritePre", {
            group = augroupFormat,
            buffer = bufnr,
            callback = function()
              vim.lsp.buf.format({ async = false })
            end,
          })
        end
      end,
    },
    dependencies = { "nvim-lua/plenary.nvim", "neovim/nvim-lspconfig" },
  },
-- other dependencies like mason-null-ls and nvim-lspconfig
}

Initially in my setup, only tools registered through none-ls were used for formatting so it was working for me to declare the on_attach function (similar to what you have) through none-ls. The problem is that rust_analyzer is handled through nvim-lspconfig (with help of mason and mason-lspconfig) so it doesn't get the on_attach function which is why the formatting doesn't work with it. Once I installed rustfmt through mason-null-ls then my current on_attach setup work and I could use it but since this usage gets deprecated it's not an acceptable solution. Setting the same on_attach function in my handlers setup code in mason-lspconfig should fix it. I'll report once it's fixed. I decided to share it since it might help somebody else.

Nargonath commented 7 months ago

Doing what I mentioned above fixed my problem. Thanks @g-plane for your insight. šŸ™

niksingh710 commented 6 months ago

Doing what I mentioned above fixed my problem. Thanks @g-plane for your insight. šŸ™

found any way to use the rustfmt of system instead of the one from mason?

Nargonath commented 6 months ago

@niksingh710 Yes I gave the solution above. Basically assigning an on_attach handler (same as the one I shared) on your rust_analyzer LSP will allow enabling formatting through rust_analyzer instead of mason rustfmt installation.

niksingh710 commented 6 months ago

@niksingh710 Yes I gave the solution above. Basically assigning an on_attach handler (same as the one I shared) on your rust_analyzer LSP will allow enabling formatting through rust_analyzer instead of mason rustfmt installation.

means currently rust_analyzer not having any way to format? as i don't see any config option to enable for formatting.

Nargonath commented 6 months ago

@niksingh710 You're right. There isn't a specific options like format: true or something like that. It's rather a matter of calling the vim.lsp.buf.format() function that will send a formatting command to the LSP. If the LSP supports the formatting command then it will work. You can find an example in rust_analyzer docs: https://rust-analyzer.github.io/manual.html#nvim-lsp.

My own config if you're curious ```lua local augroupFormat = vim.api.nvim_create_augroup("LspFormatting", {}) local formattingOnAttach = function(client, bufnr) if client.supports_method("textDocument/formatting") then vim.api.nvim_clear_autocmds({ group = augroupFormat, buffer = bufnr }) vim.api.nvim_create_autocmd("BufWritePre", { group = augroupFormat, buffer = bufnr, callback = function() vim.lsp.buf.format({ async = false }) end, }) end end return { { "williamboman/mason.nvim", config = true }, { "williamboman/mason-lspconfig.nvim", opts = { ensure_installed = { "tsserver", "dockerls", "bashls", "terraformls", "tailwindcss", "rust_analyzer", "taplo", "graphql", }, handlers = { function(server_name) local settings = { common = { capabilities = require("cmp_nvim_lsp").default_capabilities(), flags = { debounce_text_changes = 150, }, }, rust_analyzer = { on_attach = formattingOnAttach, }, } require("lspconfig")[server_name].setup( vim.tbl_deep_extend("force", settings.common, settings[server_name] or {}) ) end, }, }, }, -- other plugins like none-ls, mason-null-ls } ```
niksingh710 commented 6 months ago

@Nargonath according to ur config also rustfmt required to be installed by mason na? As mine is quite similar to ur's I just don't prefer auto formatting. Instead, I like auto save and I format it manually. (Much predicted behavior for me šŸ˜…)

Nargonath commented 6 months ago

@niksingh710 Why do you say that rustfmt is required to be installed by mason? I'm not sure whether rust-analyzer has rustfmt shipped with it or if it needs a globally available rustfmt binary. I do have it installed with rustup component add rustfmt though. I should try to remove it and see if it still works.

On my side I like autoformat on save that's why I added the autocmd. If you don't want it to happen automatically, you can bind vim.lsp.buf.format to a keymap instead and trigger it when you want.

niksingh710 commented 6 months ago

@Nargonath Mason gives deprecated warning for rustfmt. if i install rustfmt via rustup then lsp format fails to format but if i install it via mason it works. rn i have it by both but in neovim it only works if i have it through Mason. (so the deprecated warning kindda bothers me)

AlexKurisu commented 6 months ago

Please, don't deprecate shellcheck as bash-ls is 1) too heavy (requires nodejs) and 2) a bash LSP, so it does not support any other POSIX-compatible shell and cannot enforce POSIX compatibility

Nargonath commented 6 months ago

@niksingh710 I had the same problem as you before when I didn't add the on_attach that I shared in my previous post. If it still doesn't work for you then perhaps you didn't pass the on_attach properly IMO.

mochaaP commented 6 months ago

@AlexKurisu:

Please, don't deprecate shellcheck as bash-ls is 1) too heavy (requires nodejs) and 2) a bash LSP, so it does not support any other POSIX-compatible shell and cannot enforce POSIX compatibility

(1) makes sense, but I tested (2) and bash-ls reports POSIX-sh violations correctly when using the right shebang, since it also uses shellcheck if it's installed.

I roughly reviewed the code of bash-ls, it's essentially just tree-sitter + shellcheck + explainshell. How about creating a separate plugin to include all these features + none-ls.nvim integration in a package, so people could have fewer reasons to depend on node?

I will keep the shellcheck builtin in the meantime. edit: I forgot https://github.com/gbprod/none-ls-shellcheck.nvim exists. Please use that instead.

mochaaP commented 6 months ago

also cc @gbprod on the last one: do you want to take this?

niksingh710 commented 6 months ago

@niksingh710 I had the same problem as you before when I didn't add the on_attach that I shared in my previous post. If it still doesn't work for you then perhaps you didn't pass the on_attach properly IMO.

ig i have implemented on_attach correctly but the also if you have time and can look at my config

Nargonath commented 6 months ago

@niksingh710 I quickly looked at your config but it's not easily identifiable what's wrong with it. Your organization is not that straightforward. IMO you should strip everything and simplify your neovim/lspconfig config as much as you can to make it work. It might help you understand what's going on and then apply your findings to your real config.

gbprod commented 6 months ago

also cc @gbprod on the last one: do you want to take this?

You're talking about explainshell? I'm not using this tool currently and I prefer (for the moment) only create plugins for tools that I actually use :sweat_smile: