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

Vogen 3.0.21 -> 3.0.22 - Compilation Error on Custom Equality #516

Closed
cmeyertons opened this issue Nov 1, 2023 · 5 comments
Closed

Vogen 3.0.21 -> 3.0.22 - Compilation Error on Custom Equality #516

cmeyertons opened this issue Nov 1, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@cmeyertons
Copy link

cmeyertons commented Nov 1, 2023

Describe the bug

We have a Value Object that implements its own Equals and GetHashCode and it fails to compile when upgrading from 3.0.21 -> 3.0.22

Steps to reproduce

[ValueObject<string>(Conversions.Default | Conversions.NewtonsoftJson)]
public readonly partial record struct AttributeId
{
    public readonly bool Equals(AttributeId other)
    {
        // custom code
        return Value == other.Value;
    }

    public override int GetHashCode()
    {
        // custom code
        return 0;
    }
}

Error message:
image

Expected behaviour

For a patch version release, I would not expect this kind of breakage.

@cmeyertons cmeyertons added the bug Something isn't working label Nov 1, 2023
@SteveDunn
Copy link
Owner

Apologies @cmeyertons - I completely missed this scenario. I'll fix ASAP.

@SteveDunn
Copy link
Owner

The 2nd beta is there now on NuGet: https://www.nuget.org/packages/Vogen/3.0.23-beta.2

@cmeyertons
Copy link
Author

Apologies @cmeyertons - I completely missed this scenario. I'll fix ASAP.

No worries at all! Appreciate the quick fix, no rush on my end. Just like to stay up-to-date

@SteveDunn
Copy link
Owner

Sorry @cmeyertons - the comment above was meant for another issue.

I'm taking a look at this issue now. Previously, for records, GetHashCode and Equals were not generated because this was auto-generated as part of the record.

But there was a feature request to support case-insensitive comparison and as part of that, I made the change to bring records more in line with classes and structs.

It seems reasonable to allow users to provide their own GetHashCode, so the change I just made skips the generation of this.

Regarding Equals:
May I ask why you override Equals? The latest version of Vogen now generates Equals for everything, including records (hence your issue). In addition, it now generates an Equals that takes an IEqualityComparer<T> where T is the underlying type.

This causes a problem for Vogen if users have supplied their own Equals method(s). It could check for user supplied instances, but I originally imagined that equality would be something that wouldn't require an overload from the user (aside from case-insensitivity which has now been done).

I'm wondering, before I start the work on checking for overloaded Equals methods (and probably the == and != operators too (😱 ) - if the generated Equals that takes an IEqualityComparer is enough of an extension point for users.

@SteveDunn
Copy link
Owner

Thanks for the report @cmeyertons - this is now fixed in 3.0.23 - apologies for the mess up. I've documented what can be overridden here: https://github.com/SteveDunn/Vogen/wiki/Overriding-methods

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants