fix set rename issue ~xerool/fennel-ls#8

Now unused variables and var-not-set lints use a different system.

References are now of type:
{:symbol sym :target ast :ref-type (or :read :write :mutate)}
instead of ast.

unused var will warn if all the references are :write references.

src/fennel-ls/compiler.fnl

@@ -3,11 +3,21 @@ This file is responsible for the low level tasks of analysis. Its main job
 is to recieve a file object and run all of the basic analysis that will be used
 later by fennel-ls.language to answer requests from the client."
 
-(local {: sym? : list? : sequence? : table? : sym &as fennel} (require :fennel))
+(local {: sym? : list? : sequence? : table? : sym : view &as fennel} (require :fennel))
 (local message (require :fennel-ls.message))
 (local utils (require :fennel-ls.utils))
 (local searcher (require :fennel-ls.searcher))
 
+(fn scope? [candidate]
+  ;; just checking a couple of the fields
+  (and
+    (= (type candidate) :table)
+    (= (type candidate.includes) :table)
+    (= (type candidate.macros) :table)
+    (= (type candidate.manglings) :table)
+    (= (type candidate.specials) :table)
+    (= (type candidate.gensyms) :table)))
+
 ;; words surrounded by - are symbols,
 ;; because fennel doesn't allow 'require in a runtime file
 (local -require- (sym :require))
