Permalink
Browse files

Bugfix: weird interaction between `let`, macros, and file position.

  • Loading branch information...
1 parent 6b180ab commit a13f78f0764b62f1aaf33ce7c532f1239f354cdf @marick committed May 8, 2013
Showing with 57 additions and 12 deletions.
  1. +26 −12 src/midje/parsing/util/file_position.clj
  2. +31 −0 test/user/fus_midje_forms_in_macros.clj
@@ -1,6 +1,7 @@
(ns ^{:doc "Functions to help in finding the lines you care about."}
midje.parsing.util.file-position
- (:use midje.parsing.util.core
+ (:use midje.clojure.backwards-compatibility
+ midje.parsing.util.core
midje.parsing.util.zip
midje.parsing.arrow-symbols
[midje.parsing.util.zip :only [skip-to-rightmost-leaf]])
@@ -17,16 +18,25 @@
(defn set-fallback-line-number-from [form]
(reset! fallback-line-number (or (:line (meta form)) (Integer. 0))))
-(letfn [(raw-arrow-line-number [arrow-loc]
- (try
- (or (-> arrow-loc zip/left zip/node meta :line )
- (-> arrow-loc zip/right zip/node meta :line )
- (inc (-> arrow-loc zip/prev zip/left zip/node meta :line )))
- (catch Throwable ex nil)))]
-
- (defn arrow-line-number [arrow-loc]
- (if-let [raw-lineno (raw-arrow-line-number arrow-loc)]
- (reset! fallback-line-number raw-lineno)
+(defn arrow-line-number
+ "Return the best guess for what line an arrow symbol is on."
+ [arrow-loc]
+ ;; A 'lineish' is either a line number or nil.
+ (letfn [(lineish [loc] (-> loc zip/node meta :line))
+ (left-lineish [arrow-loc] (-> arrow-loc zip/left lineish))
+ (right-lineish [arrow-loc] (-> arrow-loc zip/right lineish))
+ (previous-lineish [arrow-loc] (-> arrow-loc zip/prev zip/left lineish))
+ ;; Note that the preceding function only works when the form before the arrow has no :line
+
+ (best-lineish [arrow-loc]
+ (try
+ ( (some-fn-m left-lineish right-lineish #(inc (previous-lineish %))) arrow-loc)
+ (catch Throwable ex
+ ;; `previous-lineish` returned nil: use the fallback-line-number
+ nil)))]
+
+ (if-let [lineish (best-lineish arrow-loc)]
+ (reset! fallback-line-number lineish)
(swap! fallback-line-number inc))))
(defn arrow-line-number-from-form
@@ -102,6 +112,10 @@
(integer? number-source)
(vary-meta form assoc :line number-source)
- :else
+ ;; If the line number source was generated by a macro, it won't have metadata.
+ (not (contains? (meta number-source) :line))
+ (vary-meta form assoc :line (Integer. 0))
+
+ :else
(vary-meta form assoc :line (:line (meta number-source)))))
@@ -0,0 +1,31 @@
+(ns user.fus-midje-forms-in-macros
+ (:use midje.sweet
+ midje.test-util))
+
+;; Because of the way that Midje does its parsing, there are
+;; complications when a user writes macros that wrap Midje forms. This
+;; shows that some of those complications are handled.
+
+;; This is pretty incomplete so far.
+
+
+
+;; A macro with a let in it that surrounded a fact used to blow up.
+
+(def random-atom (atom 0))
+(def count-of-facts-checked (atom 0))
+
+(defmacro with-memory-store [& body]
+ `(let [increment# 1]
+ (with-state-changes [(before :facts (reset! random-atom increment#))
+ (after :facts (swap! count-of-facts-checked inc))]
+ ~@body)))
+
+(with-memory-store
+ (fact "one fact"
+ @random-atom => 1)
+ (fact "another"
+ @count-of-facts-checked => 1))
+
+(fact @count-of-facts-checked => 2)
+

0 comments on commit a13f78f

Please sign in to comment.