From 7345d1bcd648527e7b524df0d23959c1c3322411 Mon Sep 17 00:00:00 2001 From: Michele Campeotto Date: Sat, 22 Mar 2025 12:05:08 +0100 Subject: [PATCH] Update diagnostics to be spec compliant. The spec says diagnostic.codeDescription should be an object with an URI linking to information about the error, we were using it as a string identifier for the diagnostic. This was making Helix ignore all diagnostics from fennel-ls. Changed diagnostic.code to be the string identifier, removed numeric identifiers and codeDescription field. --- src/fennel-ls/lint.fnl | 38 ++++++++------------- src/fennel-ls/message.fnl | 4 +-- test/lint.fnl | 69 +++++++++++++++++++-------------------- 3 files changed, 49 insertions(+), 62 deletions(-) diff --git a/src/fennel-ls/lint.fnl b/src/fennel-ls/lint.fnl index 1af180f..3827abe 100644 --- a/src/fennel-ls/lint.fnl +++ b/src/fennel-ls/lint.fnl @@ -33,8 +33,7 @@ the `file.diagnostics` field, filling it with diagnostics." {:range (message.ast->range server file symbol) :message (.. "unused definition: " (tostring symbol)) :severity message.severity.WARN - :code 301 - :codeDescription "unused-definition"} + :code :unused-definition} #[{:range (message.ast->range server file symbol) :newText (.. "_" (tostring symbol))}]))) @@ -57,8 +56,7 @@ the `file.diagnostics` field, filling it with diagnostics." {:range (message.ast->range server file symbol) :message (.. "unknown field: " (tostring symbol)) :severity message.severity.WARN - :code 302 - :codeDescription "unknown-module-field"})))) + :code :unknown-module-field})))) (λ unknown-module-field [server file] "any multisym whose definition can't be found through a (require) call" @@ -83,8 +81,7 @@ the `file.diagnostics` field, filling it with diagnostics." :message (.. "unnecessary : call: use (" (tostring (. call 2)) ":" method ")") :severity message.severity.WARN - :code 303 - :codeDescription "unnecessary-method"})))) + :code :unnecessary-method})))) (λ unnecessary-tset [server file head call] (λ all-syms? [call start end] @@ -109,8 +106,7 @@ the `file.diagnostics` field, filling it with diagnostics." (diagnostic {:range (message.ast->range server file call) :message (.. "unnecessary " (tostring head)) :severity message.severity.WARN - :code 309 - :codeDescription "unnecessary-tset"} + :code :unnecessary-tset} #[{:range (message.ast->range server file call) :newText (make-new-text call)}]))) @@ -120,8 +116,7 @@ the `file.diagnostics` field, filling it with diagnostics." (diagnostic {:range (message.ast->range server file call) :message (.. "unnecessary " (tostring head)) :severity message.severity.WARN - :code 310 - :codeDescription "unnecessary-do-values"} + :code :unnecessary-do-values} #[{:range (message.ast->range server file call) :newText (view (. call 2))}]))) @@ -135,8 +130,7 @@ the `file.diagnostics` field, filling it with diagnostics." (diagnostic {:range (message.ast->range server file last-body) :message "redundant do" :severity message.severity.WARN - :code 311 - :codeDescription "redundant-do"} + :code :redundant-do} #[{:range (message.ast->range server file last-body) :newText (table.concat (fcollect [i 2 (length last-body)] @@ -165,8 +159,7 @@ the `file.diagnostics` field, filling it with diagnostics." (.. " Use a loop when you have a dynamic number of " "arguments to (" (tostring op) ")"))) :severity message.severity.WARN - :code 304 - :codeDescription "bad-unpack"} + :code :bad-unpack} (if (and (= (length call) 2) (= (length (. call 2)) 2) (sym? op "..")) @@ -181,8 +174,7 @@ the `file.diagnostics` field, filling it with diagnostics." :message (.. "var is never set: " (tostring symbol) " Consider using (local) instead of (var)") :severity message.severity.WARN - :code 305 - :codeDescription "var-never-set"}))) + :code :var-never-set}))) (local op-identity-value {:+ 0 :* 1 :and true :or false :band -1 :bor 0 :.. ""}) (λ op-with-no-arguments [server file op call] @@ -196,8 +188,7 @@ the `file.diagnostics` field, filling it with diagnostics." {:range (message.ast->range server file call) :message (.. "write " (view identity) " instead of (" (tostring op) ")") :severity message.severity.WARN - :code 306 - :codeDescription "op-with-no-arguments"} + :code :op-with-no-arguments} #[{:range (message.ast->range server file call) :newText (view identity)}])))) @@ -207,8 +198,7 @@ the `file.diagnostics` field, filling it with diagnostics." {:range (message.ast->range server file call) :message "Use increasing operator instead of decreasing" :severity message.severity.WARN - :code 312 - :codeDescription "no-decreasing-comparison"} + :code :no-decreasing-comparison} #[{:range (message.ast->range server file call) :newText (let [new (if (sym? op :>=) (fennel.sym :<=) (fennel.sym :<)) reversed (fcollect [i (length call) 2 -1 @@ -230,8 +220,7 @@ the `file.diagnostics` field, filling it with diagnostics." (diagnostic {:range (message.ast->range server file (. ast 1)) :message "no pinned patterns; use case instead of match" :severity message.severity.WARN - :code 308 - :codeDescription "match-should-case"} + :code :match-should-case} #[{:range (message.ast->range server file (. ast 1)) :newText "case"}]))) @@ -250,8 +239,7 @@ the `file.diagnostics` field, filling it with diagnostics." :message (.. "bad " (tostring (. arg 1)) " call: only the first value of the multival will be used") :severity message.severity.WARN - :code 307 - :codeDescription "bad-unpack"})) + :code :inline-unpack})) (λ add-lint-diagnostics [server file] "fill up the file.diagnostics table with linting things" @@ -269,7 +257,7 @@ the `file.diagnostics` field, filling it with diagnostics." ;; all non-macro calls. This only covers specials and function calls. (each [[head &as call] (pairs file.calls)] (when head - (when lints.bad-unpack + (when (or lints.bad-unpack lints.inline-unpack) (table.insert diagnostics (bad-unpack server file head call))) (when lints.unnecessary-method (table.insert diagnostics (unnecessary-method server file head call))) diff --git a/src/fennel-ls/message.fnl b/src/fennel-ls/message.fnl index a142a6c..8419f95 100644 --- a/src/fennel-ls/message.fnl +++ b/src/fennel-ls/message.fnl @@ -82,8 +82,8 @@ LSP json objects." :unnecessary-do-values "Remove unnecessary do or values" :redundant-do "Remove redundant do" :bad-unpack "Replace with table.concat call"}] - (or (. titles diag.codeDescription) - (.. "Action title missing - " diag.codeDescription)))) + (or (. titles diag.code) + (.. "Action title missing - " diag.code)))) (λ diagnostic->code-action [_server file diagnostic ?kind] (let [{: uri} file] diff --git a/test/lint.fnl b/test/lint.fnl index 1d11eb1..3faed0e 100644 --- a/test/lint.fnl +++ b/test/lint.fnl @@ -43,23 +43,24 @@ (fn test-unused [] (check "(local x 10)" [{:message "unused definition: x" - :code 301 + :code :unused-definition :range {:start {:character 7 :line 0} :end {:character 8 :line 0}}}]) (check "(fn x [])" [{:message "unused definition: x" - :code 301 + :code :unused-definition :range {:start {:character 4 :line 0} :end {:character 5 :line 0}}}]) (check "(let [(x y) (values 1 2)] x)" - [{:code 301 + [{:code :unused-definition :range {:start {:character 9 :line 0} :end {:character 10 :line 0}}}]) - (check "(case [1 1 2 3 5 8] [a a] (print :first-two-equal))" [{:code 301}]) + (check "(case [1 1 2 3 5 8] [a a] (print :first-two-equal))" + [{:code :unused-definition}]) (assert-ok "(case [1 1 2 3 5 8] [a_ a_] (print :first-two-equal))") ;; setting a var without reading (check "(var x 1) (set x 2) (set [x] [3])" - [{:code 301 + [{:code :unused-definition :range {:start {:character 5 :line 0} :end {:character 6 :line 0}}}]) ;; setting a field without reading is okay @@ -93,39 +94,39 @@ :main.fnl "(local {: a : c &as guy} (require :the-guy-they-tell-you-not-to-worry-about)) (print guy.b guy.d)"} - [{:code 302 :message "unknown field: c"} - {:code 302 :message "unknown field: guy.d"}] - [{:code 302 :message "unknown field: a"} - {:code 302 :message "unknown field: b"}]) + [{:code :unknown-module-field :message "unknown field: c"} + {:code :unknown-module-field :message "unknown field: guy.d"}] + [{:code :unknown-module-field :message "unknown field: a"} + {:code :unknown-module-field :message "unknown field: b"}]) (check "table.insert2 table.insert" - [{:code 302 :message "unknown field: table.insert2"}] - [{:code 302 :message "unknown field: table.insert"}]) + [{:code :unknown-module-field :message "unknown field: table.insert2"}] + [{:code :unknown-module-field :message "unknown field: table.insert"}]) ;; if you explicitly write "_G", it should turn off this test. ;; Hardcoded at the top of analyzer.fnl/search-document. (check "_G.insert2" [] - [{:code 302}]) + [{:code :unknown-module-field}]) ;; we don't care about nested (check {:requireme.fnl "{:field []}" :main.fnl "(local {: field} (require :requireme)) field.unknown"} [] - [{:code 302}]) + [{:code :unknown-module-field}]) ;; specials are OK too (check {:unpacker.fnl "(local unpack (or table.unpack _G.unpack)) {: unpack}" :main.fnl "(local u (require :unpacker)) (print (u.unpack [:haha :lol]))"} [] - [{:code 302}]) + [{:code :unknown-module-field}]) (check "package.loaded.mymodule io.stderr.write" [] - [{:code 302}]) + [{:code :unknown-module-field}]) nil) -(fn test-unnecessary-colon [] +(fn test-unnecessary-method [] (check "(let [x :haha] (: x :find :a))" [{:message "unnecessary : call: use (x:find)" - :code 303 + :code :unnecessary-method :range {:start {:character 15 :line 0} :end {:character 29 :line 0}}}]) @@ -140,13 +141,13 @@ (fn test-unpack-into-op [] (check "(+ (unpack [1 2 3]))" - [{:code 304}]) + [{:code :bad-unpack}]) (check "(.. (table.unpack [\"hello\" \"world\"]))" - [{:code 304 :message #($:find "table.concat")}]) + [{:code :bad-unpack :message #($:find "table.concat")}]) (check "(* (table.unpack [\"hello\" \"world\"]))" - [{:code 304 :message #(not ($:find "table%.concat"))}]) + [{:code :bad-unpack :message #(not ($:find "table%.concat"))}]) ;; only when lexical (assert-ok "(-> [1 2 3] table.unpack +)") @@ -154,7 +155,7 @@ (fn test-unset-var [] (check "(var x nil) (print x)" - [{:code 305 + [{:code :var-never-set :range {:start {:character 5 :line 0} :end {:character 6 :line 0}}}]) @@ -166,7 +167,7 @@ (fn test-unpack-in-middle [] (check "(+ 1 2 3 (values 4 5) 6)" - [{:code 307 + [{:code :inline-unpack :range {:start {:line 0 :character 9} :end {:line 0 :character 21}}}]) @@ -180,14 +181,15 @@ (assert-ok "(local [tbl key] [{} :k]) (tset tbl key 249)") ;; never a good use of tset (check "(local tbl {}) (tset tbl :key 9)" - [{:code 309 - :codeDescription "unnecessary-tset" + [{:code :unnecessary-tset :message "unnecessary tset" :range {:start {:character 15 :line 0} :end {:character 32 :line 0}}}]) - (check "(local tbl {}) (tset tbl :key :nested 9)" [{:code 309}]) + (check "(local tbl {}) (tset tbl :key :nested 9)" + [{:code :unnecessary-tset}]) ;; Lint only triggers on keys that can be written as a sym - (check "(local tbl {}) (tset tbl \"hello-world\" 249)" [{:code 309}]) + (check "(local tbl {}) (tset tbl \"hello-world\" 249)" + [{:code :unnecessary-tset}]) (assert-ok "(local tbl {}) (tset tbl \"01234567\" 249)") (assert-ok "(local tbl {}) (tset tbl \"hello world\" 1)") (assert-ok "(local tbl {}) (tset tbl \"0123.4567\" 1)") @@ -198,14 +200,12 @@ (assert-ok "(do (print :x) 11)") ;; unnecessary do (check "(do 9)" [{:message "unnecessary do" - :code 310 - :codeDescription "unnecessary-do-values" + :code :unnecessary-do-values :range {:start {:character 0 :line 0} :end {:character 6 :line 0}}}]) ;; unnecessary values (check "(print :hey (values :lol))" - [{:code 310 - :codeDescription "unnecessary-do-values" + [{:code :unnecessary-do-values :message "unnecessary values" :range {:start {:character 12 :line 0} :end {:character 25 :line 0}}}]) @@ -216,8 +216,7 @@ (assert-ok "(case 134 x (do (print :x x) 11))") ;; unnecessary one (check "(let [x 29] (do (print 9) x))" - [{:code 311 - :codeDescription "redundant-do" + [{:code :redundant-do :message "redundant do" :range {:start {:character 12 :line 0} :end {:character 28 :line 0}}}]) @@ -242,13 +241,13 @@ ;; warn: basic no pinning (check "(match 91 z (print :yeah2 z))" [{:message "no pinned patterns; use case instead of match" - :code 308 + :code :match-should-case :range {:start {:character 1 :line 0} :end {:character 6 :line 0}}}]) ;; warn: nested no pinning (check "(match [32] [lol] (print :nested-no-pin lol))" [{:message "no pinned patterns; use case instead of match" - :code 308 + :code :match-should-case :range {:start {:character 1 :line 0} :end {:character 6 :line 0}}}]) nil) @@ -268,7 +267,7 @@ {: test-unused : test-ampersand : test-unknown-module-field - : test-unnecessary-colon + : test-unnecessary-method : test-unnecessary-tset : test-unnecessary-do : test-redundant-do