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.39k stars 69 forks source link

golangci_lint: parse text line instead of relying on object from linter #144

Closed academo closed 2 months ago

academo commented 2 months ago

While working with go and the golangci-lint I noticed that the linter was reporting all the errors in line 1.

This is an example of a wrong report:

{
  ExpectNoLint = false,
  ExpectedNoLintLinter = "",
  FromLinter = "typecheck",
  Pos = {
    Column = 0,
    Filename = "pkg/analysis/passes/includesbundled/includesbundled.go",
    Line = 1,
    Offset = 0
  },
  Replacement = vim.NIL,
  Severity = "",
  SourceLines = { "package includesbundled" },
  Text = ": # github.com/grafana/plugin-validator/pkg/analysis/passes/includesbundled\npkg/analysis/passes/includes
bundled/includesbundled.go:43:2: mainMetadata declared and not used"
}

Notice how it reports Line=1 and Column=0. even though the text clearly indicates an error in line 43.

This fix instead relies on parsing the text to indicate the line and column of the error which is more reliable.

Interestingly if I manually run golangci-lint in my terminal against this same file I get the correct Line and Column which makes me suspect this is something about how the file is sent to the linter.

I am not sure if this an error in the linter (which most likely is) but this fixes the problem for me and might fix it for others.

A fallback is implemented in case the text can't be parsed

mochaaP commented 2 months ago

I think this is an issue in golangci-lint. Please report it there instead.

academo commented 2 months ago

I think this is an issue in golangci-lint. Please report it there instead.

most likely, but I do want it fixed in my environment so I am proposing this fix no idea if they will even fix it.

mochaaP commented 2 months ago

most likely, but I do want it fixed in my environment so I am proposing this fix no idea if they will even fix it.

You can use the .with method to override builtin logic, if you'd like a hotfix in your environment. :) I will try to raise an issue upstream later.

mochaaP commented 2 months ago

I could not find the typecheck linter in upstream golangci-lint linters. Is this a custom configuration?

academo commented 2 months ago

@mochaaP this is the linter https://github.com/golangci/golangci-lint

I don't have a custom configuration though.

Great idea about using with. I'll go with that. It is strange I've been doing tests and sometimes golangci-lint works as expected and sometimes it does this depending of the project so I do suspect something in my environment.

I'll go with the with solution for now. Feel free to close this PR