Fix when get-enclosing-sexp sometimes selects a non enclosing sexp

Author: benedekfazekas

src/refactor_nrepl/s_expressions.clj

@@ -30,24 +30,29 @@
                        (zip/string zloc)))
           (take-while (complement nil?) (iterate zip/left zloc)))))
 
+(defn- node-at-loc?
+  "True if node encloses point defined by `loc-line` and `loc-column`."
+  [zloc ^long loc-line ^long loc-column]
+  (let [[line end-line column end-column] (->> (zip/node zloc)
+                                               meta
+                                               ((juxt :row :end-row :col :end-col))
+                                               (map (comp dec long)))]
+    (or (< line loc-line end-line)
+        (and (or (= line loc-line)
+                 (= end-line loc-line))
+             (<= column loc-column end-column)))))
+
 (defn- zip-to
-  "Move the zipper to the node at line and col"
-  [zipper ^long line ^long col]
-  (let [distance (fn [zloc]
-                   (let [node (zip/node zloc)
-                         line-beg (dec (long (:row (meta node))))
-                         line-end (dec (long (:end-row (meta node))))
-                         col-beg (dec (long (:col (meta node))))
-                         col-end (dec (long (:end-col (meta node))))]
-                     (+ (* 1000 (Math/abs (- line line-beg)))
-                        (* 100 (Math/abs (- line line-end)))
-                        (* 10 (Math/abs (- col col-beg)))
-                        (Math/abs (- col col-end)))))]
-    (reduce (fn [best zloc] (if (< (long (distance zloc)) (long (distance best)))
-                              zloc
-                              best))
-            zipper
-            (all-zlocs zipper))))
+  "Move the zipper to the node at `loc-line` and `loc-col`.
+
+  Implementation uses `all-zlocs` and exploits the fact that it generates
+  a seq of nodes in depth-first order."
+  [zipper ^long loc-line ^long loc-column]
+  (reduce
+   (fn [node-at-loc zloc]
+     (if (node-at-loc? zloc loc-line loc-column) zloc node-at-loc))
+   zipper
+   (all-zlocs zipper)))
 
 (defn get-enclosing-sexp
   "Extracts the sexp enclosing point at LINE and COLUMN in FILE-CONTENT,

test/refactor_nrepl/s_expressions_test.clj

@@ -11,11 +11,11 @@
   ;; some other stuff
   (foobar baz)")
 (def binding-location [3 8])
-(def funcall-location [6 8])
 (def set-location [7 35])
 (def map-location [7 28])
 (def weird-location [1 5])
 (def println-location [5 8])
+(def when-not-location [10 9])
 
 (t/deftest get-enclosing-sexp-test
   (t/is (= "[some :bindings
@@ -28,5 +28,8 @@
   (t/is (=  "#{more}" (apply sut/get-enclosing-sexp file-content set-location)))
   (t/is (=  "{:qux [#{more}]}" (apply sut/get-enclosing-sexp file-content map-location)))
   (t/is (=  nil (apply sut/get-enclosing-sexp weird-file-content weird-location)))
+  (t/is (= "(when-not (= true true)
+    (= 5 (* 2 2)))"
+           (apply sut/get-enclosing-sexp file-content when-not-location)))
   (t/is (= nil (sut/get-first-sexp weird-file-content)))
   (t/is (=  "#{foo bar baz}" (sut/get-first-sexp file-content-with-set))))

test/resources/testproject/src/com/example/sexp_test.clj

@@ -6,3 +6,7 @@
     (println #{some}
              ;; unhelpful comment )
              (prn {"foo" {:qux [#{more}]}}))))
+
+(defn bar []
+  (when-not (= true true)
+    (= 5 (* 2 2))))