api-diff icon indicating copy to clipboard operation
api-diff copied to clipboard

Distinguish lang (aka platform aka dialect, i.e. Clojure, ClojureScript)

Open lread opened this issue 4 years ago • 4 comments

Scenario One var can be defined differently for different langs.

A contrived example:

#?(:cljs (defn mixed [a cljs sig])
   :clj  (defn mixed [a different clj sig]))

A real-world example might be the different signatures for assert-expr between Clojure and ClojureScript.

Api-diff currently does not consider the lang when comparing APIs, and will currently sometimes target the wrong v2 target.

Proposal The unique key for a var is currently :ns + :name. Make it :ns + :name + :lang. Report on lang.

Reporting option ideas:

  1. A line for each diff:

    test-resources/older/example.cljc:11:1:clj error: example/y was removed.
    test-resources/older/example.cljc:11:1:cljs error: example/y was removed.
    

    Accurate, but could be considered noisy.

  2. A line for each unique diff. We could consider optimizing this to a single line by listing platforms when diff finding is same for all platforms for single var:

    test-resources/older/example.cljc:11:1:clj,cljs error: example/y was removed.
    
  3. Omit lang from reporting when vars compared are Clojure only or ClojureScript only.

Option 3 can be combined with option 1 or 2. I'm leaning toward only option 1 for an initial release. What do you think? (Any edn #16 return would be the verbose finding for each diff)

Other info

  • Somewhat related to #5, at least in topic
  • Clojure documented platforms are :clj :cljs :cljr and :default.
  • We'll just pass along what clj-kondo provides us (including :bb etc)
  • We'll infer :lang when clj-kondo does not fill it by looking at filename extension.

Next steps After we agree on an approach, I can take work on a PR.

lread avatar Oct 25 '21 17:10 lread

Yes, I think we should take into account the language.

borkdude avatar Oct 25 '21 17:10 borkdude

I think we have a similar issue for clj-kondo where we show the language context for .cljc.

borkdude avatar Oct 25 '21 17:10 borkdude

Yeah, here it is: https://github.com/clj-kondo/clj-kondo/issues/1196

Forgot we did some good analysis on reporting there, we can steal that.

lread avatar Oct 25 '21 17:10 lread

So I don't forget when I come back to this: ClojureScript & macros

I'm at rambly thoughts stage here:

While working on cljdoc, I noticed that when analyzing for ClojureScript, it only includes macros that have been loaded by a library from ClojureScript. I'll double-check, but I expect this is codox behavior as well.

I had a good chat with @borkdude about this on Slack a while ago. He helped me to understand this assumption is not technically correct. Just because a library does not load its own macros from ClojureScript does not make that library's macros unavailable from ClojureScript.

So, when describing an API from the ClojureScript view, I suppose it should include all Clojure macros.

But, as a library author, how would I express: "no, this macro is supported for Clojure only" - or even ClojureScript only.

Our only widely used metadata mechanism for excluding something from an API is currently the boolean :no-doc (and its synonym :skip-wiki). I'm not sure if reader-conditionals would help here, but have yet to experiment.

Perhaps an easier to understand alternative would be to extend :no-doc to also optionally include the platform to exclude? Maybe something like ^{:no-doc [:cljs]}? But that would mean tooling that did not understand this extension would exclude the macro entirely...

So more thinking is required, just wanted to make sure I didn't lose these thoughts.

lread avatar Apr 23 '22 15:04 lread