source-tracking-eval

0001-source-tracking-eval-wip.patch

From 1248e8505235d177f325d469c4a1b2166ee27d2b Mon Sep 17 00:00:00 2001
From: Michael Griffiths <mikey@cich.li>
Date: Thu, 26 Mar 2015 14:55:28 +0000
Subject: [PATCH] source tracking eval wip

---
 META-INF/MANIFEST.MF                               |  2 +-
 doc/ops.md                                         |  2 +-
 pom.xml                                            |  8 +++++-
 .../tools/nrepl/middleware/interruptible_eval.clj  | 29 ++++++++++++++++++----
 src/test/clojure/clojure/tools/nrepl_test.clj      | 13 +++++++++-
 5 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/META-INF/MANIFEST.MF b/META-INF/MANIFEST.MF
index 96a7b6c..813a069 100644
--- a/META-INF/MANIFEST.MF
+++ b/META-INF/MANIFEST.MF
@@ -10,5 +10,5 @@ Created-By: Apache Maven Bundle Plugin
 Build-Jdk: 1.6.0_22
 Export-Package: clojure.tools,
  clojure.tools.nrepl
-Import-Package: clojure;version="1.3.0",
+Import-Package: clojure;version="1.4.0",
  clojure.lang
diff --git a/doc/ops.md b/doc/ops.md
index 6fd1b4e..048573f 100644
--- a/doc/ops.md
+++ b/doc/ops.md
@@ -2,7 +2,7 @@
    **Do not edit!** -->
 # Supported nREPL operations
 
-<small>generated from a verbose 'describe' response (nREPL v0.2.9-SNAPSHOT)</small>
+<small>generated from a verbose 'describe' response (nREPL v0.2.10-SNAPSHOT)</small>
 
 ## Operations
 
diff --git a/pom.xml b/pom.xml
index aed0833..db6d354 100644
--- a/pom.xml
+++ b/pom.xml
@@ -27,7 +27,7 @@
   </scm>
 
     <properties>
-        <clojure.version>1.2.0</clojure.version>
+        <clojure.version>1.4.0</clojure.version>
         <clojure.warnOnReflection>true</clojure.warnOnReflection>
     </properties>
 
@@ -38,6 +38,12 @@
         <version>0.2.3</version>
         <optional>true</optional>
       </dependency>
+      <dependency>
+        <groupId>org.clojure</groupId>
+        <artifactId>tools.reader</artifactId>
+        <version>0.8.16</version>
+        <optional>false</optional>
+      </dependency>
     </dependencies>
 
     <build>
