Annotate nullability of reference types #582
Conversation
|
Build is failing on AppVeyor because Visual Studio 2019 image is not yet ready. |
|
I'm unlikely to be able to find time to do this any time soon, I'm afraid - I've got a lot on my plate right now. |
Don't we all. 😉 I'm not in a big hurry to merge this if you think you'll find some time in a few days. I don't expect it to be a huge effort since all warnings are addressed and tests are passing. There's just a couple of corner cases but which might be just compiler bugs for now unless I've misunderstood something. It would have been great to get quick review from you given your experience with the same for NodaTime. I'll remove you as a reviewer for now but if you'd like to be back on the panel then let me know.
Thanks! Using the Visual Studio 2019 Preview image with 52406ab seems to have done the trick for MSBuild meanwhile though it is still failing due to SDK version mismatch. |
This reverts commit 2e5edfc.
|
@fsateler, @leandromoh I've requested your reviews, but I imagine you might not find time in the coming days. On the other hand, this PR makes sweeping changes that I'd like to merge soon to avoid divergence and merge conflicts; it's also been open for far too long. There should be no behavioural changes so I don't expect introduction of any bugs. That said, I did refactor some code to be more honest in the face of nullability. I will merge this PR by this weekend if I don't hear back on premise that there will always be time to make improvements. If you get round to reviewing the changes at a later time then I'd suggest just dropping your comments here (although that would unusual for a PR that will have been closed) or opening new new PRs to propose your changes/improvements. Needless to say, reviews are equally welcome from everyone else who has contributed or commented on this PR so far. |
| @@ -111,11 +111,11 @@ public static IEnumerable<TSource> Pad<TSource>(this IEnumerable<TSource> source | |||
| if (source == null) throw new ArgumentNullException(nameof(source)); | |||
| if (paddingSelector == null) throw new ArgumentNullException(nameof(paddingSelector)); | |||
| if (width < 0) throw new ArgumentException(null, nameof(width)); | |||
| return PadImpl(source, width, default, paddingSelector); | |||
| return PadImpl(source, width, default!, paddingSelector); | |||
There was a problem hiding this comment.
Bummer. Sounds like there will be permanent warts (like null!) then
This is a note to credit other contributors of "Annotate nullability of reference types" (commit 8dec9ca). git shortlog --summary --email --first-parent 8b7e21b..861585c 51 Atif Aziz <[email protected]> 3 Orace <[email protected]> 5 moh-hassan <[email protected]> Co-authored-by: Pierre Lando <[email protected]> Co-authored-by: Mohamed Hassan <[email protected]>
This PR enables the nullable context for the entire project and annotates all reference types with respect to their nullability using nullable references that are being introduced with C# 8.
There are a few cases marked with
TODO(nullable)that I had trouble reasoning about.Merging this PR would mean that the code base will be moved to C# 8. Is it too early to impose?