iampeterbanjo / vscode-elixir-linter

Provides linting for Elixir files using Credo for Visual Studio Code
11 stars 9 forks source link

can't get the linting to work #35

Open happysalada opened 6 years ago

happysalada commented 6 years ago

Is there something specific to do besides having credo and adding the extension to vs code ?

when I run mix credo --strict I get an output that I can't see linted in vscode The only plugin I'm running on vscode related to elixir is the elixirLs one.

Has anyone had this before ?

iampeterbanjo commented 6 years ago

you should see the output of the linter when you open an Elixir language file in VS code

iampeterbanjo commented 6 years ago

if you're still having problems maybe your settings are causing an issue like in this case

happysalada commented 6 years ago

thanks for helping on this. After checking, the issue you pointed out isn't related to my problem though.

iampeterbanjo commented 6 years ago

Are you using windows? I recently saw an issue while I was debugging the extension that VS code will sometimes report the file type as "plainttext" instead of "elixir" which of course meant that the file would not get linted. I didn't get to the root cause of it but I'll keep a look out for it in the future.

My scenario is that I'm now running Windows 10, the Elixir project is on a separate SSD drive with Fat32 formatting. What is your setup?

happysalada commented 6 years ago

Thanks for taking a look at this. I'm on a mac with the latest Sierra update as of now.

alex88 commented 6 years ago

Same here, I've credo installed in an umbrella app, mix credo return code readability errors but I don't see any in the problems tab nor the editor, the only other elixir-related extension I have is ElixirLS

iampeterbanjo commented 6 years ago

@alex88 thanks for joining. what OS are you on (Mac/Windows/Linux)? If you run credo from the command-line do you see any lint issues?

alex88 commented 6 years ago

@iampeterbanjo OSX, VSCode 1.28.1, credo 0.10.2. I'm in the root of an umbrella app. Config file is in the root (.credo.exs) Example file (mix.exs):

defmodule MyApp.Umbrella.MixProject do
  use Mix.Project

  def project do
    [
      apps_path: "apps",
      start_permanent: Mix.env() == :prod,
      deps: deps(),
      aliases: aliases()
    ]
  end

  defp deps do
    [
      {:credo, "~> 0.10.0", only: [:dev, :test], runtime: false}
    ]
  end

  defp aliases() do
    [
      lint: ["format", "credo --strict"]
    ]
  end
end

saving file doesn't show any error, running mix credo triggers a warning:

  Code Readability
┃
┃ [R] ↘ Do not use parentheses when defining a function which has no arguments.
┃       mix.exs:32 #(MyApp.Umbrella.MixProject.aliases)

No errors in vscode problems tab

iampeterbanjo commented 6 years ago

OS X, nice. Are you working on a public repo?

alex88 commented 6 years ago

Nope, private, I've also tried to use:

"elixirLinter.useStrict": true,
"elixirLinter.defaultSeverity": 3,
"elixirLinter.consistencySeverity": 3,
"elixirLinter.designSeverity": 3,
"elixirLinter.readabilitySeverity": 3,
"elixirLinter.refactoringSeverity": 3,
"elixirLinter.warningsSeverity": 3

in the settings (not sure if this would help) but nothing changed

alex88 commented 6 years ago

I've just tried to do this:

$ mix new testapp
$ cd testapp
$ nano mix.exs #add credo here and change `defp deps do` in `defp deps() do`
$ mix deps.get
$ mix credo gen.config
$ nano .credo.exs #add `"."` into the included files
$ mix credo --strict
Checking 5 source files ...

  Code Readability
┃
┃ [R] ↘ Do not use parentheses when defining a function which has no arguments.
┃       mix.exs:22 #(Testapp.MixProject.deps)

Please report incorrect results: https://github.com/rrrene/credo/issues

Analysis took 0.1 seconds (0.03s to load, 0.08s running checks)
7 mods/funs, found 1 code readability issue.

My custom vscode config only has "elixirLinter.useStrict": true now, restarted vscode, no errors on the page

iampeterbanjo commented 6 years ago

I suspect the root cause may be a vscode api change or an api i'm not using correctly. I do have some technical debt i need to pay-off in this extension :cry:

thanks for chiming in - I realise now its definitely not a platform or config issue.

alex88 commented 6 years ago

I've also just tried to disable any extension other than elixir linter, for some reason I get this:

screen shot 2018-10-16 at 12 49 47 am

but no other issue in the file

iampeterbanjo commented 6 years ago

hmm... could be some kind of conflict. which other elixir lang extensions do you have installed?

alex88 commented 6 years ago

I first tried with none (as you can see from the no syntax highlighting) and with ElixirLS

Update:

PS: I had ElixirLS and vscode-elixir but those were both disabled so they shouldn't matter.

Is there a way to debug your extension code? I've tried to open the debug menu and search for your extension code in the sources tab but no luck

Update 2:

Well that message goes away if I disable vscode-elixir-linter exception (probably that [R:1] is a readability issue from credo), so maybe it's just not catching up the rules or the config file

alex88 commented 6 years ago

So, debugging the extension, I see that in the case the credo output is an array like this:

0:"[R] ↘ stdin:21 There should be no more than 1 consecutive blank lines."
1:"[R] ↘ stdin:23 Do not use parentheses when defining a function which has no arguments."
2:"[R] → stdin:1:11 Modules should have a @moduledoc tag."

since the first two problems doesn't have a column, https://github.com/iampeterbanjo/vscode-elixir-linter/blob/master/src/credoProvider.ts#L78 this returns an empty value so the first two lines are skipped because isNotAnumber is true

Removing the || isNaN(parseInt(lineInfo.column, 10)) make the extension working again, it doesn't show the error on the full line, but at least it shows the error

iampeterbanjo commented 6 years ago

@alex88 that's great detective work! thanks for getting to the bottom of it. we were discussing in #37 to upgrade to a json output from credo but that would be a breaking change because it's only in 0.9.0-rc3. But at this rate I think it would help a lot with this extension.

alex88 commented 6 years ago

Definitely :) it'll probably make the extension more future-proof rather than parsing other kind of credo output 😄 Plus you're both < 1.0.0 so some BC shouldn't matter 😄