elm-lang / elm-package

Command line tool to share Elm libraries
BSD 3-Clause "New" or "Revised" License
213 stars 66 forks source link

Certain record type signatures changes should be a minor update. #256

Closed AnthonyJacob closed 7 years ago

AnthonyJacob commented 7 years ago

Currently if you change foo : {a | x : Int} to foo : {a | x : Int, y : Int} it will be considered as major change, because you changed the type signature. Or did you?

Generic record signatures ( {a | ... } ) are weak toward field deletion! That means you can change {x = 2} to {} anywhere just by changing its signature. That is significant for library authors, because they can make non-breaking changes, even though they can look like breaking one.

For whatever reason {a | } is not valid type...

Few examples:

{a | x:Int} ==> {b | x:Int, y:Int} {a | x:Int, y:Int} =/=> {b | x:Int}

{a | x:Int, y:Int} -> _ ==> {b | x:Int} -> _ {a | x:Int} -> _ =/=> {b | x:Int, y:Int} -> _

({a | x:Int} -> {a | x:Int}) -> _ ==> ({b | x:Int, y:Int} -> {b | x:Int, y:Int}) -> _ ({a | x:Int, y:Int} -> {a | x:Int, y:Int}) -> _ =/=> ({b | x:Int} -> {b | x:Int}) -> _

Explanation:

==> first signature implies the second one (side effect : changing first to second won't break anybody's code) =/=> first signature does not imply the second one (side effect : changing first to second will break the users code)

You can think of it as type alias that works only in one way.

process-bot commented 7 years ago

Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

JoeyEremondi commented 7 years ago

I think Co and contravariance will show up here. Whether it's safe to add or delete a field depends on which side of the function arrow the record is.

On Dec 10, 2016 11:29 PM, "Process Bot" notifications@github.com wrote:

Thanks for the issue! Make sure it satisfies this checklist https://github.com/process-bot/contribution-checklist/blob/master/issues.md. My human colleagues will appreciate it!

Here is what to expect next https://github.com/process-bot/contribution-checklist/blob/master/expectations.md, and if anyone wants to comment, keep these things https://github.com/process-bot/contribution-checklist/blob/master/participation.md in mind.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/elm-lang/elm-package/issues/256#issuecomment-266267860, or mute the thread https://github.com/notifications/unsubscribe-auth/ACzn0owR-8fN1I4UJ4xt1ZYj6xEY1D7lks5rG6ZRgaJpZM4LJ3LR .

jvoigtlaender commented 7 years ago

Actually, the assertions in the original comment here are wrong. It's not true that by changing foo : {} to foo : {x : Int} you won't break anybody's code.

If somebody has this in their code:

import Package exposing (foo)

f : {} -> Int
f _ = 42

e = f foo

then as long as Package has foo : {}, that somebody's code will compile, but as soon as Package switches to foo : {x : Int}, that somebody's code will not compile anymore.

@AnthonyJacob, I think you have a wrong understanding of how record type annotations work.

AnthonyJacob commented 7 years ago

@jvoigtlaender Oops, seems like I have overlooked something.

Never mind though, it is still possible if you use{a | ...} instead.

jvoigtlaender commented 7 years ago

Your examples are still wrong, though. Because you use the same type variable (a) on both sides. Your assertion about what ==> means in those cases is simply not true.

I suggest you close the issue here and instead start a topic on the mailing list to come to a full understanding and proposal.

AnthonyJacob commented 7 years ago

I didn't expect somebody is going to read it as forall a. ... ==> ... instead of forall a. ... ==> forall a. .... Never mind.

What mailing list do you have on mind?

jvoigtlaender commented 7 years ago

The thing you dismiss as "Never mind" is not the real problem with your examples I meant.

As for discussion forum, see http://elm-lang.org/community. (And what process-bot's links have already pointed out about what the role of this issue tracker here is and is not.)