Remove `StringConv` instance for `ByteString`

[seagreen]

I apologize for suggesting such a big change to the library, but I think this would be a huge improvement.

Lazy Text, Text, and String are all fundamentally Unicode. Keeping six different conversion functions between them is a pain, and being able to use a single function for this is really nice. string-conv fills this role really well.

ByteString and lazy ByteString don't really fit in here though. I'd like to use string-conv, but as long as there's a StringConv instance for ByteString I'm afraid to, because I risk silently overwriting data. For every use case I've ever run into I don't want ByteString -> Text, I want ByteString -> Maybe Text or some other function that admits the possibility of failure.**

** I realize this doesn't represent everybody -- I'm sure there are people dealing with more ephemeral data who like the convenience of having a ByteString -> Text function. Perhaps there could be two classes, one that includes messy conversions and one that does not?

[MichaelXavier]

Not sure what the answer is here but something I wanted to point out: string-conv has the concept of strict and lenient decoding (strict is overloaded, I realize, but it looks like we borrowed naming from the text library). toS defaults to strict. This will throw a UnicodeException when conversion goes wrong. This kind of sucks because it would effectively be a pure exception. I think it would be easy for a library to export an exception-safe conversion function. A Maybe is a bit questionable as it loses error information like which character caused the issue. However, if we wanted an Either based interface we'd have to reconcile that with the fact that the typeclass is open for extension and any type we provided would need to be able to capture all possible errors. Perhaps a file alternative is to use an associated datatype with the instance, so type ConversionError ByteString Text = UnicodeException and that would give us a function toSSafe :: (StringConv a b) => a -> Either (ConversionError a b) b. You may have to fold that into the typeclass too as not every conversion path will use exceptions, so you may have a default implementation for toSSafe :: (StringConv a b, Exception (ConversionError a b) => a -> Either (ConversionError a b) b

[MichaelXavier]

Another small idea to ponder. You could use 2 typeclasses, one that guarantees conversions are always safe and another subclass of that that cannot make that guarantee but provides an exception-safe interface. That way people doing conversions between Text and String needn't pollute their program with lots of pointless error handling.

[seagreen]

Agreed that making people doing Text to String conversions handle errors that will never happen isn't a good option.

I think the one thing I'm sure about is that there should be a typeclass for handling conversions that never fail.

Other than that I'm not sure. Should that typeclass handle conversions from Text to ByteString? Those will never fail, but it would also mean continuing to endorse one implementation (UTF-8) even though there could be others. This seems like a matter of taste.

And should there be a second typeclass that handles conversions that can fail? IMO this is also a matter of taste because it suffers from the previous multiple implementations problem (UTF-8/UTF-16 etc.) and then additionally will also have a more complicated type signature. Perhaps this kind of decoding should just be done explicitly? But I don't know how convenient people might find it to be able to use a typeclass here, so I'm not sure.

[PiDelport]

Adding another voice of support: we use this library in projects, but the unsafe / unchecked UTF-8 decoding is a bit of a foot-gun.

It would be really nice to be able to import a variant of toS with safe conversions only: perhaps a Data.String.Conv.Safe module?

Another approach might be to use toS only for conversion between Unicode string types, with encodeS and decodeS for conversions to and from byte string types.

Another approach might be to take inspiration from https://hackage.haskell.org/package/witch, which separates its conversions into Rust-style From and TryFrom classes.

[mightybyte]

I understand the complaint here, as I have also been bitten by unsafe UTF8 decoding in the past. But I also think it's useful for the most common ByteString instances / conversions to exist somewhere if for no other reason than as a kind of documentation. Perhaps this and the backwards compatibility concerns can be addressed by moving the offending instances into a self-contained module, and maybe also that module could include safe conversion functions as well? Then people relying on the existing behavior would have a relatively simple upgrade path of adding one additional import in the places where they rely on the ByteString instances.

[MichaelXavier]

@mightybyte I could see that being a bit confusing if the module just exports instances just by way of how typeclasses get introduced basically if anybody imports them. I could see a scenario where the offending instances get imported by some library I'm using and then I import the safe versions and there's an overlap. Or maybe I'm not understanding the suggestion well enough.

I think there are 2 main use cases and that we should support both: 1) best-effort conversions that will use the unicode replacement character whatever rather than throw a pure exception, 2) cautious conversions that return a Left on error.

I'd like to avoid breaking changes if at all possible but I'm not sure what the best way is to do that.