SteveDunn / Vogen

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

Why implicit comparison is allowed? #561

Closed arteny closed 9 months ago

arteny commented 9 months ago

Describe the bug

Not sure that it is correct behaior for ValueObject as strong typed value

            _ = EventId.From("123456789*") == "ddd";

There is no any compilation error.

Steps to reproduce

Compile.

Expected behaviour

Compilation error.

arteny commented 9 months ago

And I meet this issue in real usage now with EFCore. As for me, it breaks all concepts of ValueObject. When I use this code:

await db.Exposures.Where(x => x.Market.Id == market.Id).ExecuteDeleteAsync();

there are no errors or warnings during compilation, but at execution time, I get the exception:

System.InvalidCastException: Invalid cast from 'System.String' to 'Domain.Enums.MarketId'

market.Id is string here, so to get it working i need to change it to MarketId.From(market.Id), but I expected to know this before running my program. MarketId type has the following definition:

[ValueObject<string>(conversions: Conversions.EfCoreValueConverter | Conversions.Default)]
SteveDunn commented 9 months ago

Hi @arteny , this is correct behaviour. Or at least it's behaviour that has been there since inception.

Vogen generates the following equality methods for all underlying types. For a string it's

        public static bool operator ==(EventId left, string right) => Equals(left.Value, right);
        public static bool operator !=(EventId left, string right) => !Equals(left.Value, right);

As for me, it breaks all concepts of ValueObject

Two of the main concepts of a Value Object are:

For the latter, encapsulation isn't broken; Equals(this, string other) doesn't allow you to access this if it isn't valid, and the comparison to the underlying string doesn't create any other value object.

I think that EF is taking your where expression and rewriting it in such a way that the Equals is being ignored and a direct cast is used instead. Looking online reveals lots of issues related to comparisons in expressions (most are related to casing of the string).

There are a few additional things that you need to do before using Vogen types with EF. These are described in this How To article. I hope this solves your problem. If you could let me have some feedback on that article (or better still, modify it if there's something missing/incorrect/confusing), that'd be great.

SteveDunn commented 9 months ago

Closing this as I'm confident that the additional setup shown in the documentation will solve this issue, but please let me know if it doesn't.

Tvde1 commented 8 months ago

Is there a way to opt out of this? I have a value object which is a record struct backed by a string. I only want these two structs to be able to be compared, as comparing one with a string allows comparing a a non-nullable struct to null (which I'd like to prevent compile time).

In this case it allows

[ValueObject<string>]
public partial record struct MyThing;

MyThing.From("abc") == null;