Fork me on GitHub
#clojure-spec
<
2017-03-23
>
danbunea12:03:49

Hi, I am trying to do a simple function check using spec in ClojureScript (:require [cljs.spec :as s] [cljs.spec.test :as test]... (defn my-index-of "Returns the index at which search appears in source" [source search] (clojure.string/index-of source search)) (s/fdef my-index-of :args (s/cat :source string? :search string?) :ret nat-int? :fn #(<= (:ret %) (-> % :args :source count))) and (test/check 'my-index-of) instead of returning a problem , always returns [] meaning basically that it doesn't do anything?

danbunea12:03:00

I am basically trying this in ClojureScript https://www.youtube.com/watch?v=W6crrbF7s2s

thheller12:03:55

so this is giving me a headache, I can't figure out why this doesn't work

thheller12:03:20

I wrote a spec impl for (map-spec :req {:foo string?})

thheller12:03:36

it works just fine if I use it directly in explain or conform

thheller12:03:50

(s/def ::y (map-spec :req {:foo string?}))

(s/explain ::y {:bar 1})

thheller12:03:17

this however fails with CompilerException java.lang.Exception: Unable to resolve spec: :user/y, ...

thheller12:03:12

it works everywhere else but I can't def it?

thheller12:03:28

found it, apparently you can't use deftype to create a spec impl, defrecord works

thheller12:03:35

(deftype MySpec []
  s/Spec
  (conform* [_ x]
    ::s/invalid))

(s/def ::x (MySpec.))

(s/explain ::x :foo)

thheller12:03:23

@alexmiller is that a bug? I suspect the problem is in with-name which is called in def-impl. deftype isn't IObj, defrecord is, as is reify which is used everywhere in spec

yonatanel12:03:49

I've added a keyz spec that's like s/keys but with :deny :rest that will allow only keys in :req, :req-un, :opt and :opt-un

yonatanel12:03:43

It's currently just a hack and doesn't have nice explain data. That will require implementing Spec just for the deny part.

thheller12:03:50

@yonatanel was that intended for me?

yonatanel12:03:13

@thheller No, but now that I saw what you wrote, are we working on the same thing?

thheller12:03:07

hehe had me confused for a second there 🙂

thheller12:03:41

not exactly the same thing, but I thought about adding the do not allow unspecified keys thing

thheller12:03:35

I just have a bunch of old data and otherwise open maps that I can't easily create namespaced keys for

thheller12:03:32

you could just use s/and

yonatanel12:03:35

@thheller Isn't :req-un enough for that?

thheller12:03:03

:req-un means that I add ::foo which I must s/def somewhere

thheller12:03:21

if I have :foo with 2 different meaning that gets annoying

yonatanel12:03:41

Regarding just using s/and I am doing it, combining s/keys and a set predicate on the map keys, but the explain message is crap.

thheller12:03:49

ah right, well if you stick to defrecord and avoid the headache that caused the impl for explain/conform is rather straightforward

yonatanel12:03:52

@thheller Got you. I wanted to do the same thing, so would be nice if you release your inlined map spec in github.

thheller12:03:47

need to figure out unform and the gen stuff

thheller12:03:55

but conform/`explain` work just fine

yonatanel13:03:18

@thheller spec-tools have something similar that you might find interesting: https://github.com/metosin/spec-tools

thheller13:03:46

ah didn't know about that, thx

yonatanel13:03:01

I like how you implemented explain unlike spec which creates a contains? pred for each key.

thheller13:03:52

yeah dunno map-spec-impl looked way too complicated for what I want

thheller13:03:26

started out by using it but made things way too complicated

hospadar13:03:20

funny that ya'll are talking about this right now

hospadar13:03:30

I am implementing something very similar as we speak

hospadar13:03:04

I have to validate a datastructure that already exists in some systems that's a) not using namespaced keys b) not always using keywords as the keys for maps

hospadar13:03:00

and sometimes the value types of the (non-namespaced) keys are conditional on something else

hospadar13:03:15

i.e.

{:job-type :simple 
 :output "is a string}
{:job-type :complicated
 :output {:is "a map of something}}

thheller13:03:16

@yonatanel added the closed version

thheller13:03:26

(s/def ::y (map-spec :req {:foo string?} :closed? true))

(s/explain ::y {:foo "1" :x 1})

thheller13:03:39

val: {:foo "1", :x 1} fails spec: :shadow.spec/y predicate: (not (contains? % :x))

thheller13:03:34

might not be the best idea but I want to use that to validate react component props, it might actually be better to use closed maps there

yonatanel14:03:14

@hospadar Are you using multimethods for that?

yonatanel14:03:31

@thheller Do you think the closed map explain data should show all the allowed keys instead of just the illegal one from the input?

hospadar14:03:24

@yonatanel I tried using a multi spec that points and keys specs in different namespaces but that seemed to not work the way I needed

hospadar15:03:17

^ that basically does what I want

hospadar15:03:24

but I didn't like the namespace hopping

hospadar15:03:50

ohhh, I guess I could probably s/def with namespaced keywords instead of using :: to autoresolve?

hospadar15:03:34

learning all the time 🙂

hospadar15:03:32

ohhhh yeah that's a lot nicer

thheller15:03:47

@yonatanel not sure about the explain data, good error messages are hard. could put anything in there really

spieden16:03:35

@hospadar i think that’s typical usage as specs are meant to augment a separate namespace that contains the keywords

spieden16:03:49

although i do use them inline sometimes too

anmonteiro17:03:09

how does clojure.spec/instrument play with direct linking?

anmonteiro17:03:32

wondering if someone has tried that before I go and play with it

Alex Miller (Clojure team)17:03:48

it won’t do anything

Alex Miller (Clojure team)17:03:00

because direct linking doesn’t use vars

anmonteiro17:03:46

@alexmiller got me a little confused there. by “won’t do anything” do you mean it won’t instrument or it won’t be impacted by direct linking?

Alex Miller (Clojure team)17:03:50

I mean it won’t instrument

Alex Miller (Clojure team)17:03:15

or rather it will instrument, but direct linked calls don’t use either the original var or the instrumented var, so instrumentation will have no effect on those call sites

Alex Miller (Clojure team)17:03:42

new, non direct linked invocations will use the instrumented vars

Alex Miller (Clojure team)17:03:00

@thheller re your question yesterday, there is an implicit assumption in the code that spec impls also implement IObj. so in one sense, I’d say your example is at fault of that. And in another sense that spec code could fall through to throwing an error in that case to better let you know that.

mpenet18:03:58

Direct linking in dev makes little sense anyway doesnt it?

mpenet18:03:47

(since instrument is also dev time)

seancorfield18:03:48

Agreed. We only enable direct linking on QA/prod. It gets in the way of REPL-driven development.

thheller18:03:12

@alexmiller didn't know about the implicit assumption about IObj, took me about 2h to figure that out. an error would have helped a lot.

Alex Miller (Clojure team)18:03:20

@mpenet right, there are no plans to change wrt direct linking

anmonteiro19:03:31

@mpenet @seancorfield we have some functions for which we want to turn instrumentation on in prod, hence my question

anmonteiro19:03:35

we don’t have direct linking in dev

mpenet19:03:21

Dont do that. Instrument can/will trigger gen. Use pre/post

anmonteiro19:03:36

that would be my next question

anmonteiro19:03:53

we’re currently using Plumatic Schema

anmonteiro19:03:08

and we have (s/set-fn-validation! true) for the namespace

anmonteiro19:03:20

apparently instrument won’t do what I expect it to

seancorfield20:03:04

I would say that if you want the checks to always be performed (i.e., in production) then explicitly code that into the functions themselves — that gives you more control over the error handling anyway.

seancorfield20:03:29

As @mpenet notes, even if you did instrument functions in production, any higher-order functions that get instrumented would trigger generative testing, and instrumentation doesn’t check function returns anyway, just arguments.

Oliver George22:03:52

@anmonteiro would s/assert be a better choice for production code?

anmonteiro22:03:12

hrm I thought s/assert‘s point was to be turned off in Prod

anmonteiro22:03:35

Currently I’m thinking about going with a combination of s/valid? and s/explain that we send to Sentry

Oliver George23:03:44

Sounds sensible.