@@ -64,24 +74,34 @@ later by fennel-ls.language to answer requests from the client."
         require-calls {}] ; the keys are all the calls that start with `require
 
     (λ find-definition [name ?scope]
-      (when ?scope
+      (if ?scope
         (or (. definitions-by-scope ?scope name)
             (find-definition name ?scope.parent))))
 
-    (λ reference [ast scope]
+    (λ reference [symbol scope ref-type]
-      ;; Add a reference to the references
+      (assert (or (= ref-type :read) (= ref-type :write) (= ref-type :mutate)) "wrong ref-type")
-      (assert (sym? ast))
+      (assert (sym? symbol) :not-a-symbol)
+      (assert (scope? scope) :not-a-scope)
       ;; find reference
-      (let [name (string.match (tostring ast) "[^%.:]+")]
+      (let [name (string.match (tostring symbol) "[^%.:]+")]
         (case (find-definition (tostring name) scope)
           target
-          (do
+          (if (. references symbol)
-            (tset references ast target)
+            (do ;; already exists
-            (table.insert target.referenced-by ast)))))
+              (assert (= symbol (. references symbol :symbol)) (.. "the symbol should always be the same")))
+              ;; (assert (= target (. references symbol :target)) (.. "different targets: " (view target) (view (. references symbol :target)))))
+              ;; (print "old and new" ref-type (. references symbol :ref-type)))
+            (let [ref {: symbol : target : ref-type}]
+              (tset references symbol ref)
+              (table.insert target.referenced-by ref))))))
 
     (λ symbol-to-expression [ast scope ?reference?]
-      (when (or ?reference? (fennel.multi-sym? ast))
+      (assert (sym? ast) "symbols only")
-        (reference ast scope)))
+      (reference ast scope (if ?reference?
+                             :read
+                             (not (multisym? ast))
+                             :write
+                             :mutate)))
 
     (λ define [?definition binding scope ?opts]
       ;; Add a definition to the definitions
@@ -124,22 +144,19 @@ later by fennel-ls.language to answer requests from the client."
       (recurse binding []))
 
     (λ mutate [_?definition binding scope]
-      ;; for now, mutating a field counts as a reference I guess
       (λ recurse [binding keys]
         (if (sym? binding)
-            (let [
+            ;; (let [;; future work may need to care about mutations
-                  ;; ;; future work may need to care about mutations
+            ;;       _mutation
-                  ;; _mutation
+            ;;       {: binding
-                  ;; {: binding
+            ;;        :new-definition ?definition
-                  ;;  :new-definition ?definition
+            ;;        :keys (if (< 0 (length keys))
-                  ;;  :keys (if (< 0 (length keys))
+            ;;                (fcollect [i 1 (length keys)]
-                  ;;          (fcollect [i 1 (length keys)]
+            ;;                  (. keys i)))}]
-                  ;;            (. keys i)))}
+            (when (not (multisym? binding))
-                  name (string.match (tostring binding) "[^%.:]+")]
+              (reference binding scope :write)
-              (case (find-definition (tostring name) scope)
+              (if (. references binding)
-                target (if (multisym? binding)
+                (tset (. references binding :target) :var-set true)))
-                           (table.insert target.referenced-by binding)
-                           (set target.var-set true))))
             (= :table (type binding))
             (each [k v (iter binding)]
               (table.insert keys k)
@@ -147,7 +164,7 @@ later by fennel-ls.language to answer requests from the client."
               (table.remove keys))))
       (recurse binding []))
 
-    (λ destructure [to from scope {:declaration ?declaration? &as opts}]
+    (λ destructure [to from scope {:declaration ?declaration? : symtype &as opts}]
       ;; I really don't understand symtype
       ;; I think I need an explanation
       (if ?declaration?
@@ -164,9 +181,9 @@ later by fennel-ls.language to answer requests from the client."
           (set target.fields (or target.fields {}))
           (tset target.fields field
             {:binding multisym
-             :definition ast
+             :definition ast}))))
-             ;; referenced-by inherits from all other symbols
+             ;; ;; referenced-by inherits from all other symbols
-             :referenced-by (or (?. definitions multisym :referenced-by) [])}))))
+             ;; :referenced-by (or (?. definitions multisym :referenced-by) [])}))))
 
     (λ define-function-name [ast scope]
       ;; add a function definition to the definitions
@@ -221,7 +238,7 @@ later by fennel-ls.language to answer requests from the client."
         (tset require-calls ast true)
         ;; fennel expands multisym calls into the `:` special, so we need to reference the symbol while we still can
         (where [sym] (multisym? sym) (: (tostring sym) :find ":"))
-        (reference sym scope)))
+        (reference sym scope :read)))
 
     (λ recoverable? [msg]
       (or (= 1 (msg:find "unknown identifier"))
@@ -295,6 +312,8 @@ later by fennel-ls.language to answer requests from the client."
                             (true ?item1 ?item2) (values ?item1 ?item2)
                             (where (or (nil err) (false err)) (not (err:find "^[^\n]-__NOT_AN_ERROR\n")))
                             (if (os.getenv :TESTING)
+                              (print (.. "\nYou have crashed fennel-ls (or the fennel " component ") with the following message\n:" err
+                                         "\n\n^^^ the error message above here is the root problem\n\n"))
                               (error (.. "\nYou have crashed fennel-ls (or the fennel " component ") with the following message\n:" err
                                          "\n\n^^^ the error message above here is the root problem\n\n"))
                               (table.insert diagnostics

src/fennel-ls/diagnostics.fnl

@@ -10,8 +10,12 @@ Goes through a file and mutates the `file.diagnostics` field, filling it with di
 (λ unused-definition [self file]
   "local variable that is defined but not used"
   (icollect [symbol definition (pairs file.definitions) &into file.diagnostics]
-    (if (and (= 0 (length definition.referenced-by))
+    (if (and (not= "_" (: (tostring symbol) :sub 1 1))
-             (not= "_" (: (tostring symbol) :sub 1 1)))
+             (not (accumulate [reference false
+                               _ ref (ipairs definition.referenced-by)
+                               &until reference]
+                    (or (= ref.ref-type :read)
+                        (= ref.ref-type :mutate)))))
       {:range (message.ast->range self file symbol)
        :message (.. "unused definition: " (tostring symbol))
        :severity message.severity.WARN
@@ -77,9 +81,9 @@ Goes through a file and mutates the `file.diagnostics` field, filling it with di
   (icollect [symbol definition (pairs file.definitions) &into file.diagnostics]
     (if (and definition.var? (not definition.var-set))
         {:range (message.ast->range self file symbol)
-         :message (.. "var is never set: " (tostring symbol))
+         :message (.. "var is never set: " (tostring symbol) " Consider using (local) instead of (var)")
          :severity message.severity.WARN
-         :code 301
+         :code 305
          :codeDescription "var-never-set"})))
 
 (λ check [self file]

src/fennel-ls/handlers.fnl

@@ -90,7 +90,7 @@ Every time the client sends a message, it gets handled by a function in the corr
       symbol
       (language.find-nearest-definition self file symbol byte)
       (where (definition def-file) (not= definition.referenced-by nil))
-      (let [result (icollect [_ symbol (ipairs definition.referenced-by)]
+      (let [result (icollect [_ {: symbol} (ipairs definition.referenced-by)]
                      (message.range-and-uri self def-file symbol))]
         (if ?include-declaration?
           (table.insert result
@@ -188,7 +188,7 @@ Every time the client sends a message, it gets handled by a function in the corr
       (language.find-nearest-definition self file symbol symbol.bytestart)
       ;; TODO we are assuming that every reference is in the same file
       (where (definition def-file) (not= definition.referenced-by nil))
-      (let [usages (icollect [_ symbol (ipairs definition.referenced-by)
+      (let [usages (icollect [_ {: symbol} (ipairs definition.referenced-by)
                               &into [{:range (message.multisym->range self def-file definition.binding 1)
                                       :newText new-name}]]
                      (if (not (rawequal symbol definition.binding))

src/fennel-ls/language.fnl

@@ -2,7 +2,7 @@
 The high level analysis system that does deep searches following
 the data provided by compiler.fnl."
 
-(local {: sym? : list? : sequence? : varg? : sym} (require :fennel))
+(local {: sym? : list? : sequence? : varg? : sym &as fennel} (require :fennel))
 (local utils (require :fennel-ls.utils))
 (local state (require :fennel-ls.state))
 
@@ -35,15 +35,16 @@ the data provided by compiler.fnl."
   (stack-add-split! stack (utils.multi-sym-split symbol)))
 
 (λ search-assignment [self file assignment stack opts]
-  (let [{:binding _
+  (assert assignment.target (.. "WRONG TYPE" (fennel.traceback)))
-         :definition ?definition
+  (let [{:target {:binding _
-         :keys ?keys
+                  :definition ?definition
-         :fields ?fields} assignment]
+                  :keys ?keys
+                  :fields ?fields}} assignment]
     (if (and (= 0 (length stack)) opts.stop-early?)
-        (values assignment file) ;; BASE CASE!!
+        (values assignment.target file) ;; BASE CASE!!
         ;; search a virtual field from :fields
         (and (not= 0 (length stack)) (?. ?fields (. stack (length stack))))
-        (search-assignment self file (. ?fields (table.remove stack)) stack opts)
+        (search-assignment self file {:target (. ?fields (table.remove stack))} stack opts)
         (search-ast self file ?definition (stack-add-keys! stack ?keys) opts))))
 
 (λ search-symbol [self file symbol stack opts]

test/diagnostic-test.fnl

@@ -88,7 +88,7 @@
         [{:params {: diagnostics}}]
         (is (find [_ v (ipairs diagnostics)]
              (match v
-               {:message "var is never set: x"
+               {:code 305
                 :range {:start {:character 5 :line 0}
                         :end   {:character 6 :line 0}}}
                v))
@@ -112,7 +112,7 @@
   (it "does not warn if a field is used"
     (let [self (create-client)
           responses (self:open-file! filename "(fn [a b] (set a.x 10) (fn b.f []))")]
-      (assert (not (?. responses 1 :params :diagnostics 1)))))
+      (assert (not (?. responses 1 :params :diagnostics 1)) (?. responses 1 :params :diagnostics 1 :message))))
 
   (it "warns when using the : special when a multisym would do"
     (let [self (create-client)]
@@ -150,20 +150,42 @@
                    v)))
         _ (error "did not match"))))
 
-  (it "warns if a var is written but not read"
+  (it "warns 'unused' if a var is written but not read"
     (let [self (create-client)
           responses (self:open-file! filename "(var x 1) (set x 2) (set [x] [3])")]
       (match responses
         [{:params {: diagnostics}}]
         (is (find [_ v (ipairs diagnostics)]
              (match v
-               {:message "unused definition: x"
+               {:code 301
                 :range {:start {:character 5 :line 0}
                         :end   {:character 6 :line 0}}}
                v))
             "not found")
         _ (error "did not match"))))
 
+  (it "warns 'var-never-set' if a var is not written"
+    (let [self (create-client)
+          responses (self:open-file! filename "(var x 1) (print x)")]
+      (match responses
+        [{:params {: diagnostics}}]
+        (is (find [_ v (ipairs diagnostics)]
+             (match v
+               {:code 305
+                :range {:start {:character 5 :line 0}
+                        :end   {:character 6 :line 0}}}
+               v))
+            "not found")
+        _ (error "did not match"))))
+
+  (it "does not warn 'var-never-set' if a var is written"
+    (let [self (create-client)
+          responses (self:open-file! filename "(var x 1) (set x 2) (print x)")]
+      (match responses
+        [{:params {: diagnostics}}]
+        (is.equal 0 (length diagnostics) "this code has no problems")
+        _ (error "did not match"))))
+
   (it "does not warn on ampersand in destructuring"
     (let [self (create-client)
           responses (self:open-file! filename "(let [[x & y] [1 2 3]] (print x (. y 1) (. y 2)))")]

test/rename-test.fnl

@@ -39,4 +39,16 @@
 
  (it "renames a sym inside of lambda"
    (check-rename "(λ [foo] (print foo))" 0 6 :something
-                 "(λ [something] (print something))")))
+                 "(λ [something] (print something))"))
+
+ (it "renames a sym inside of set"
+   (check-rename "(var x 10)\n(set x 20)" 1 6 :something
+                 "(var something 10)\n(set something 20)"))
+
+ (it "renames a sym inside of set 2"
+   (check-rename "(var x 10)\n(var m 0)\n(set (m x) (values 10 20))" 2 8 :something
+                 "(var something 10)\n(var m 0)\n(set (m something) (values 10 20))"))
+
+ (it "renames a sym inside of macro that uses multiple times"
+   (check-rename "(var x 10)\n(doto x (set 20) (set 30))" 1 6 :something
+                 "(var something 10)\n(doto something (set 20) (set 30))")))