Several bugs with lispy-eval in clojure

[yuhan0]

lispy-clojure appears to be pre-processing forms to be evaluated, which causes unexpected behaviors:

let bindings escaping into "global namespace"

This is probably the most serious of these issues and cost me quite a bit of time chasing non-existent bugs... it makes local definitions in a let binding vector globally available after being evaluated, and silently overshadows all subsequent bindings of the same name:

Unable to eval def with docstring:

Error when evaluating first expression in a vector:

Returned strings are displayed un-stringified

Is this the intended behavior? It seems inconsistent and might cause confusion.

Note: All the above examples behave as expected with cider-eval-current-sexp.

[danieroux]

I confirm all of these on lispy 20190219.1125 and cider 20181016.2217

[yuhan0]

Another bug with evaluating repeated sub-expressions in a threading macro, this one due to using (position) to determine context, which only takes the first matching form:

[abo-abo]

let bindings escaping into "global namespace"

This is a feature, not a bug. The variables aren't escaping into "global namespace", that's why cider-eval still works correctly.

This feature binds locals in the shadows. Then lispy-eval merges shadows into the current namespace so that you can eval things faster. You can use (lispy-clojure/shadow-unmap *ns*) to null those bindings.

The intent is to allow a more fine-grained eval. Suppose you have this code:

(ns lispy-clojure)

(def e-str "(+ 1 2)")
(def context-str "[x (+ 1 2) y (+ x x)]")
(def file "foo")
(def line 10)

(defn reval [e-str context-str & {:keys [file line]}]
  (let [expr (read-string e-str)
        context (try
                  (read-string context-str)
                  (catch Exception _))
        full-expr (read-string (format "[%s]" e-str))
        expr1 (xcond
                ((nil? context-str)
                 (cons 'do full-expr))
                ((= (count full-expr) 2)
                 (shadow-dest full-expr))
                ((add-location-to-deflike expr file line))
                (:else
                 (guess-intent expr context)))]
    (eval `(with-shadows
             (try
               (do ~expr1)
               (catch Exception ~'e
                 (clojure.core/str "error: " ~ 'e)))))))

Now you're trying to step through reval to see/fix what's going on. With the regular cider-eval, all you can eval is:

• (read-string e-str)
• the whole let expression

That's it. If there's a bug or something interesting in between, you're out of luck.

But with lispy-eval, you can eval in turn the whole thing:

(defn reval [e-str context-str & {:keys [file line]}]
  (let [expr (read-string e-str)
        ;; => (+ 1 2)
        context (try
                  (read-string context-str)
                  (catch Exception _))
        ;; => [x (+ 1 2) y (+ x x)]
        full-expr (read-string (format "[%s]" e-str))
        ;; => [(+ 1 2)]
        expr1 (xcond
                ((nil? context-str)
                 ;; => false
                 (cons 'do full-expr))
                ((= (count full-expr) 2)
                 ;; => false
                 (shadow-dest full-expr))
                ((add-location-to-deflike expr file line)
                 ;; => nil
                 )
                (:else
                 (guess-intent expr context)
                 ;; => (clojure.core/let [x (+ 1 2)] (lispy-clojure/shadow-def 'x x) {:x x})
                 ))
        ;; => (clojure.core/let [x (+ 1 2)] (lispy-clojure/shadow-def 'x x) {:x x})
        ]
    (eval `(with-shadows
             (try
               (do ~expr1)
               (catch Exception ~'e
                 (clojure.core/str "error: " ~ 'e))))
          ;; =>
          ;; (lispy-clojure/with-shadows
          ;;  (try
          ;;   (do
          ;;    (clojure.core/let
          ;;     [x (+ 1 2)]
          ;;     (lispy-clojure/shadow-def 'x x)
          ;;     {:x x}))
          ;;   (catch java.lang.Exception e (clojure.core/str "error: " e))))
          )
    ;; => {:x 3}
    ))

[abo-abo]

Returned strings are displayed un-stringified

Can't reproduce:

(let [xs ["100"]]
  (first xs))
;; => "100"

I think I went through all the reported bugs. Please close the issue if you confirm that they're fixed.

[yuhan0]

Thanks for the replies! The problem I faced with the first bug/feature was that it was completely undocumented anywhere on the Readme, the github.io page and even the docstrings in lispy-eval or le-clojure.el - there's no mention of this concept of "shadows" or any other "guess-intent" features.

I totally understand how this feature can help with debugging, but it could also very easily trip up users who don't realise that Lispy is intercepting and transforming all eval'ed forms, silently trying to "dwim" based on context.

Anecdotally, it caused me many headaches over the course of a few days, when it would shadow commonly vars like coll or x and make it seem like some function logic was flawed when it was actually splicing in arguments from some distant let binding. I would restart the entire REPL, thinking that it was some sort of CIDER/nREPL middleware issue and never suspecting Lispy had anything to do with it.

Would it make sense to automatically un-shadow bindings upon the first eval of a form outside the original let context? Or at the least expose an interactive command for (lispy-clojure/shadow-unmap *ns*) and properly document the different "intent-guessing" contexts and behaviours? Thanks for your consideration, I'd be willing to help with any implementation :)

[abo-abo]

Thanks. I've added the 0e binding to remove most of the pain, now that you know what's going on.

Next, we could add some documentation. I was thinking of some customization, on by default, that makes guess-intent also println what it did, maybe cross-link to some (to be created) documentation that explains why it's useful.

[yuhan0]

I updated Lispy but the last issue with un-stringification is still reproducible on my end - I'm on Emacs 26.1, latest 0.22 version of Cider and and cider-nrepl middleware - not sure what else could be causing the discrepancy.

There was also another guess-intent related bug I reported in a comment above:

Another bug with evaluating repeated sub-expressions in a threading macro, this one due to using (position) to determine context, which only takes the first matching form:

(-> [[[:a] [:b] [:c]]]
  (first)
  (reverse)
  (first)
  (first))

[abo-abo]

using (position) to determine context

OK, I can reproduce. But fixing this needs a rewrite of reval API. This will take me some time.