Migrate to tools.reader to solve aliased keyword pains

kibit/project.clj

@@ -8,7 +8,8 @@
   :dependencies [[org.clojure/clojure "1.8.0"]
                  [org.clojure/core.logic "0.8.11"]
                  [org.clojure/tools.cli "0.3.5"]
-                 [rewrite-clj "0.4.12"]]
+                 [rewrite-clj "0.4.12"]
+                 [org.clojure/tools.reader "1.0.2"]]
   :profiles {:dev {:dependencies [[lein-marginalia "0.9.0"]]
                    :resource-paths ["test/resources"]}}
   :deploy-repositories [["releases" :clojars]

kibit/src/kibit/check.clj

@@ -6,7 +6,8 @@
             [kibit.rules :as core-rules]
             [kibit.reporters :as reporters]
             [kibit.monkeypatch
-             :refer [with-monkeypatches kibit-redefs]])
+             :refer [with-monkeypatches kibit-redefs]]
+            [kibit.check.reader :as kibit-read])
   (:import [clojure.lang LineNumberingPushbackReader]))
 
 ;; ### Overview
@@ -32,47 +33,6 @@
 ;; For more information, see: [rules](#kibit.rules) namespace
 (def all-rules core-rules/all-rules)
 
-;; Reading source files
-;; --------------------
-;; ### Extracting forms
-
-;; `read-file` is intended to be used with a Clojure source file,
-;; read in by Clojure's LineNumberingPushbackReader *(LNPR)*. Expressions are
-;; extracted using the clojure reader (ala `read`), and line numbers
-;; are added as `:line` metadata to the forms (via LNPR).
-
-(defn- careful-refer
-  "Refers into the provided namespace all public vars from clojure.core
-except for those that would clobber any existing interned vars in that
-namespace.  This is needed to ensure that symbols read within syntax-quote
-end up being fully-qualified to clojure.core as appropriate, and only
-to *ns* if they're not available there.  AFAICT, this will work for all
-symbols in syntax-quote except for those referring to vars that are referred
-into the namespace."
-  [ns]
-  (binding [*ns* ns]
-    (refer 'clojure.core :exclude (or (keys (ns-interns ns)) ())))
-  ns)
-
-(def eof (Object.))
-
-(defn read-file
-  "Generate a lazy sequence of top level forms from a
-   LineNumberingPushbackReader"
-  [^LineNumberingPushbackReader r init-ns]
-  (let [do-read (fn do-read [ns]
-                  (lazy-seq
-                    (let [form (binding [*ns* ns]
-                                 (read r false eof))
-                          [ns? new-ns k] (when (sequential? form) form)
-                          ns (if (and (symbol? new-ns)
-                                      (or (= ns? 'ns) (= ns? 'in-ns)))
-                               (careful-refer (create-ns new-ns))
-                               ns)]
-                      (when-not (= form eof)
-                        (cons form (do-read ns))))))]
-    (do-read (careful-refer (create-ns init-ns)))))
-
 ;; ### Analyzing the pieces
 
 ;; `tree-seq` returns a lazy-seq of nodes for a tree.
@@ -160,9 +120,9 @@ into the namespace."
 
 (def ^:private res->read-seq
   {:toplevel (fn [reader init-ns]
-               (read-file (LineNumberingPushbackReader. reader) init-ns))
+               (kibit-read/read-file (LineNumberingPushbackReader. reader) init-ns))
    :subform  (fn [reader init-ns]
-               (mapcat expr-seq (read-file (LineNumberingPushbackReader. reader) init-ns)))})
+               (mapcat expr-seq (kibit-read/read-file (LineNumberingPushbackReader. reader) init-ns)))})
 
 ;; Checking the expressions
 ;; ------------------------

kibit/src/kibit/check/reader.clj

