informatikr / hedis

A Redis client library for Haskell.
http://hackage.haskell.org/package/hedis
BSD 3-Clause "New" or "Revised" License
327 stars 121 forks source link

Make SREM safer using `Data.List.NonEmpty` #175

Closed ivanbakel closed 1 year ago

ivanbakel commented 2 years ago

We had an error recently at runtime from passing srem the empty list []. The members argument of SREM is actually non-optional - at least one value must be supplied. So it makes sense to generate a signature which uses a type that would catch the use of the empty list at compile-time, like the type in Data.List.NonEmpty.

This modifies the codegen to generate signatures and code using NonEmpty for every multiple argument of a command which isn't optional. Whether or not this is correct is unclear to me - this forbids some valid commands, like SCRIPT EXISTS with zero arguments. It's probable that there's some mistakes in the commands.json that need to be corrected so that this codegen can be more discerning.

This also adds some changes into the codegen that were introduced by #157 - that PR manually modified the file produced by the codegen in a way that meant the changes were lost when re-generating.

k-bx commented 2 years ago

Please see https://github.com/informatikr/hedis/issues/184 if interested

qnikst commented 1 year ago

Hello @ivanbakel thanks for your work!

It seems that we will have to make an API breaking release anyway so it's better accumulate good API changes. However it's not possible to merge this PR in it's current form, as it removes and changes a lot of code in the Commands.hs and removes a lot of functionality.

If you are still interested can you please make another version that will have only relevant changes, I would love to merge it! Or feel free to tell if you are no longer interested in this work.

ivanbakel commented 1 year ago

Unfortunately I'm no longer in a position to maintain this kind of change, since I don't have access to a large codebase using Hedis.

qnikst commented 1 year ago

Ok, not a problem! Thanks a lot anyway