borkdude / grasp

Grep Clojure code using clojure.spec regexes
Eclipse Public License 1.0
242 stars 7 forks source link

Grasp doesn't seem to descend into macro forms #28

Open dpsutton opened 2 years ago

dpsutton commented 2 years ago

I'm looking for all strings in our codebase at Metabase that need translations. To this end i'm doing

(s/def ::translate (s/and
                     (complement vector?)
                     (s/cat :translate-symbol (fn [x]
                                               (and (symbol? x)
                                                    (#{"trs" "deferred-trs"
                                                       "tru" "deferred-tru"}
                                                      (name x))))
                            :args (s/+ any?))))

(I only just now realized I can resolve symbols for smarter matching but not relevant here). This spec works great and finds almost all usages. However I've found one that it does not find:

(defmacro ^:private deffingerprinter
  [field-type transducer]
  {:pre [(keyword? field-type)]}
  (let [field-type [field-type :Semantic/* :Relation/*]]
    `(defmethod fingerprinter ~field-type
       [field#]
       (with-error-handling
         (with-global-fingerprinter
           (redux/post-complete
            ~transducer
            (fn [fingerprint#]
              {:type {~(first field-type) fingerprint#}})))
         (trs "Error generating fingerprint for {0}" (sync-util/name-for-logging field#))))))

I'm going to try to come up with a minimal reproduction because there's a lot going on.

  (g/grasp "/Users/dan/projects/work/metabase/src/metabase/sync/analyze/fingerprint/fingerprinters.clj"
           ::translate)
;; misses the example above
[(deferred-trs "Error reducing {0}" (name k))]

Note I'm not expecting to find the usage at macro invocations and call sites, just in the literal form here which I would expect it to be able to descend into and match on the trs.

borkdude commented 2 years ago

I can look at this hopefully when I have some time on the train to ClojureD this week, else next week, but a minimal repro would surely help to look into this faster.

borkdude commented 2 years ago

I think I found the issue. When reading, a macro looks like this:

user=> (read-string "(defmacro my-macro [] `(foo))")
(defmacro my-macro [] (clojure.core/seq (clojure.core/concat (clojure.core/list (quote user/foo)))))

and this is the form that grasp "grasps" against. So it is the fully expanded but unevaluated form that the syntax quote gives back.

dpsutton commented 2 years ago

ah so (list quote user/foo) won't match against (s/cat :foo ::foo) spec. Not clear if this a close as not a bug, close as a bug but won't fix, or what. Thank you for looking into it.

borkdude commented 2 years ago

@dpsutton This is what the unevaluated form looks like, basically. Always evaluating what comes back from the reader won't work/isn't safe. To find stuff in syntax-quote expression I suggest inspecting them in a REPL:

user=> '`(trs "foobar")
(clojure.core/seq (clojure.core/concat (clojure.core/list (quote user/trs)) (clojure.core/list "foobar")))

and then making your spec account for that.

We could document this.

shaunlebron commented 1 year ago

I wonder if this is a good case for using rewrite-clj to allow descending into unexpanded syntax quotes.