@@ -0,0 +1,111 @@
+(ns kibit.check.reader
+  (:require [clojure.tools.reader :as reader])
+  (:import [clojure.lang LineNumberingPushbackReader]))
+
+;; Preprocessing
+;; -------------
+;; Alias pre-processing
+
+;; It is necessary at read time to maintain a small amount of state
+;; about the contents of the stream read so far to enable the proper
+;; reading of aliased keywords. Clojure accomplishes this during
+;; `(:require ...)` because evaluation and compilation is interlaced
+;; with reading so as to establish aliases in a namespace as it is
+;; being loaded. To do this statically we need to maintain a temporary
+;; table of namespaces to aliases. Additionally, we cannot simply use
+;; a map of aliases as it is possible to switch namespaces mid-file
+;; and get one stream to effectively hop between two namespaces.
+
+
+(defmulti derive-aliases first :default 'ns)
+
+(defn derive-alias-from-dep
+  [[dep & rest]]
+  (let [seq? (seq rest)
+        index (and seq?
+                   (first (keep-indexed #(if (= :as %2) %1)
+                                        rest)))
+        alias (and index
+                   (nth rest (inc index)))]
+    (when alias
+      [alias dep])))
+
+(defn unquote-if-quoted
+  [form]
+  (if (and (seq? form)
+           (= 'quote (first form)))
+    (second form)
+    form))
+
+(defmethod derive-aliases 'ns
+  [[_ _ns & ns-asserts]]
+  (->> ns-asserts
+       (group-by first)
+       (reduce (fn [m [k v]]
+                 (assoc m k (mapcat rest v)))
+               {})
+       ((juxt :require :require-macros))
+       (apply concat)
+       (map derive-alias-from-dep)
+       (remove nil?)
+       (into {})))
+
+(defmethod derive-aliases 'require
+  [[_ & deps]]
+  (->> deps
+       (map (comp derive-alias-from-dep unquote-if-quoted))
+       (remove nil?)
+       (into {})))
+
+;; Reading source files
+;; --------------------
+;; ### Extracting forms
+
+;; `read-file` is intended to be used with a Clojure source file,
+;; read in by Clojure's LineNumberingPushbackReader *(LNPR)*. Expressions are
+;; extracted using the clojure reader (ala `read`), and line numbers
+;; are added as `:line` metadata to the forms (via LNPR).
+
+(defn- careful-refer
+  "Refers into the provided namespace all public vars from clojure.core
+except for those that would clobber any existing interned vars in that
+namespace.  This is needed to ensure that symbols read within syntax-quote
+end up being fully-qualified to clojure.core as appropriate, and only
+to *ns* if they're not available there.  AFAICT, this will work for all
+symbols in syntax-quote except for those referring to vars that are referred
+into the namespace."
+  [ns]
+  (binding [*ns* ns]
+    (refer 'clojure.core :exclude (or (keys (ns-interns ns)) ())))
+  ns)
+
+(def eof (Object.))
+
+(defn trace [v] (println v) v)
+
+(defn read-file
+  "Generate a lazy sequence of top level forms from a
+   LineNumberingPushbackReader"
+  [^LineNumberingPushbackReader r init-ns]
+  (let [ns (careful-refer (create-ns init-ns))
+        do-read (fn do-read [ns alias-map]
+                  (lazy-seq
+                   (let [form (binding [*ns* ns
+                                        reader/*alias-map* (merge (ns-aliases ns)
+                                                                  (alias-map ns))]
+                                (reader/read r false eof))
+                         [ns? new-ns k] (when (sequential? form) form)
+                         new-ns (unquote-if-quoted new-ns)
+                         ns (if (and (symbol? new-ns)
+                                     (#{'ns 'in-ns} ns?))
+                              (careful-refer (create-ns new-ns))
+                              ns)
+                         alias-map (if (#{'require 'ns} ns?)
+                                     (update alias-map ns
+                                             merge
+                                             (derive-aliases form))
+                                     alias-map)]
+                     (when-not (= form eof)
+                       (cons form (do-read ns alias-map))))))]
+    (do-read ns {ns {}})))
+

kibit/test/kibit/test/check_reader.clj

@@ -0,0 +1,14 @@
+(ns kibit.test.check-reader
+  (:require [kibit.check.reader :as reader])
+  (:use [clojure.test]))
+
+(deftest derive-aliases-test
+  (are [expected-alias-map ns-form]
+      (= expected-alias-map (reader/derive-aliases ns-form))
+      '{foo foo.bar.baz} '(ns derive.test.one
+                            (:require [foo.bar.baz :as foo]))
+      '{foo foo.bar.baz
+        foom foo.bar.baz.macros} '(ns derive.test.one
+                                    (:require [foo.bar.baz :as foo])
+                                    (:require-macros [foo.bar.baz.macros :as foom]))
+      '{str clojure.string} '(require (quote [clojure.string :as str]))))

kibit/test/kibit/test/driver.clj

@@ -1,7 +1,8 @@
 (ns kibit.test.driver
   (:require [kibit.driver :as driver]
             [clojure.test :refer :all]
-            [clojure.java.io :as io]))
+            [clojure.java.io :as io])
+  (:import (java.io ByteArrayOutputStream PrintWriter)))
 
 (deftest clojure-file-are
   (are [expected file] (= expected (driver/clojure-file? (io/file file)))
@@ -12,10 +13,19 @@
 
 (deftest find-clojure-sources-are
   (is (= [(io/file "test/resources/first.clj")
+          (io/file "test/resources/keywords.clj")
           (io/file "test/resources/second.cljx")
           (io/file "test/resources/sets.clj")
           (io/file "test/resources/third.cljs")]
          (driver/find-clojure-sources-in-dir (io/file "test/resources")))))
 
 (deftest test-set-file
   (is (driver/run ["test/resources/sets.clj"] nil)))
+
+(deftest test-keywords-file
+  (let [test-buf (ByteArrayOutputStream.)
+        test-err (PrintWriter. test-buf)]
+    (binding [*err* test-err]
+      (driver/run ["test/resources/keywords.clj"] nil))
+    (is (zero? (.size test-buf))
+        (format "Test err buffer contained '%s'" (.toString test-buf)))))

kibit/test/resources/keywords.clj

@@ -0,0 +1,20 @@
+(ns resources.keywords
+  (:require [clojure.java.io :as io]))
+
+(defn aliased-keyword-access
+  [x]
+  (::io/some-fake-key x))
+
+(ns resources.non-conforming)
+
+(require '[clojure.string :as str])
+
+(defn using-a-different-keyword-alias-in-different-ns
+  [z]
+  (::str/another-fake-key z))
+
+(in-ns 'resources.keywords)
+
+(defn flipped-back-aliases-still-there
+  [y]
+  (::io/last-fake-key y))