Make MonadDeclare more useful

[mossprescott]

Fixes #96.

The first commit adds instances for the standard transformers. Note: an instance is provided for ErrorT, which produces a deprecation warning when it's compiled.

The second commit changes the types in Schema.hs to use MonadDeclare. Note: this changes the type of declareNamedSchema which may effect users of the library who provide instances of ToSchema, so it may be considered a breaking change. However, I believe most reasonable implementations will continue to compile.

[fizruk]

New instances look great. Thanks!

However, I'm not entirely sure that switching from Declare to MonadDeclare m => m is a good idea. It can be a breaking change for some folks and I'm not yet convinced it buys us anything. What is your rationale here?

@phadej can you advise on this?

[phadej]

it's about the same as which one to pick forall m. Monad m=> m a or just a (see https://twitter.com/mattoflambda/status/847836232453210112).
So in ToSchema it really doesn't add much.

I'd rather add and

liftDeclare :: MonadDeclare d m => Declare d a -> m a
liftedDeclaredNamedSchema :: MonadDeclare (Definitions Schema) m => proxy a -> m NamedSchema

and maybr a dependency on mmorph to add MFunctor instance so we could also have

hoist :: (forall x. m x -> n x) -> DeclareT d m a -> DeclareT d n a

for completeness

[mossprescott]

I threw that change in mostly to get feedback, so thanks for jumping on it ;)

In our project, where we’re using MonadDeclare constraints, I had to write this adapter:

schemaRef :: (MonadDeclare (Definitions Schema) m, ToSchema a) => Proxy a -> m (Referenced Schema)
schemaRef = liftD . declareSchemaRef
  where
    liftD :: MonadDeclare d m => Declare d a -> m a
    liftD da = do
      (d', a) <- looks (runDeclare da)
      declare d'
      pure a

If we were using the other functions that yield Declare ..., we'd have to apply liftD to them, too. It would make our life easier if everything just used MonadDeclare, and it doesn’t prevent using the simple type. I guess if the library provided something like liftD we could just use that, or maybe there’s another simple solution.

[mossprescott]

Thanks, @phadej. Guess I'd be happy with just adding liftDeclare; I don’t think it makes sense to provide a lifted variant of each function in Schema.hs.

[mossprescott]

Unrelatedly, the build is failing on ghc 7.8 because ExceptT is not available. Since I'm not using it, I’ll simply remove that instance.

[fizruk]

You can use CPP like this:

#if __GLASGOW_HASKELL__ >= 710
instance MonadDeclare d m => MonadDeclare d (ExceptT e m) where
  declare = lift . declare
  look = lift look
#endif

[fizruk]

I think I would rather drop instance for ErrorT than for ExceptT. After all the former is deprecated in favour of the latter.

[phadej]

We should depend on transformers-compat >= 0.3 (if we don't), then ExceptT will always be there.

[phadej]

In Control.Monad.Trans.Except

[fizruk]

Right, I forgot about transformers-compat.

[phadej]

LGTM, Let's merge this when Travis is green.

Adding State and RWST (both strict and lazy), would be great too; but not necessary.

[mossprescott]

Looks like there was already an (indirect) dependency on transformers-compat, but I suppose it is reasonable to make it explicit.

Happy to add State and RWST. I just missed them because I happened to be looking at StateT's definition when I wrote mine.

Thanks for all the detailed feedback, by the way!

[fizruk]

Perfect! Many thanks @mossprescott and @phadej!

I'll make a release right away.

[mossprescott]

Awesome. Thanks, @fizruk!