frasertweedale / hs-jose

Haskell JOSE and JWT library
http://hackage.haskell.org/package/jose
Apache License 2.0
122 stars 46 forks source link

Document or re-name protection indicators #125

Open ericpashman opened 5 months ago

ericpashman commented 5 months ago

This is a suggestion for improving the discoverability and ergonomics of the library.

I was initially confused by the usage of the ProtectionIndicator class, mainly due to its name and the name of its members. My first thought was that a type parameter p :: ProtectionIndicator p => p indicates whether a header is protected or unprotected. Apparently that's wrong: it indicates whether the header is required to be protected (p :: ()) versus whether protection is optional (p :: Protection).

This is not very clearly documented, in my opinion. Better than merely documenting it, I think, would be to re-name the class and its members to more clearly indicate their usage:

class ProtectionOptionality p where
  ...

data ProtectionOptional = Protected | Unprotected

instance ProtectionOptionality ProtectionOptional where
  ...

data ProtectionRequired = ProtectionRequired

instance ProtectionOptionality ProtectionRequired where
  ...

That would be a breaking change. A middle ground that would preserve compatibility while still improving things meaningfully, I think, might be just to add another class member that could be used in preference to ().

data ProtectionRequired = ProtectionRequired

instance ProtectionIndicator ProtectionRequired where
  ...

This would permit deprecating the use of () to indicate required protection, which is an analogue of Boolean blindness.

frasertweedale commented 3 months ago

@ericpashman could you please review #128 and provide feedback?

Thanks!