Fix analyzer false positives for collection asserts by @Youssef1313 in #6300 (backport to rel/3.10) (#6301)

Co-authored-by: Youssef1313 <youssefvictor00@gmail.com>

Author: youssef-backport-bot

src/Analyzers/MSTest.Analyzers/UseProperAssertMethodsAnalyzer.cs

@@ -219,11 +219,13 @@ public override void Initialize(AnalysisContext context)
                 return;
             }
 
-            context.RegisterOperationAction(context => AnalyzeInvocationOperation(context, assertTypeSymbol), OperationKind.Invocation);
+            INamedTypeSymbol objectTypeSymbol = context.Compilation.GetSpecialType(SpecialType.System_Object);
+
+            context.RegisterOperationAction(context => AnalyzeInvocationOperation(context, assertTypeSymbol, objectTypeSymbol), OperationKind.Invocation);
         });
     }
 
-    private static void AnalyzeInvocationOperation(OperationAnalysisContext context, INamedTypeSymbol assertTypeSymbol)
+    private static void AnalyzeInvocationOperation(OperationAnalysisContext context, INamedTypeSymbol assertTypeSymbol, INamedTypeSymbol objectTypeSymbol)
     {
         var operation = (IInvocationOperation)context.Operation;
         IMethodSymbol targetMethod = operation.TargetMethod;
@@ -240,19 +242,19 @@ private static void AnalyzeInvocationOperation(OperationAnalysisContext context,
         switch (targetMethod.Name)
         {
             case "IsTrue":
-                AnalyzeIsTrueOrIsFalseInvocation(context, firstArgument, isTrueInvocation: true);
+                AnalyzeIsTrueOrIsFalseInvocation(context, firstArgument, isTrueInvocation: true, objectTypeSymbol);
                 break;
 
             case "IsFalse":
-                AnalyzeIsTrueOrIsFalseInvocation(context, firstArgument, isTrueInvocation: false);
+                AnalyzeIsTrueOrIsFalseInvocation(context, firstArgument, isTrueInvocation: false, objectTypeSymbol);
                 break;
 
             case "AreEqual":
-                AnalyzeAreEqualOrAreNotEqualInvocation(context, firstArgument, isAreEqualInvocation: true);
+                AnalyzeAreEqualOrAreNotEqualInvocation(context, firstArgument, isAreEqualInvocation: true, objectTypeSymbol);
                 break;
 
             case "AreNotEqual":
-                AnalyzeAreEqualOrAreNotEqualInvocation(context, firstArgument, isAreEqualInvocation: false);
+                AnalyzeAreEqualOrAreNotEqualInvocation(context, firstArgument, isAreEqualInvocation: false, objectTypeSymbol);
                 break;
         }
     }
@@ -429,6 +431,7 @@ private static StringMethodCheckStatus RecognizeStringMethodCheck(
 
     private static CollectionCheckStatus RecognizeCollectionMethodCheck(
         IOperation operation,
+        INamedTypeSymbol objectTypeSymbol,
         out SyntaxNode? collectionExpression,
         out SyntaxNode? itemExpression)
     {
@@ -439,10 +442,7 @@ private static CollectionCheckStatus RecognizeCollectionMethodCheck(
             // Check for Collection.Contains(item)
             if (methodName == "Contains" && invocation.Arguments.Length == 1)
             {
-                // Ensure it's a collection type (implements IEnumerable)
-                ITypeSymbol? receiverType = invocation.Instance?.Type;
-                if (receiverType is not null &&
-                    IsCollectionType(receiverType))
+                if (IsBCLCollectionType(invocation.TargetMethod.ContainingType, objectTypeSymbol))
                 {
                     collectionExpression = invocation.Instance?.Syntax;
                     itemExpression = invocation.Arguments[0].Value.Syntax;
@@ -456,11 +456,14 @@ private static CollectionCheckStatus RecognizeCollectionMethodCheck(
         return CollectionCheckStatus.Unknown;
     }
 
-    private static bool IsCollectionType(ITypeSymbol type)
-        // Check if the type implements IEnumerable (but is not string)
+    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.SpecialType == SpecialType.System_Collections_IEnumerable ||
-            (i.OriginalDefinition?.SpecialType == SpecialType.System_Collections_Generic_IEnumerable_T));
+            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);
 
     private static ComparisonCheckStatus RecognizeComparisonCheck(
         IOperation operation,
@@ -488,7 +491,7 @@ private static ComparisonCheckStatus RecognizeComparisonCheck(
         return ComparisonCheckStatus.Unknown;
     }
 
-    private static void AnalyzeIsTrueOrIsFalseInvocation(OperationAnalysisContext context, IOperation conditionArgument, bool isTrueInvocation)
+    private static void AnalyzeIsTrueOrIsFalseInvocation(OperationAnalysisContext context, IOperation conditionArgument, bool isTrueInvocation, INamedTypeSymbol objectTypeSymbol)
     {
         RoslynDebug.Assert(context.Operation is IInvocationOperation, "Expected IInvocationOperation.");
 
@@ -575,7 +578,7 @@ private static void AnalyzeIsTrueOrIsFalseInvocation(OperationAnalysisContext co
         }
 
         // Check for collection method patterns: myCollection.Contains(...)
-        CollectionCheckStatus collectionMethodStatus = RecognizeCollectionMethodCheck(conditionArgument, out SyntaxNode? collectionExpr, out SyntaxNode? itemExpr);
+        CollectionCheckStatus collectionMethodStatus = RecognizeCollectionMethodCheck(conditionArgument, objectTypeSymbol, out SyntaxNode? collectionExpr, out SyntaxNode? itemExpr);
         if (collectionMethodStatus != CollectionCheckStatus.Unknown)
         {
             if (collectionMethodStatus == CollectionCheckStatus.Contains)
@@ -705,7 +708,7 @@ private static void AnalyzeIsTrueOrIsFalseInvocation(OperationAnalysisContext co
         }
     }
 
-    private static void AnalyzeAreEqualOrAreNotEqualInvocation(OperationAnalysisContext context, IOperation expectedArgument, bool isAreEqualInvocation)
+    private static void AnalyzeAreEqualOrAreNotEqualInvocation(OperationAnalysisContext context, IOperation expectedArgument, bool isAreEqualInvocation, INamedTypeSymbol objectTypeSymbol)
     {
         // Check for collection count patterns: collection.Count/Length == 0 or collection.Count/Length == X
         if (isAreEqualInvocation)
@@ -716,6 +719,7 @@ private static void AnalyzeAreEqualOrAreNotEqualInvocation(OperationAnalysisCont
                 CountCheckStatus countStatus = RecognizeCountCheck(
                     expectedArgument,
                     actualArgumentValue,
+                    objectTypeSymbol,
                     out SyntaxNode? collectionExpr,
                     out _,
                     out _);
@@ -822,6 +826,7 @@ actualArgumentValue.Type is { } actualType &&
     private static CountCheckStatus RecognizeCountCheck(
         IOperation expectedArgument,
         IOperation actualArgument,
+        INamedTypeSymbol objectTypeSymbol,
         out SyntaxNode? collectionExpression,
         out SyntaxNode? countExpression,
         out int countValue)
@@ -833,7 +838,7 @@ expectedArgument.ConstantValue.Value is int expectedValue &&
             actualArgument is IPropertyReferenceOperation propertyRef &&
             propertyRef.Property.Name is "Count" or "Length" &&
             propertyRef.Instance?.Type is not null &&
-            IsCollectionType(propertyRef.Instance.Type))
+            IsBCLCollectionType(propertyRef.Property.ContainingType, objectTypeSymbol))
         {
             collectionExpression = propertyRef.Instance.Syntax;
             countExpression = propertyRef.Syntax;
@@ -848,7 +853,7 @@ actualArgument.ConstantValue.Value is int actualValue &&
             expectedArgument is IPropertyReferenceOperation propertyRef2 &&
             propertyRef2.Property.Name is "Count" or "Length" &&
             propertyRef2.Instance?.Type is not null &&
-            IsCollectionType(propertyRef2.Instance.Type))
+            IsBCLCollectionType(propertyRef2.Property.ContainingType, objectTypeSymbol))
         {
             collectionExpression = propertyRef2.Instance.Syntax;
             countExpression = propertyRef2.Syntax;

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

@@ -2091,5 +2091,73 @@ await VerifyCS.VerifyCodeFixAsync(
             fixedCode);
     }
 
+    [TestMethod]
+    public async Task WhenAssertAreEqualWithCollectionCountUsingCustomCollection()
+    {
+        string code = """
+            using System;
+            using System.Collections;
+            using System.Collections.Generic;
+            using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+            internal sealed class MyCustomCollection<T> : IEnumerable<T>
+            {
+                public IEnumerator<T> GetEnumerator()
+                {
+                    throw new NotImplementedException();
+                }
+
+                IEnumerator IEnumerable.GetEnumerator()
+                {
+                    throw new NotImplementedException();
+                }
+
+                public int Count => 5;
+            }
+
+            [TestClass]
+            public class MyTestClass
+            {
+                [TestMethod]
+                public void MyTestMethod()
+                {
+                    var x = new MyCustomCollection<string>();
+                    Assert.AreEqual(4, x.Count);
+                }
+            }
+            """;
+
+        await VerifyCS.VerifyCodeFixAsync(code, code);
+    }
+
+    [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();
+                    Assert.AreEqual(4, x.Count);
+                    // error CS0411: The type arguments for method 'Assert.HasCount<T>(int, IEnumerable<T>)' cannot be inferred from the usage. Try specifying the type arguments explicitly.
+                    // When we add a non-generic IEnumerable overload, this test will fail because CS0411 is no longer reported.
+                    // In that case, the analyzer should start reporting a diagnostic for the AreEqual call above.
+                    // The codefix should suggest to switch to HasCount.
+                    // Tracking issue https://github.com/microsoft/testfx/issues/6184.
+                    Assert.{|CS0411:HasCount|}(4, x);
+                }
+            }
+            """;
+
+        await VerifyCS.VerifyCodeFixAsync(code, code);
+    }
     #endregion
 }