[3.0 regression] AreEqual/AreNotEqual with nullable structs and implicit casts don't compile any more

[cremor]

Describe the bug

After updating MSTest.TestFramework to 3.0.2 unit tests that use Assert.AreEqual/AreNotEqual with a nullable struct parameter that has an implicit cast operator to the other parameter type implemented don't compile any more.

Steps To Reproduce

1. Create a new test project. (It will be created with MSTest.TestFramework 2.2.10.)
2. Add the following code.

public struct MyValue
{
    public int Value;

    public static implicit operator int(MyValue v) => v.Value;
}

[TestClass]
public class UnitTest1
{
    [TestMethod]
    public void TestMethod1()
    {
        var v = new MyValue();
        MyValue? nullable = null;

        // Still compiles without errors.
        Assert.AreEqual(0, v);
        Assert.AreNotEqual(1, v);
        Assert.AreEqual(null, nullable);
        Assert.AreNotEqual(v, nullable);

        // Doesn't compile any more.
        Assert.AreNotEqual(1, nullable);
        Assert.AreEqual(0, nullable); // Note: This fails. I don't know if there is a useful use-case for this.
    }
}

4. Try to compile the project, see that it compiles just fine.
5. Update MSTest.TestFramework to 3.0.2
6. See compile errors

Expected behavior

Code should compile without errors.

Actual behavior

Code has compile errors:
CS0411 The type arguments for method 'Assert.AreNotEqual(T?, T?)' cannot be inferred from the usage. Try specifying the type arguments explicitly.

Additional context

The Assert.AreEqual case might not make sense. But at least the Assert.AreNotEqual case has valid use-cases and should work.

It seems like the reason for this is #254/#1429 because version 2.2.10 used this overload.
Cc @whoisj @Evangelink

[Evangelink]

Hi @cremor,

Thanks for raising this issue/regression with a proper analysis and reproducers!

We have had a lot of reports about "runtime" errors that could be caught at compilation because of object, object overload on Assert.AreEqual where having this T, T actually helps a lot. When I did the implementation I assumed that the opposite was interesting but didn't thought at all about implicit use cases. As you properly stated, the AreEqual and AreNotEqual might need to be different as they may want to capture different use cases.

[cremor]

I found a few additional things:

• If you define the implicit operator the other way around (implicit operator MyValue(int v)) then the code still compiles with the new version. It then uses the Are(Not)Equal<MyValue?>() overload.
• If you define both implicit operators then the first two Are(Not)Equal calls (right after the comment Still compiles without errors, which are not using a nullable variable) are the ones which don't compile any more.
  Note: In the old version (where these calls do compile) the AreEqual call will fail now because it will use the object, object overload and have different types for both parameters.
• Based on the second point: If I then change those two not compiling lines to pass a long instead of an int as the first parameter it compiles again. It then uses Are(Not)Equal<long>(). So double implicit casts work but single ones don't? 😕
  Note: And now those two lines not only compile, but even pass again. Because the twice-converted long values are correctly compared.

[Evangelink]

I just did a quick investigation and before it was always boxing to object and then using object.Equals(expected, actual). If you rewrite

Assert.AreNotEqual(1, nullable);
Assert.AreEqual(0, nullable);

into

Assert.AreNotEqual<object>(1, nullable);
Assert.AreEqual<object>(0, nullable);

or

Assert.AreNotEqual<int?>(1, nullable);
Assert.AreEqual<int?>(0, nullable);

it will compile and provide expected behavior.

I am exploring the options because bringing back object, object overload makes it always favored over generic version.

[Evangelink]

Hi @cremor, I did more tests and discussed with the team the strategy we want to adopt here and we decided that we want to keep the current behavior as we want to allow strict comparison.

If you try to use the EqualityComparer with the 0 and nullable case (i.e. EqualityComparer<MyValue>.Equal(0, nullable)) you will see this is using the (object?, object?) orverload and not the (MyValue?, MyValue?) one because you defined the implicit operator only on a non-nullable MyValueType. If you start adding the following implicit operator:

public static implicit operator int(MyValue? v);

You will see that the assertions will start working. Lastly, you can use Assert.Are(Not)Equal<object> to force using the object equality.

Given what I wrote above and the fact we didn't receive other feedback on this change, I would move forward by closing this issue as won't fix. If we receive more feedback, we will of course revisit this decision.

[Cjewett]

@Evangelink Just to confirm when upgrading to 3.0.x we can specify the type as object and it'll provide the same result as the 2.0.x versions?

[Evangelink]

That's correct.