Retain resource URL in java-parser results, improve caching

The java parser/legacy-parser returns a resource's `:file`, actually a relative
path as a string, and `:path`, the relative path converted to absolute. It does
this conversion through an `io/resource` lookup to find the file system location
of the artifact. There are two issues with this:

1. The return value of `io/resource` is discarded, even though it is needed
later on in `orchard.java`, causing unnecessary resource lookups.

2. The path is obtained through `(.getPath (io/resource path))`, which yields
nonsensical results when used on resources that are inside a JAR.

This change keeps the `:file` returned from java-parser (relative path string),
but removes the `:path` value in favor of a `:resource-url`, i.e. the return
value of `(io/resource path)`. It then provides utility functions to work with
this resource URL directly, namely mapping it to a filesystem artifact, or
retrieving its modification time.

java.parser/parser already does a `io/resource` lookup for the resource it is
parsing, meaning it has a full URL (jar: or file:). By including this URL in the
map it returns callers can do further checks or operations on the resource
without having to re-scan the classpath.

This in turn allows us to simplify and optimize `orchard.java/class-info`. The
old version would call `io/resource` on each invocation, even when the result
was already cached, in order to find the artifact's modification time.

This can noticably speed up cider-nrepl's `"stacktrace"` op, which calls
`orchard.java/class-info` on each stack frame, in extreme cases taking multiple
seconds to analyze all stack frames and return a result. This leads to rather
jarring UX, with the stacktrace buffering popping up in Emacs seconds after the
evaluation and exception happened. This is exarcerbated when using the nREPL
sideloader, which adds overhead to each resource lookup.

This also makes the return value of java-parser more correct. As mentioned the
previous version would always call `(.getPath (io/resource path))`, but this
only makes sense for `file:` resources, not for `jar:` resources. For `file:`
URLs `.getPath` returns the path of the file. For `jar:` URLs it returns the
nested url+the jar entry, so a `file:` URL but with a dangling `!/jar/entry`.

Illustration of why calling `getPath` on `jar:` URLs is not useful:

```clojure
(io/resource "lambdaisland/witchcraft.clj")
;;=> #java.net.URL "file:/srv/mc/witchcraft/src/lambdaisland/witchcraft.clj"

(.getPath (io/resource "lambdaisland/witchcraft.clj"))
;;=> "/srv/mc/witchcraft/src/lambdaisland/witchcraft.clj"

(io/resource "clojure/lang/RT.class")
;;=> #java.net.URL "jar:file:/root/.m2/repository/org/clojure/clojure/1.10.3/clojure-1.10.3.jar!/clojure/lang/RT.class"

(.getPath (io/resource "clojure/lang/RT.class"))
;;=> "file:/root/.m2/repository/org/clojure/clojure/1.10.3/clojure-1.10.3.jar!/clojure/lang/RT.class"
```

Author: plexus

.clj-kondo/config.edn

@@ -1,4 +1,4 @@
 {:output {:progress true
           :exclude-files ["analysis.cljc" "meta.cljc" "inspect_test.clj"]}
- :linters {:unused-private-var {:level :warning :exclude [orchard.query-test/a-private orchard.query-test/docd-fn]}}}
-
+ :linters {:unused-private-var {:level :warning :exclude [orchard.query-test/a-private orchard.query-test/docd-fn]}
+           :refer-all {:exclude [clojure.test]}}}

CHANGELOG.md

@@ -21,6 +21,7 @@
 * [#123](https://github.com/clojure-emacs/orchard/pull/123): Fix info lookups from namespaces that don't yet exist
 * [#125](https://github.com/clojure-emacs/orchard/issues/125): Don't fail if the classpath references a non-existing .jar
 * [#128](https://github.com/clojure-emacs/orchard/issues/128): Strengthen `apropos`
+* [#124](https://github.com/clojure-emacs/orchard/pull/124): Remove costly `io/resource` lookup
 
 ## 0.7.1 (2021-04-18)
 

dev/user.clj

@@ -14,5 +14,6 @@
 
 (cond->> ["dev" "src" "test"]
   jdk8?       (into ["src-jdk8"])
-  (not jdk8?) (into ["src-newer-jdks"])
+  (not jdk8?) (into ["src-newer-jdks"
+                     "test-newer-jdks"])
   true        (apply set-refresh-dirs))

project.clj

@@ -87,6 +87,9 @@
              "-Dclojure.main.report=stderr"]
 
   :source-paths ["src" "src-jdk8" "src-newer-jdks"]
+  :test-paths ~(cond-> ["test"]
+                 (not jdk8?)
+                 (conj "test-newer-jdks"))
 
   :profiles {
              ;; Clojure versions matrix
@@ -96,8 +99,8 @@
                                   [org.clojure/clojure "1.8.0" :classifier "sources"]]}
              :1.9 {:dependencies [[org.clojure/clojure "1.9.0"]
                                   [org.clojure/clojure "1.9.0" :classifier "sources"]]}
-             :1.10 {:dependencies [[org.clojure/clojure "1.10.1"]
-                                   [org.clojure/clojure "1.10.1" :classifier "sources"]]}
+             :1.10 {:dependencies [[org.clojure/clojure "1.10.3"]
+                                   [org.clojure/clojure "1.10.3" :classifier "sources"]]}
              :master {:repositories [["snapshots"
                                       "https://oss.sonatype.org/content/repositories/snapshots"]]
                       :dependencies [[org.clojure/clojure "1.11.0-master-SNAPSHOT"]

src-jdk8/orchard/java/legacy_parser.clj

@@ -273,6 +273,8 @@
         (assoc (->> (map parse-info (.classes root))
                     (filter #(= klass (:class %)))
                     (first))
+               ;; relative path on the classpath
                :file path
-               :path (. (io/resource path) getPath))))
+               ;; Full URL, e.g. file:.. or jar:...
+               :resource-url (io/resource path))))
     (catch Abort _)))

