Reflection warnings indicate compile time type ambiguity that is resolved at runtime with a dynamic dispatch. This causes a small runtime cost that is not usually a problem, but can also mask type errors, for example set-body! which claims to accept a javax.mail.Message but actually requires a javax.mail.internet.MimeMessage.
Even if postal does not benefit from resolving type ambiguities, downstream users may have very good reasons for enabling reflection warnings so it would be nice if postal (and similar libraries) could avoid generating reflection warnings in their core namespaces so that they don't generate noise for downstream users.
This change is split into three commits:
1) Enable reflection warnings: this commit is optional, but will help avoid regressions
2) Resolve ambiguities in core namespaces: this commit is the meat of the change
3) Resolve ambiguities in test namespaces: if you accept (1) you probably want this too
I have tried to avoid any changes to behaviour, or the signatures of public vars. The exceptions to this are message->str, where I added a return type hint to resolve a lot of warnings in test code, and add-body!, where I resolved a potential bug where a MimeMessage was expected but the API claims to take a Message. I changed the code to support Message in both paths so that the API remains consistent, but you may prefer to change to MimeMessage and eliminate the extra conditional.
Reflection warnings indicate compile time type ambiguity that is resolved at runtime with a dynamic dispatch. This causes a small runtime cost that is not usually a problem, but can also mask type errors, for example
set-body!
which claims to accept ajavax.mail.Message
but actually requires ajavax.mail.internet.MimeMessage
.Even if postal does not benefit from resolving type ambiguities, downstream users may have very good reasons for enabling reflection warnings so it would be nice if postal (and similar libraries) could avoid generating reflection warnings in their core namespaces so that they don't generate noise for downstream users.
This change is split into three commits:
1) Enable reflection warnings: this commit is optional, but will help avoid regressions 2) Resolve ambiguities in core namespaces: this commit is the meat of the change 3) Resolve ambiguities in test namespaces: if you accept (1) you probably want this too
I have tried to avoid any changes to behaviour, or the signatures of public vars. The exceptions to this are
message->str
, where I added a return type hint to resolve a lot of warnings in test code, andadd-body!
, where I resolved a potential bug where aMimeMessage
was expected but the API claims to take aMessage
. I changed the code to supportMessage
in both paths so that the API remains consistent, but you may prefer to change toMimeMessage
and eliminate the extra conditional.