Grab arglists meta outside of with-redefs

[ProjectFrank]

Arity checking in bond/with-stub! does not work with multimethods. We attempted to work around this by adding :arglists metadata our var. In some cases (unable to consistently reproduce), with-redefs will lose the original var's metadata.

This change makes it so that the original var's arglists are computed outside of the with-redefs and passed to the stub.

[codecov[bot]]

Codecov Report

Merging #38 into master will increase coverage by 0.29%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #38      +/-   ##
==========================================
+ Coverage   98.59%   98.88%   +0.29%     
==========================================
  Files           1        1              
  Lines          71       90      +19     
  Branches        1        1              
==========================================
+ Hits           70       89      +19     
  Partials        1        1

Impacted Files	Coverage Δ
src/bond/james.cljc	98.88% <100%> (+0.29%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 09b9d31...43f3f98. Read the comment docs.

[ProjectFrank]

I initially tried to use arglists-map# but because I'm using it in two different syntax-quoted forms, the autogensym that comes out is different.

+1 for factoring out the zipmap; I'll do that

[technomancy]

Oh, I see what you mean now about the nested backticks. Yeah, that could probably be cleaned up but doesn't need to happen for this PR as it was already there before.

[ProjectFrank]

@gordonsyme I added the positive case for the multimethod and it became apparent that you can't set :arglists metadata on the var in cljs. stub! should still work but the test function where we are setting custom :arglists metadata will not work.

Would it be appropriate to have reader conditionals in the test code so that the defmulti would be a plain old defn in cljs?

[gordonsyme]

@ProjectFrank stub! does not appear to work for ClojureScript multimethods, the :arglists metadata is stubbornly empty:

Clojure:

user=> (require '[bond.james :as bond])
nil

user=> (defmulti some-multimethod {:arglists '([arg1 arg2 arg3])} (fn [arg1 arg2 arg3] :foo))
#'user/some-multimethod

user=> (defmethod some-multimethod :foo [arg1 arg2 arg3] (list arg1 arg2 arg3))
#object[clojure.lang.MultiFn 0x11a24d3c "clojure.lang.MultiFn@11a24d3c"]

user=> (some-multimethod :a :b :c)
(:a :b :c)

user=> ((bond/stub! #'some-multimethod (fn [arg1 arg2 arg3])) :a :b :c)
nil

ClojureScript:

cljs.user=> (require '[bond.james :as bond])
nil

cljs.user=> (defmulti some-multimethod {:arglists '([arg1 arg2 arg3])} (fn [arg1 arg2 arg3] :foo))
#'cljs.user/some-multimethod

cljs.user=> (defmethod some-multimethod :foo [arg1 arg2 arg3] (list arg1 arg2 arg3))
#object[cljs.core.MultiFn]

cljs.user=> (some-multimethod :a :b :c)
(:a :b :c)

cljs.user=> ((bond/stub! #'some-multimethod (fn [arg1 arg2 arg3])) :a :b :c)
org.mozilla.javascript.JavaScriptException: Error: 3 (.cljs_rhino_repl/goog/../bond/james.js#278)
     (.cljs_rhino_repl/bond/james.cljc:89:21)
     (.cljs_rhino_repl/bond/james.cljc:86:24)
     (.cljs_rhino_repl/cljs/core.cljs:1871:5)
     (.cljs_rhino_repl/cljs/core.cljs:1856:1)
     (NO_SOURCE_FILE <cljs repl>:1:0)
     (NO_SOURCE_FILE <cljs repl>:1:0)

[gordonsyme]

Since the problem is due to incomplete multimethod (var, really) support from ClojureScript I am fine with making the body of the multimethod tests conditional on Clojure only, and declaring in the README that stubbing multimethods is not supported in ClojureScript.

[ProjectFrank]

Apparently this whole PR is not necessary (and might not even do anything). The reason why the multimethod metadata was being stripped had nothing to do with with-redefs. It had more to do with the fact that var metadata gets stripped every time you reevaluate the defmulti form. This is a bug with the implementation of defmulti in Clojure 1.8, fixed in 1.9 (https://dev.clojure.org/jira/browse/CLJ-1870).

I thought that with-redefs had something to do with it only because running tests in Cursive would reload all the relevant namespaces, and consequently, defmulti forms.

The workaround for now is to attach the var metadata to the symbol for the var name, instead of as a third argument to defmulti. That way, re-evaluating defmulti forms does not strip var metadata.