jose-elias-alvarez / null-ls.nvim

Use Neovim as a language server to inject LSP diagnostics, code actions, and more via Lua.
Other
3.63k stars 793 forks source link

phpmd built-in not working out of the box #1316

Open matiascaniete opened 1 year ago

matiascaniete commented 1 year ago

Hi, I recently started to migrate all of the php analysis tools I use in VSCode and migrate them to NVim. So I found this awesome plugin that really help me specially with phpmd. Although didn't work out of the box, the reason why is beause the binary requires 3 arguments: filename or directory, output format and finally the ruleset which is missing in this line:

https://github.com/jose-elias-alvarez/null-ls.nvim/blob/eaacba0b93c416252894f8bdc68e6b50b4e4c3b4/lua/null-ls/builtins/diagnostics/phpmd.lua#L14

phpmd usage docs: https://phpmd.org/documentation/index.html

I don't know if this was intended or just a bug.

In the meanwhile I made it work by configuring the plugin:

    require("null-ls").builtins.diagnostics.phpmd.with({
      extra_args = function()
        if vim.fn.filereadable("phpmd.xml") == 1 then
          return { "phpmd.xml" }
        else
          return { "cleancode,codesize,controversial,design,naming,unusedcode" }
        end
      end
    }),

Even this works, I believe may be a better way to solve it.

jose-elias-alvarez commented 1 year ago

I don't really know about the structure of PHP projects, but your solution looks good. We have other built-ins that require user configuration, so I think adding a note to the documentation that users need to specify a ruleset is enough.

matiascaniete commented 1 year ago

Independent of how a PHP project may be structured, I think could be nice to have a minimal working out-of-the-box experience when this built-in is installed. It could be fixed by just adding a 3rd argument as a default:

Replacing this:

args = { "$FILENAME", "json" },

By this

args = { "$FILENAME", "json", "cleancode,codesize,controversial,design,naming,unusedcode" },

If you agree I could do a PR

jose-elias-alvarez commented 1 year ago

Wouldn't that make it difficult for users who want to modify the default ruleset? Adding extra arguments is fairly easy, but removing them is more involved - users would have to copy-paste the default arguments and override them completely.

HalibutGitWiz commented 1 year ago

I agree, enforcing a value for the third parameter would make things more complicated. A note in the documentation is fine.

I got the same problem and I'm trying a slightly different approach : a global XML ruleset file stored in my home folder and applied to all projects.

I manage to make it work with the following config:

null_ls.builtins.diagnostics.phpmd.with { extra_args = { "/home/user/.config/nvim/lua/custom/lsp/settings/phpmd.xml"} },

This is acceptable, but for the portability of my config, I'd rather use ~ or $HOME instead of /home/user. Curiously, neither works. This does not seem to be related to PHPMD because both work fine in the terminal:

phpmd index.php json ~/.config/nvim/lua/custom/lsp/settings/phpmd.xml
phpmd index.php json $HOME/.config/nvim/lua/custom/lsp/settings/phpmd.xml

Any idea why ?

jose-elias-alvarez commented 1 year ago

That's because your shell is performing expansion. Try using vim.fn.expand() on the path and it should work.

HalibutGitWiz commented 1 year ago

That was it, thanks !