bad-unpack lint warns about (+ (unpack t))

Also, there's a special message for the .. operator to let people know
about table.concat

src/fennel-ls/diagnostics.fnl

@@ -16,7 +16,7 @@ Goes through a file and mutates the `file.diagnostics` field, filling it with di
        :message (.. "unused definition: " (tostring symbol))
        :severity message.severity.WARN
        :code 301
-       :codeDescription "I don't know"})))
+       :codeDescription "unused-definition"})))
 
 (λ unknown-module-field [self file]
   "any multisym whose definition can't be found through a (require) call"
@@ -29,9 +29,10 @@ Goes through a file and mutates the `file.diagnostics` field, filling it with di
            :message (.. "unknown field: " (tostring symbol))
            :severity message.severity.WARN
            :code 302
-           :codeDescription "field checking I guess"})))))
+           :codeDescription "unknown-module-field"})))))
 
 (λ unnecessary-method [self file]
+  "a call to the : builtin that could just be a multisym"
   (icollect [[colon receiver method &as call] (pairs file.calls)
              &into file.diagnostics]
     (if (and (fennel.sym? colon ":")
@@ -39,14 +40,38 @@ Goes through a file and mutates the `file.diagnostics` field, filling it with di
              (. file.lexical call)
              (= :string (type method))
              (not (method:find "^[0-9]"))
-             ;; questions: #
              (not (method:find "[^!$%*+-/0-9<=>?A-Z\\^_a-z|\128-\255]")))
         (case (message.ast->range self file call)
           range {: range
                  :message (.. "unnecessary : call: use (" (tostring receiver) ":" method ")")
                  :severity message.severity.WARN
                  :code 303
-                 :codeDescription "unnecessary colon"}))))
+                 :codeDescription "unnecessary-method"}))))
+
+(local ops {"+" 1 "-" 1 "*" 1 "/" 1 "//" 1 "%" 1 ".." 1})
+(λ bad-unpack [self file]
+  "an unpack call leading into an operator"
+  (icollect [[op &as call] (pairs file.calls)
+             &into file.diagnostics]
+    (if (and (fennel.sym? op)
+             (. ops (tostring op))
+             ;; last item is an unpack call
+             (fennel.list? (. call (length call)))
+             (or (fennel.sym? (. call (length call) 1) :unpack)
+                 (fennel.sym? (. call (length call) 1) :_G.unpack)
+                 (fennel.sym? (. call (length call) 1) :table.unpack))
+             ;; Only the unpack call needs to be present in the original file.
+             (. file.lexical (. call (length call))))
+        (case (message.ast->range self file (. call (length call)))
+          range {: range
+                 :message (.. "faulty unpack call: " (tostring op) " isn't variadic at runtime."
+                              (if (fennel.sym? op "..")
+                                (let [unpackme (fennel.view (. call (length call) 2))]
+                                  (.. " Use (table.concat " unpackme ") instead of (.. (unpack " unpackme "))"))
+                                (.. " Use a loop when you have a dynamic number of arguments to " (tostring op))))
+                 :severity message.severity.WARN
+                 :code 304
+                 :codeDescription "bad-unpack"}))))
 
 (λ check [self file]
   "fill up the file.diagnostics table with linting things"
@@ -55,6 +80,8 @@ Goes through a file and mutates the `file.diagnostics` field, filling it with di
   (if self.configuration.checks.unknown-module-field
     (unknown-module-field self file))
   (if self.configuration.checks.unnecessary-method
-    (unnecessary-method self file)))
+    (unnecessary-method self file))
+  (if self.configuration.checks.bad-unpack
+    (bad-unpack self file)))
 
 {: check}

src/fennel-ls/state.fnl

@@ -96,7 +96,8 @@ in the \"self\" object."
    :version (option "lua54")
    :checks {:unused-definition (option true)
             :unknown-module-field (option true)
-            :unnecessary-method (option true)}})
+            :unnecessary-method (option true)
+            :bad-unpack (option true)}})
 
 (λ make-configuration [?c]
   (make-configuration-from-template default-configuration ?c))

test/diagnostic-test.fnl

@@ -170,7 +170,96 @@
         (is.nil (find [_ v (ipairs diagnostics)]
                  (match v
                    {:message "unused definition: &"}
-                   v)))))))
+                   v))))))
+
+  (it "does not warn about a generated unpack"
+    (let [self (create-client)
+          responses (self:open-file! filename "(-> [1 2 3] unpack +)")]
+      (match responses
+        [{:params {: diagnostics}}]
+        (is.nil (find [_ v (ipairs diagnostics)]
+                 (match v
+                   {:code 304}
+                   v))))))
+
+  (it "warns about unpack into +"
+    (let [self (create-client)
+          responses (self:open-file! filename "(+ (unpack [1 2 3]))")]
+      (match responses
+        [{:params {: diagnostics}}]
+        (is (find [_ v (ipairs diagnostics)]
+             (match v
+               {:code 304}
+               v))))))
+
+  (it "mentions table.concat if you use unpack into .."
+    (let [self (create-client)
+          responses (self:open-file! filename "(.. (table.unpack [\"hello\" \"world\"]))")]
+      (match responses
+        [{:params {: diagnostics}}]
+        (is (find [_ v (ipairs diagnostics)]
+             (and
+               (match v
+                 {:code 304}
+                 v)
+               (v.message:find "table.concat")))))))
+
+  (it "doesn't mention table.concat if you use unpack into another op"
+    (let [self (create-client)
+          responses (self:open-file! filename "(* (table.unpack [\"hello\" \"world\"]))")]
+      (match responses
+        [{:params {: diagnostics}}]
+        (is.nil (find [_ v (ipairs diagnostics)]
+                 (and
+                   (match v
+                     {:code 304}
+                     v)
+                   (v.message:find "table.concat"))))))))
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+