Vogen icon indicating copy to clipboard operation
Vogen copied to clipboard

Nullable references are not enable in generated files

Open DPDmancul opened this issue 1 year ago • 1 comments

Describe the bug

C# does not observe global nullable settings inside generated files, thus generated files without #nullable enable do not use nullable reference types. This, for example, causes the From factory to silently accept a null value without any compile time errors, postponing the error discovery only a runtime!

Steps to reproduce

[Vogen<string>]
public readonly partial struct MyVogenType;

MyVogenType.From(null);

Expected behaviour

This should not compile, but instead it compiles and throws at runtime.

This issue is easily solved by adding #nullable enable at the beginning of generated sources

DPDmancul avatar Aug 26 '24 14:08 DPDmancul

Many thanks for the bug report @DPDmancul . I'll take a look at this soon.

SteveDunn avatar Aug 27 '24 16:08 SteveDunn

I'll hopefully have something ready tomorrow. I'll need to bump up the major version as it's a breaking change.

SteveDunn avatar Sep 08 '24 18:09 SteveDunn

@DPDmancul - this is now in version 5.0.0 (beta 1). Would you mind giving it a try to see if it works as expected?

SteveDunn avatar Sep 09 '24 15:09 SteveDunn

Yes it now gives compilation error as expected, thanks!

PS: I don't know if it is a real use case scenario, but I noticed it is impossible to wrap a nullable reference type:

[Vogen<Guid>]
public readonly partial struct MyGuid;

MyGuid.From(null); // this correctly won't compile

[Vogen<Guid?>]
public readonly partial struct MyNullableGuid;

MyNullableGuid.From(null); // this compiles fine

[Vogen<string>]
public readonly partial struct MyString;

MyString.From(null); // now this correctly won't compile (previously, incorrectly, would have compiled)

[Vogen<string?>] // here we got error because of C# limitations on nullable reference types
public readonly partial struct MyNullableString;

[Vogen(typeof(string?))] // this fails for the same reason
public readonly partial struct MyNullableString;

Before this bugfix one could have used [Vogen<string>] to wrap a nullable string (since in c# string? = string), but now it could no more be don. This saves us from terrible nasty bugs in the case we want a real string, but also prevent us to wrap a nullable string (without using other wrappers, e.g. Option).

I found a workaround for this, but maybe there is a better solution:

#nullable disable
[ValueObject<string>]
public partial class MyNullableString;
#nullable restore

MyNullableString.From(null);

DPDmancul avatar Sep 10 '24 07:09 DPDmancul

Thanks for trying it out @DPDmancul . It's an interesting scenario regarding wrapping nullable reference types. In my opinion, Vogen's primary purpose is to avoid nulls and strongly type things that were previously expressed as primitives (and/or nulls).

I've seen one other issue that was reported, and I think that was an issue relating to a nullable value type.

I still think that the best alternative is an Option/Maybe. Hopefully with discriminated unions comping up in C#, things like this will be easier to manage.

Thanks again for your feedback. I'll close this issue, but wait a week or so before releasing, just in case there's any reported issues.

SteveDunn avatar Sep 10 '24 20:09 SteveDunn