bitemyapp / esqueleto

New home of Esqueleto, please file issues so we can get things caught up!
BSD 3-Clause "New" or "Revised" License
370 stars 107 forks source link

'SqlExpr' can be unsafely coerced using the safe 'coerce' function #270

Open arthurxavierx opened 3 years ago

arthurxavierx commented 3 years ago

It's now possible to write

let
  fromNull :: SqlExpr (Maybe (Entity e)) -> SqlExpr (Entity e)
  fromNull = coerce

  myNonNullableEntity = fromNull (myNullableEntity :: SqlExpr (Maybe (Entity SomeEntity)))

which could cause unexpected runtime errors.

belevy commented 3 years ago

Is there a way to prevent this outside of the library code? This is a feature in the internal code, we get free coercion.

Basically coerce is not part of our contract so if someone makes a runtime error using it I don't really care.

parsonsmatt commented 3 years ago

Hm.

We could have:

newtype SqlExpr a 
type role SqlExpr nominal

veryUnsafeCoerceSqlExpr :: SqlExpr a -> SqlExpr b
veryUnsafeCoerceSqlExpr = unsafeCoerce

unsafeCoerce is safe in this sense, since we're explicitly blocking the normal coerce behavior that would otherwise be used. Clients of the library need to write the scary name. And we can still use it internally. Is this an acceptable compromise?

belevy commented 3 years ago

that seems fine, we should probably be using veryUnsafeCoerceSqlExpr everywhere anyways though I think we lose the fmap (f . coerce) == coerce . fmap f rewrite rule

parsonsmatt commented 3 years ago

I think an INLINE pragma on veryUnsafeCoerceSqlExpr would work to get the rewrites firing again