SteveDunn / Vogen

A semi-opinionated library which is a source generator and a code analyser. It Source generates Value Objects
Apache License 2.0
888 stars 46 forks source link

`ToString()`'s return type shouldn't be nullable #697

Closed aradalvand closed 3 days ago

aradalvand commented 3 weeks ago

Describe the bug

Vogen generates this:

image

Steps to reproduce

Create any value object.

Expected behaviour

The ToString() object never returns null, its return type should not be nullable.

philippbeckmann commented 2 weeks ago

It overrides Object.ToString(), which returns a nullable string.

https://learn.microsoft.com/en-us/dotnet/api/system.object.tostring?view=net-8.0

SteveDunn commented 2 weeks ago

There was a huge thread on the CLR repo about this. I'll see if I can find the link.

SteveDunn commented 2 weeks ago

@aradalvand can you share your exact definition? I guess it's a record struct or struct? Is it readonly?

aradalvand commented 2 weeks ago

@SteveDunn Sure:

[ValueObject<decimal>]
public readonly partial struct Rial;
SteveDunn commented 2 weeks ago

Actually, there were some minor tweeks in the latest versions of Vogen around this. Quite a lot of the ToString code was changed to facilitate IFormattable hoisting. I can see in the latest RC versions, that structs that wraps a decimal, it generates this:

        /// <inheritdoc cref = "System.Decimal.ToString()"/>
        public override global::System.String ToString() => IsInitialized() ? Value.ToString() ?? "" : "[UNINITIALIZED]";

So, even though Object.ToString and ValueType.ToString return string?, Vogen doesn't because it checks for null and returns an empty string.

Please let me know if the RC versions better match what you expect.

SteveDunn commented 3 days ago

Closing this for now, but if you feel it needs further investigation, please shout. Thanks for the feedback!