From 2b2c31c57765870af621ab308eb0127879f7210f Mon Sep 17 00:00:00 2001 From: Phil Hagelberg Date: Mon, 6 Oct 2025 11:26:02 -0700 Subject: [PATCH] Add legacy-multival/legacy-multival-case lint and fix. As of Fennel 1.6.0, parens in destructuring is deprecated, and we want to make it easy for people to convert to using table destructuring instead since it's optimized to compile to the same thing. --- src/fennel-ls/compiler.fnl | 5 +++ src/fennel-ls/lint.fnl | 66 ++++++++++++++++++++++++++++++++++++++ test/lint.fnl | 24 ++++++++++++-- 3 files changed, 93 insertions(+), 2 deletions(-) diff --git a/src/fennel-ls/compiler.fnl b/src/fennel-ls/compiler.fnl index 0785f68..64b86fd 100644 --- a/src/fennel-ls/compiler.fnl +++ b/src/fennel-ls/compiler.fnl @@ -70,6 +70,7 @@ identifiers are declared / referenced in which places." ;; The useful information being recorded: (let [definitions-by-scope (doto {} (setmetatable has-tables-mt)) definitions {} ; symbol -> binding + multi-binds {} compile-diagnostics {} ; [diagnostic] references {} ; symbol -> references macro-refs {} ; symbol -> macro @@ -140,6 +141,9 @@ identifiers are declared / referenced in which places." (recurse binding [] 0)) (λ define [?definition binding scope ?opts] + ;; just storing lists for now; can add others if a lint needs them + (when (list? binding) + (table.insert multi-binds {:left binding :right ?definition})) (for-each-binding-in binding ?definition (fn [symbol ?definition keys ?multival] (when (not (or (list? symbol) (multisym? symbol))) @@ -462,6 +466,7 @@ identifiers are declared / referenced in which places." (set file.scopes scopes) (set file.definitions definitions) (set file.definitions-by-scope definitions-by-scope) + (set file.multi-binds multi-binds) (set file.compile-errors compile-diagnostics) (set file.references references) (set file.require-calls require-calls) diff --git a/src/fennel-ls/lint.fnl b/src/fennel-ls/lint.fnl index cd8c33a..312714d 100644 --- a/src/fennel-ls/lint.fnl +++ b/src/fennel-ls/lint.fnl @@ -798,6 +798,72 @@ You can read more about how to add lints in docs/linting.md" {:message "indexing a table with 0; did you forget that Lua is 1-indexed?" : ast}))}) +(add-lint :legacy-multival + {:what-it-does "Detects usage of legacy (paren) multival destructuring." + :why-care? "It's more consistent to use table destructuring." + :type :other + :example "```fnl +(let [input \"whatever\" + (v1 v2) (input:match \"([aeiou]).*([aeiou])\")] + (print :vowels v1 v2)) +``` + +Instead, use: +```fnl +(let [input \"whatever\" + [v1 v2] [(input:match \"([aeiou]).*([aeiou])\")]] + (print :vowels v1 v2)) +```" + :since "0.2.2-dev" + :disabled true + :impl (fn [server file] + (each [_ {: left : right} (ipairs file.multi-binds)] + (when (and (list? left) (. file.lexical left)) + (coroutine.yield + {:message "Legacy multival destructure can be replaced with table destructure." + :ast left + :fix #{:title "Replace legacy multival destructure with table." + :changes [{:range (message.ast->range server file left) + :newText (-> (tostring left) + (: :gsub "^%(" "[") + (: :gsub "%)$" "]"))} + {:range (message.ast->range server file right) + :newText (.. "[" (tostring right) "]")}]}}))))}) + +(fn match-call? [[callee &as ast]] + (and (list? ast) + (case (tostring callee) + :case true :match true :case-try true :match-try true))) + +(add-lint :legacy-multival-case + {:what-it-does "Detects usage of legacy (paren) multival destructuring in pattern match." + :why-care? "It's more consistent to use table destructuring." + :type :macro-call + :since "0.2.2-dev" + :disabled true + :example "```fnl +(case (input:match \"([aeiou]).*([aeiou])\") + (v1 v2) (print \"Two vowels:\" v1 v2) + _ (print \"Less than two\")) +``` + +Instead, use: +(case [(input:match \"([aeiou]).*([aeiou])\")] + [v1 v2] (print \"Two vowels:\" v1 v2) + _ (print \"Less than two\")) +" + :impl (fn [_server _file ast] + (if (match-call? ast) + (faccumulate [r nil i 3 (length ast) 2 &until r] + (if (and (list? (. ast i)) + (not (sym? (. ast i 1) :catch)) + (not (sym? (. ast i 1) :where))) + ;; would be nice to get fixes here but it needs to be smart + ;; about case (fix val+every pattern) vs case-try (fix + ;; pattern and previous body) + {:message "Legacy multival destructure can be replaced with table destructure." + :ast (. ast i)}))))}) + (add-lint :invalid-flsproject-settings {:what-it-does "Checks if the flsproject file's settings are valid." diff --git a/test/lint.fnl b/test/lint.fnl index 7e1b0bb..850ade6 100644 --- a/test/lint.fnl +++ b/test/lint.fnl @@ -38,11 +38,18 @@ :escape-newlines? true}))) (table.remove diagnostics i))))) +(macro check-form [lints form expected ?unexpected] + `(check {:main.fnl ,(view form) :flsproject.fnl ,(view {: lints})} + ,expected ,?unexpected)) + (fn assert-ok [file-contents] (let [{: uri : client} (create-client file-contents) [{:result {:items diagnostics}}] (client:diagnostic uri)] (faith.= nil (next diagnostics) (view diagnostics)))) +(macro assert-ok-form [lints form] + `(assert-ok {:main.fnl ,(view form) :flsproject.fnl ,(view {: lints})})) + (fn test-unused [] (check "(local x 10)" [{:message "unused definition: x" @@ -414,7 +421,19 @@ (assert-ok (add-opts "(. math :0)"))) nil) - +(fn test-legacy-multival [] + (check-form {:legacy-multival true} + (let [(x y z) (table.unpack [10 9 8])] (+ x y z)) + [{:message "Legacy multival destructure can be replaced with table destructure." + :range {:end {:character 13 :line 0} :start {:character 6 :line 0}}}]) + (check-form {:legacy-multival-case true} + (case (table.unpack [9 0 3]) (n x) (print :yes n x)) + [{:message "Legacy multival destructure can be replaced with table destructure."}]) + (assert-ok-form {:legacy-multival-case true} + (case-try (values 1 2 3) + nil (print :lol) + (where (or a [a])) (print (+ a 3)) + (catch (x) x)))) {: test-unused : test-ampersand @@ -434,4 +453,5 @@ : test-arg-count : test-duplicate-keys : test-nested-associative-operator - : test-zero-indexed} + : test-zero-indexed + : test-legacy-multival}