elixir-tools / credo-language-server

LSP implementation for Credo.
MIT License
96 stars 11 forks source link

Different results from 'mix credo' #39

Closed buildreactive closed 1 year ago

buildreactive commented 1 year ago

I'm trying to figure out why I get different results from mix credo versus using the elixir-tools VSCode plugin.

In both cases, I'm working from the same directory (an umbrella project). But on the command line I'm seeing 11 issues (1 consistency issue, 8 code readability issues, 3 software design suggestions), whereas in VSCode I'm seeing 6 issues.

The really odd bit is that it's not a clear subset. There are issues being reported on the command line that are unique to the CLI, and issues in VSCode that are unique to VSCode.

Examples: VSCode:

CLI:

It's basically as if each is running a different .credo.exs (but, I only have one, at the root of the umbrella project).

mhanberg commented 1 year ago

The extension runs credo with --strict --all, can you double check and run the mix task with that flags?

Do you have any custom checks or 3rd party checks?

buildreactive commented 1 year ago

Did a little bit of experimenting:

  1. Turned off the 'Credo (Elixir Linter)' plugin
  2. Reload
  3. Turned on 'elixir-tools.vscode'

Preconditions:

  1. Code is pointing at my umbrella project (umbrella has two apps in it)
  2. Confirmed there is a default '.credo.exs' in the umbrella root as well as application roots (I've observed in the past that having it in the root is necessary; having it in the child projects is not, but not doing so causes 'Credo (Elixir Linter)' plugin to complain)
  3. No custom or third party checks that I'm aware of

Test:

  1. Code output immediately popped up and shows me a total of 7 issues.
  2. Ran 'mix credo --strict --all' in the CLI, and see a total of 20 issues reported.

Notable differences include the following, which are NOT appearing in Code:

mhanberg commented 1 year ago

I'll have to try it out in an umbrella app.

Are there issues being reported for each child app? or are they all coming from a single child?

buildreactive commented 1 year ago

I resolved a bunch of them... I believe they were cross-app. I'll keep my eyes on it and let you know if that's wrong.

mhanberg commented 1 year ago

another thing I realized...

VSCode doesn't show you the "hints" in the two little numbers at the bottom, nor do they show up in the 'problems' tab, only warnings and errors.

mhanberg commented 1 year ago

Ahh, it only shows Errors, Warnings, and Information (but not Hints).

I'll probably just change the hints checks to information. it was unclear to me how to map them. then they should all show up.

this will make it simpler to see what the problem you are facing is

mhanberg commented 1 year ago

I am fairly convinced this was just the hints not showing up in the problems tab.

The change to make hints -> information severity will be published in the next day or so.

tensiondriven commented 10 months ago

I'm also seeing inconsistencies between mix credo and the Problems displayed in vscode. It looks like the differences are due to --all --strict.

Our team runs mix credo without any arguments, so the use of --all --strict is adding a lot of noise to my vscode Problems output.

For now, I've deleted my local .credo.exs and added it to my global gitignore, but ideally I'd like to be able to set the flags that elixir-tools uses when it detects a credo exs.

Another option would be to add a checkbox to the elixir-tools config for "Disable credo checks" so i coudl disable them entirely, even when I have a .credo.exs.

Not sure this rises to the level of a separate issue, so commenting here. Thanks for all your work on elixir-tools, this is sorely needed!!

mhanberg commented 10 months ago

Adding configuration for the flags the LSP uses is on my roadmap, but for the Credo extension in Next LS

But, I'll add it to Credo Language Server as well once I (or someone) gets to it

tensiondriven commented 10 months ago

Thanks - I found an issue in the Next LS project and and added a comment there. Thanks again for all your work on this!

mhanberg commented 10 months ago

Are you using Next LS or Credo Language Server?

tensiondriven commented 10 months ago

Next LS 💯