Trust ClojureScript :file analysis metadata

Updated ClojureScript's reader to use the :file metadata returned by analysis.
It formerly set :file to the actual path of the :file it was analysing. This
change matches the Clojure reader behaviour and allows for potemkin import-vars
type manipulation of metadata to, for example, point at an alternate var's docs
in a different source file.

To verify readers are still returning data that we expect, I added some tests
based on the existing playgound under test-sources. The tests are a good start
but not comprehensive and should be extended as needed. Circleci config
is included to automatically run tests on commit.

While adding tests, I noticed small bugs and made corrections:
1. The ClojureScript reader now sorts protocol methods by name. This
   matches the behaviour of the Clojure reader.
2. The Clojure reader will now add a single var for each defrecord
   with the name of the defrecord. This matches the ClojureScript
   behaviour and is apparently the desired behaviour for cljdoc.

For consistency and testability:
- we are now removing keys with empty/nil values from reader analysis maps.
- to be conistent with the ClojureScript reader, the Clojure reader no longer
  returns :file :line :column :end-column and :end-line on the namespace
  element.

Notes:

As part of this change, the ClojureScript reader will stop supressing protocol
functions when the actual source does not match the :file metadata. This allows
for a tool such as import-vars to point to, and have documented, an imported
protocol function. No action was needed for the Clojure reader to make this
work.

This change was made a bit challenging due to cljdoc's usage of codox's
root-dir and :file metadata:
- The original intent of codox is that the config :root-dir point to the
  project (aka git) root of a project and that the :file metadata
  was simply what was returned by analysis. Codox added the :path to analysis
  metadata to resolve :file to the :root-dir.
- Cljdoc does not set the :root-dir to the project root, ignores the :path and
  assumes that the :file is always relative to configured :source-path. Although
  this usage works for cljdoc, it did sprain my brain while making this change.
- To compensate, I have been careful to ensure that :file continues to always
  be relative to the the configured :source-path.

Another oddity was that altered :file metadata is being resolved to paths
within the jar file when codox is called from cljdoc. This might be due to the
way the classpath is setup when doing an analysis run. My change is coded to
handle this situation when it arises.

Author: lread

.circleci/config.yml

@@ -0,0 +1,49 @@
+# Clojure CircleCI 2.0 configuration file
+#
+# Check https://circleci.com/docs/2.0/language-clojure/ for more details
+#
+version: 2
+jobs:
+  build:
+    branches:
+      only:
+        - /cljs-proper.*/
+
+    docker:
+      - image: circleci/clojure:tools-deps-1.10.0.442
+
+    working_directory: ~/repo
+
+    steps:
+      - checkout
+
+      - restore_cache:
+          keys:
+            - v1-dependencies-{{ checksum "codox/deps.edn" }}
+            - v1-dependencies- # fallback if cache not found
+
+      - run:
+          name: Dump tool versions
+          command: clojure -e '(println (System/getProperty "java.runtime.name") (System/getProperty "java.runtime.version") "\nClojure" (clojure-version))'
+          working_directory: ~/repo/codox
+
+      - run:
+          name: Dump classpath
+          command: clojure -Spath
+          working_directory: ~/repo/codox
+
+      - run:
+          name: Run tests
+          command: clojure -Atest --reporter documentation --plugin kaocha.plugin/junit-xml --junit-xml-file target/test-results/unit/results.xml
+          working_directory: ~/repo/codox
+
+      - save_cache:
+          paths:
+            - ~/.m2
+          key: v1-dependencies-{{ checksum "codox/deps.edn" }}
+
+      - store_test_results:
+          path: codox/target/test-results
+
+      - store_artifacts:
+          path: codox/target/test-results

.gitignore

@@ -1,9 +1,11 @@
 pom.xml
 pom.xml.asc
 *jar
+.cpcache/
 lib/
 classes/
 target/
+out/
 checkouts/
 .lein-*
 .nrepl-port

README.md

@@ -1,9 +1,12 @@
 ### Notes about this fork
 
