fp-ts icon indicating copy to clipboard operation
fp-ts copied to clipboard

fromRefinement

Open waynevanson opened this issue 5 years ago • 16 comments

🚀 Feature request

Just like fromPredicate but only works on refinements and only has one argument. This means we get the type of the second argument for free.

Current Behavior

// We want `E` to be `number` only
//
// (a: string | number) => E.Either<string | number, string>
const result = E.fromPredicate(
  (a: string | number) => typeof a === "string",
  (e) => e
)

Desired Behavior

Currently we need to manually assert the E type:

// We want `E` to be `number` only, but we must specify type in second argument
//
// (a: string | number) => E.Either<number, string>
E.fromPredicate(
  (a: string | number) => typeof a === "string",
  (e) => e as number
)

Suggested Solution

This example is for an Either.

export function fromRefinement<A, B extends A>(
  refinement: Refinement<A, B>
): (a: A) => E.Either<Exclude<A, B>, B> {
  return (a) => (refinement(a) ? E.right(a) : E.left(a as Exclude<A, B>))
}

In example:

// `E` is now a number because it's not a string
//
// (a: string | number) => Either<number, string>
const result = fromRefinement(
  (a: string | number): a is string => typeof a === "string"
)

Who does this impact? Who is this for?

All levels. As a beginner, I was looking for this but could only come up with this:

Describe alternatives you've considered

Additional context

Your environment

Software Version(s)
fp-ts ^2.8.3
TypeScript ^4.0.0

waynevanson avatar Oct 14 '20 10:10 waynevanson

Hey @waynevanson

The overloaded implementation of fromPredicate in fp-ts for a Refinement accepts a third type parameter E. You can avoid the assertion in the onLeft argument by specifying the generic types.

Lets use ReadonlyArray<A> as the type to refine and ReadonlyNonEmptyArray<A> as our refined type. For clarity, I will define the type aliases E, A, and B to show the positions of the generic types.

import * as E from 'fp-ts/Either'

type E = ReadonlyArray<number>
type A = ReadonlyArray<number>
type B = ReadonlyNonEmptyArray<number>

E.fromPredicate<E, A, B>(
  (as): as is B => RA.isNonEmpty(as),
  (a) => a
)

You can also avoid the (a) => a with the identity function from fp-ts/function.

import { identity } from 'fp-ts/function'

E.fromPredicate<E, A, B>((as): as is B => RA.isNonEmpty(as), identity)

However, I certainly understand wanting to avoid all of this ceremony with the addition of a fromRefinement constructor, so I am happy to continue the discussion!

IMax153 avatar Oct 14 '20 20:10 IMax153

Thanks for pointing out alternatives.

So this would be added to any typeclass that is Filterable.

Shall I write a PR?

waynevanson avatar Oct 15 '20 02:10 waynevanson

Does A = B + Exclude<A, B> hold when B <= A?

The suggested solution relies on this property I guess.

However the type system doesn't agree, i.e. you need a a as Exclude<A, B> cast.

gcanti avatar Oct 15 '20 07:10 gcanti

@gcanti When using ternary operators and switch statements, Typescript automatically infers what the remaining types are:

declare const a: string | number

const A = typeof a === "string" // operation asserts `a is string`
  ? a // always a string
  : a // always Exclude<typeof a, string>, in this case a number.

declare const b: object |  Array<any>

const B = Array.isArray(b) // operation asserts `b is any[]`
  ? b // always a string
  : b // always Exclude<typeof b, Array<any>>, in this case an object.

declare const c: object |  Array<any>

const C = typeof c === "object" // operation asserts Extract<typeof c, object>
  ? c // always object |  Array<any>
  : c // always Exclude<typeof b, object>, in this case `never`.

This means that the type system DOES agree, but it doesn't implement it. This technique can be used to do what is already possible in the language, but apply it to our functions.

Does A = B + Exclude<A, B> hold when B <= A?

The suggested solution relies on this property I guess.

Does this mean "B is a subtype of A"? The signature for a refinement is always B extends A, which I believe means this holds true.

interface RefinementA, B extends A>{
  (a: A): a is B
}

I also saw something somewhere in a library of yours mentioning how fromRefinement in io-ts* is unsafe, but not sure where that issue is. Whatever viewpoint was in that issue was not considered for this proposal.

waynevanson avatar Oct 15 '20 23:10 waynevanson

Sorry for the close.

Just stumbled about the problem myself. Is there any counter example why a fromRefinement should not work as we all thing? Could this be simply a typescript bug?

