Dapper
Dapper copied to clipboard
Nullable reference types
I annotated the public API in https://github.com/StackExchange/Dapper/pull/1392I for fun and in case it could be helpful. I don't have a use case for this feature as a Dapper user right now, and I'm not asking for it.
@NickCraver had good thoughts and asked me to open an issue for discussion. There are some big drawbacks:
- It would hose pretty much all open PRs.
- Fixing the new warnings results in a big change to review. (Flip side: there are definitely bugs that got caught.)
- Using C# 8 with any target framework other than .NET Core 3+ is officially unsupported by Microsoft, even though they tell library authors to use this unsupported configuration for the purpose of NRT annotation.
- If I'm paraphrasing correctly, Nick is concerned that enabling NRTs could make it easier for nulls to leak in or out of Dapper APIs in places where they are currently supposed to never be null.
Is there anything that Dapper would stand to gain by paying these costs?
📝 https://github.com/tunnelvisionlabs/ReferenceAssemblyAnnotator aims to fill the tooling gap by copying nullability annotations from .NET Core 3+ into the reference assemblies you compile against and by providing internal nullability attributes as source. I use it for active projects from net35 to net48 and netstandard2.0. There aren't any technical limitations I know of compared to the NRT experience on .NET Core 3 itself. (Besides bugs in this community tool, of course.)
Tagging this for consideration in v3 - we're not sure when the net4x
drop will happen but Dapper is very popular and this one doesn't cross the boundary well so...we need to see.
Update here: we will be dropping net4x support in vNext.
Dapper not being annotated means that I don't have a way to know whether Dapper might pass null to either value
parameter:
internal sealed class FooTypeHandler : SqlMapper.TypeHandler<Foo>
{
public override Foo Parse(object value)
{
throw new NotImplementedException();
}
public override void SetValue(IDbDataParameter parameter, Foo value)
{
throw new NotImplementedException();
}
}
Looking at the Microsoft blog the advise for library authors is quite clear. So I'd like to see the annotations in the library as well. It's a bit unfortunate though that you need to turn on C# 8 for the netstandard2.0 target which is "unsupported" but then most things work anyway (including NRT).