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

Require generator for :gen/fmap #1120

Merged
merged 3 commits into from
Nov 23, 2024

Conversation

frenchy64
Copy link
Collaborator

@frenchy64 frenchy64 commented Nov 13, 2024

:gen/fmap is fed nil if its schema cannot create a generator. This does not seem like desirable semantics because silent failures could happen if the :gen/fmap function is permissive, like first or a keyword, making the entire generator return nils.

Could we reconsider this? Perhaps :gen/fmap predated :gen/return and :gen/elements, but we can certainly now force the user to be explicit here. Importantly, is this worth the risk breaking things by fixing it?

Also includes a fix for the CLJS version of alphanumeric-char? in the tests.

@ikitommi
Copy link
Member

I think this is a good idea.

@ikitommi
Copy link
Member

Perhaps :gen/fmap predated :gen/return and :gen/elements

it is, but I as the orginal test showed, I think this was just not thought of. clojure.test.check.generators/fmap always requires a generator, so malli should too.

@ikitommi
Copy link
Member

thanks!

@ikitommi ikitommi merged commit c715a85 into metosin:master Nov 23, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants