Enable explicit null checking of reference types #501
Conversation
|
With regards to the When looking at the FxCop/analyzer warning stuff I saw multiple suggestions to replace allocations of [0] length arrays with Array.Empty<>. |
Since |
|
I thought i'd seen some other project do that, but maybe i'm misremembering, though the implementation is apparently only something like internal static class EmptyArray<T>
{
public static readonly T[] Value = new T[0];
}anyway. |
| /// <param name="isStreamOwner">Both streams are closed on completion if true.</param> | ||
| /// <exception cref="ArgumentNullException">Input or output stream is null</exception> | ||
| public static void Decompress(Stream inStream, Stream outStream, bool isStreamOwner) | ||
| public static void Decompress(Stream? inStream, Stream? outStream, bool isStreamOwner) |
There was a problem hiding this comment.
should these streams be nullable if the function throws on null?
There was a problem hiding this comment.
Not sure, I don't think we can check for null if we say that they're non-nullable. I have to read up on how to interface with consumer code with and without Nullable enabled.
There was a problem hiding this comment.
Callers without Nullable enabled will just behave as they do before (i.e treat it as though it can be null), and you can still check for null and throw for those cases (or in general really, given that the checks don't actually prevent it from being null even if marked as non-nullable).
FxCop still gives warnings that non-nullable parameters to public functions should be checked before use as well.
There was a problem hiding this comment.
Yep you're right. There are some instances where you branch on null that the compiler thinks it should be nullable.
Yeah, that is both the implementation in mscorlib and the way I polyfilled it now. I haven't pushed those changes yet though, since I'm not happy with the naming yet. |
|
Would it be worth breaking the Array.Empty bits out into a seperate change? (it could pass as a trivial optimization, and shou;dn't rely on the other nullability changes?) |
* Add the 'EmptyRefs' helper, to support Array.Empty on .NET 4.5 (ref: #501) * Replace uses of 'new T[0]' with 'Empty.Array<T>'
|
Closing this as it would have to be redone now anyway. |
Adds explicit null checking for reference types using C# 8.
Running InferNull on the main source project made it go down from 661 warnings to 214.
After some cleaning and refactoring, the project now happily compiles without any warnings and with almost complete null-checking (some places were just explicitly ignored since they would have a much greater performance impact).
I certify that I own, and have sufficient rights to contribute, all source code and related material intended to be compiled or integrated with the source code for the SharpZipLib open source product (the "Contribution"). My Contribution is licensed under the MIT License.