From f3d59840917fe6d900d15d262e001a9a205570bb Mon Sep 17 00:00:00 2001 From: Michele Campeotto Date: Wed, 2 Apr 2025 15:59:22 +0200 Subject: [PATCH] Improved signature help and active argument detection. This now uses a more correct algorithm to find the function call the user is typing and also adds active argument detection. --- src/fennel-ls/analyzer.fnl | 41 ++++++++-- src/fennel-ls/formatter.fnl | 15 ++-- src/fennel-ls/handlers.fnl | 12 +-- src/fennel-ls/message.fnl | 13 +++- test/signature-help.fnl | 149 +++++++++++++++++++++++++++++++----- 5 files changed, 191 insertions(+), 39 deletions(-) diff --git a/src/fennel-ls/analyzer.fnl b/src/fennel-ls/analyzer.fnl index be286a9..9c653e8 100644 --- a/src/fennel-ls/analyzer.fnl +++ b/src/fennel-ls/analyzer.fnl @@ -39,7 +39,6 @@ find the definition `10`, but if `opts.stop-early?` is set, it would find " (local {: sym? : list? : sequence? : varg?} (require :fennel)) -(local {: special?} (require :fennel-ls.compiler)) (local {: get-ast-info &as utils} (require :fennel-ls.utils)) (local files (require :fennel-ls.files)) (local docs (require :fennel-ls.docs)) @@ -218,7 +217,7 @@ find the definition `10`, but if `opts.stop-early?` is set, it would find _ (case (find-local-definition file name scope) def (search-val server file def.definition (stack-add-keys! stack def.keys) opts)))))) -(λ _past? [?ast byte] +(λ past? [?ast byte] ;; check if a byte is past an ast object (and (= (type ?ast) :table) (get-ast-info ?ast :byteend) @@ -277,12 +276,37 @@ find the definition `10`, but if `opts.stop-early?` is set, it would find (λ find-nearest-call [_server file byte] "Find the nearest call -returns the called symbol and the argument number position points to" - (let [(_ [[call] parent-call]) (find-symbol file.ast byte) - parent (?. parent-call 1)] - (if (and parent (special? parent)) - (values parent -1) - (values call -1)))) +returns the called symbol and the number of the argument closest to byte" + (λ find-list [[call & parents]] + (if (. file.calls call) + call + (if (next parents) + (find-list parents)))) + + (λ arg-index [call byte] + ;; TODO: special handling for binding forms so we can point to the + ;; individual arguments in an each or accumulate call. + ;; Also need to split them up in formatter.fnl + (faccumulate [index nil + i (length call) 1 -1 &until index] + (if (contains? (. call i) byte) + ; -2 because this is the 3rd element of the list, but + ; the 2nd argument to the call, and LSP is 0-indexed + (- i 2) + (past? (. call i) byte) + ; this means we are either at the end of the list or + ; inserting between two arguments + (- i 1)))) + + (case-try (find-symbol file.ast byte) + (_symbol parents) (find-list parents) + [callee &as call] (values callee (arg-index call byte)) + (catch _ nil))) + +(λ find-definition [server file symbol ?byte] + (if (. file.definitions symbol) + (. file.definitions symbol) + (search-main server file symbol {:stop-early? false} {:byte ?byte}))) (λ find-nearest-definition [server file symbol ?byte] (if (. file.definitions symbol) @@ -292,6 +316,7 @@ returns the called symbol and the argument number position points to" {: find-symbol : find-nearest-call : find-nearest-definition + : find-definition : search-main : search-name-and-scope :search-ast search-val} diff --git a/src/fennel-ls/formatter.fnl b/src/fennel-ls/formatter.fnl index 9f82860..ec80fd3 100644 --- a/src/fennel-ls/formatter.fnl +++ b/src/fennel-ls/formatter.fnl @@ -19,6 +19,11 @@ user code. Fennel-ls doesn't support user-code formatting as of now." (.. ": " $2 $3))) _ (tostring arg))) +(fn render-arglist [?arglist] + (if ?arglist + (icollect [_ arg (ipairs ?arglist)] + {:label (render-arg arg)}))) + (fn fn-signature-format [special name args] (let [args (case (type (?. args 1)) :table (icollect [_ v (ipairs args)] @@ -95,14 +100,14 @@ fntype is one of fn or λ or lambda" {:fntype ?fntype :name ?name :arglist ?arglist :docstring ?docstring} {:label (fn-signature-format ?fntype ?name ?arglist) :documentation ?docstring - :parameters (if ?arglist - (icollect [_ arg (ipairs ?arglist)] - {:label (render-arg arg)}))} + :parameters (render-arglist ?arglist)} _ (case symbol {: binding :metadata {:fnl/arglist arglist :fnl/docstring docstring}} {:label (fn-signature-format :fn binding arglist) - :documentation docstring} - _ {:label (.. "ERROR: don't know how to format " (tostring symbol)) + :documentation docstring + :parameters (render-arglist arglist)} + _ {:label (.. "ERROR: don't know how to format " + (view symbol {:one-line? true :depth 3})) :documentation (code-block (view symbol {:depth 3}))}))) diff --git a/src/fennel-ls/handlers.fnl b/src/fennel-ls/handlers.fnl index b64513f..13574aa 100644 --- a/src/fennel-ls/handlers.fnl +++ b/src/fennel-ls/handlers.fnl @@ -138,16 +138,18 @@ Every time the client sends a message, it gets handled by a function in the corr (catch _ nil)))) (λ requests.textDocument/signatureHelp [server - _send - {:textDocument {: uri} : position}] + _send + {:textDocument {: uri} : position}] (let [file (files.get-by-uri server uri) byte (utils.position->byte file.text position server.position-encoding)] (case-try (analyzer.find-nearest-call server file byte) (symbol active-parameter) - (analyzer.find-nearest-definition server file symbol) - {:indeterminate nil &as result} + (analyzer.find-definition server file symbol) + {:indeterminate nil &as definition} + (formatter.signature-help-format definition) + signature (message.symbol->signature-help server file symbol - (formatter.signature-help-format result) + signature active-parameter) (catch _ nil)))) diff --git a/src/fennel-ls/message.fnl b/src/fennel-ls/message.fnl index 5b36af8..3519a21 100644 --- a/src/fennel-ls/message.fnl +++ b/src/fennel-ls/message.fnl @@ -91,9 +91,16 @@ LSP json objects." :kind ?kind :edit {:changes {uri (diagnostic.quickfix)}}})) -(λ symbol->signature-help [_server _file _call signature _active-parameter] - {:signatures [signature] - :activeSignature 0}) +(λ symbol->signature-help [_server _file _call signature active-parameter] + (let [params-count (length signature.parameters) + {:label last} (. signature.parameters params-count)] + {:signatures [signature] + :activeSignature 0 ; we only ever have one signature + :activeParameter (if (and (>= active-parameter params-count) + (= last "...")) + (- params-count 1) + (>= active-parameter 0) + active-parameter)})) (λ multisym->range [server file ast n] (let [spl (utils.multi-sym-split ast) diff --git a/test/signature-help.fnl b/test/signature-help.fnl index f3e459d..17b4c56 100644 --- a/test/signature-help.fnl +++ b/test/signature-help.fnl @@ -5,7 +5,9 @@ (λ check-signature [expected response] (case response {:signatures [{:label signature}]} - (faith.= expected.signature signature) + (do + (faith.= expected.signature signature) + (faith.= expected.activeParameter response.activeParameter)) ;; fail _ (faith.is nil (.. "Invalid response: " (view response))))) @@ -15,62 +17,173 @@ (check-signature expected result))) (fn test-fn-definition [] + (check "(fn |)" + {:signature "(fn name? args docstring? ...)" + :activeParameter 0}) + + (check "(fn some-nam|)" + {:signature "(fn name? args docstring? ...)" + :activeParameter 0}) + + (check "(fn some-name [|]" + {:signature "(fn name? args docstring? ...)" + :activeParameter 1}) + + (check "(fn some-name [arg|])" + {:signature "(fn name? args docstring? ...)" + :activeParameter 1}) + + (check "(fn some-name [arg1 arg2] + \"docstring|\")" + {:signature "(fn name? args docstring? ...)" + :activeParameter 2}) + + (check "(fn some-name [arg1 arg2] + \"docstring\" + |" + {:signature "(fn name? args docstring? ...)" + :activeParameter 2})) + +(fn test-local-function [] (check "(fn func [arg1 arg2] (print :hello)) (func|)" - {:signature "(func arg1 arg2)"}) + {:signature "(func arg1 arg2)" + :activeParameter nil}) (check "(fn func [arg1 arg2] (print :hello)) (func |)" - {:signature "(func arg1 arg2)"}) + {:signature "(func arg1 arg2)" + :activeParameter 0}) (check "(fn func [arg1 arg2] (print :hello)) (func a1|)" - {:signature "(func arg1 arg2)"}) + {:signature "(func arg1 arg2)" + :activeParameter 0}) (check "(fn func [arg1 arg2] (print :hello)) (func a|1 a2)" - {:signature "(func arg1 arg2)"}) + {:signature "(func arg1 arg2)" + :activeParameter 0}) (check "(fn func [arg1 arg2] (print :hello)) (func a1 a2|)" - {:signature "(func arg1 arg2)"}) + {:signature "(func arg1 arg2)" + :activeParameter 1}) (check "(fn func [arg1 arg2] (print :hello)) (func a1 a|2)" - {:signature "(func arg1 arg2)"})) + {:signature "(func arg1 arg2)" + :activeParameter 1})) + +(fn test-literals [] + (check "(fn func [arg1 arg2] (print :hello)) + (func arg 10|2)" + {:signature "(func arg1 arg2)" + :activeParameter 1}) + + (check "(fn func [arg1 arg2] (print :hello)) + (func arg \"10|2\")" + {:signature "(func arg1 arg2)" + :activeParameter 1})) + +(fn test-vararg [] + (check "(or a b|)" + {:signature "(or a b ...)" + :activeParameter 1}) + + (check "(or a b c|)" + {:signature "(or a b ...)" + :activeParameter 2}) + + (check "(or a b c d e|)" + {:signature "(or a b ...)" + :activeParameter 2})) (fn test-lua-builtin [] (check "(error msg lvl|)" - {:signature "(error message ?level)"})) + {:signature "(error message ?level)" + :activeParameter 1}) + + (check "(error msg lvl extr|)" + {:signature "(error message ?level)" + :activeParameter 2})) (fn test-multisym [] (check "(table.concat tbl s|)" - {:signature "(table.concat list ?sep ?i ?j)"})) + {:signature "(table.concat list ?sep ?i ?j)" + :activeParameter 1})) (fn test-destructuring-arg [] (check "(fn dstr [{:field name} arg2] {}) (dstr |)" - {:signature "(dstr {:field name} arg2)"}) + {:signature "(dstr {:field name} arg2)" + :activeParameter 0}) (check "(fn dstr [{:field name} arg2] {}) (dstr arg1 ar|)" - {:signature "(dstr {:field name} arg2)"})) + {:signature "(dstr {:field name} arg2)" + :activeParameter 1}) -(fn test-special [] + (check "(fn dstr [{:field |} arg2] {})" + {:signature "(fn name? args docstring? ...)" + :activeParameter 1})) + +(fn test-binding-form [] (check "(each |)" - {:signature "(each [key value (iterator)] ...)"}) + {:signature "(each [key value (iterator)] ...)" + :activeParameter 0}) (check "(each [|])" - {:signature "(each [key value (iterator)] ...)"}) + {:signature "(each [key value (iterator)] ...)" + :activeParameter 0}) (check "(each [k val|])" - {:signature "(each [key value (iterator)] ...)"}) + {:signature "(each [key value (iterator)] ...)" + :activeParameter 0}) - (check "(each [33|])" - {:signature "(each [key value (iterator)] ...)"})) + (check "(let [a 0] + (error |))" + {:signature "(error message ?level)" + :activeParameter 0}) + + (check "(let [a|] (error))" + {:signature "(let [name1 val1 ... nameN valN] ...)" + :activeParameter 0})) + +(fn test-indirect-definition [] + (check "(let [a error] + (a |))" + {:signature "(error message ?level)" + :activeParameter 0})) + +(fn test-destructuring-binding [] + (check "(let [(a b|) {}]" + {:signature "(let [name1 val1 ... nameN valN] ...)" + :activeParameter 0}) + + (check "(let [(a {:b |}) {}]" + {:signature "(let [name1 val1 ... nameN valN] ...)" + :activeParameter 0}) + + (check "(let [{:field |} {}]" + {:signature "(let [name1 val1 ... nameN valN] ...)" + :activeParameter 0}) + + (check "(let [{:field {: nested |" + {:signature "(let [name1 val1 ... nameN valN] ...)" + :activeParameter 0}) + + (check "(let [{:field {: nested &as |}} {}]" + {:signature "(let [name1 val1 ... nameN valN] ...)" + :activeParameter 0})) {: test-fn-definition + : test-local-function + : test-literals + : test-vararg : test-lua-builtin : test-multisym : test-destructuring-arg - : test-special} + : test-binding-form + : test-indirect-definition + : test-destructuring-binding}