Eliminate many incorrect unused-namespaces warnings

Two main categories of problems fixed here are (a) better parsing of
ns forms to correctly reconstruct namespace name when they are given
in 'nested' form, and (b) no longer ignore macro invocations when
looking for uses of vars in a namespace.

There is still at least one bug where uses of Vars in syntax-quoted
forms are not recognized as uses of a namespace, leading to some
remaining incorrect unused-namespace warnings, including one that is
checked in as part of the Eastwood test suite, marked with a "TBD" in
a comment as needing fixing.

On crucible, these changes cause raw number of unused-namespaces
warnings to go down from 618 to 390.

cases/testcases/unusednss.clj

@@ -0,0 +1,33 @@
+(ns testcases.unusednss
+  (:use testcases.unusednss2)
+  (:require [clojure [string :as s]]
+            [clojure [repl :as r]]
+            [clojure.data :as d]))
+
+;; clojure.repl should be considered used because of the macro
+;; invocation r/doc in foo1.
+
+(defn foo1 [x]
+  (r/doc x))
+
+
+;; TBD: clojure.data should be considered used because of the function
+;; call d/diff in macro foo2.  However, as of right now Eastwood does
+;; not search through syntax-quoted forms for uses of namespaced vars.
+
+(defmacro foo2 [x y]
+  `(d/diff ~x ~y))
+
+
+;; testcases.unusednss2 should be considered used because of the use
+;; of atom1 in foo3.
+
+(defn foo3 [n]
+  (swap! atom1 conj n))
+
+
+;; Should be no warning for this require, because it is outside of ns
+;; form.
+
+(defn foo4 [ns]
+  (require ns))

cases/testcases/unusednss2.clj

@@ -0,0 +1,3 @@
+(ns testcases.unusednss2)
+
+(def atom1 (atom []))

src/eastwood/linters/unused.clj

@@ -3,6 +3,7 @@
             [clojure.string :as str]
             [clojure.java.io :as io]
             [eastwood.copieddeps.dep10.clojure.tools.reader.edn :as edn]
+            [eastwood.copieddeps.dep9.clojure.tools.namespace.parse :as parse]
             [clojure.pprint :as pp]
             [eastwood.util :as util]
             [eastwood.passes :as pass]
@@ -31,6 +32,14 @@
        (map :var)
        set))
 
+(defn macros-invoked [asts]
+  (->> asts
+       (mapcat ast/nodes)
+       (mapcat :eastwood/partly-resolved-forms)
+       (map util/safe-first)
+       (remove nil?)
+       set))
+
 (defn unused-private-vars [{:keys [asts]} opt]
   (let [pdefs (private-non-const-defs asts)
         vars-used-set (vars-used asts)]
@@ -199,25 +208,20 @@ Example: (all-suffixes [1 2 3])
 
 ;; Unused namespaces
 
-;; If require is called outside of an ns form, on an argument that is
-;; not a constant, the #(-> % :expr :form) step below can introduce
-;; nil values into the sequence.  For now, just filter those out to
-;; avoid warnings about namespace 'null' not being used.  Think about
-;; if there is a better way to handle such expressions.  Examples are
-;; in core namespace clojure.main, and contrib library namespaces
-;; clojure.tools.namespace.reload and clojure.test.generative.runner.
-(defn required-namespaces [exprs]
-  (->> (mapcat ast/nodes exprs)
-       (filter #(and (= (:op %) :invoke)
-                     (let [v (-> % :fn :var)]
-                       (#{'clojure.core/require 'clojure.core/use}
-                        (util/var-to-fqsym v)))))
-       (mapcat :args)
-       (map #(-> % :expr :form))
-       (remove nil?)
-       (map #(if (coll? %) (first %) %))
-       (remove keyword?)
-       (into #{})))
+(defn ns-form-asts [asts]
+  (->> (mapcat ast/nodes asts)
+       (filter #(= 'clojure.core/ns
+                   (util/safe-first
+                    (first (:eastwood/partly-resolved-forms %)))))))
+
+(defn required-namespaces [ns-asts]
+  (->> ns-asts
+       (mapcat #(nnext (first (:eastwood/partly-resolved-forms %))))
+       (filter (fn [f] (#{:require :use} (first f))))
+       (mapcat rest)
+       (mapcat #(#'parse/deps-from-libspec nil %))
+       set))
+
 
 ;; TBD: (-> asts first ...) below is written with the assumption that
 ;; the ns form must be first in a file.  It usually is, but
@@ -232,13 +236,29 @@ Example: (all-suffixes [1 2 3])
 ;; beginning of the ns form it is in.
 
 (defn unused-namespaces [{:keys [asts]} opt]
-  (let [curr-ns (-> asts first :statements first :args first :expr :form)
-        required (required-namespaces asts)
-        used (set (map #(-> ^clojure.lang.Var % .ns .getName)
-                       (vars-used asts)))]
-    (for [ns (set/difference required used)]
-      {:linter :unused-namespaces
-       :msg (format "Namespace %s is never used in %s" ns curr-ns)})))
+  (let [;curr-ns (-> asts first :statements first :args first :expr :form)
+        ns-asts (ns-form-asts asts)
+        loc (pass/code-loc (first ns-asts))
+        curr-ns (-> ns-asts first :form second second second)
+        required (required-namespaces ns-asts)
+        used-vars (vars-used asts)
+        used-macros (macros-invoked asts)
+;;        _ (do
+;;            (println "dbg: required namespaces:")
+;;            (pp/pprint required)
+;;            (println "dbg: vars used:")
+;;            (pp/pprint (map (juxt #(.getName (.ns %)) #(.sym %)) used-vars))
+;;            (println "dbg: macros used:")
+;;            (pp/pprint used-macros))
+        used-namespaces (set
+                         (concat (map #(-> ^clojure.lang.Var % .ns .getName)
+                                      used-vars)
+                                 (keep #(if-let [n (namespace %)] (symbol n))
+                                       used-macros)))]
+    (for [ns (set/difference required used-namespaces)]
+      (util/add-loc-info loc
+       {:linter :unused-namespaces
+        :msg (format "Namespace %s is never used in %s" ns curr-ns)}))))
 
 
 ;; Unused return values

src/eastwood/util.clj

@@ -118,6 +118,13 @@ more interesting keys earlier."
   (every? #(contains? m %) key-seq))
 
 
+(defn safe-first [f]
+  (try
+    (first f)
+    (catch Exception e
+      nil)))
+
+
 (defn nil-safe-rseq [s]
   (if (nil? s)
     nil

test/eastwood/test/linters_test.clj

@@ -1572,6 +1572,22 @@ the next."
      :line 164, :column 1}
     1,
     })
+  (lint-test
+   'testcases.unusednss
+   [:unused-namespaces]
+   default-opts
+   {
+    {:linter :unused-namespaces,
+     :msg "Namespace clojure.data is never used in testcases.unusednss",
+     :file (fname-from-parts "testcases" "unusednss.clj"),
+     :line 1, :column 1}
+    1,
+    {:linter :unused-namespaces,
+     :msg "Namespace clojure.string is never used in testcases.unusednss",
+     :file (fname-from-parts "testcases" "unusednss.clj"),
+     :line 1, :column 1}
+    1,
+    })
   ;; I would prefer if this threw an exception, but I think it does
   ;; not because Clojure reads, analyzes, and evaluates the namespace
   ;; before lint-test does, and thus the namespace is already there