feat: add diagnostic filtering

[nawordar]

Closes #55.

[jose-elias-alvarez]

Thanks for putting this together! I quickly scanned it and it looks good (I especially appreciate the test coverage), but I'll have time to fully review it and test it out locally tomorrow. A couple of quick impressions:

1. I think it would be nice to allow users to set the severities they want to ignore as strings (e.g. "hint" instead of 4), since it's not immediately obvious what the numbers correspond to without referring to the LSP documentation.
2. Could we mention the option in the readme?

Edit: also, if possible could you run stylua on the code to avoid having to format it later?

[nawordar]

What about having an enum-like table of severities? If so, what are your naming conventions here?

local ts_utils = require("nvim-lsp-ts-utils")
print(ts_utils.severity.hint) -- 4

Edit: I added documentation and filtering by code.

1. Filtering by code works in my editor, but the test "should filter out diagnostics by code" doesn't pass and I don't know why. Could you look at it?

2. "e2e import_all should import both missing types" test fails. I don't think that it fails because of my changes (I checked out commit before my changes and it still fails), but this made me thinking, if filtering diagnostics can impact importing. I'm afraid that it can. What should we do about it?

   Testing:        /home/nawordar/projects/nvim-lsp-ts-utils/test/spec/e2e_spec.lua
   Fail    ||      e2e import_all should import both missing types
           ...wordar/projects/nvim-lsp-ts-utils/test/spec/e2e_spec.lua:61: Expected objects to be equal.
           Passed in:
           (string) 'import { User, UserNotification } from "./test-types";'
           Expected:
           (string) 'const testUser: User = { name: "Jose" };'
   
           stack traceback:
               ...wordar/projects/nvim-lsp-ts-utils/test/spec/e2e_spec.lua:61: in function <...wordar/projects/nvim-lsp-ts-utils/test/spec/e2e_spec.lua:57>

[jose-elias-alvarez]

I'm not opposed to a table of severities, but what's the advantage of having a dedicated variable vs. allowing strings and converting them with a simple map like this?

local severities = {
    ["error"] = 1,
    ["warning"] = 2,
    -- etc
}

For the tests: I think the first test failure is because diagnostics_result is getting mutated in the first test. This diff solves it on my end:

diff --git a/test/spec/client_spec.lua b/test/spec/client_spec.lua
index 9ebb6a2..9b1ca3e 100644
--- a/test/spec/client_spec.lua
+++ b/test/spec/client_spec.lua
@@ -97,14 +97,18 @@ describe("client", function()
         end)

         describe("diagnostics_handler", function()
-            local diagnostics_result = {
-                diagnostics = {
-                    { source = "eslint", severity = 4, code = 80001 },
-                    { source = "typescript", severity = 1, code = 80001 },
-                    { source = "typescript", severity = 3, code = 80001 },
-                    { source = "typescript", severity = 4, code = 80000 },
-                },
-            }
+            local diagnostics_result
+            before_each(function()
+                diagnostics_result = {
+                    diagnostics = {
+                        { source = "eslint", severity = 4, code = 80001 },
+                        { source = "typescript", severity = 1, code = 80001 },
+                        { source = "typescript", severity = 3, code = 80001 },
+                        { source = "typescript", severity = 4, code = 80000 },
+                    },
+                }
+            end)

             it("should filter out hints", function()
                 o.get.returns({

I wouldn't worry too much about the import all test, since it's flaky, but you're correct that filtering out errors does affect :TSLspImportAll. I don't know if users will really want to filter out errors, so if you think it's okay, I think we can just add a warning in the documentation about the interaction and deal with it later if it comes up.

Edit: out of curiosity, does the import all test work when you actually open the file and run :TSLspImportAll?

[nawordar]

I'm not opposed to a table of severities, but what's the advantage of having a dedicated variable vs. allowing strings and converting them with a simple map like this?

local severities = {
    ["error"] = 1,
    ["warning"] = 2,
    -- etc
}

This is a nice idea. I think I will put severities in utils if it will ever be needed in other files.

Edit: out of curiosity, does the import all test work when you actually open the file and run :TSLspImportAll?

It does.

[jose-elias-alvarez]

LGTM, thanks for working on this!

I have to think about how to make testing in general more reliable, since Plenary / Luassert weren't really built to handle cases like this, but as long as it actually works it should be fine for now.