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

pmd v7 not supported #47

Closed xixiaofinland closed 8 months ago

xixiaofinland commented 9 months ago

Issues

Feature description

pmd version7 code analyzer now switches to use "pmd check" as the main command instead of "pmd", thus null-ls throws a "not recognized parameter" error. Adding "check" into the extra_args table doesn't work.

Help

Yes, but I don't know how to start. I would need guidance

Implementation help

I locate the cmd is constructed here: https://github.com/nvimtools/none-ls.nvim/blob/ef09f14eab78ca6ce3bee1ddc73db5511f5cd953/lua/null-ls/helpers/make_builtin.lua#L101

but not sure how to change from "pmd" to "pmd check"

jakenvac commented 9 months ago

Hi, thanks for raising an issues. As this is a community maintained project it would be amazing if you could have a go at fixing it yourself. I'd be more than happy to provide whatever guidance I can.

The first place you should take a look is here: https://github.com/nvimtools/none-ls.nvim/blob/ef09f14eab78ca6ce3bee1ddc73db5511f5cd953/lua/null-ls/builtins/diagnostics/pmd.lua#L104

This is the specific builtin for the PMD diagnostics feature. The cmd field is the root command and then args is where any subcommands/flags etc. should go. So I'd imagine adding check to the args should at least get you moving in the right direction.

Typically, any builtin is configured in its respective type (Formatter, Diagnostic etc.) directory here: https://github.com/nvimtools/none-ls.nvim/tree/main/lua/null-ls/builtins

If you'd like any extra help, please could provide a link to the pmd docs for this command and a java code snippet that you'd expect PMD to provide some diagnostics for. This way those of us that are unfamiliar with java and its tools (me 😆) should still be able to assist you.

Feel free to tag me directly if you're struggling, I'm more likely to see the notification that way.

xixiaofinland commented 9 months ago

@jakenvac Thanks Jake for your kind help! In pmd v6, which null-ls supports, the command is: **pmd** + parameters, like below

pmd --format json --rulesets apex_ruleset.xml  --dir ./nebula-logger

in pmd v7, the command is **pmd check** + parameters

pmd check --format json --rulesets apex_ruleset.xml  --dir ./nebula-logger

In my null-ls configuration, I attempt to add check as part of the extra_args to form a pmd check, but it doesn't work probably because check is not a real parameter but part of the root command.

So the question boils down to, how to let none-ls to invoke pmd check instead of pmd ? I assume the root command you mentioned is the right direction for me. I will take a look later today.

xixiaofinland commented 9 months ago

Here is the error in nvim none-ls, as we can see it doesn't invoke pmd check Image 002

it is the same error in console, when I run pmd instead of pmd check Image 003

jakenvac commented 9 months ago

Hm, it's strange to me that adding check into the args doesn't work. While it's a subcommand of pmd it's still technically an argument. We've quite a few integrations that call subcommands with the args.

What error, if any, do you get when adding check to the args?

xixiaofinland commented 9 months ago

Hm, it's strange to me that adding check into the args doesn't work. While it's a subcommand of pmd it's still technically an argument. We've quite a few integrations that call subcommands with the args.

What error, if any, do you get when adding check to the args?

When I uncomment this line in my dotfiles, nvim shows exactly the same error as my first screenshot above, as if the check is ignored totally.

xixiaofinland commented 9 months ago

Image 005 When I print require'null-ls'.builtins.diagnostics.pmd I got this. It seems with in this table is a function.. not sure how to continue troubleshooting

xixiaofinland commented 9 months ago

If I try manually overwrite command to pmd check in my setting, Despite no error, null-ls isn't happy either

Image 006

Image 007

xixiaofinland commented 9 months ago

As check in extra_args not working, I created the args and put check into it. I'm not sure if this is a good practice, but it works. Here is my change in dotfiles.

Now at the beginning of the file it still shows an odd error, but PMD diagnostics display (like in line 8) Image 008

jakenvac commented 8 months ago

Could you provide a code sample to run pmd against and a link to the pmd docs? I'll see if I can recreate your issue and debug it.

xixiaofinland commented 8 months ago

Could you provide a code sample to run pmd against and a link to the pmd docs? I'll see if I can recreate your issue and debug it.

Not that I don't want to, it's a salesforce related stuff that has highly custom dotfiles. kinda difficult to reproduce in a clean setup. Since it is working now (only the odd error at the beginning, it shows the rules already. IMO, we can treat it as case resolved unless Java/Salesforce developers complains with it in the future :). Thanks Jake and happy Xmas! @jakenvac

FedeAbella commented 5 months ago

@xixiaofinland I don't know if you've completely solved this since closing, but with the stable release of pmd 7, I managed to make it work with no error messages. The null-ls doc says we need a wrapper pmd script in $PATH, since pmd v6.* used to provide only a run.sh script, and gives an example of a wrapper script that can be used:

#!/usr/bin/env bash
path/to/pmd/bin/run.sh pmd "$@"

With pmd 7, this worked just fine for me just by changing the wrapper to:

#!/usr/bin/env bash
path/to/pmd/bin/pmd check "$@"
xixiaofinland commented 3 months ago

@FedeAbella Thanks, your solution indeed works! The only downside is the pmd is now pmd check, which is not a big issue :)

ognjen-vuceljic commented 1 month ago

@xixiaofinland @FedeAbella

Thanks for the insightful conversation.

I am not sure if this thread is still active, or if we can post messages, but I'll give it a go :)

I've followed this thread closely, and applied all the suggested fixes for pmd v7. Seems like pmd check is indeed running against the directory with SF classes, however I get the following error:

image

Based on my limited knowledge, I would guess that PMD check generated a fairly large JSON that is not readable (aka out of memory) anymore. I tried excluding some classes from PMD checks, and this number from screenshot gets down, but the error still remains. I do have to note that repo I'm trying to use PMD against is huge, with a ton of classes.

Appreciating in advance any advice and suggestions.

Have a great day!

xixiaofinland commented 1 month ago

I don't use v7 in nvim at the moment as my team still uses v6.

I recall the v7 still gave an error at the first line of file (don't recall what it is) despite linting diagnostic messages display as expected.

FedeAbella commented 1 month ago

I am using v7, but have not come across that. The only issue I've come across when using v7 is with the new UnusedMethod rule on large codebases, as it would cause null-ls to timeout. But never got a json too large to parse.