src-newer-jdks/orchard/java/parser.clj

@@ -304,7 +304,9 @@
                       (map #(parse-info % root))
                       (filter #(= klass (:class %)))
                       (first))
+                 ;; relative path on the classpath
                  :file path
-                 :path (.getPath (io/resource path)))
+                 ;; Full URL, e.g. file:.. or jar:...
+                 :resource-url (io/resource path))
           (finally (.close (.getJavaFileManager root))))))
     (catch Throwable _)))

src/orchard/info.clj

@@ -195,3 +195,7 @@
   classes. If no source is available, return the relative path as is."
   [^String path]
   {:javadoc (java/resolve-javadoc-path path)})
+
+(comment
+  (info-java 'clojure.lang.RT 'baseLoader)
+  (file-info "clojure/core.clj"))

src/orchard/java.clj

@@ -250,16 +250,6 @@
 
 (def cache (atom {}))
 
-(defn- immutable-source-file?
-  "Return true if the source file is effectively immutable. Specifically, this
-  returns true if no source file is available, or if the source file is in a
-  jar/zip archive."
-  [info]
-  (let [path (:file info)
-        src  (when path (io/resource path))]
-    (or (not src)
-        (re-find #"\.(jar|zip)!" (str src)))))
-
 (defn class-info
   "For the class symbol, return (possibly cached) Java class and member info.
   Members are indexed first by name, and then by argument types to list all
@@ -269,9 +259,11 @@
         info (if cached
                (:info cached)
                (class-info* class))
-        last-modified (if (immutable-source-file? info)
+        resource (:resource-url info)
+        last-modified (if (or (nil? resource)
+                              (util.io/url-to-file-within-archive? resource))
                         0
-                        (.lastModified ^File (io/file (:path info))))
+                        (util.io/last-modified-time resource))
         stale (not= last-modified (:last-modified cached))
         ;; If last-modified in cache mismatches last-modified of the file,
         ;; regenerate class-info.

src/orchard/java/resource.clj

@@ -15,7 +15,9 @@
     s))
 
 (defn project-resources
-  "Get a list of classpath resources."
+  "Get a list of classpath resources, i.e. files that are not clojure/java source
+  or class files. Only consider classpath entries that are directories, does not
+  consider jars."
   []
   (mapcat
    (fn [^File directory]
@@ -32,7 +34,7 @@
                    {:root directory
                     :file file
                     :relpath relpath
-                    :url (io/resource relpath)})))
+                    :url (io/as-url file)})))
           (remove #(.startsWith ^String (:relpath %) "META-INF/"))
           (remove #(re-matches #".*\.(clj[cs]?|java|class)" (:relpath %)))))
    (filter (memfn ^File isDirectory) (map io/as-file (cp/classpath (cp/boot-aware-classloader))))))

src/orchard/util/io.clj

@@ -1,4 +1,10 @@
-(ns orchard.util.io)
+(ns orchard.util.io
+  "Utility functions for dealing with file system objects, and in/out streams."
+  (:require
+   [clojure.java.io :as io])
+  (:import
+   (java.io File)
+   (java.net URL JarURLConnection)))
 
 (defn wrap-silently
   "Middleware that executes `(f)` without printing to `System/out` or `System/err`.
@@ -22,3 +28,55 @@
             (System/setOut old-out))
           (when (= ps System/err) ;; `System/err` may have changed in the meantime (in face of concurrency)
             (System/setErr old-err)))))))
+
+(defn url-protocol
+  "Get the URL protocol as a string, e.g. http, file, jar."
+  [^java.net.URL url]
+  (.getProtocol url))
+
+(defn url-to-file-within-archive?
+  "Does this URL point to a file inside a jar (or zip) archive.
+  i.e. does it use the jar: protocol."
+  [^java.net.URL url]
+  (= "jar" (url-protocol url)))
+
+(defn resource-jarfile
+  "Given a jar:file:...!/... URL, return the location of the jar file on the
+  filesystem. Returns nil on any other URL."
+  ^File [^URL jar-resource]
+  (assert (= "jar" (url-protocol jar-resource)))
+  (let [^JarURLConnection conn (.openConnection jar-resource)
+        inner-url (.getJarFileURL conn)]
+    (when (= "file" (url-protocol inner-url))
+      (io/as-file inner-url))))
+
+(defn resource-artifact
+  "Return the File from which the given resource URL would be loaded.
+
+  For `file:` URLs returns the location of the resource itself, for
+  `jar:..!/...` URLs returns the location of the archive containing the
+  resource. Returns a fully qualified File, even when the URL is relative.
+  Throws when the URL is not a `file:` or `jar:` URL."
+  ^File [^java.net.URL resource]
+  (let [protocol (url-protocol resource)]
+    (case protocol
+      "file"
+      (io/as-file resource)
+      "jar"
+      (resource-jarfile resource)
+      #_else
+      (throw (ex-info (str "URLs with a " protocol
+                           " protocol can't be situated on the filesystem.")
+                      {:resource resource})))))
+
+(defprotocol LastModifiedTime
+  (last-modified-time [this]
+    "Return the last modified time of a File or resource URL."))
+
+(extend-protocol LastModifiedTime
+  java.net.URL
+  (last-modified-time [this]
+    (last-modified-time (resource-artifact this)))
+  java.io.File
+  (last-modified-time [this]
+    (.lastModified this)))