Implement assert contains overload to accept non-generic collection

[AtolagbeMuiz]

This Pull Request fixes Collection assert methods should accept IEnumerable collections #6184

This implementation involves an overload of Assert.Contains which allows non-generic IEnumerable collection like ArrayList and Stack.

Current Behaviour
Currently, the Assert.Contains function doesn't allow non-generic collection like ArrayList as this throws compile time error as seen below

Expected Behaviour
With this Implementation, Assert.Contains will allow non-generic collection as seen below;

[Youssef1313]

I think we need similar non-generic overloads also for IsEmpty and HasCount. Then MSTEST0037 analyzer need to be adjusted as well to account for the existence of the non-generic overloads:

testfx/src/Analyzers/MSTest.Analyzers/UseProperAssertMethodsAnalyzer.cs

Lines 464 to 471 in 8f99258

	private static bool IsBCLCollectionType(ITypeSymbol type, INamedTypeSymbol objectTypeSymbol)
	// Check if the type implements IEnumerable<T> (but is not string)
	// Note: Assert.Contains/IsEmpty/HasCount for collections accept IEnumerable<T>, but not IEnumerable.
	=> type.SpecialType != SpecialType.System_String && type.AllInterfaces.Any(i =>
	i.OriginalDefinition.SpecialType == SpecialType.System_Collections_Generic_IEnumerable_T) &&
	// object is coming from BCL and it's expected to always have a public key.
	type.ContainingAssembly.Identity.HasPublicKey == objectTypeSymbol.ContainingAssembly.Identity.HasPublicKey &&
	type.ContainingAssembly.Identity.PublicKey.SequenceEqual(objectTypeSymbol.ContainingAssembly.Identity.PublicKey);

[AtolagbeMuiz]

Yes, I intend to implement this for IsEmpty etc. as well.. still yet to have a good understanding of the Analyzer part.

[Youssef1313]

Once you add the overload for HasCount, this analyzer test will fail:

testfx/test/UnitTests/MSTest.Analyzers.UnitTests/UseProperAssertMethodsAnalyzerTests.cs

Line 2134 in 8f99258

public async Task WhenAssertAreEqualWithCollectionCountUsingNonGenericCollection()

The test will need to be updated as following (note: I'm writing it directly on GitHub so there might be small mistakes):

    [TestMethod]
    public async Task WhenAssertAreEqualWithCollectionCountUsingNonGenericCollection()
    {
        string code = """
            using System;
            using System.Collections;
            using System.Collections.Generic;
            using Microsoft.VisualStudio.TestTools.UnitTesting;

            [TestClass]
            public class MyTestClass
            {
                [TestMethod]
                public void MyTestMethod()
                {
                    var x = new Hashtable();
                    {|#0:Assert.AreEqual(4, x.Count)|};
                }
            }
            """;

        string fixedCode = """
            using System;
            using System.Collections;
            using System.Collections.Generic;
            using Microsoft.VisualStudio.TestTools.UnitTesting;

            [TestClass]
            public class MyTestClass
            {
                [TestMethod]
                public void MyTestMethod()
                {
                    var x = new Hashtable();
                    Assert.HasCount(4, x);
                }
            }
            """;

        await VerifyCS.VerifyCodeFixAsync(
            code,
            // /0/Test0.cs(13,9): info MSTEST0037: Use 'Assert.HasCount' instead of 'Assert.AreEqual'
            VerifyCS.DiagnosticIgnoringAdditionalLocations().WithLocation(0).WithArguments("HasCount", "AreEqual"),
            fixedCode);
    }

The analyzer implementation then will need to check System_Collections_Generic_IEnumerable instead of System_Collections_Generic_IEnumerable_T

Note: because we use AllInterfaces, and IEnumerable<T> is IEnumerable, we need only IEnumerable check. We don't need both IEnumerable<T> and IEnumerable.
Note 2: OriginalDefinition access will become redundant, because we are no longer dealing with generics.

[AtolagbeMuiz]

i will revert back to this once I want to implement for HasCount

[Youssef1313]

Thanks @AtolagbeMuiz.

Are you planning to continue that soon? It's best to have all the overloads in the same PR as the analyzer is likely handling all these methods in the same way (even if we have only test coverage for HasCount)

[AtolagbeMuiz]

Hi @Youssef1313, should the implementation for other Assert methods like HasCount be in this same PR and branch and not as a different branch??

was intending to work on it on a different branch.. I should commence work on it this week.

[Youssef1313]

Yes, I prefer if it's the same PR in this case. Thanks @AtolagbeMuiz!