juxt / clip

Light structure and support for dependency injection
MIT License
228 stars 15 forks source link

oddity when component functions args are dereferenced structures containing lists #15

Closed henryw374 closed 4 years ago

henryw374 commented 4 years ago

Hi. Using clip 0.1.18, I see some strange behaviour when an expression deref's a structure that contains a list. Here is a repro:

(ns foo-ns)

(defn foo-component [x]
  (println x)
  x)

(ns clip
  (:require [juxt.clip.core :as clip]
            [foo-ns]))

(defn sys []
  (let [val {:bar (list :a :b :c)}]
    {:components
     {:foo {:start `(foo-ns/foo-component ~val)}}}))

(comment

  (clip/start (sys))
  )

starting the system, instead of the component getting {:bar (:a :b :c)}, it comes through as:

{:foo {:bar #object[juxt.clip.impl.core$prevent_eval$reify__1811 0x36a83a42 "#blocked-eval [:c]"]}}

SevereOverfl0w commented 4 years ago

There's a bug here, but your example will try and eval :a as a function, which I don't think you intend.

SevereOverfl0w commented 4 years ago

Fixed in 0.19.0.

henryw374 commented 4 years ago

Thanks, yes the example should have been `(foo-ns/foo-component '~val).

I think what I need when using clip is a sanity checking fn that applies to every :start and :stop form

Here is a first pass of the kind of thing... the arg x is a start/stop form

(clojure.walk/prewalk
        (fn [x]
          (cond 
                (and (qualified-symbol? x)
                  (nil? (requiring-resolve x)))
                (println "warning, " x " does not resolve to a value")
                (and (list? x)
                  (not (and (qualified-symbol? x) (first x))))
                (println "warning, " x " will evaluate as a function expression unless quoted"))
          x
          )))

so in the example above, I see:

warning, (:a :b :c) will evaluate as a function unless quoted.

also any stray symbols in there will get the "does not resolve" message.

Needs a bit more thought ... but I think the delayed execution part is something many ppl will not be very familiar with and prone to subtle bugs - so some kind of guard rail could be useful there.

SevereOverfl0w commented 4 years ago

(:a :b :c) can't be distinguished from (:k m :backup).

Btw, you might want to use with-deps instead of generating code in this exact way.

henryw374 commented 4 years ago

yep, but I'll happily be prompted of that and change to (get m :k backup) to avoid subtle errors. I guess we'll wrap the start fn we use.

Thanks for pointing out those macros.