mlegenhausen avatar Mar 11 '21 08:03 mlegenhausen

Is there any counter example

@mlegenhausen I haven't found one so far

gcanti avatar Mar 11 '21 11:03 gcanti

btw there's no need for a new API

import { Either } from 'fp-ts/Either'
import { identity, Predicate, Refinement } from 'fp-ts/function'

declare const fromPredicate: {
  <E, A, B extends A>(refinement: Refinement<A, B>, onFalse: (a: Exclude<A, B>) => E): (
    a: A
  ) => Either<E, B>
  <E, A>(predicate: Predicate<A>, onFalse: (a: A) => E): (a: A) => Either<E, A>
}

// const f: (a: string | number) => Either<number, string>
const f = fromPredicate((sn: string | number): sn is string => typeof sn === 'string', identity)

gcanti avatar Mar 11 '21 11:03 gcanti

Also, the same trick could be applied to any API using a Refinement I guess:

import { Either, getFilterable } from 'fp-ts/Either'
import { Predicate, Refinement } from 'fp-ts/function'
import { Separated } from 'fp-ts/Separated'
import * as S from 'fp-ts/string'

declare const e: Either<string, string | number>

// const x: Separated<Either<string, string | number>, Either<string, string>>
const x = getFilterable(S.Monoid).partition(
  e,
  (sn: string | number): sn is string => typeof sn === 'string'
)

declare const partition2: {
  <A, B extends A>(fa: Either<string, A>, refinement: Refinement<A, B>): Separated<
    Either<string, Exclude<A, B>>,
    Either<string, B>
  >
  <A>(fa: Either<string, A>, predicate: Predicate<A>): Separated<
    Either<string, A>,
    Either<string, A>
  >
}

//            here's the difference ---v
// const y: Separated<Either<string, number>, Either<string, string>>
const y = partition2(e, (sn: string | number): sn is string => typeof sn === 'string')

gcanti avatar Mar 11 '21 13:03 gcanti

Sounds good to me!

Id like to see this in the pipe able module until v3 is released also.

waynevanson avatar Mar 11 '21 22:03 waynevanson

Here's an example where this can cause unexpected behavior

Which is not to say that fromRefinement couldn't be useful - just that maybe it should live separately from the existing fromPredicate function, in case this behavior isn't desired

(Edit: Never mind - the problem isn't unique to fromRefinement. Updated the example to show similar wonkiness w/ a simple if statement)

anthonyjoeseph avatar May 14 '21 18:05 anthonyjoeseph

@anthonyjoeseph nice example. The problem seems that your Refinement only tells half of the truth. The correct type should need to be the following to get the desired error:

const isBlueCar = (v: Vehicle): v is (Car & { color: 'blue' }) => v._type === 'car' && v.color === 'blue'

mlegenhausen avatar May 17 '21 05:05 mlegenhausen

Agreed - good catch! Never mind, I take it all back - I like fromPredicate!

anthonyjoeseph avatar May 17 '21 05:05 anthonyjoeseph

@waynevanson do you still want to write a PR for this for version 2.12? It would be very helpful for me personally - I could help out/do it myself if it's too big of a lift (or ofc I'll hold off if it's not desired)

anthonyjoeseph avatar May 22 '21 14:05 anthonyjoeseph

Feel free to write it if you wish :)

waynevanson avatar May 24 '21 04:05 waynevanson

I'm running into a small type inference bug w/ the above definition re: Predicate (non-Refinement) functions - playground

Does anyone have an idea why this is happening? Also, is this bug prohibitive? I'm not sure if it's bad enough to prevent this from going thru (I've got a working branch otherwise)

anthonyjoeseph avatar May 28 '21 04:05 anthonyjoeseph

I think I figured it out - w/o type hints, the onFalse argument is inferred as Exclude<A, A>, aka never

Here's a version w/ more robust inference:

declare const fromPredicate: <E, A, F extends (a: A) => boolean>(
  refinement: F,
  onFalse: (a: F extends (a: any) => a is infer B ? Exclude<A, B> : A) => E
) => (a: A) => 
  E.Either<E, F extends (a: any) => a is infer B ? B : A>

I'm hoping it's alright to mess w/ the type parameters since this is a breaking change anyway

Here's a playground that stress-tests it

Even though this is listed as a milestone for 2.12, imo this ought to go into 3.0.0 first just in case there are still kinks to work out.

Does this look OK?

anthonyjoeseph avatar May 29 '21 16:05 anthonyjoeseph