-- Focused on extracting structured data (EDN) from jars.
+[![CircleCI](https://circleci.com/gh/cljdoc/codox/tree/cljs-proper.svg?style=svg)](https://circleci.com/gh/cljdoc/codox/tree/cljs-proper)
+
+- Focused on extracting structured data (EDN) from jars for cljdoc which makes use of work under codox subdir only.
 - Any writing-related (HTML etc.) dependencies have been removed.
 - Any remaining unused files are kept only to keep diff to mainline manageable.
 - Various tweaks have been done to align the results of Clojure and ClojureScript analysis.
+- During dev, tests can be run via `clojure -A:test --watch`
 
 
 ---

codox/deps.edn

@@ -1,4 +1,9 @@
-{:paths ["src" "resources" "test-sources"]
+{:paths ["src"]
  :deps {org.clojure/clojure {:mvn/version "1.9.0"}
         org.clojure/tools.namespace {:mvn/version "0.2.11"}
-        org.clojure/clojurescript {:mvn/version "1.10.339"}}}
+        org.clojure/clojurescript {:mvn/version "1.10.520"}}
+ :aliases {:test
+            {:extra-paths ["test" "test-sources"]
+             :extra-deps {lambdaisland/kaocha {:mvn/version "0.0-413"}
+                          lambdaisland/kaocha-junit-xml {:mvn/version "0.0-70"}}
+             :main-opts ["-m" "kaocha.runner"]}}}

codox/src/codox/main.clj

@@ -4,9 +4,11 @@
   (:require [clojure.string :as str]
             [clojure.pprint]
             [clojure.java.shell :as shell]
+            [clojure.java.io :as io]
             [codox.reader.clojure :as clj]
             [codox.reader.clojurescript :as cljs]
-            [codox.reader.plaintext :as text]))
+            [codox.reader.plaintext :as text]
+            [codox.utils :as util]))
 
 (defn- writer [{:keys [writer]}]
   (let [writer-sym (or writer 'codox.writer.html/write-docs)
@@ -44,9 +46,11 @@
         (update-in [:members] add-var-defaults defaults))))
 
 (defn- add-ns-defaults [namespaces defaults]
-  (for [namespace namespaces]
-    (-> (merge defaults namespace)
-        (update-in [:publics] add-var-defaults defaults))))
+  (if (seq defaults)
+    (for [namespace namespaces]
+      (-> (merge defaults namespace)
+          (update-in [:publics] add-var-defaults defaults)))
+    namespaces))
 
 (defn- ns-matches? [{ns-name :name} pattern]
   (cond
@@ -100,13 +104,15 @@
   ([]
      (generate-docs {}))
   ([options]
-     (let [options    (merge defaults options)
-           write-fn   (writer options)
-           namespaces (read-namespaces options)
-           documents  (read-documents options)]
-       (write-fn (assoc options
-                        :namespaces namespaces
-                        :documents  documents)))))
+   (let [options    (-> (merge defaults options)
+                        (update :root-path util/canonical-path)
+                        (update :souce-paths #(map util/canonical-path %)))
+         write-fn   (writer options)
+         namespaces (read-namespaces options)
+         documents  (read-documents options)]
+     (write-fn (assoc options
+                      :namespaces namespaces
+                      :documents  documents)))))
 
 (defn -main
   "The main entry point for reading API information from files in a directory.

codox/src/codox/reader/clojure.clj

@@ -5,7 +5,8 @@
   (:use [codox.utils :only (assoc-some update-some correct-indent)])
   (:require [clojure.java.io :as io]
             [clojure.tools.namespace.find :as ns]
-            [clojure.string :as str]))
+            [clojure.string :as str]
+            [codox.utils :as util]))
 
 (defn try-require [namespace]
   (try
@@ -53,6 +54,14 @@
   (if-let [p (:protocol (meta var))]
     (some #{p} vars)))
 
+(defn- include-record-factory-as-defrecord [var-meta]
+  (let [n (str (:name var-meta))]
+    (if (re-find #"map->\p{Upper}" n)
+      (-> var-meta
+          (assoc :name (symbol (subs n 5)))
+          (dissoc :doc :arglists))
+      var-meta)))
+
 (defn- protocol-methods [protocol vars]
   (filter #(= protocol (:protocol (meta %))) vars))
 
@@ -68,35 +77,40 @@
     (if (empty? delayed-errors)
       (:t ret))))
 
-(defn- read-var [vars var]
-  (-> (meta var)
-      (select-keys [:name :file :line :arglists :doc :dynamic
-                    :added :deprecated :doc/format])
-      (update-some :doc correct-indent)
-      (assoc-some  :type (var-type var)
-                   :type-sig (if (core-typed?) (core-typed-type var))
-                   :members (seq (map (partial read-var vars)
-                                      (protocol-methods var vars))))))
-
-(defn- read-publics [namespace]
+(defn- read-var [source-path vars var]
+  (let [normalize (partial util/normalize-to-source-path source-path)]
+    (-> (meta var)
+        (include-record-factory-as-defrecord)
+        (select-keys [:name :file :line :arglists :doc :dynamic
+                      :added :deprecated :doc/format])
+        (update-some :doc correct-indent)
+        (update-some :file normalize)
+        (assoc-some  :type (var-type var)
+                     :type-sig (if (core-typed?) (core-typed-type var))
+                     :members (seq (map (partial read-var source-path vars)
+                                        (protocol-methods var vars))))
+        util/remove-empties)))
+
+(defn- read-publics [source-path namespace]
   (let [vars (sorted-public-vars namespace)]
     (->> vars
          (remove proxy?)
          (remove no-doc?)
          (remove (partial protocol-method? vars))
-         (map (partial read-var vars))
+         (map (partial read-var source-path vars))
          (sort-by (comp str/lower-case :name)))))
 
-(defn- read-ns [namespace exception-handler]
+(defn- read-ns [namespace source-path exception-handler]
   (try-require 'clojure.core.typed.check)
   (when (core-typed?)
     (typecheck-namespace namespace))
   (try
     (require namespace)
     (-> (find-ns namespace)
         (meta)
+        (dissoc :file :line :column :end-column :end-line)
         (assoc :name namespace)
-        (assoc :publics (read-publics namespace))
+        (assoc :publics (read-publics source-path namespace))
         (update-some :doc correct-indent)
         (list))
     (catch Exception e
@@ -148,8 +162,9 @@
   ([paths {:keys [exception-handler]
            :or {exception-handler default-exception-handler}}]
    (mapcat (fn [path]
-             (->> (io/file path)
-                  (find-namespaces)
-                  (mapcat #(read-ns % exception-handler))
-                  (remove :no-doc)))
+             (let [path (util/canonical-path path)]
+               (->> (io/file path)
+                    (find-namespaces)
+                    (mapcat #(read-ns % path exception-handler))
+                    (remove :no-doc))))
            paths)))

codox/src/codox/reader/clojurescript.clj

@@ -6,7 +6,8 @@
             [cljs.analyzer.api :as ana]
             [cljs.closure]
             [cljs.env]
-            [clojure.string :as str]))
+            [clojure.string :as str]
+            [codox.utils :as util]))
 
 (defn- cljs-filename? [filename]
   (or (.endsWith filename ".cljs")
@@ -51,25 +52,39 @@
    (multimethod? opts)     :multimethod
    :else                   :var))
 
-(defn- read-var [file vars var]
-  (let [vt (var-type var)]
+(defn- read-var [source-path file vars var]
+  (let [vt (var-type var)
+        normalize (partial util/normalize-to-source-path source-path)]
     (-> var
-        (select-keys [:name :line :arglists :doc :dynamic :added :deprecated :doc/format])
+        (select-keys [:name :file :line :arglists :doc :dynamic :added :deprecated :doc/format])
         (update-some :name (comp symbol name))
         (update-some :arglists remove-quote)
         (update-some :doc correct-indent)
-        (assoc-some  :file    (if (= vt :macro) (:file var) (.getPath file))
-                     :type    vt
-                     :members (map (partial read-var file vars)
-                                   (protocol-methods var vars))))))
-
-(defn- read-publics [state namespace file]
-  (let [vars (vals (ana/ns-publics state namespace))]
+        (update-some :file normalize)
+        (assoc-some  :type    vt
+                     :members (->> (protocol-methods var vars)
+                                   (map (partial read-var source-path file vars))
+                                   (map util/remove-empties)
+                                   (map #(dissoc % :file :line))
+                                   (sort-by :name)))
+        util/remove-empties)))
+
+(defn- unreferenced-protocol-fn?
+  "Tools like potemkin import-vars can create a new function in one namespace point to an existing function within a protocol.
+  In these cases, we want to include the new function."
+  [source-path actual-file vars]
+  (let [meta-file (util/normalize-to-source-path source-path (:file vars))
+        actual-file (util/normalize-to-source-path source-path (str actual-file))]
+    (and (:protocol vars) (= meta-file actual-file))))
+
+(defn- read-publics [state namespace source-path file]
+  (let [vars (vals (ana/ns-publics state namespace))
+        unreferenced-protocol? (partial unreferenced-protocol-fn? source-path file)]
     (->> vars
-         (remove :protocol)
          (remove :anonymous)
+         (remove unreferenced-protocol?)
          (remove no-doc?)
-         (map (partial read-var file vars))
+         (map (partial read-var source-path file vars))
          (sort-by (comp str/lower-case :name)))))
 
 (defn- analyze-file [file]
@@ -80,17 +95,19 @@
      (ana/analyze-file state file opts))
     state))
 
-(defn- read-file [path file exception-handler]
+(defn- read-file [source-path file exception-handler]
   (try
-    (let [source  (io/file path file)
+
+    (let [source  (io/file source-path file)
           ns-name (:ns (ana/parse-ns source))
           state   (analyze-file source)]
       {ns-name
        (-> (ana/find-ns state ns-name)
            (select-keys [:name :doc])
            (update-some :doc correct-indent)
            (merge (-> ns-name meta (select-keys [:no-doc])))
-           (assoc :publics (read-publics state ns-name file)))})
+           (util/remove-empties)
+           (assoc :publics (read-publics state ns-name source-path file)))})
     (catch Exception e
       (exception-handler e file))))
 
@@ -129,7 +146,7 @@
   ([paths {:keys [exception-handler]
            :or {exception-handler default-exception-handler}}]
    (mapcat (fn [path]
-             (let [path (io/file path)
+             (let [path (io/file (util/canonical-path path))
                    file-reader #(read-file path % exception-handler)]
                (->> (find-files path)
                     (map file-reader)