add should.equals

[Meir017]

closes #94

[Meir017]

@bordecal does it make sense to assume that the right assertion would be .Should().Be()?

[jnyrup]

If the subject is a collection Equals should probably just be Equal.
I think it might be difficult to guess the intended assertion.

[bordecal]

@Meir017 Do you mean for a fixer? If so, for a primitive type, .Should().Be() would probably be the only reasonable choice in most cases.

For complex types, it's more difficult to predict the right option. Maybe .Should().BeSameAs() would be the safest choice. At least it would probably break the test if it's the wrong choice, which wouldn't necessarily be the case for the less restrictive assertions.

[Meir017]

@jnyrup I'm pretty sure it's possible to detect the type of actual object used in the assertion. would that make sense?

[JamesDriscoll]

Have just run into this very issue in several of our code-bases today.
There are multiple copy and paste Should().Equals(...) calls that have started propagating like wild fire.
I was going to offer up a PR myself, but found this one sat dormant.
It would be great if this could be resurrected; if not, I'll try and contribute :)

If the object is a collection, Should().Be() is not available, so Should().Equal() seems correct.

For my needs, I think that Should().Be() would be more than sufficient.

For what it's worth, I think this analyzer is extremely important.
We have non-English speaking developers working offshore who don't spot that x.Should().Equals(y); is a natural English code smell vs x.Should().Be(y);

[Meir017]

@JamesDriscoll I have been really busy lately, I really want to get back to this project too :)

[JamesDriscoll]

Let me know if you need any help :)

[Meir017]

@JamesDriscoll @jnyrup Maybe we could have two code fixes suggested and each having a different message, one for primitives and the other for collection and custom classes?

[JamesDriscoll]

I think one message for collections going to Equal and another one for everything else going to Be.

A lot of tests we write create different but equivalent objects. I personally think it is much, much less likely to have tests where the actual versus expected would satisfy BeSameAs.

Perhaps the compromise would be to offer Be as the fix, but to acknowledge that BeSameAs might be preferable in the message.

[jnyrup]

Some considerations:

• TaskCompletionSourceAssertions does not have any of Equal, Be, BeSameAs or BeEquivalentTo.
• StreamAssertions has BeSameAs, but if I had written Equals I imagine what I wanted was to compare the content of the streams.

So it comes down to whether the fixer has to be perfect.
I'm not sure it can be perfect, not because of technical limitations, but because it will possibly have to guess among equal candidates.

[Meir017]

@jnyrup Great suggestions!

[JamesDriscoll]

Thanks for picking this up so swiftly - I think it will really adds a lot of value once it gets out into the wild :)

Some considerations:

• TaskCompletionSourceAssertions does not have any of Equal, Be, BeSameAs or BeEquivalentTo.
• StreamAssertions has BeSameAs, but if I had written Equals I imagine what I wanted was to compare the content of the streams.

So it comes down to whether the fixer has to be perfect. I'm not sure it can be perfect, not because of technical limitations, but because it will possibly have to guess among equal candidates.

I know this is already merged, but the other alternative would be not to offer a fix in all cases, but still warn.

[jnyrup]

I know this is already merged, but the other alternative would be not to offer a fix in all cases, but still warn.

That's probably the direction I would have taken.