purescript / purescript-record

Functions for working with records and polymorphic labels
BSD 3-Clause "New" or "Revised" License
70 stars 31 forks source link

Builder.merge could be "safer" #38

Closed safareli closed 6 years ago

safareli commented 6 years ago

At this moment this program is compiled with any issues

module Main where

import Data.Record.Builder as Builder

union :: forall r3 r1 r2. Union r1 r2 r3 => { | r1 } -> { | r2 } -> { | r3 }
union r1 r2 = Builder.build (Builder.merge r2) r1

y :: {a :: Int, a :: String }
y = {a: 1 } `union` {a: "foo" }

z :: {a :: Int, a :: Int }
z = {a: 1 } `union` {a: 1 }

Which I guess should be rejected.

changing type of union fixes the issue

import Type.Row (class RowListNub, class RowToList)

union
 :: forall r1 r2 r3 r3l
  . Union r1 r2 r3
 => RowToList r3 r3l
 => RowListNub r3l r3l
 => { | r1 }
 -> { | r2 }
 -> { | r3 }
union r1 r2 = Builder.build (Builder.merge r2) r1

I think adding similar constraints to Builder.merge will make it much safer.

If we add such constraints error is not that informative, this is what you would get for:

{a: 1 } `union` {a: "foo" }

error: Could not match type Nil with type Cons "a" String Nil

With special class (for example SafeMerge) + Fail constraint the error could be made more useful.

/cc @MonoidMusician @justinwoo

garyb commented 6 years ago

Resolved in the 0.12 branch