devlooped / moq

The most popular and friendly mocking framework for .NET
Other
5.83k stars 799 forks source link

The public API is still missing nullable reference type annotations #1418

Open stakx opened 10 months ago

stakx commented 10 months ago

They should perhaps be added. Ideally not just for the public API surface, but for the whole code base. This could then replace quite a few Debug.Assertions that guard against null references internally.

(This might potentially be a good first issue for new contributors. I expect that there will be some code locations where it isn't immediately clear whether null is allowed or not, and where one would have to study the code & execution paths through it for a bit... but even that shouldn't be too tough.)

rikrak commented 10 months ago

I had a quick look at this last night and noticed that adding nullable markings started to cause a few false-positive warnings. This looks to be due to targeting of net462 and netstandard20, which are missing some of the nicer nullable compiler enhancements.

I could work around this by adding augmenting the Guard class so that is decorated with attributes that help the compiler understand what's going on (Example here, which uses polyfill attributes.

Is this the sort of direction that feels appropriate? One thing to note is that it results in a change in behaviour as there is potential for exceptions where previously there was not (although, if the nullability is interpreted correctly, all should be good).