From 88114544565860811cca1d36bdd4cb9edca2aaca Mon Sep 17 00:00:00 2001 From: XeroOl Date: Tue, 15 Jul 2025 22:56:20 -0500 Subject: [PATCH] split mismatched-argument-count lint into not-enough-arguments and too-many-arguments --- changelog.md | 7 +-- flsproject.fnl | 2 +- src/fennel-ls/lint.fnl | 96 +++++++++++++++++++++++++++++------------- test/lint.fnl | 53 ++++++++++++----------- 4 files changed, 100 insertions(+), 58 deletions(-) diff --git a/changelog.md b/changelog.md index f4f99e5..fbf81ed 100644 --- a/changelog.md +++ b/changelog.md @@ -3,10 +3,11 @@ ### Features * Code action "Expand macro" lets you see what a macro expands to * Macro expansion is shown when hovering over a macro -* New lint `empty-let` for replacing (let [] ...) with (do ...) +* New lint `empty-let` for replacing `(let [] ...)` with `(do ...)` * New lint `duplicate-table-keys` for detecting duplicate keys (eg. `{:a 1 :a 2}`) -* New lint `mismatched-argument-count` for ensuring function calls have the right argument count - * Disabled by default because it gives false positives when your arglist doesn't follow fennel's naming conventions +* New lint `too-many-arguments` for function calls that provide extra, useless arguments +* New lint `not-enough-arguments` for ensuring you don't accidentally implicitly pass nil + * Disabled by default because it requires following fennel's optional argument naming conventions * New lint `invalid-flsproject-settings` checks your `flsproject.fnl` file as you edit it ### Changes diff --git a/flsproject.fnl b/flsproject.fnl index d143b88..38fb917 100644 --- a/flsproject.fnl +++ b/flsproject.fnl @@ -1,3 +1,3 @@ {:lua-version "intersection" - :lints {:mismatched-argument-count true} + :lints {:not-enough-arguments true} :fennel-path "./?.fnl;./?/init.fnl;src/?.fnl;src/?/init.fnl;deps/?.fnl;deps/?/init.fnl"} diff --git a/src/fennel-ls/lint.fnl b/src/fennel-ls/lint.fnl index 8abe50a..2bce5fa 100644 --- a/src/fennel-ls/lint.fnl +++ b/src/fennel-ls/lint.fnl @@ -593,22 +593,20 @@ the `file.diagnostics` field, filling it with diagnostics." (or (and (list? ast) (not (op? (. ast 1))) (not (sym? (. ast 1) :not))) (varg? ast))) -(add-lint :mismatched-argument-count +(add-lint :not-enough-arguments {:what-it-does - "Checks if function calls have the correct number of arguments based on the function's signature." + "Checks if function calls have enough number of arguments based on the function's signature." :why-care? - "Calling functions with the wrong number of arguments can lead to runtime errors - or unexpected behavior. This lint helps catch these issues early." + "Calling functions without all the arguments fills in the extra arguments with `nil` which + can cause unexpected behavior. This lint helps catch these issues early." :example "```fnl (string.sub \"hello\") ; missing required arguments - (string.sub \"hello\" 1 2 3) ; too many arguments ``` Instead, use: ```fnl (string.sub \"hello\" 1) ; provide all required arguments - (string.sub \"hello\" 1 2) ; remove extra arguments ```" :limitations "This lint is disabled by default because it can produce false positives. @@ -616,10 +614,7 @@ the `file.diagnostics` field, filling it with diagnostics." and any other arguments can be assumed to be required. This is reasonably accurate if the code follows Fennel conventions. Also this lint is very new and may have issues, so I'd like to let people try it on their own terms before - enabling it by default. - - In the future I may split it into \"too-many-arguments\" (which is accurate regardless of code style) - and \"not-enough-arguments\" (which needs the arglist to be annotated properly)" + enabling it by default." :since "0.2.2-dev" :type [:function-call :special-call :macro-call] :disabled true @@ -635,19 +630,15 @@ the `file.diagnostics` field, filling it with diagnostics." 1)) ; function call; the head doesn't count as an argument passes-extra-args (and (not= 1 (length ast)) (possibly-multival? (. ast (length ast)))) - (min-params infinite-params?) (faccumulate [(last-required-argument vararg) nil - i (length signature) 1 -1] - (let [s (tostring (. signature i)) - m (or (= s "...") (= s "&"))] - (values - (if m - nil - last-required-argument - last-required-argument - (let [first-char (string.sub s 1 1)] - (and (not= first-char "?") (not= first-char "_"))) - i) - (or vararg m)))) + min-params (accumulate [last-required-argument nil + i arg (ipairs signature) + &until (let [s (tostring arg)] + (or (= s "...") + (= s "&")))] + (let [first-char (string.sub (tostring arg) 1 1)] + (if (and (not= first-char "?") (not= first-char "_")) + i + last-required-argument))) ;; exception: (- a b) only needs 1 argument ;; exception: (/ a b) only needs 1 argument min-params (if (= result (docs.get-builtin server :-)) 1 @@ -655,17 +646,62 @@ the `file.diagnostics` field, filling it with diagnostics." ;; TODO Fennel 1.5.4+ has `fn`'s arglist fixed ;; exception: fn only needs one argument (= result (docs.get-builtin server :fn)) 1 - (or min-params 0)) - ;; exception: (table.insert table item) can take a third argument - max-params (if (= result (. (docs.get-global server :table) :fields :insert)) - 3 - (length signature))] + (or min-params 0))] (if (and (< number-of-args min-params) (not passes-extra-args)) {:range (message.ast->range server file ast) :message (.. "not enough args. my analysis of the signature says you need at least " min-params " arguments but I only see " number-of-args) - :severity message.severity.WARN} - (and (< max-params number-of-args) + :severity message.severity.WARN})))))}) + +(add-lint :too-many-arguments + {:what-it-does + "Checks if function calls have the correct number of arguments based on the function's signature." + :why-care? + "Calling functions with the wrong number of arguments can lead to runtime errors + or unexpected behavior. This lint helps catch these issues early." + :example + "```fnl + (string.sub \"hello\" 1 2 3) ; too many arguments + + (assert (< x y) + (.. \"x=\" + (tostring x)) ; mismatched parens can cause too many arguments to a function + \" is less than y=\" + (tostring y)) + ``` + + Instead, use: + ```fnl + (string.sub \"hello\" 1 2) ; remove extra arguments + + (assert (< x y) + (.. \"x=\" + (tostring x) + \" is less than y=\" + (tostring y))) ; fixed parens + ```" + :since "0.2.2-dev" + :type [:function-call :special-call :macro-call] + :impl (fn [server file ast] + (case (analyzer.search server file (. ast 1) {} {}) + {:indeterminate nil &as result} + (case (?. (navigate.getmetadata server result) :fnl/arglist) + signature + (let [number-of-args (- (length ast) + (if (and (sym? (. ast 1)) + (string.find (tostring (. ast 1)) ".:")) + 0 ; method call; the head counts as an argument + 1)) ; function call; the head doesn't count as an argument + (infinite-params?) (accumulate [vararg nil + _ arg (ipairs signature) + &until vararg] + (let [s (tostring arg)] + (or (= s "...") (= s "&")))) + ;; exception: (table.insert table item) can take a third argument + max-params (if (= result (. (docs.get-global server :table) :fields :insert)) + 3 + (length signature))] + (if (and (< max-params number-of-args) (not infinite-params?)) {:range (message.ast->range server file ast) :message (.. "too many args. my analysis of the signature says we ignore any arguments past " max-params " arguments but you've provided " number-of-args) diff --git a/test/lint.fnl b/test/lint.fnl index ad98d9a..58db33c 100644 --- a/test/lint.fnl +++ b/test/lint.fnl @@ -287,33 +287,38 @@ (assert-ok "(let [x 5] (<= 1 x 4))") (assert-ok "(let [x 5] (> 4 x 1))") (assert-ok "(let [x 5] (>= 4 x 1))") - (assert-ok {:main.fnl "(let [x 5] (< 1 x 4))" - :flsproject.fnl "{:lints {:no-decreasing-comparison true}}"}) - (assert-ok {:main.fnl "(let [x 5] (<= 1 x 4))" - :flsproject.fnl "{:lints {:no-decreasing-comparison true}}"}) - (check {:main.fnl "(let [x 5] (> 4 x 1))" - :flsproject.fnl "{:lints {:no-decreasing-comparison true}}"} - [{:message "Use increasing operator instead of decreasing" - :code :no-decreasing-comparison - :range {:start {:character 11 :line 0} - :end {:character 20 :line 0}}}]) - (check {:main.fnl "(let [x 5] (>= 4 x 1))" - :flsproject.fnl "{:lints {:no-decreasing-comparison true}}"} - [{:message "Use increasing operator instead of decreasing" - :code :no-decreasing-comparison - :range {:start {:character 11 :line 0} - :end {:character 21 :line 0}}}]) - nil) + (let [add-opts #{:main.fnl $ :flsproject.fnl "{:lints {:no-decreasing-comparison true}}"}] + (assert-ok (add-opts "(let [x 5] (< 1 x 4))")) + (assert-ok (add-opts "(let [x 5] (<= 1 x 4))")) + (check (add-opts "(let [x 5] (> 4 x 1))") + [{:message "Use increasing operator instead of decreasing" + :code :no-decreasing-comparison + :range {:start {:character 11 :line 0} + :end {:character 20 :line 0}}}]) + (check (add-opts "(let [x 5] (>= 4 x 1))") + [{:message "Use increasing operator instead of decreasing" + :code :no-decreasing-comparison + :range {:start {:character 11 :line 0} + :end {:character 21 :line 0}}}]) + nil)) (fn test-arg-count [] ;; methods - (assert-ok {:main.fnl "(let [f :hi] (f:byte))" - :flsproject.fnl "{:lints {:mismatched-argument-count true}}"}) - (assert-ok {:main.fnl "(let [foo 10] (fn [] foo))" - :flsproject.fnl "{:lints {:mismatched-argument-count true}}"}) - (assert-ok {:main.fnl "(fn [])" - :flsproject.fnl "{:lints {:mismatched-argument-count true}}"}) - nil) + (let [add-opts #{:main.fnl $ :flsproject.fnl "{:lints {:not-enough-arguments true}}"}] + (check (add-opts "(fn foo [a b c ?d ?e] (print a b c ?d ?e))\n(foo 1 2)") + [{:code :not-enough-arguments}]) + (assert-ok (add-opts "(fn foo [a b c ?d ?e] (print a b c ?d ?e))\n(foo 1 2 3)")) + (assert-ok (add-opts "(fn foo [a b c ?d ?e] (print a b c ?d ?e))\n(foo 1 2 3 4 5)")) + (check (add-opts "(fn foo [a b c ?d ?e] (print a b c ?d ?e))\n(foo 1 2 3 4 5 6)") + [{:code :too-many-arguments}]) + (assert-ok (add-opts "(let [f :hi] (f:byte))")) + (check (add-opts "(let [f :hi] (f:sub))") + [{:code :not-enough-arguments}]) + (check (add-opts "(let [f :hi] (f:sub 1 2 3))") + [{:code :too-many-arguments}]) + (assert-ok (add-opts "(let [foo 10] (fn [] foo))")) + (assert-ok (add-opts "(fn [])")) + nil)) (fn test-duplicate-keys [] (assert-ok "{:a 1 :b 2}")