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

Macros and functions that are replaced from clojure.core are shown with the incorrect namespace when using backtick #509

Closed
bhb opened this issue Jan 21, 2021 · 5 comments

Comments

@bhb
Copy link

bhb commented Jan 21, 2021

version

I believe it's f724f0e (I'm using babaska 0.2.7 and it looks like that's the version of sci included)

platform

Mac binary

problem

If you replace a function from clojure.core with a new implementation, and then you use the backtick to resolve the function, sci will assume you are referencing the clojure.core version.

repro

My repro is in babaska: run the below with bb repro.clj

;; repro.clj
(ns repro)

(alias 'c 'clojure.core)

(defmacro and
  [& pred-forms]
  `(do
     (prn "in custom 'and'")
     (c/and ~@pred-forms)))

(prn {:expr `(and true false)
      :result (and true false)})

(defn inc [x]
  (prn "in custom 'inc'")
  (c/int x))

(prn {:expr `(inc 1)
      :result (inc 1)})

This will print out:

"in custom 'and'"
{:expr (clojure.core/and true false), :result false}
"in custom 'inc'"
{:expr (clojure.core/inc 1), :result 1}

You can see that:

  1. the custom function and macro are being called correctly
  2. but when using the backtick, the namespace is clojure.core, not repro

expected behavior

Run the same code with the clojure CLI (version 1.10.1.763) with clj -M repro.clj and it will print out:

WARNING: and already refers to: #'clojure.core/and in namespace: repro, being replaced by: #'repro/and
"in custom 'and'"
{:expr (repro/and true false), :result false}
WARNING: inc already refers to: #'clojure.core/inc in namespace: repro, being replaced by: #'repro/inc
"in custom 'inc'"
{:expr (repro/inc 1), :result 1}

I would expect the backtick to expand to repro/and and repro/inc, like it does in the Clojure example.

Note: the reason I ran into this that https://github.com/borkdude/spartan.spec prints a confusing output when you call s/form:

(require '[babashka.deps :as deps])

(deps/add-deps
 '{:deps {borkdude/spartan.spec {:git/url "https://github.com/borkdude/spartan.spec"
                                 :sha "bf4ace4a857c29cbcbb934f6a4035cfabe173ff1"}}})

;; Loading spartan.spec will create a namespace clojure.spec.alpha for compatibility:
(require 'spartan.spec)
(require '[clojure.spec.alpha :as s])


(s/def ::foo (s/and string?
                    (fn [x] (< 10 (count x)))))


(s/form ::foo)
;; ^---------------
;; Expected:  (clojure.spec.alpha/and clojure.core/string? (fn [%] (clojure.core/< 10 (clojure.core/count %))))
;; Actual: (clojure.core/and clojure.core/string? (fn [%] (clojure.core/< 10 (clojure.core/count %))))
@borkdude
Copy link
Collaborator

@bhb Thanks, this is a bug. I noticed you can work around this by excluding those names:

(ns repro (:refer-clojure :exclude [and inc]))

but the bug still needs to be addressed. Thanks for reporting.

@borkdude
Copy link
Collaborator

Minimal repro:

(defn inc []) `inc

@borkdude
Copy link
Collaborator

Fixed with 014da0e. I will bump bb to this commit and you can try a new bb from the CircleCI builds.

@borkdude
Copy link
Collaborator

@bhb
Copy link
Author

bhb commented Jan 21, 2021

Wow, thanks for the fast fix! I'll try this out ASAP!

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