-
Notifications
You must be signed in to change notification settings - Fork 981
Description
C# 8.0 introduced Nullable reference types and enabled a new static analyzer to make it easier for developers to see where their code is making an assumption about the nullability of reference types.
In Humanizer, there are most certainly assumptions being made about reference types, such as strings, being non-null. If users try to call .Humanize()
on a string that happens to be null, a null ref will be thrown by the current version of this SDK.
Unfortunately, even if users have nullable enabled in their projects it's not good enough. If a particular dependency (Humanizer) wasn't built with the same setting turned on, the analyzer has no way of knowing the expectations of the library. So it just doesn't flag nullability warnings on interactions with that SDK.
E.g.
#nullable enable
public void MyMethod(string? foo)
{
// This compiles fine, but will throw a NullRef if `null`e is passed to this method
Console.WriteLine(foo.Humanize());
}
Proposal:
If Humanizer were to add a simple property to the .csproj, this problem would go away entirely:
<Nullable>enable</Nullable>
Then, in situations like the MyMethod()
example from above, the compiler would show a warning on this line like "Possible null reference", reminding the user to handle that case as desired.
It's also an opportunity for the Humanizer library to improve - I'd be willing to bet enabling this setting would reveal a whole bunch of places where you're currently making assumptions about the nullability of things! Additionally, the APIs could potentially be updated to accept string?
instead of just string
, and return human-readable strings in case null
is humanized.
Just something to consider! Even if you don't update the API to allow humanization of null values, it'd be nice if you simply applied this setting so that nullability analysis worked when interfacing with this library.