Fantasy-land: proposal: remove laws directory from repository

[davidchambers]

Earlier today I created #170 and #172, but I now realize that many of the property tests are dysfunctional. To provide one more example, the test for semigroup associativity seeks to assert:

(a `concat` b) `concat` c = a `concat` (b `concat` c)

Since it uses the same value for a, b, and c, it actually asserts:

(a `concat` a) `concat` a = a `concat` (a `concat` a)

If a is Identity('x') and both sides evaluate to Identity('xxx'), we haven't proven the fantasy-land/concat method of our Identity type to be associative, have we?

The functions provided in fantasy-land/laws are not very useful. To _actually_ test many of these properties we can't use a universally applicable function such as identity :: a -> a. Instead, we must use functions specialized to the data types being tested. Choosing these functions requires a great deal of care, and _this_ is the thing which makes testing these properties challenging, not copying a line from the spec and adding the fantasy-land/ prefixes where appropriate.

[SimonRichardson]

fyi: So the issue arose when removing the laws from their original tests and making the laws more generalised. This obviously isn't ideal and not fully baked, but I do think we should provide a solution to how do you test the spec. This might not be the way, but we should have an avenue for discussing it.

Easy: Provide more arguments to allow to test more values (like the original tests when they where in fantasy-check).

[davidchambers]

Easy: Provide more arguments to allow to test more values (like the original tests when they where in fantasy-check).

Let's consider a concrete example: asserting associativity of Maybe#fantasy-land/concat.

Without fantasy-land/laws:

const fl = require('fantasy-land');
const jsc = require('jsverify');

const equals = ...;

//    JustArb :: Arbitrary a -> Arbitrary (Maybe a)
const JustArb = arb => arb.smap(S.Just, m => m.value, R.toString);

//    MaybeArb :: Arbitrary a -> Arbitrary (Maybe a)
const MaybeArb = arb => jsc.oneof(JustArb(arb), jsc.constant(S.Nothing));

describe('Maybe', () => {

  describe('Semigroup', () => {

    it('satisfies associativity', () => {
      jsc.assert(jsc.forall(MaybeArb(jsc.string), MaybeArb(jsc.string), MaybeArb(jsc.string), (a, b, c) => {
        const lhs = a[fl.concat](b)[fl.concat](c);
        const rhs = a[fl.concat](b[fl.concat](c));
        return equals(lhs, rhs);
      }));
    });

  });

});

There's a lot of preparation required, but this is the important part:

const lhs = a[fl.concat](b)[fl.concat](c);
const rhs = a[fl.concat](b[fl.concat](c));
return equals(lhs, rhs);

How might this look with fantasy-land/laws?

return require('fantasy-land/laws/semigroup').associativity(equals, a, b, c);

This doesn't strike me as a big improvement. I actually prefer the lhs/rhs approach because I can see the equivalence we're hoping to prove, and if my test fails I can simply console.log(lhs, rhs); whereas with fantasy-land/laws I'd need to add logging to a third-party module.

[SimonRichardson]

The idea for the laws was to provide the laws in a easy to reference function setup, so you didn't have to keep remembering what they are. This also prevents the issue of subtle bugs, which is how we've ended up in the left/right identity issue.

I like the lhs, rhs setup as well as it eradicates the equality issue (not everything can be equal as some structures are lazy and require executing in certain ways), by putting that on the implementer.

Something like (just an idea):

const {lhs, rhs} = require('fantasy-land/laws/semigroup').associativity;
return equals(lhs(a,b,c), rhs(a,b,c))

[davidchambers]

I like the idea of the equality check living in the user's test suite, @SimonRichardson. :thumbsup:

[rpominov]

Yea, it's not always possible to provide synchronous equals function, and equality check outside can be asynchronous.

[safareli]

equals could return Promise<Boolean> and in tests just return Promise.All like in ramda-fantasy

Also you as you are passing your equals function you can log in there

