Skip to content
This repository has been archived by the owner on Aug 12, 2023. It is now read-only.

feat(builtins/diagnostics/credo): toggle per file/full workspace diagnostics #1465

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(builtins/diagnostics/credo): toggle per file/full workspace diagnostics #1465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion doc/BUILTINS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,22 @@ local sources = { null_ls.builtins.diagnostics.credo }
- Filetypes: `{ "elixir" }`
- Method: `diagnostics`
- Command: `mix`
- Args: `{ "credo", "suggest", "--format", "json", "--read-from-stdin", "$FILENAME" }`
- Args: dynamically resolved (see [source](https://github.com/jose-elias-alvarez/null-ls.nvim/blob/main/lua/null-ls/builtins/diagnostics/credo.lua))

#### Config

##### `full_workspace` (boolean)

- `false` (default) - run credo for a single file
- `true` - run credo on the entire workspace. If this is slow on large projects, you may wish to set `method = null_ls.methods.DIAGNOSTICS_ON_SAVE` in `with()` call.

```lua
local credo = null_ls.builtins.diagnostics.credo.with({
config = {
full_workspace = true
},
})
```

#### Notes

Expand Down
32 changes: 31 additions & 1 deletion lua/null-ls/builtins/diagnostics/credo.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ return h.make_builtin({
notes = {
"Searches upwards from the buffer to the project root and tries to find the first `.credo.exs` file in case the project has nested `credo` configs.",
},
config = {
{
key = "full_workspace",
type = "boolean",
description = [[- `false` (default) - run credo for a single file
- `true` - run credo on the entire workspace. If this is slow on large projects, you may wish to set `method = null_ls.methods.DIAGNOSTICS_ON_SAVE` in `with()` call.]],
usage = [[true]],
},
},
},
method = DIAGNOSTICS,
filetypes = { "elixir" },
Expand All @@ -34,12 +43,29 @@ return h.make_builtin({
return params.root
end
end,
args = { "credo", "suggest", "--format", "json", "--read-from-stdin", "$FILENAME" },
args = function(params)
if params:get_config().full_workspace then
return { "credo", "suggest", "--format", "json" }
else
return { "credo", "suggest", "--format", "json", "--read-from-stdin", "$FILENAME" }
end
end,
format = "raw",
to_stdin = true,
on_output = function(params, done)
local issues = {}

-- `multiple_files = true` must be set ONLY if running
-- `full_workspace=true`.
-- If it is set when credo is only generating diagnostics per file,
-- then existing diagnostics in open buffers will be cleared on
-- each subsequent execution in a different buffer
if params:get_config().full_workspace then
-- this is hacky, but there isn't any way to set `multiple_files`
-- dynamically based on user config properly
params:get_source().generator.multiple_files = true
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried a few different ways to dynamically set multiple_files based on user config, but without much luck.

Happy to modify this to a better approach if there's something I'm missing. Mutating the generator during on_output doesn't seem great, but couldn't find a better way that didn't involve messing with a bunch of core logic in null-ls.

end

-- credo is behaving in a bit of a tricky way:
-- 1. if there are no elixir warnings, it will give its output
-- on stderr, and stdout will be nil
Expand Down Expand Up @@ -84,6 +110,10 @@ return h.make_builtin({
source = "credo",
}

if params:get_config().full_workspace then
err.filename = issue.filename
end

-- using the dynamic priority ranges from credo source
if issue.priority >= 10 then
err.severity = h.diagnostics.severities.error
Expand Down
90 changes: 81 additions & 9 deletions test/spec/builtins/diagnostics_spec.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,15 @@ describe("diagnostics", function()
describe("credo", function()
local linter = diagnostics.credo
local parser = linter._opts.on_output
local args = linter._opts.args
local credo_diagnostics
local done = function(_diagnostics)
credo_diagnostics = _diagnostics
end
local config = {}
local get_config = function()
return config
end
after_each(function()
credo_diagnostics = nil
end)
Expand All @@ -125,7 +130,7 @@ describe("diagnostics", function()
}
]
} ]]
parser({ output = output }, done)
parser({ output = output, get_config = get_config }, done)
assert.same({
{
source = "credo",
Expand Down Expand Up @@ -153,7 +158,7 @@ describe("diagnostics", function()
"trigger": "@impl true"
}]
} ]]
parser({ output = output }, done)
parser({ output = output, get_config = get_config }, done)
assert.same({
{
source = "credo",
Expand Down Expand Up @@ -181,7 +186,7 @@ describe("diagnostics", function()
"trigger": "TODO: implement check"
}]
} ]]
parser({ output = output }, done)
parser({ output = output, get_config = get_config }, done)
assert.same({
{
source = "credo",
Expand Down Expand Up @@ -209,7 +214,7 @@ describe("diagnostics", function()
"trigger": "|>"
}]
} ]]
parser({ output = output }, done)
parser({ output = output, get_config = get_config }, done)
assert.same({
{
source = "credo",
Expand All @@ -224,7 +229,7 @@ describe("diagnostics", function()
it("returns errors as diagnostics", function()
local error =
[[** (Mix) The task "credo" could not be found\nNote no mix.exs was found in the current directory]]
parser({ err = error }, done)
parser({ err = error, get_config = get_config }, done)
assert.same({
{
source = "credo",
Expand All @@ -233,7 +238,7 @@ describe("diagnostics", function()
},
}, credo_diagnostics)
end)
it("should handle compile warnings preceeding output", function()
it("should handle compile warnings preceding output", function()
local output = [[
00:00:00.000 [warn] IMPORTING DEV.SECRET

Expand All @@ -253,7 +258,8 @@ describe("diagnostics", function()
}
]
} ]]
parser({ output = output }, done)

parser({ output = output, get_config = get_config }, done)
assert.same({
{
source = "credo",
Expand All @@ -267,7 +273,7 @@ describe("diagnostics", function()
end)
it("should handle messages with incomplete json", function()
local output = [[Some incomplete message that shouldn't really happen { "issues": ]]
parser({ output = output }, done)
parser({ output = output, get_config = get_config }, done)
assert.same({
{
source = "credo",
Expand All @@ -278,7 +284,7 @@ describe("diagnostics", function()
end)
it("should handle messages without json", function()
local output = [[Another message that shouldn't really happen]]
parser({ output = output }, done)
parser({ output = output, get_config = get_config }, done)
assert.same({
{
source = "credo",
Expand All @@ -287,6 +293,72 @@ describe("diagnostics", function()
},
}, credo_diagnostics)
end)

describe("full_workspace is falsey", function()
config = {
full_workspace = false,
}

it("creates diagnostic without filename", function()
local output = [[
{
"issues": [{
"category": "design",
"check": "Credo.Check.Design.TagTODO",
"column": null,
"column_end": null,
"filename": "./foo.ex",
"line_no": 8,
"message": "Found a TODO tag in a comment: \"TODO: implement check\"",
"priority": -5,
"scope": null,
"trigger": "TODO: implement check"
}]
} ]]
parser({ output = output, get_config = get_config }, done)
assert.same(credo_diagnostics[1].filename, nil)
end)

it("calls credo for a specific file", function()
assert.same(
args({ get_config = get_config }),
{ "credo", "suggest", "--format", "json", "--read-from-stdin", "$FILENAME" }
)
end)
end)

describe("full_workspace is truthy", function()
local get_source = function()
return { generator = {} }
end
config = {
full_workspace = true,
}

it("creates diagnostic with filename", function()
local output = [[
{
"issues": [{
"category": "design",
"check": "Credo.Check.Design.TagTODO",
"column": null,
"column_end": null,
"filename": "./foo.ex",
"line_no": 8,
"message": "Found a TODO tag in a comment: \"TODO: implement check\"",
"priority": -5,
"scope": null,
"trigger": "TODO: implement check"
}]
} ]]
parser({ output = output, get_config = get_config, get_source = get_source }, done)
assert.same("./foo.ex", credo_diagnostics[1].filename)
end)

it("calls credo for workspace", function()
assert.same(args({ get_config = get_config }), { "credo", "suggest", "--format", "json" })
end)
end)
end)

describe("luacheck", function()
Expand Down