Error formatting HoneySQL that has nested metadata

[camsaul]

Hi Sean, sorry to bother you with nonsense, but after bumping my Honey SQL version to the latest I'm running into issues running tests with Cloverage because it adds an :original key to metadata for instruments forms, and :original itself has :line/:column/:end-line/:end-column metadata.

Here's a repro for the error:

(honey.sql/format
 ^{:line 179, :column 77, :end-line 179, :end-column 79}
 {:select
  ^{:line 130,
    :column 44,
    :end-line 131,
    :end-column 48,
    :original
    ^{:line 131, :column 44, :end-line 131, :end-column 48}
    [:*]}
  [:*],
  :from
  ^{:line 134, :column 38, :end-line 134, :end-column 51}
  [^{:line 67, :column 5, :end-line 67, :end-column 15} [:venues]],
  :where ^{:line 42, :column 5, :end-line 42, :end-column 13} [:= :id 4]})

1. Caused by java.lang.ClassCastException
   class clojure.lang.PersistentVector cannot be cast to class clojure.lang.Named (clojure.lang.PersistentVector and
   clojure.lang.Named are in unnamed module of loader 'app')

                  core.clj: 1610  clojure.core/name
                  core.clj: 1604  clojure.core/name
                  sql.cljc:  342  honey.sql$sql_kw/invokeStatic
                  sql.cljc:  329  honey.sql$sql_kw/invoke
                  core.clj: 6979  clojure.core/mapv/fn
     PersistentVector.java:  343  clojure.lang.PersistentVector/reduce
                  core.clj: 6885  clojure.core/reduce
                  core.clj: 6970  clojure.core/mapv
                  core.clj: 6970  clojure.core/mapv
                  sql.cljc:  572  honey.sql$format_meta/invokeStatic
                  sql.cljc:  554  honey.sql$format_meta/doInvoke
               RestFn.java:  410  clojure.lang.RestFn/invoke
                  sql.cljc:  729  honey.sql$format_selects_common/invokeStatic
                  sql.cljc:  728  honey.sql$format_selects_common/invoke
                  sql.cljc:  744  honey.sql$format_selects/invokeStatic
                  sql.cljc:  743  honey.sql$format_selects/invoke
                  Var.java:  388  clojure.lang.Var/invoke
                  sql.cljc: 1574  honey.sql$format_dsl$fn__17379/invoke
     PersistentVector.java:  343  clojure.lang.PersistentVector/reduce
                  core.clj: 6885  clojure.core/reduce
                  core.clj: 6868  clojure.core/reduce
                  sql.cljc: 1568  honey.sql$format_dsl/invokeStatic
                  sql.cljc: 1558  honey.sql$format_dsl/doInvoke
               RestFn.java:  423  clojure.lang.RestFn/invoke
                  Var.java:  388  clojure.lang.Var/invoke
                  sql.cljc: 2108  honey.sql$format/invokeStatic
                  sql.cljc: 2042  honey.sql$format/invoke
                  sql.cljc: 2057  honey.sql$format/invokeStatic
                  sql.cljc: 2042  honey.sql$format/invoke
             honeysql2.clj:  292  toucan2.honeysql2/eval27288
             honeysql2.clj:  292  toucan2.honeysql2/eval27288
             Compiler.java: 7194  clojure.lang.Compiler/eval
             Compiler.java: 7653  clojure.lang.Compiler/load
                      REPL:    1  user/eval27124
                      REPL:    1  user/eval27124
             Compiler.java: 7194  clojure.lang.Compiler/eval
             Compiler.java: 7149  clojure.lang.Compiler/eval
                  core.clj: 3215  clojure.core/eval
                  core.clj: 3211  clojure.core/eval
    interruptible_eval.clj:   87  nrepl.middleware.interruptible-eval/evaluate/fn/fn
                  AFn.java:  152  clojure.lang.AFn/applyToHelper
                  AFn.java:  144  clojure.lang.AFn/applyTo
                  core.clj:  667  clojure.core/apply
                  core.clj: 1990  clojure.core/with-bindings*
                  core.clj: 1990  clojure.core/with-bindings*
               RestFn.java:  425  clojure.lang.RestFn/invoke
    interruptible_eval.clj:   87  nrepl.middleware.interruptible-eval/evaluate/fn
                  main.clj:  437  clojure.main/repl/read-eval-print/fn
                  main.clj:  437  clojure.main/repl/read-eval-print
                  main.clj:  458  clojure.main/repl/fn
                  main.clj:  458  clojure.main/repl
                  main.clj:  368  clojure.main/repl
               RestFn.java: 1523  clojure.lang.RestFn/invoke
    interruptible_eval.clj:   84  nrepl.middleware.interruptible-eval/evaluate
    interruptible_eval.clj:   56  nrepl.middleware.interruptible-eval/evaluate
    interruptible_eval.clj:  152  nrepl.middleware.interruptible-eval/interruptible-eval/fn/fn
                  AFn.java:   22  clojure.lang.AFn/run
               session.clj:  218  nrepl.middleware.session/session-exec/main-loop/fn
               session.clj:  217  nrepl.middleware.session/session-exec/main-loop
                  AFn.java:   22  clojure.lang.AFn/run
               Thread.java: 1623  java.lang.Thread/run

I enabled tracing on a few vars and *print-meta* and this is the failing call

(honey.sql/format-meta ^{:line 130, :column 44, :end-line 131, :end-column 48, :original ^{:line 131, :column 44, :end-line 131, :end-column 48} [:*]} [:*])
;; ...
(honey.sql/sql-kw ^{:line 131, :column 44, :end-line 131, :end-column 48} [:*])
;; => Execution error (ClassCastException) at honey.sql/sql-kw (sql.cljc:342).
;; class clojure.lang.PersistentVector cannot be cast to class clojure.lang.Named 
;; (clojure.lang.PersistentVector and clojure.lang.Named are in unnamed module of loader 'app')

How would you feel about updating

https://github.com/seancorfield/honeysql/blob/a3ef21548520636168a970d780ff46a4be4fa002/src/honey/sql.cljc#L558-L576

to recursively ignore metadata inside metadata to handle weird edge cases like this? I can submit a PR if you'd like.

Thanks!

[seancorfield]

I have some work planned around metadata soon, so I'll roll this into it. I guess with cloverage you can't get at the option that tells HoneySQL to ignore :original?

[seancorfield]

Back at my desk, so I can give a more detailed answer.

Right now, HoneySQL assumes it can call sql-kw on metadata keys and values which in turn means called name on them. That's not a very robust approach, but I want to expand what can be provided as metadata in several contexts. My plan is to make it ignore all "non-scalar" data, so it will accept numbers, strings, symbols, and keywords but skip over everything else, in addition to the "standard" location metadata that popular tooling uses. With that approach, :original would be ignored since it is a map.

[seancorfield]

See if the head of develop bf34a23e6874c0c7c03b81e69b9eaddf06c2caa9 or the latest 2.6.9999-SNAPSHOT solves this for you.

[camsaul]

Hey @seancorfield I gave that a try and it seems to work. Thanks for the quick fix!