Add full support for maps

[maxnordlund]

This is an effort to fix #289 and related issues. Since I've never hacked on PropEr itself I'm not sure if I got it right, and we can always fall back on the ?LET solution, but I figured it would be good to have maps as a first class citizen.

Edit: I removed mentions of the other implementation since ?CONTAINER is the way to go.

[kostis]

Thanks for the PR! It's clearly something that PropEr needs.

FYI, I've also started working on this, back in April/May or so, also following the ?CONTAINER approach, but soon hit a wall in some of my tests and then I needed to do other stuff, so I've put it on the side. I'll try to dig up that work and see how I can merge it here.

One thing that currently bothers me (big time, actually) on this PR is that it contains no tests, even though the issues that it references do contain some. I think this is top priority.

[kostis]

@maxnordlund Rebasing this PR on the current master should now get past the point that GitHub actions fail when generating documentation.

[maxnordlund]

Thanks for the PR! It's clearly something that PropEr needs.

❤️

FYI, I've also started working on this, back in April/May or so, also following the ?CONTAINER approach, but soon hit a wall in some of my tests and then I needed to do other stuff, so I've put it on the side. I'll try to dig up that work and see how I can merge it here.

That's good; means I was on the right track.

One thing that currently bothers me (big time, actually) on this PR is that it contains no tests, even though the issues that it references do contain some. I think this is top priority.

I was wonder about that. Can I use PropEr to test PropEr itself, or do you use something else? Or just plain ol' EUnit? I also wanted to validate the basic design before spending too much time writing tests.

[maxnordlund]

I've hacked a bit more on it and hit a major snag. The shrinking framework assumes get_indices works the same for both types and instances. But in the case of maps that doesn't hold because we want to allow proper types as keys.

I'll take another stab at it hopefully tomorrow, and try to teach the shrinkers about keys and not just indices.

---

A small aside, I found it very confusing how you're supposed to test stuff. I guess I'm used to having EUnit tests at the bottom of the file, but after I found proper_test, I still have a hard time reading it. I just spliced in a couple of examples after fixed_list and it generated some failures, so I assume that's what one needs to do.

[maxnordlund]

@kostis do you have any ideas of how to go forward with this PR?

I don't want to have to rewrite too much of the shrinking if I can help it. Maybe have separate strategies for maps instead of trying to shoehorn them into the existing shrinkers.

[maxnordlund]

I decided to try the custom shrinker approach and now the tests pass, which is promising. Fixed maps never shrinks their keys, normal maps do (both change and remove keys). Both types shrink their values.

[maxnordlund]

@kostis I rebased on latest master (and fixuped a couple of commits) and this apparently made it so that you must approve the workflow again.

[maxnordlund]

So I kept hacking on it yesterday, but now I'm stuck and need your help. You can see what I've done in the two commits, and if you run the tests in ab18886 you get this weird error where it can't find the type map/6, but how/why did it even try such a high arity?

The last commit gets a different error, but I can't figure out why. Also, the code in proper_typeserver is really hard to understand. What/how does rec work? Why is every argument prefixed with a boolean? Etc.

Anyway, could you please take a look and see if you can figure it out?

[maxnordlund]

Ping @kostis

[asabil]

Any update on this?