Fix error when deriving instances of type synonyms

[zweimach]

First, I want to thank you for developing and maintaining PureScript. It is a great language and compiler.

I don't know if my changes are idiomatic Haskell or not. This is my first big Haskell project, after all.
If there are things that I could do to improve my code, please let me know. Hopefully, this will fix #3510.

[rhendric]

@garyb, what's your disposition on #3539? Do you think changing the error message (this PR) temporarily until that can land is a good idea?

@zweimach, welcome! In terms of code quality, this PR looks pretty good to me. In fact, I think this is the rare time I can tell a PR author that they probably need to add fewer tests—I think the new failing/3510 case tests exactly the same thing as the failing/3405 test you edited, so unless a real maintainer says otherwise I would take it out and save the quarter-second of build time (and however many seconds it takes @garyb to delete it if/when he gets rid of TypeSynonymInstance).

[garyb]

@rhendric Oops, I've not put any thought about how to fix the current issue with #3539 since it was raised by Harry, so that PR isn't really going anywhere just now. I'd be fine with it if someone else wants to carry it on. 😄

But yeah, given that PR is somewhat stalled, I think fixing this error is a good step in the interim.

[kl0tl]

Allowing instances for type synonyms isn‘t enough for deriving them because the desugaring of derived type instances happens before type checking. We still need a way to map the type synonym declarations returned by findTypeDecl to the underlying data declaration.

[hdgarrood]

Did you mean to comment on the other PR?

[kl0tl]

No, I meant to say that this pull request can’t simply be closed once #3539 is merged. I didn‘t meant to suggest that the other should also allow to derive instances for type synonyms.

[hdgarrood]

Ohh sorry, I see what you’re getting at now, thanks. In that case how about getting #3539 merged first since it’s basically there now, and come back to this afterwards?

[rhendric]

Well, hold on a sec. If we merge #3539 and type synonyms are allowed in instances, then the solution used here (throwing an error message that states that type synonyms aren't allowed in instances) won't work; the error constructor itself will have been deleted! At the very least, this PR would have to be amended to add a new error constructor for the specific message of ‘type synonyms aren't allowed in derived instances, despite being allowed in explicitly written instances’. And if that inconsistency is undesirable, then either we need to do the work to implement type synonyms in derived instances (which would surely not be this PR—maybe it really should be a part of #3539, or maybe it's its own PR), or we need not to merge #3539 and merge this PR instead.

[kl0tl]

I may have misunderstood your intentions about this pull request, sorry about that. I think we should try to add support for type synonyms in derived instances in another pull request (it should be feasible to turn externs files into a map from type synonyms to their underlying data declaration in deriveInstances). If it turns out not to be that straightforward it’ll be done when I manage to desugar type classes and instances during type checking anyway so we can always update and merge this to improve error messages in the meantime.

[hdgarrood]

Right - I understand that this PR will need adjustment after merging #3539, but I would still prefer to get that one merged first since it has been in the queue for longer, and I suspect it will provide more value than this one will. Both proposed approaches (ie adding a new error constructor for the issue that derived type synonym instances aren’t yet supported, or adding proper support for derived type synonym instances) sound fine to me.