Fix instrumentation of small map literals (#665)

Map literal instrumentation was relying on some limited heuristic before.

Now we use Clojure's underlying array-map implementation for small map
literals and fall back to a sorted order of keys if that is not the
case. If this fails as well we simply instrument only the values of
the map in the order they appear in a sequential pass.

Author: FiV0

CHANGELOG.md

@@ -8,6 +8,7 @@
 
 * [#663](https://github.com/clojure-emacs/cider-nrepl/pull/663): Fix non-recognized `recur` inside `case` in the debugger.
 * [#664](https://github.com/clojure-emacs/cider-nrepl/pull/664): Fix continue-all in conditional break.
+* [#665](https://github.com/clojure-emacs/cider-nrepl/pull/665): Fix form location in map literals.
 
 ## 0.23.0 (2019-01-18)
 

src/cider/nrepl/middleware/util/instrument.clj

@@ -26,7 +26,7 @@
 
 ;;;; ## Instrumentation
 ;;;
-;;; The top-level instrumenting function is `read-and-instrument`. See
+;;; The top-level instrumenting function is `instrument-tagged-code`. See
 ;;; its doc for more information.
 ;;;
 ;;; Each of the other `instrument-*` functions is responsible for
@@ -208,7 +208,7 @@
   Only forms with a ::breakfunction metadata will be
   instrumented, and even then only if it makes sense (e.g., the
   binding names in a let form are never instrumented).
-  See `read-and-instrument` for more information."
+  See `instrument-tagged-code` for more information."
   [form]
   (condp #(%1 %2) form
     ;; Function call, macro call, or special form.
@@ -225,7 +225,7 @@
 ;;;
 ;;; The following functions are used to populate with metadata and
 ;;; macroexpand code before instrumenting it. This is where we
-;;; calculate all the ::coor vectors. See `read-and-instrument`.
+;;; calculate all the ::coor vectors. See `instrument-tagged-code`.
 (defn- walk-indexed
   "Walk through form calling (f coor element).
   The value of coor is a vector of indices representing element's
@@ -236,20 +236,34 @@
    (let [map-inner (fn [forms]
                      (map-indexed #(walk-indexed (conj coor %1) f %2)
                                   forms))
-         ;; Maps are unordered, but we can try to use the keys (and
-         ;; they're important enough that we're willing to risk
-         ;; getting the position wrong).
-         result (cond (map? form)  (into {} (map (fn [[k v]]
-                                                   [k (walk-indexed (conj coor (pr-str k)) f v)])
-                                                 form))
-                      ;; Order of sets is unpredictable, unfortunately.
-                      (set? form)  form
-                      ;; Borrowed from clojure.walk/walk
-                      (list? form) (apply list (map-inner form))
-                      (instance? clojure.lang.IMapEntry form) (vec (map-inner form))
-                      (seq? form)  (doall (map-inner form))
-                      (coll? form) (into (empty form) (map-inner form))
-                      :else form)]
+         ;; Clojure uses array-maps up to some map size (8 currently).
+         ;; So for small maps we take advantage of that, otherwise fall
+         ;; back to the heuristic below.
+         ;; Maps are unordered, but we can try to use the keys as order
+         ;; hoping they can be compared one by one and that the user
+         ;; has specified them in that order. If that fails we don't
+         ;; instrument the map. We also don't instrument sets.
+         ;; This depends on Clojure implementation details.
+         walk-indexed-map (fn [map]
+                            (map-indexed (fn [i [k v]]
+                                           [(walk-indexed (conj coor (* 2 i)) f k)
+                                            (walk-indexed (conj coor (inc (* 2 i))) f v)])
+                                         map))
+         result (cond
+                  (map? form) (if (<= (count form) 8)
+                                (into {} (walk-indexed-map form))
+                                (try
+                                  (into (sorted-map) (walk-indexed-map (into (sorted-map) form)))
+                                  (catch Exception e
+                                    form)))
+                  ;; Order of sets is unpredictable, unfortunately.
+                  (set? form)  form
+                  ;; Borrowed from clojure.walk/walk
+                  (list? form) (apply list (map-inner form))
+                  (instance? clojure.lang.IMapEntry form) (vec (map-inner form))
+                  (seq? form)  (doall (map-inner form))
+                  (coll? form) (into (empty form) (map-inner form))
+                  :else form)]
      (f coor (m/merge-meta result (meta form))))))
 
 (defn coord<

test/clj/cider/nrepl/middleware/debug_integration_test.clj

@@ -429,6 +429,37 @@
   (<-- {:value "4"})
   (<-- {:status ["done"]}))
 
+;;; Tests for map literals
+
+(deftest small-literal-map-test
+  (--> :eval
+       "#dbg
+        (seq
+          {1 (+ 2 (+ 3 2))
+           (inc 3)  (+ 12345)})")
+  (<-- {:debug-value "5" :coor [1 1 2]}) ; (+ 3 2)
+  (--> :next)
+  (<-- {:debug-value "7" :coor [1 1]}) ; (+ 2 (+ 3 2))
+  (--> :next)
+  (<-- {:debug-value "4" :coor [1 2]}) ; (inc 3) 
+  (--> :next)
+  (<-- {:debug-value "12345" :coor [1 3]}) ; (+ 12345) 
+  (--> :next)
+  (<-- {:value "([1 7] [4 12345])"}) ; (seq ...) 
+  (<-- {:status ["done"]}))
+
+(deftest larger-literal-map-test
+  (--> :eval
+       "#dbg (count {1 2 3 (inc 4) 5 6 7 8 9 10 11 12 13 14 15 (inc 16) 17 (inc 18)})")
+  (<-- {:debug-value "5" :coor [1 3]}) ; (inc 4)
+  (--> :next)
+  (<-- {:debug-value "17" :coor [1 15]}) ; (inc 15)
+  (--> :next)
+  (<-- {:debug-value "19" :coor [1 17]}) ; (inc 18) 
+  (--> :next)
+  (<-- {:value "9"}) ; (count ...)
+  (<-- {:status ["done"]}))
+
 ;;; Tests for conditional breakpoints
 
 (deftest conditional-in-for-test