Pattern Matching [Maybe] Issue with Aff

[jmzagorski]

I ran into a problem while unit testing my application, and wanted to get your opinion to determine if this is a bug or it behaves as expected. Really, I got lazy with unit testing on a generic method so I used object with Aff<T>.MapFail, which resulted in the Error object succeeding because the pattern matching has the generic type matched before the Error object.

https://github.com/louthy/language-ext/blob/d42c12a2c09bc04f9185587fd4f74bafa27d987f/LanguageExt.Core/Effects/Eff/Extensions/Eff.Extensions.cs#L43-L48

Here is a simple replication:

Unit Test

public class Test
{
    [Fact]
    public async Task ShouldFail()
    {
        var error = Error.New("foobar");

        var fin = await FailAff<object>(error).MapFail(e => e).Run();

        Assert.True(fin.IsFail); // This fails
    }
}

Would you expect IsFail to be true or false?

[louthy]

I'd expect it to be in a Fail state, yes. Could you step into the functions to see how it's happened? And what the actual state of the Fin is? (I'm assuming Bottom).

[louthy]

Sorry, just read your comment fully. It looks like you're right that anything object would match first, so yes, that's a bug. If you want to submit a fix, I'm due to do a new release soon.

[jmzagorski]

How large of a changeset would you like this to be? For example, change just that one method, or look over that file, or include other files that have the generic before the Error in pattern matching? I noticed some instances in the Eff<T> as well.

[louthy]

I will accept any amount of help! If you're willing to find all of the similar issues and fix (search for '.Case'), that's awesome. If you fix this one instance, that's awesome too!

[jmzagorski]

I apologize I never got around to this. I had a lot of changes at work that never gave me the time. However, it looks like this may be fixed with the version 5? I changed the Act part of the test to FailEff<object>(error).MapFail(e => e).Run(); and it does fail as expected. I am not sure if that is true for all methods given the number of changes to the source code since this issue was submitted.

Great work on the changes by the way. I am still pushing my team to invest time into understanding this library.

[louthy]

I am not sure if that is true for all methods given the number of changes to the source code since this issue was submitted.

Many of the core types have been refactored or completely rewritten and I had this in mind when updating, so yes, it's probably solved for all -- I just haven't got around to building unit tests for the new v5 functionality yet.

[jmzagorski]

Closing since the new version takes care of this issue. Thanks!