Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Please enable nullable in Humanizer.csproj #1556

Open
DrEsteban opened this issue Dec 8, 2024 · 0 comments
Open

Please enable nullable in Humanizer.csproj #1556

DrEsteban opened this issue Dec 8, 2024 · 0 comments

Comments

@DrEsteban
Copy link

DrEsteban commented Dec 8, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant