From 2a23d2f950c9c450d12ab7de5ddce9ddc4a0096f Mon Sep 17 00:00:00 2001 From: XeroOl Date: Wed, 30 Jul 2025 00:29:32 -0500 Subject: [PATCH] fix completion scope bug and fix completion macro call special case and add completions for nil and false and true and improve completion test speeds I did a lot in one big commit. fix bug where (each [foo (ipairs [])] |) wouldn't suggest foo fix bug with (doto foo |) wouldn't suggest specials and macros, even though it expands to | in a call position completions for true, false, nil, .nan, .inf docs for true, false, nil clarify error message when passing a folder to --lint improved test performance for completion tests --- src/fennel-ls/analyzer.fnl | 4 +- src/fennel-ls/compiler.fnl | 44 ++++---- src/fennel-ls/completion.fnl | 18 +++- src/fennel-ls/docs.fnl | 34 +++++-- src/fennel-ls/files.fnl | 2 + src/fennel-ls/lint.fnl | 2 +- src/fennel-ls/navigate.fnl | 10 +- src/fennel-ls/utils.fnl | 4 +- test/completion.fnl | 191 +++++++++++++---------------------- 9 files changed, 150 insertions(+), 159 deletions(-) diff --git a/src/fennel-ls/analyzer.fnl b/src/fennel-ls/analyzer.fnl index 03e19da..c99a3ee 100644 --- a/src/fennel-ls/analyzer.fnl +++ b/src/fennel-ls/analyzer.fnl @@ -183,7 +183,7 @@ find the definition `10`, but if `opts.stop-early?` is set, it would find (if (sym? ast) (search-symbol server file ast stack opts) (= 0 (length stack)) {:definition ast : file} ;; BASE CASE !! (= :table (type ast)) (search-table server file ast stack opts) - (= :string (type ast)) (search-document server (docs.get-global server :string) stack opts)) + (= :string (type ast)) (search-document server (docs.get-global server nil :string) stack opts)) nil)))) @@ -239,7 +239,7 @@ initialization-opts: {:stack ?list[ast] metadata (search-document server metadata stack opts) _ (case (find-local-definition file base-name scope) def (search-definition server file def stack opts) - _ (case (docs.get-global server base-name) + _ (case (docs.get-global server scope base-name) metadata (search-document server metadata stack opts)))))) (λ past? [?ast byte] diff --git a/src/fennel-ls/compiler.fnl b/src/fennel-ls/compiler.fnl index 52f2db6..74db5c6 100644 --- a/src/fennel-ls/compiler.fnl +++ b/src/fennel-ls/compiler.fnl @@ -6,13 +6,20 @@ compiler's plugin hook callbacks. It stores lexical info about which identifiers are declared / referenced in which places." (local {: sym? : list? : sequence? : table? : varg? : sym : view &as fennel} (require :fennel)) -(local {:scopes {:global {: specials}}} (require :fennel.compiler)) +(local {:scopes {:global {: specials} :compiler compiler-scope}} (require :fennel.compiler)) +(local {: make-compiler-env} (require :fennel.specials)) + (local docs (require :fennel-ls.docs)) (local message (require :fennel-ls.message)) (local searcher (require :fennel-ls.searcher)) (local utils (require :fennel-ls.utils)) + (local print print) +(local macro-globals + (icollect [k (pairs (make-compiler-env))] + k)) + (local nil* (sym :nil)) (fn special? [item] @@ -39,10 +46,6 @@ identifiers are declared / referenced in which places." (= (type candidate.specials) :table) (= (type candidate.gensyms) :table))) -(λ ast->macro-ast [ast] - [(fennel.list (sym :eval-compiler) - ((or (. table :unpack) _G.unpack) ast))]) - (λ multisym? [t] ;; check if t is a symbol with multiple parts, eg. foo.bar.baz (and (sym? t) @@ -97,7 +100,7 @@ identifiers are declared / referenced in which places." ;; find reference (let [name (string.match (tostring symbol) "[^%.:]+")] (case (or (find-definition (tostring name) scope) - (docs.get-global server name)) + (docs.get-global server scope name)) target (let [ref {: symbol : target : ref-type}] (tset references symbol ref) (when target.referenced-by @@ -214,12 +217,16 @@ identifiers are declared / referenced in which places." ;; handle the definitions of a function (define-function-name ast scope)) - (λ compile-for [_ast scope binding] - (define nil* binding scope)) + (λ compile-for [ast scope binding] + (tset scopes ast scope) + (define nil* binding scope)) - (λ compile-each [_ast scope bindings] + (λ compile-each [ast scope bindings] + (tset scopes ast scope) (each [_ binding (ipairs bindings)] - (define nil* binding scope))) + (if (and (sym? binding) + (not (. scope.gensyms (tostring binding)))) + (define nil* binding scope)))) (λ compile-fn [ast scope] (tset scopes ast scope) @@ -231,7 +238,8 @@ identifiers are declared / referenced in which places." (λ call [ast scope] "every list that is a call to a special or function" (tset calls ast true) - (tset scopes ast scope) + (when (not (. scopes ast)) + (tset scopes ast scope)) ;; Most calls aren't interesting, but here's the list of the ones that are: (let [head (. ast 1)] (case (and (sym? head) (tostring head)) @@ -341,13 +349,13 @@ identifiers are declared / referenced in which places." :severity message.severity.WARN :code :compiler-warning}))) - (local allowed-globals (docs.get-all-globals server)) - (icollect [extra-global (server.configuration.extra-globals:gmatch "[^ ]+") - &into allowed-globals] - extra-global) (let [macro-file? (or (file.uri:match "%.fnlm$") (= (file.text:sub 1 24) ";; fennel-ls: macro-file")) + allowed-globals (if macro-file? macro-globals + (icollect [extra-global (server.configuration.extra-globals:gmatch "[^ ]+") + &into (docs.get-all-globals server)] + extra-global)) plugin {:name "fennel-ls" :versions ["1.4.1" "1.4.2" "1.5.0" "1.5.1" "1.5.3" "1.5.4"] @@ -364,7 +372,9 @@ identifiers are declared / referenced in which places." :pre-each compile-each :pre-fn compile-fn :pre-do compile-do} - scope (fennel.scope) + scope (if macro-file? + (fennel.scope compiler-scope) + (fennel.scope)) opts {:filename file.uri :plugins [plugin] :allowedGlobals allowed-globals @@ -426,7 +436,7 @@ identifiers are declared / referenced in which places." (set _G.print #(values)) (table.insert defer #(set _G.print print)) - (each [_i form (ipairs (if macro-file? (ast->macro-ast ast) ast))] + (each [_i form (ipairs ast)] (filter-errors :compiler ;; this entire block of code is for making the instruction limit work (xpcall #(let [limiting? (not= server.configuration.compiler-instruction-limit -1)] diff --git a/src/fennel-ls/completion.fnl b/src/fennel-ls/completion.fnl index f98886d..7a4ff21 100644 --- a/src/fennel-ls/completion.fnl +++ b/src/fennel-ls/completion.fnl @@ -28,15 +28,17 @@ the client to forward information to the resolve request by setting the `data` to {: uri : byte}. When this capability exists, `server.can-do-good-completions?` is set to true and we report that we support completionItem/resolve." +(local fennel (require :fennel)) +(local {:metadata METADATA} (require :fennel.compiler)) + (local files (require :fennel-ls.files)) (local utils (require :fennel-ls.utils)) (local analyzer (require :fennel-ls.analyzer)) -(local fennel (require :fennel)) (local message (require :fennel-ls.message)) (local format (require :fennel-ls.formatter)) (local navigate (require :fennel-ls.navigate)) (local compiler (require :fennel-ls.compiler)) -(local {:metadata METADATA} (require :fennel.compiler)) +(local docs (require :fennel-ls.docs)) (λ textDocument/completion [server _send {: position :textDocument {: uri}}] ;; get the file @@ -49,9 +51,12 @@ is set to true and we report that we support completionItem/resolve." ;; find what ast objects are under the cursor (symbol parents) (analyzer.find-symbol file.ast byte) ;; check what context I'm in - in-call-position? (and (fennel.list? (. parents 1)) - (= symbol (. parents 1 1))) - ;; find the first one that contains a scope + in-call-position? (or (and (fennel.list? (. parents 1)) + (= symbol (. parents 1 1))) + (accumulate [?find nil ast (pairs file.calls) + &until ?find] + (= (. ast 1) symbol))) + ;; find the first parent that contains a scope scope (or (accumulate [?find nil _ parent (ipairs parents) &until ?find] (. file.scopes parent)) file.scope) @@ -100,6 +105,9 @@ is set to true and we report that we support completionItem/resolve." (io.stderr:write "BAD!!!! undocumented global: " (tostring global*) "\n") (add-completion! global* {}))))) + (each [k v (pairs docs.literals)] + (add-completion! k v)) + (var scope scope) (while scope (each [mangling (pairs scope.manglings)] diff --git a/src/fennel-ls/docs.fnl b/src/fennel-ls/docs.fnl index 8239f7d..1a43f87 100644 --- a/src/fennel-ls/docs.fnl +++ b/src/fennel-ls/docs.fnl @@ -9,7 +9,8 @@ Handles grabbing the documentation from sources other than fennel code; (local fennel (require :fennel)) (local {:metadata METADATA :scopes {:global {:specials SPECIALS - :macros MACROS}}} + :macros MACROS} + :compiler compiler-scope}} (require :fennel.compiler)) (local docset-ext ".lua") @@ -93,26 +94,36 @@ Handles grabbing the documentation from sources other than fennel code; &until g] (and enabled? (. (get-library library-name) global-name)))) -(fn get-global [server global-name] +(fn get-global [server ?scope global-name] (or (get-library-global server global-name) - (. (get-lua-version server.configuration.lua-version) global-name))) + (. (get-lua-version server.configuration.lua-version) global-name) + (if (do (var x ?scope) + (while (and x (not= x compiler-scope)) + (set x x.parent)) + (= x compiler-scope)) + (. (require :fennel-ls.docs.generated.compiler-env) global-name)))) -(local hardcoded-special-items - {:nil {:metadata {:fnl/docstring "Represents the absence of a useful value." +(local literals + {:nil {:definition (fennel.sym :nil) + :metadata {:fnl/docstring "Represents the absence of a useful value." :fls/itemKind "Keyword"}} - :true {:metadata {:fnl/docstring "A boolean value representing truth." + :true {:definition true + :metadata {:fnl/docstring "A boolean value representing truth." :fls/itemKind "Keyword"}} - :false {:metadata {:fnl/docstring "A boolean value representing falsehood." + :false {:definition false + :metadata {:fnl/docstring "A boolean value representing falsehood." :fls/itemKind "Keyword"}} - :.nan {:metadata {:fnl/docstring "NaN" + :.nan {:definition .nan + :metadata {:fnl/docstring "NaN" :fls/itemKind "Constant"}} - :.inf {:metadata {:fnl/docstring "inf" + :.inf {:definition .inf + :metadata {:fnl/docstring "inf" :fls/itemKind "Constant"}}}) (fn get-builtin [_server builtin-name] (or (. specials builtin-name) (. macros* builtin-name) - (. hardcoded-special-items builtin-name))) + (. literals builtin-name))) (λ validate-lua-version [lua-version invalid] (case (. lua-versions lua-version) @@ -133,4 +144,5 @@ Handles grabbing the documentation from sources other than fennel code; : get-builtin : get-all-globals : validate-lua-version - : validate-libraries} + : validate-libraries + : literals} diff --git a/src/fennel-ls/files.fnl b/src/fennel-ls/files.fnl index d78b15a..527923b 100644 --- a/src/fennel-ls/files.fnl +++ b/src/fennel-ls/files.fnl @@ -14,6 +14,8 @@ This module has high level helpers for creating/getting \"file\" objects." {: uri : text}) _ (case (io.open (utils.uri->path uri) "r") file (let [text (file:read :*a)] + (when (not text) + (error (.. "could not read file:" (utils.uri->path uri)))) (file:close) {: uri : text}))))) diff --git a/src/fennel-ls/lint.fnl b/src/fennel-ls/lint.fnl index 1a5e538..d1407e4 100644 --- a/src/fennel-ls/lint.fnl +++ b/src/fennel-ls/lint.fnl @@ -693,7 +693,7 @@ You can read more about how to add lints in docs/linting.md" (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)) + max-params (if (= result (. (docs.get-global server nil :table) :fields :insert)) 3 (length signature)) method-call? (and (sym? (. ast 1)) diff --git a/src/fennel-ls/navigate.fnl b/src/fennel-ls/navigate.fnl index dedfcd7..9ff3460 100644 --- a/src/fennel-ls/navigate.fnl +++ b/src/fennel-ls/navigate.fnl @@ -6,7 +6,7 @@ "TODO name this thing" (coroutine.wrap #(if (= (type definition.definition) :string) - (each [key value (pairs (-> (docs.get-global server :string) (. :fields)))] + (each [key value (pairs (-> (docs.get-global server nil :string) (. :fields)))] (when (or (= (type key) :string) (= (type key) :number)) (coroutine.yield key value true))) (do @@ -22,10 +22,15 @@ (when (or (= (type key) :string) (= (type key) :number)) (coroutine.yield key value)))))))) +(λ has-fields [server_ definition] + (or (= (type definition.definition) :string) + (fennel.table? definition.definition) + definition.fields)) + (λ _get-field [server definition key] (let [fields (or definition.fields (when (= (type definition.definition) :string) - (. docs.get-global server :string)))] + (. (docs.get-global server nil :string) :fields)))] (or (?. fields key) (when (fennel.table? definition.definition) (analyzer.search server definition.file definition.definition {} {:stack [key]}))))) @@ -53,4 +58,5 @@ {: _get-field : iter-fields + : has-fields : getmetadata} diff --git a/src/fennel-ls/utils.fnl b/src/fennel-ls/utils.fnl index 55776ea..2bdfb1c 100644 --- a/src/fennel-ls/utils.fnl +++ b/src/fennel-ls/utils.fnl @@ -160,9 +160,7 @@ WARNING: this is only used in the test code, not in the real language server" (fn multi-sym-split [symbol ?offset] (local symbol (tostring symbol)) - (if (or (= symbol ".") - (= symbol "..") - (= symbol "...") + (if (or (symbol:find "^%.") (= symbol ":") (= symbol "?.")) [symbol] diff --git a/test/completion.fnl b/test/completion.fnl index a4cf817..16f5d9b 100644 --- a/test/completion.fnl +++ b/test/completion.fnl @@ -85,45 +85,30 @@ ""))))) (expected completions)))) -(fn test-global [] - (check "(" [{:label :setmetatable :kind kinds.Function}] []) - (check "(" [:_G :debug :table :io :getmetatable :setmetatable :_VERSION - :ipairs :pairs :next] [:this-is-not-a-global]) - (check "#nil\n(" [:_G :debug :table :io :getmetatable :setmetatable - :_VERSION :ipairs :pairs :next] []) - (check "(if ge" [:getmetatable] []) - (check "(table.i" [:table.insert] []) - (check "(tablei" [:table.insert] []) - nil) - -(fn test-local [] - (check "(local x 10)\n(print |)" [:x] [:+]) - (check "(local x (doto 10 or and +))\n(print |)" [:x] []) - (check "(local x 10)\n|\n" [:x] []) - (check "(do (local x 10))\n|" [] [:x]) - (check "(let [foo 10 bar 20] - |)" [:foo :bar] []) - (check "(let [foo 10] - (let [bar 20] - |))" [:foo :bar] []) - (check "(let [foo 10] - (let [bar 20] - fo|))" [:foo :bar] []) - (check "(let [foo 10] - (let [bar 20] - |" [:foo :bar] []) - (check "(let [foo 10] - (let [bar 20] - fo|" [:foo :bar] []) - (check "(local foo 10) - (local bar (let [y foo] |" [:foo :y] []) - (check "(let [foo 10 - bar 20 - _ |" [:foo :bar] []) - (check "(let [foo 10 - bar 20 - _ fo|" [:foo :bar] []) - (check "(local x {:field 100})\n(if x.fi" [:x.field] []) +(fn test-basic [] + ;; basic scoping rules + (check "(local yes1 10) + (fn yes2 [no2]) + (do (local no1 11)) + (let [yes3 (fn [no3] no3)] + (let [{:item y} {:item {:es4 12}}] + (fn [yes5 {: yes6}] + (each [no4 {: no5} (pairs _G)] + nil) + (each [yes7 {: yes8} (pairs _G)] + |" + [:yes1 :yes2 :yes3 :y.es4 :yes5 :yes6 :yes7 :yes8 + :_G :debug :table :table.insert :io :getmetatable :_VERSION :ipairs :pairs :next {:label :setmetatable :kind kinds.Function} + :true :false :.nan :.inf :nil + {:label "coroutine.yield" + :documentation #(and $.value ($.value:find "```fnl\n(coroutine.yield ...)\n```" 1 true))}] + [:no1 :no2 :no3 :no4 :no5 :+ + :this-variable-does-not-exist + :_G.coroutine.yield + :_G._G.coroutine.yield + #(and (= nil $.documentation) + (not= $.label :yes8))]) + ;; no duplicates (check "(let [x 10] (let [x 10] x" (fn [completions] (faith.= 1 (accumulate [number-of-x 0 _ completion (ipairs completions.items)] @@ -131,72 +116,51 @@ (+ number-of-x 1) number-of-x)))) []) - ;; stretchy completions - (check "(local x {:field 100})\n(if fi" [:x.field] []) - (check "(local x {:field {:deep 100}})\n(if de" [:x.field.deep] []) - (check "(local t {:field (fn [foo] nil)})\n(t|" [:t.field] []) - (check "(local t {:field (fn [self] nil)})\n(t|" [:t:field] []) - (check "(local t {})\n(fn t.field [foo] nil)\n(t|" [:t.field] []) - (check "(local t {})\n(fn t.field [self] nil)\n(t|" [:t:field] []) + ;; completions of fields (nested) + (check "(local x {:y {:z {:a {:b 1}}}}) ; deep tables + (local m {}) ; split modules + (fn m.function []) + (fn m.method [self]) + (local m2 {:function m.function :method m.method}) + |" + [:x :x.y :x.y.z.a.b + :m.function + :m:method + :m2.function + :m2:method] + []) + + ;; regression test for not crashing + (check "(local x {:field (fn [self])})\n(x::f" [:x:field] []) nil) (fn test-builtin [] - (check "(|)" [:do :let :fn :doto :-> :-?>> :?.] []) - ;; it's not the language server's job to do filtering, - ;; so there's no negative assertions here for other symbols - (check "(d|)" [:do :doto] []) - ;; in fact, for fuzzy-matching clients, you especially want to make sure the server isn't filtering - (check "(t|)" [:doto :setmetatable] []) - ;; specials only are suggested in callable positions - (check "(do |)" [] [:do :let :fn :-> :-?>> :?.]) - (check "|\n" [] [:do :let :fn :-> :-?>> :?.]) - (check "d|\n" [] [:do :let :fn :-> :-?>> :?.]) + ;; specials and macros are only suggested in callable positions + (check "(macro funny [] `nil) + (|)" + [:do :let :fn :doto :-> :-?>> :?. :funny] + []) + (check "(do |)" + [] + [:do :let :fn :doto :-> :-?>> :?.]) + ;; tricky case + (check "(doto f |)" + [:do :let :fn :doto :-> :-?>> :?.] + []) nil) -(fn test-macro [] - (check "(macro funny [] `nil)\n(|)" [:funny] []) - nil) (fn test-local-in-macro [] - (check "(local item 10)\n(doto it|)" [:item] []) - (check "(local item 10)\n(doto |)" [:item] []) (check "(local item 10)\n(case 1 1 it|)" [:item] []) - (check "(local item 10)\n(case 1 1 |)" [:item] []) - nil) - -(fn test-fn-arg [] - (check "(fn [x] (print x))\n" [] [:x]) - (check "(fn [x] (print x))\n(print " [] [:x]) - (check "(fn foo [z]\n (let [x 10 y 20]\n |" [:x :y :z] []) - (check "(fn foo [arg1 arg2 arg3]\n |)" [:arg1 :arg2 :arg3] []) - (check "(fn foo [arg1 arg2 arg3]\n (do (do (do |))))" [:arg1 :arg2 :arg3] []) nil) (fn test-field [] - (check "(local x {:field (fn [self])})\n(x:" [:x:field] []) - (check "(local x {:field (fn [self])})\n(x:fi|" [:x:field] []) - ;; regression test for not crashing - (check "(local x {:field (fn [self])})\n(x::f" [] []) (check "(let [my-table {:foo 10 :bar 20}]\n my-table.|)))" [{:label :my-table.foo :kind kinds.Value} {:label :my-table.bar :kind kinds.Value}] []) - (check - {:main.fnl "(let [foo (require :fooo)] - foo.|)))" - :fooo.fnl "(fn my-export [x] (print x)) - {: my-export :constant 10}"} - [:foo.my-export :foo.constant] - []) - (check - {:main.fnl "(let [foo (require :fooo)] - foo.|)))" - :fooo.fnl "(local M {:constant 10}) - (fn M.my-export [x] (print x)) - M"} - [:foo.my-export :foo.constant] - []) + nil) (fn test-docs [] @@ -251,21 +215,17 @@ nil) (fn test-module [] - (check "(coroutine.y|" - [{:label "coroutine.yield" - :documentation #(and $.value ($.value:find "```fnl\n(coroutine.yield ...)\n```" 1 true))}] - [{:documentation #(= nil $)}]) - (check "(local c coroutine) - (c.y" - ["coroutine.yield" "c.yield"] - [{:documentation #(= nil $)}]) - (check "(local t table) - (t.i" - ["table.insert" "t.insert"] - [{:documentation #(= nil $)}]) - (check "debug.deb|" - [{:label "debug.debug" - :documentation #(and $.value ($.value:find "```fnl\n(debug.debug)\n```" 1 true))}] + (check + {:main.fnl "(let [foo (require :fooo) + bar (require :baar)] + |)" + :fooo.fnl "(fn my-export [x] (print x)) + {: my-export :constant 10}" + :baar.fnl "(local M {:constant 10}) + (fn M.my-export [x] (print x)) + M"} + [:foo.my-export :foo.constant + :bar.my-export :bar.constant] []) nil) @@ -275,18 +235,10 @@ (print foo)" [:foo] [:math]) - (check "(local f|) - (print foo)" - [:foo] - [:math]) (check "(let [f|] (print foo))" [:foo] [:math]) - (check "(let [foo |] ; cursor is in an expression so we want expressions now - (print foo))" - [:math] - []) nil) (fn test-no-completion [] @@ -303,11 +255,16 @@ [] [:math]) - (check "(fn foo|)" + (check "(fn foo| [])" [] ["foo|"]) nil) +(fn test-compiler-env [] + (check ";; fennel-ls: macro-file\n(" + [:sym :sym? :list :quote :icollect :math] + [:os])) + ;; ;; Future tests / features ;; ;; Scope Ordering Rules ;; (it "does not suggest locals past the suggestion location when a symbol is partially typed") @@ -326,14 +283,12 @@ ;; (it "suggests keys when typing out destructuring, as in `(local {: typinghere} (require :mod))`") ;; (it "only suggests tables for `ipairs` / begin work on type checking system") -{: test-global - : test-local +{: test-basic : test-builtin - : test-macro : test-local-in-macro - : test-fn-arg : test-field : test-docs : test-module : test-destructure - : test-no-completion} + : test-no-completion + : test-compiler-env}