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

ClojureScript compatibility + Primitive type hint support + spec->hint inference #45

Closed
wants to merge 17 commits into from

Conversation

vemv
Copy link
Contributor

@vemv vemv commented Apr 14, 2019

  • JVM ^longs, ^long ^byte*
  • primitives: accept clj? arg
  • primitive?: accept clj? arg
  • clj-class-mapping completeness
  • the tails should be potentially hinted out of the name-ann
  • unit.nedap.utils.speced.defn*: cover 'alt' positions
  • use stronger every? in tests
  • any remaining thrown? Exception?
  • 64 vs 64.0
  • primitives should be forbidden for protocols
  • Add extra coverage for primitives
  • ::nilable should be forbidden for primitives
  • unit.nedap.utils.speced.defprotocol.type-hinting and others should be cljc'ed
  • maybe-tag-tails: add a :pre asserting that valid js metadata is being passed
  • mv + cover: ensure-proper-type-hint
  • {:post [(if-not % true (-> % :spec))]}
  • Enable metadata tests for cljs
  • testing "Arguments hinting for multi-arity functions" needs a single-arity counterpart
  • unit test for (extract-specs-from-metadata {:tag 'js/Number} false) and such
  • cover consistent-tagging? and other similar fns
  • Ensure unit.nedap.utils.speced.defn.pre-post is in sync
  • Upgrade :pres to use check!
  • Restore Cloverage
  • Maybe: if there's a primitive hint, turn :pres into :posts
    • Rationale: Note that boolean return values will be turned into Booleans, chars will become Characters, and numeric primitives will become Numbers unless they are immediately consumed by a method taking a primitive. https://clojure.org/reference/java_interop
  • README
    • Mention as improvements:
      • ret metadata forbidden in nonsensical places
      • primitive metadata forbidden in nonsensical places
      • Unified ret metadata placement (clj/cljs)

@vemv vemv force-pushed the cljs branch 5 times, most recently from 46ecbf9 to 41904a9 Compare April 15, 2019 04:15
@vemv
Copy link
Contributor Author

vemv commented Apr 15, 2019

@lennartbuit: check out this branch, I got it almost working!

@vemv
Copy link
Contributor Author

vemv commented Apr 15, 2019

...the unit.nedap.utils.api.satisfies test is failing, no idea why. Could be the macro thing you mentioned once, or another cljs intricacy.

Would you give it a 👀?

@lennartbuit
Copy link
Contributor

Yep, I sadly found out why: :extend-via-metadata doesn't work in CLJS. For example reported in stuartsierra/component.

@vemv
Copy link
Contributor Author

vemv commented Apr 15, 2019

clojure/clojurescript@23ab9a0 is there though... its tag matches our version

I couldn't get even the basic case (plain compile-time non-partial extension) to work, so sth is off I believe

@lennartbuit
Copy link
Contributor

Yeah, I was prematurely concluding. I am having the same issue indeed. Meh, will need to set up a cljs repl somehow then.

@vemv
Copy link
Contributor Author

vemv commented Apr 17, 2019

Removed intended support of satisfies? for cljs, for now. We simply don't emit the fn under the api ns

Not a big deal, we can add it later

Tests are passing for both suites, with better coverage than before

Pending:

  • Enable unit.nedap.utils.speced.defn for JS and similar cases
    • i.e. ns's completely disabled
  • Enabled unit.nedap.utils.speced.defn.nilable-specs for JS and similar cases
    • i.e. specific parts of ns's disabled

Basically I have to make utils.spec expect/understand ^string instead of ^String. Info:

@vemv vemv changed the title ClojureScript compatibility ClojureScript compatibility + Primitive type hint support + spec->hint inference May 21, 2019
@vemv
Copy link
Contributor Author

vemv commented May 21, 2019

(will re-open in its final form tomorrow)

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

Successfully merging this pull request may close these issues.

2 participants