diff --git a/src/main/clojure/clojure/tools/nrepl/middleware/interruptible_eval.clj b/src/main/clojure/clojure/tools/nrepl/middleware/interruptible_eval.clj
index f682f0c..062fcef 100644
--- a/src/main/clojure/clojure/tools/nrepl/middleware/interruptible_eval.clj
+++ b/src/main/clojure/clojure/tools/nrepl/middleware/interruptible_eval.clj
@@ -2,9 +2,13 @@
      clojure.tools.nrepl.middleware.interruptible-eval
   (:require [clojure.tools.nrepl.transport :as t]
             clojure.tools.nrepl.middleware.pr-values
+            [clojure.tools.reader :as r]
             clojure.main)
   (:use [clojure.tools.nrepl.misc :only (response-for returning)]
-        [clojure.tools.nrepl.middleware :only (set-descriptor!)])
+        [clojure.tools.nrepl.middleware :only (set-descriptor!)]
+        [clojure.tools.reader.impl.utils :only [make-var]]
+        [clojure.tools.reader.reader-types :only (->SourceLoggingPushbackReader
+                                                  string-push-back-reader)])
   (:import clojure.lang.LineNumberingPushbackReader
            (java.io StringReader Writer)
            java.util.concurrent.atomic.AtomicLong
@@ -24,6 +28,20 @@
   []
   (dissoc (get-thread-bindings) #'*msg* #'*eval*))
 
+(defn- source-logging-string-reader
+  [code file line column]
+  (->SourceLoggingPushbackReader
+   (string-push-back-reader code)
+   (or line 0)
+   (or column 0)
+   true
+   nil
+   nil
+   file
+   (doto (make-var)
+     (alter-var-root (constantly {:buffer (StringBuilder.)
+                                  :offset 0})))))
+
 (defn evaluate
   "Evaluates some code within the dynamic context defined by a map of `bindings`,
    as per `clojure.core/get-thread-bindings`.
@@ -39,7 +57,7 @@
 
    It is assumed that `bindings` already contains useful/appropriate entries
    for all vars indicated by `clojure.main/with-bindings`."
-  [bindings {:keys [code ns transport session eval] :as msg}]
+  [bindings {:keys [code ns transport session eval file line column] :as msg}]
   (let [explicit-ns-binding (when-let [ns (and ns (-> ns symbol find-ns))]
                               {#'*ns* ns})
         original-ns (bindings #'*ns*)
@@ -47,7 +65,8 @@
                                     (if-not explicit-ns-binding
                                       bindings
                                       (assoc bindings #'*ns* original-ns)))
-        bindings (atom (merge bindings explicit-ns-binding))
+        file (or file (get bindings #'*file*))
+        bindings (atom (merge bindings explicit-ns-binding {#'*file* file}))
         session (or session (atom nil))
         out (@bindings #'*out*)
         err (@bindings #'*err*)]
@@ -64,8 +83,8 @@
                      (set! *3 (@bindings #'*3))
                      (set! *e (@bindings #'*e)))   
             :read (if (string? code)
-                    (let [reader (LineNumberingPushbackReader. (StringReader. code))]
-                      #(read reader false %2))
+                    (let [reader (source-logging-string-reader code file line column)]
+                      #(r/read reader false %2))
                     (let [code (.iterator ^Iterable code)]
                       #(or (and (.hasNext code) (.next code)) %2)))
             :prompt (fn [])
diff --git a/src/test/clojure/clojure/tools/nrepl_test.clj b/src/test/clojure/clojure/tools/nrepl_test.clj
index e30cc21..9b5fb9f 100644
--- a/src/test/clojure/clojure/tools/nrepl_test.clj
+++ b/src/test/clojure/clojure/tools/nrepl_test.clj
@@ -5,6 +5,7 @@
   (:require (clojure.tools.nrepl [transport :as transport]
                                  [server :as server]
                                  [ack :as ack])
+            [clojure.tools.reader :as reader]
             [clojure.set :as set]))
 
 (def project-base-dir (File. (System/getProperty "nrepl.basedir" ".")))
@@ -36,7 +37,7 @@
 
 (def-repl-test eval-literals
   (are [literal] (= (binding [*ns* (find-ns 'user)] ; needed for the ::keyword
-                      (-> literal read-string eval list))
+                      (-> literal reader/read-string eval list))
                     (repl-values client literal))
     "5"
     "0xff"
@@ -81,6 +82,16 @@
              combine-responses
              (select-keys [:value])))))
 
+(def-repl-test source-tracking-eval
+  (is (= {:file "test.clj" :line 42}
+         (-> (message timeout-client {:op :eval
+                                      :code "(do (def x 1) (select-keys (meta #'x) [:file :line]))"
+                                      :file "test.clj" :line 42 :column 10})
+             combine-responses
+             :value
+             first
+             read-string))))
+
 (def-repl-test unknown-op
   (is (= {:op "abc" :status #{"error" "unknown-op" "done"}}
          (-> (message timeout-client {:op :abc}) combine-responses (select-keys [:op :status])))))
-- 
2.3.3

nrepl-messages.el

(--->
  op  "eval"
  ns  "user"
  code  "(def a 1000)"
  file  "hello.clj"
  line  42
  column  10
  session  "387c75ff-3573-44d0-bf8e-8e6dc9da1541"
  id  "19"
)
(<-
  id  "19"
  ns  "user"
  session  "387c75ff-3573-44d0-bf8e-8e6dc9da1541"
  value  "#'user/a"
)
(<-
  id  "19"
  session  "387c75ff-3573-44d0-bf8e-8e6dc9da1541"
  status  ("done")
)

repl.clj

user> (meta #'a)
{:ns #<Namespace user>,
 :name a,
 :file "hello.clj",
 :end-column 16,
 :source "a",
 :column 10,
 :line 42,
 :end-line 42}
user> 

Presuming this is in preparation for http://dev.clojure.org/jira/browse/NREPL-59?

This is cool, but I don't want to pull in tools.reader. nREPL's reading should be bug-for-bug compatible with clojure.main, and thus we have to stick with LineNumberingPushbackReader. I don't see any clear reason why we can't do this, except perhaps that we'll have to use reflection to set the column number if it's provided and not zero.

Yeah, exactly that :-).

Not using tools.reader makes sense and that's the main reason I didn't just go straight to providing a patch for this. This approach will work in piggieback at least, since cljs uses tools.reader.

setLineNumber was only added to LineNumberingPushbackReader in 1.7 - in is a protected field, so we'd need to either subclass it or just use reflection for that too. Will give it a go this evening.

Well, turns out _columnNumber was only added to LineNumberingPushbackReader in 1.5 :-). Will go ahead and set it only in 1.5+.