Funcky.Xunit - Analyzer for Assert.Equal with FunctionalAssert.Ok/Some/Error/etc. in the `actual` parameter

[Mafii]

Rudimentary example without context:

Can be simplified to

[bash]

This would be the first analyzer that analyzes Funcky.Xunit 🚀

It should of course come with an accompanying code fix.

So in summary, this analyzer would suggest the following changes to users:

• [ ] Rewrite Assert.Equal(expected, FunctionalAssert.Some(actual)) to FunctionalAssert.Some(expected, actual)
• There is no overload for FunctionalAssert.None
• [ ] Rewrite Assert.Equal(expected, FunctionalAssert.Ok(actual)) to FunctionalAssert.Ok(expected, actual)
• ~Rewrite Assert.Equal(expected, FunctionalAssert.Error(actual)) to FunctionalAssert.Error(expected, actual)~ This overload does not exist.
• [ ] Rewrite Assert.Equal(expected, FunctionalAssert.Left(actual)) to FunctionalAssert.Left(expected, actual)
• [ ] Rewrite Assert.Equal(expected, FunctionalAssert.Right(actual)) to FunctionalAssert.Right(expected, actual)

Instead of having a hardcoded list of FunctionalAssert methods to check, I suggest adding an attribute (naming idea off the top of my head [HasAssertEqualOverload]).

If you want to implement this and need some guidance to get started, you can give me a shout on Discord :)

[janhohenheim]

@Mafii any updates?

[Mafii]

Sounds good! I don't know if I want to do this, or what the timeline is, but it would be a useful feature, so let's see if and when it happens :)

[bash]

We need to be very careful for which cases we emit a warning.

There are two things to consider:

• Assert.Equal has overloads which take more than two parameters, such as those that take a custom comparer or the precision for floats. \ → The solution is simple: Just don't emit a warning for these overloads.
• Assert.Equal<T> specializes for some types, such as IEnumerable. Our overloads to not account for this as we call Assert.Equal<Option<T>> losing this specialization. \ → This is a bit trickier: Do we want to account for these specialization in our analyzer or should we change the implementation of FunctionalAssert?

[Mafii]

That's tricky! Both would be fine - adding specializations for FunctionalAssert (that's a lot of effort) or not emitting the analyzer warning. I think the second one is easier, as it can be applied partially instead of having to cover all cases immediately