taoensso / truss

Assertions micro-library for Clojure/Script
https://www.taoensso.com/truss
Eclipse Public License 1.0
304 stars 14 forks source link

In-line assertion on side-effecting form is unsafe in macro contexts #12

Closed samharad closed 1 year ago

samharad commented 1 year ago

Version: 1.7.2

(ns scratch
  (:require
    [taoensso.truss :refer [have]]))

(have nil? (println "side effect!"))
; side effect!
;=> nil

(defn bar []
  (have nil? (println "side effect!")))

(bar)
; side effect!
;=> nil

(defmacro foo []
  `(have nil? (println "side effect!")))

;; Weird?!
(foo)
; side effect!
; side effect!
;=> nil

(macroexpand
  '(foo))
;=>
;(if
;  (clojure.core/nil? (clojure.core/println "side effect!"))
;  (clojure.core/println "side effect!")
;  (taoensso.truss.impl/-invar-violation!
;    true
;    (quote rpl.rama.test-helpers)
;    nil
;    (quote clojure.core/nil?)
;    (clojure.core/println "side effect!")
;    nil
;    nil))

I'm not sure why the issue presents only in the context of a macro... this led to a tough bug to track down.

I think ideally such in-line assertions on side-effecting forms would be safe; otherwise, perhaps the README should call this out?

ptaoussanis commented 1 year ago

@samharad Hi Sam, thanks for tracking this down and for the clear report. This is definitely a bug. I'm really sorry about the trouble! This must have been a lot of work to identify.

Will investigate today and get back to you ASAP.

ptaoussanis commented 1 year ago

Just pushed [com.taoensso/truss "1.8.0"] to Clojars, which should fix the issue you're seeing here.

Note that there's one remaining + unavoidable gotcha when using Truss as part of a macro expansion- the assertion's line number will unfortunately be lost by default.

This is due to CLJ-865, dating back to 2013.

While I was fixing the bug you reported, I took the opportunity to document the CLJ-865 issue in the have docstring, and to provide a manual workaround that you might like to use.

Hope that helps. And again, apologies for the trouble! Cheers

samharad commented 1 year ago

@ptaoussanis Thanks a bunch for the quick patch!