Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

borkdude.rewrite-edn/get-in gives Unhandled java.lang.ClassCastException #39

Open
teodorlu opened this issue Aug 6, 2024 · 5 comments

Comments

@teodorlu
Copy link

teodorlu commented Aug 6, 2024

Hi :)

During work on a Neil PR, I encountered an exception in rewrite-edn.

Versions for repro

;; rewrite-edn version
borkdude/rewrite-edn {:mvn/version "0.4.8"}

;; java and clojure version
user> *clojure-version*
;; => {:major 1, :minor 11, :incremental 1, :qualifier nil}
user> (System/getProperty "java.version")
;; => "21.0.4"

Repro

  (def edn-nodes (borkdude.rewrite-edn/parse-string "{:deps #:babashka{pods #:git{:url \"https://github.com/babashka/babashka.pods\", :sha \"6ad6045b94bc871c5107bfc75d39643b6c1bc8ba\"}}}"))
  (def nl-path '[:deps babashka/pods])
  (borkdude.rewrite-edn/get-in edn-nodes nl-path)

Expected: return nil or a value.

Actual: I get a crash:

1. Unhandled java.lang.ClassCastException
   class clojure.lang.Symbol cannot be cast to class java.lang.Number
   (clojure.lang.Symbol is in unnamed module of loader 'app'; java.lang.Number
   is in module java.base of loader 'bootstrap')

              Numbers.java:  265  clojure.lang.Numbers/gte
              Numbers.java: 3991  clojure.lang.Numbers/gte
                 impl.cljc:  154  borkdude.rewrite_edn.impl$get/invokeStatic
                 impl.cljc:  119  borkdude.rewrite_edn.impl$get/invoke
                 impl.cljc:  162  borkdude.rewrite_edn.impl$get_in$fn__15166/invoke
     PersistentVector.java:  343  clojure.lang.PersistentVector/reduce
                  core.clj: 6885  clojure.core/reduce
                  core.clj: 6868  clojure.core/reduce
                 impl.cljc:  159  borkdude.rewrite_edn.impl$get_in/invokeStatic
                 impl.cljc:  158  borkdude.rewrite_edn.impl$get_in/invoke
          rewrite_edn.cljc:   36  borkdude.rewrite_edn$get_in/invokeStatic
          rewrite_edn.cljc:   30  borkdude.rewrite_edn$get_in/invoke
          rewrite_edn.cljc:   34  borkdude.rewrite_edn$get_in/invokeStatic
          rewrite_edn.cljc:   30  borkdude.rewrite_edn$get_in/invoke
                      REPL:  321  babashka.neil.dep-upgrade-test/eval18159
@teodorlu
Copy link
Author

teodorlu commented Aug 6, 2024

As far as I can tell, comparing k and (count coll) fails here.

(if (>= k (count coll))
(node/coerce default)
(node/coerce (first (nth coll k))))))))

When I run the test, k appears to be 'babashka/pods (namespace qualified symbol), whereas (count coll) is 2 (number).

@borkdude
Copy link
Owner

borkdude commented Aug 7, 2024

I guess this is the branch where the tag of the node isn't a :map so it's assumed to be a vector with a numeric key k. Can you maybe print what the tag is, assuming it's not a vector?

@borkdude
Copy link
Owner

borkdude commented Aug 7, 2024

Oh I guess it's one of those namespaced maps which might not be handled so well yet.

@borkdude
Copy link
Owner

We can take a look at how antq is dealing with namespaced maps here:

https://github.com/liquidz/antq/blob/daf6321f316ca33386dbbc5787ec2807f3ba3cb1/src/antq/upgrade/clojure.clj#L21

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants