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

NUnit2010 - do not consider Equals call if it doesn't override Object.Equals #728

Closed
matode opened this issue Apr 12, 2024 · 2 comments · Fixed by #730
Closed

NUnit2010 - do not consider Equals call if it doesn't override Object.Equals #728

matode opened this issue Apr 12, 2024 · 2 comments · Fixed by #730
Assignees
Labels
Milestone

Comments

@matode
Copy link
Contributor

matode commented Apr 12, 2024

If there is a class Foo, implementing Equals in this way:

class Foo
{
   public bool Equals(Foo objectToCompare
   {
     bool areEqual;
     //some logic modifying areEqual;
     return areEqual;
   }
}

and Assert like:

Foo expectedValue = new Foo();
Foo testeeResult = new Foo(); //for simplicity. Some calculation happened instead.
Assert.That(expectedValue.Equals(testeeResult));

NUnit2010 fix changes it to

Assert.That(expectedValue, Is.EqualTo(testeeResult));

But this test fails with error message:
Expected: Foo
But was: Foo

It would work, if Foo would implement public override bool Equals(object objectToCompare) instead. But this change is not possible for legacy code reason.

It would probably better, if the Analyzer would check, if the called Equals method is an override from object or just an isolated implementation and ignore it in such case.

@manfred-brands
Copy link
Member

Agreed. If Equals is not Equals(object) and not IEquality<Foo> then it should not raise NUnit2010.

I guess there is a legacy reason Foo doesn't derive from IEquality<Foo> but does implement that method.

@manfred-brands
Copy link
Member

I added a PR to exclude Equals implementation if the class doesn't implement the corresponding IEquality interface.
However, I'm not sure if this actually should be fixed in NUnit itself instead.
If a class A implements bool Equals(B) then Assert.That(B, Is.EqualTo(A)) should pass.

@mikkelbu mikkelbu added this to the Release 4.2 milestone Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants