fast-check icon indicating copy to clipboard operation
fast-check copied to clipboard

Feedback on `oneof`

Open gcanti opened this issue 5 years ago • 3 comments

💡 Idea

Currently oneof;

  1. forces the user to add a type annotation at the call site
  2. doesn't statically enforce at least one arb in input

Repros:

import * as fc from 'fast-check'

fc.oneof(fc.string(), fc.boolean()) // error
fc.oneof<string | boolean>(fc.string(), fc.boolean()) // ok
import * as fc from 'fast-check'

fc.oneof() // no error
fc.oneof(...[]) // no error

Not sure whether you may be interested (type level tricks are nice when they work, however they could add some maintenance burden) but here's a possible solution:

import * as fc from 'fast-check'

export declare function oneof<A extends [unknown, ...Array<unknown>]>(
  ...arbs: { [K in keyof A]: fc.Arbitrary<A[K]> }
): fc.Arbitrary<A[number]>

// 1)

// const arb: fc.Arbitrary<string | boolean>
const arb = oneof(fc.string(), fc.boolean()) // ok

// 2)

oneof() // error
oneof(...[]) // error

gcanti avatar Feb 01 '20 12:02 gcanti

Having better typings for oneof would definitely be a great thing.

The fact to avoid users to explicitly type <string, number> on fc.oneof(fc.nat(), fc. string()) would be pretty cool.

If you want to I can let you open a PR for that purpose. Otherwise I can handle it in the future.


Concerning the idea of forcing at least one parameter at call site, it was originally the case but the constraint has been removed by https://github.com/dubzzz/fast-check/commit/1833a4b1576a4ba122146052faf9bbc987de3e1f

I'll need to dig a little bit more to understand why. But I believe that it caused issues when combined with other arbitraries or chain:

fc.set(fc.uuid(), 1, 10)
  .chain(allUuids => fc.record({
    oneRecord: fc.oneof(...allUuids),
    records: fc.constant(allUuids),
  }))

Enforcing at least one parameter with typings might break existing code (that is working today) 🙄

dubzzz avatar Feb 03 '20 05:02 dubzzz

Enforcing at least one parameter with typings might break existing code (that is working today)

Agree, technically is a breaking change

gcanti avatar Feb 28 '20 06:02 gcanti

As I'm planning a next major of fast-check (mostly to add a full support for esm), I'll maybe take advantage of it to include your request into it 👍

dubzzz avatar Jun 23 '20 21:06 dubzzz

I believe this issue has been resolved, right? I've had no problems w/ needing to specify .oneof generic types manually nowadays 👍

jasikpark avatar Sep 09 '22 17:09 jasikpark

It should not be that far indeed 🙂 But I think we still support the empty case from a typing point of view. If so it should not be that hard to forbid and finally close this old issue.

dubzzz avatar Sep 11 '22 07:09 dubzzz

woot

jasikpark avatar Sep 12 '22 18:09 jasikpark