return require('fantasy-land/laws/semigroup').associativity(a => b => {
  console.log(a,b);
  return equals(a,b);
}, a, b, c);

or wrap equals with some logArgsAndResult helper which logs arguments and result

update:

I like the lhs, rhs setup as well as it eradicates the equality issue (not everything can be equal as some structures are lazy and require executing in certain ways), by putting that on the implementer.

It's already implementers responsibility to pass function for equality.

[joneshf]

Did we reach a conclusion here?

[davidchambers]

I'm in favour of moving the laws to a separate repository to allow releases for the two projects to be made independently. Currently we're in an awkward position because we can't really follow SemVer for the laws without confusing those who only use the spec.

[SimonRichardson]

I'm in favour of moving the laws to a separate repository to allow releases for the two projects to be made independently

I don't agree with this because of one main point; the laws will be left and never updated with the spec and this will be sad because you'll loose the power of them. I don't take the argument around if you update the spec then you must also update the laws, because it starts off with good intensions, but _always_ fails in the long run.

[rpominov]

The problem is actually other way around: sometimes we have to update laws (fix bugs, make API better, etc.) without changing the spec. In this situation the SemVer semantics of fantasy-land package version number is confusing, because both the spec and laws implementation may have major/minor/patch changes. So the version number have to represent changes of two independent artifacts.

Another reason why separation may be a good idea, is that unfortunately it seems that we have very limited human resources to maintain and improve this project. And we must choose how to best spend these resources. If project would contain only specification document it would require less effort to improve the spec. In other words we'd better have outdated laws in a separate repo plus a more useful and powerful spec, than less powerful spec plus up to date laws in a single repo. Ideally we will have both of course.

[safareli]

We could also have multiple packages in same repo as babel or cyclejs has.

• @fantasy-land/spec
• @fantasy-land/laws

We can even go crazy and put identity, either and others packages in this repo

• @fantasy-land/identity
• @fantasy-land/either ...

I don't have strong opinion on this, just pointing out other option.

[SimonRichardson]

In this situation the SemVer semantics of fantasy-land package version number is confusing, because both the spec and laws implementation may have major/minor/patch changes.

I don't get that argument, it's a non-issue. If laws and specs are to be handled as one then the laws are the specs and vice versa.

is that unfortunately it seems that we have very limited human resources to maintain and improve this project.

So let's take it's too it's full natural end game; nobody updates the laws and people re-implement the laws over and over again, except that we don't have a good set of standard laws (ignoring ours needs updating), because people implement those laws differently and every one has a different bug. Not only is this happening with the spec (see maybe, either etc) I'll wager it'll happen with everything else.

Ideally we will have both of course.

Ideally yes.

---

So this comes down to do you want laws or not. Either someone steps up and "owns" the laws or they don't.

[rpominov]

I actually regret I brought up the "limited human resources" perspective. It's kinda sad to look at things from this angle. Still maybe this is something we should be considering.

So let's take it's too it's full natural end game; nobody updates the laws and people re-implement the laws over and over again

What we're basically doing is we made some particular piece of code related to the spec mandatory to update when spec is updated. The question is why this particular code is more important to keep up to date compared to other pieces (up to all the libraries that compatible with fantasy-land). I'm not saying that it isn't more important, but we have to answer why.

Maybe what @safareli proposing is a good idea. And after we identify criterion by which we decide what should be inside this repo and be always up to date, we should actually move _more_ stuff inside. But currently this criterion is unclear to me. And again we should consider that this would mean more work to maintain all this, so it might end up poorly maintained.

Update: anyway if we're keeping laws in this repository, having it in a separate npm package as @safareli suggests would be definitely an improvement!

[SimonRichardson]

The question is why this particular code is more important to keep up to date compared to other pieces (up to all the libraries that compatible with fantasy-land).

Initially I would hope that people could have used it to verify that there maybe, either implementations would be correct and I'm even wondering if that's even possible?

[davidchambers]

fantasyland/fantasy-laws#1