orchard.inspect: render non-accessible fields better

vemv · vemv · commit 9cb236c87577 · 2023-10-24T11:13:25.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,11 @@
 * `orchard.inspect`: don't render keyword/symbol/number values as strings.
 * `orchard.inspect`: don't use `pr-str` over the main `Value: ` being inspected. 
   * All values are already formatted as strings, so this `pr-str` was redundant.
+* `orchard.inspect`: render non-accessible fields better.
+  * If a given field cannot be inspected (because it's private and the JDK module system prevents opening it), we return the fixed symbol `<non-inspectable value>` for representing its value, clients being free to elide its rendering.
+  * Now, for a given inspected Object (except Class objects), we return these sections, if present: `Instance fields`, `Static fields` ,`Private instance fields`, `Private static fields`.
+  * For Class objects, we keep grouping the fields under a single `Fields` section.
+* `orchard.inspect`: render field names as symbols, not strings.
 
 ## 0.16.1 (2023-10-05)
 
diff --git a/project.clj b/project.clj
@@ -51,7 +51,8 @@
                                      [org.clojure/clojure "1.12.0-master-SNAPSHOT" :classifier "sources"]]}
 
 
-             :test {:dependencies [[org.clojure/java.classpath "1.0.0"]]
+             :test {:dependencies [[org.clojure/java.classpath "1.0.0"]
+                                   [nubank/matcher-combinators "3.8.8"]]
                     :resource-paths ["test-resources"
                                      "not-a.jar"
                                      "does-not-exist.jar"]
diff --git a/src/orchard/inspect.clj b/src/orchard/inspect.clj
@@ -608,50 +608,51 @@
       (render-indent-str-lines obj)
       (unindent)))
 
+(defn- field-val [^Field f, obj]
+  (try
+    (.get f obj)
+    (catch Exception _
+      ::access-denied)))
+
 (defmethod inspect :default [inspector obj]
   (let [class-chain (loop [c (class obj), res ()]
                       (if c
                         (recur (.getSuperclass c) (cons c res))
                         res))
+        memoized-field-val (memoize field-val)
         all-fields (mapcat #(.getDeclaredFields ^Class %) class-chain)
-
-        {static true, non-static false}
-        (group-by #(Modifier/isStatic (.getModifiers ^Field %)) all-fields)]
-    (letfn [(field-name [^Field f]
-              (.getName f))
-
-            (field-val [^Field f]
-              (let [^Exception e
-                    (try (.setAccessible f true)
-                         nil
-                         (catch Exception e
-                           ;; We want to handle specifically SecurityException
-                           ;; and j.l.r.InaccessibleObjectException, but the
-                           ;; latter only comes with Java9+, so let's just
-                           ;; catch everything instead.
-                           e))]
-                (try (.get f obj)
-                     (catch java.lang.IllegalAccessException _
-                       (symbol
-                        (format "<Access denied%s>"
-                                (when e (str " (" (.getName (.getClass e)) ")"))))))))
-
-            (render-fields [inspector section-name fields]
+        {static-accessible        [true true]
+         non-static-accessible    [false true]
+         static-nonaccessible     [true false]
+         non-static-nonaccessible [false false]}
+        (group-by (fn [^Field f]
+                    [(Modifier/isStatic (.getModifiers f))
+                     (not= ::access-denied (memoized-field-val f obj))])
+                  all-fields)]
+    (letfn [(render-fields [inspector section-name fields]
               (if (seq fields)
                 (-> inspector
                     (render-section-header section-name)
                     (indent)
                     (render-map-values (->> fields
-                                            (map (fn [f] [(field-name f) (field-val f)]))
+                                            (map (fn [^Field f]
+                                                   (let [v (memoized-field-val f obj)]
+                                                     [(-> f .getName symbol)
+                                                      (if (= v ::access-denied)
+                                                        ;; This is a special value that can be detected client-side:
+                                                        (symbol "<non-inspectable value>")
+                                                        v)])))
                                             (into (sorted-map))))
                     (unindent))
                 inspector))]
-      (-> inspector
-          (render-labeled-value "Class" (class obj))
-          (render-labeled-value "Value" obj)
-          (render-fields "Fields" non-static)
-          (render-fields "Static fields" static)
-          (render-datafy obj)))))
+      (cond-> inspector
+        true                           (render-labeled-value "Class" (class obj))
+        true                           (render-labeled-value "Value" obj)
+        (seq non-static-accessible)    (render-fields "Instance fields" non-static-accessible)
+        (seq static-accessible)        (render-fields "Static fields" static-accessible)
+        (seq non-static-nonaccessible) (render-fields "Private instance fields" non-static-nonaccessible)
+        (seq static-nonaccessible)     (render-fields "Private static fields" static-nonaccessible)
+        true                           (render-datafy obj)))))
 
 (defn- render-section [obj inspector [section sort-key-fn]]
   (let [method (symbol (str ".get" (name section)))
diff --git a/test/orchard/inspect_test.clj b/test/orchard/inspect_test.clj
