Warn on use of unsafe decodeUtf8

[michaelpj]

decodeUtf8 throws an exception, this is quite unexpected. There has been various discussion about this over time:

• decodeUtf8 / decodeUtf8' situation haskell/text#247
• https://github.com/NorfairKing/haskell-dangerous-functions#datatextencodingdecodeutf8
• https://www.fpcomplete.com/haskell/tutorial/string-types/

However, I think many people are unaware that this is partial - it doesn't look partial. So a hint is helpful.

I'd like some feedback, though: it's not clear to me what the RHS of such a rule should be. In particular, I don't know whether it's expected to typecheck. The "simplest" rule is what I've written here: replace decodeUtf8 with decodeUtf8', but the latter has a different type. I could instead make the RHS something like \bss -> case decodeUtf8' bs of { Left e -> error "fixme"; Right t -> t }?

[ndmitchell]

Thanks for the patch. As a general rule, the RHS should be equivalent to the RHS - in this case fromRight undefined . decodeUtf8' might be a reasonable RHS, since it's equivalent and more obviously partial.

However, if decodeUtf8 is manifestly wrong and people shouldn't use it, why not have the upstream text library deprecate it? It seems that the authors of the text library should really be making the call about which functions shouldn't be used, and if they make that call, there are better ways for them to enforce it. Given the ownership of the text library has changed several times since the linked ticket, perhaps reraise it there first?

[michaelpj]

That's fair, a deprecation would be better. I'll raise it again, but I'm not hugely hopeful.

I guess I am effectively asking for a back-door deprecation via hlint. It's fair enough if you'd rather leave that to upstream, but I do think it would be a service to the ecosystem to start nudging people to avoid it.

(This was prompted by me discovering a use of this in an upstream library that rendered a nominally-pure parsing function impure - and thus vulnerable to malicious input. It's quite a nasty problem!)

[michaelpj]

Another thing worth mentioning is that even if it gets deprecated in text, that won't reach users until they upgrade to the newest version of text, whereas hlint lets us "backport" the recommendation.

[ndmitchell]

If you get upstream agreement, even to mark it with a documentation comment saying "don't use this", that's enough to make it clear that HLint should warn against it. Happy to use HLint to backport these suggestions.

[ndmitchell]

The text maintainers seem to be active once more, with new releases, so you might have more chance of getting a response from them now.