Moq1100 incorrectly firing on nullable parameters

src/Moq.Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs

@@ -58,31 +58,72 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
 
         if (mockedMethodArguments.Count != lambdaParameters.Count)
         {
-            Diagnostic diagnostic = Diagnostic.Create(Rule, callbackLambda.ParameterList.GetLocation());
+            Diagnostic diagnostic = callbackLambda.ParameterList.GetLocation().CreateDiagnostic(Rule);
             context.ReportDiagnostic(diagnostic);
         }
         else
         {
-            for (int argumentIndex = 0; argumentIndex < mockedMethodArguments.Count; argumentIndex++)
-            {
-                TypeSyntax? lambdaParameterTypeSyntax = lambdaParameters[argumentIndex].Type;
+            ValidateParameters(context, mockedMethodArguments, lambdaParameters);
+        }
+    }
 
-                // TODO: Don't know if continue or break is the right thing to do here
-                if (lambdaParameterTypeSyntax is null) continue;
+    private static void ValidateParameters(
+        SyntaxNodeAnalysisContext context,
+        SeparatedSyntaxList<ArgumentSyntax> mockedMethodArguments,
+        SeparatedSyntaxList<ParameterSyntax> lambdaParameters)
+    {
+        for (int argumentIndex = 0; argumentIndex < mockedMethodArguments.Count; argumentIndex++)
+        {
+            TypeSyntax? lambdaParameterTypeSyntax = lambdaParameters[argumentIndex].Type;
+
+            // We're unable to get the type from the Syntax Tree, so abort the type checking because something else
+            // is happening (e.g., we're compiling on partial code) and we need the type to do the additional checks.
+            if (lambdaParameterTypeSyntax is null)
+            {
+                continue;
+            }
 
-                TypeInfo lambdaParameterType = context.SemanticModel.GetTypeInfo(lambdaParameterTypeSyntax, context.CancellationToken);
+            TypeInfo lambdaParameterType = context.SemanticModel.GetTypeInfo(lambdaParameterTypeSyntax, context.CancellationToken);
+            TypeInfo mockedMethodArgumentType = context.SemanticModel.GetTypeInfo(mockedMethodArguments[argumentIndex].Expression, context.CancellationToken);
 
-                TypeInfo mockedMethodArgumentType = context.SemanticModel.GetTypeInfo(mockedMethodArguments[argumentIndex].Expression, context.CancellationToken);
+            ITypeSymbol? lambdaParameterTypeSymbol = lambdaParameterType.Type;
+            ITypeSymbol? mockedMethodTypeSymbol = mockedMethodArgumentType.Type;
 
-                string? mockedMethodTypeName = mockedMethodArgumentType.ConvertedType?.ToString();
-                string? lambdaParameterTypeName = lambdaParameterType.ConvertedType?.ToString();
+            if (lambdaParameterTypeSymbol is null || mockedMethodTypeSymbol is null)
+            {
+                continue;
+            }
 
-                if (!string.Equals(mockedMethodTypeName, lambdaParameterTypeName, StringComparison.Ordinal))
-                {
-                    Diagnostic diagnostic = callbackLambda.ParameterList.CreateDiagnostic(Rule);
-                    context.ReportDiagnostic(diagnostic);
-                }
+            if (!HasConversion(context.SemanticModel, mockedMethodTypeSymbol, lambdaParameterTypeSymbol))
+            {
+                Diagnostic diagnostic = lambdaParameters[argumentIndex].GetLocation().CreateDiagnostic(Rule);
+                context.ReportDiagnostic(diagnostic);
             }
         }
     }
+
+    private static bool HasConversion(SemanticModel semanticModel, ITypeSymbol source, ITypeSymbol destination)
+    {
+        // This condition checks whether a valid type conversion exists between the parameter in the mocked method
+        // and the corresponding parameter in the callback lambda expression.
+        //
+        // - `conversion.Exists` checks if there is any type conversion possible between the two types
+        //
+        // The second part ensures that the conversion is either:
+        // 1. an implicit conversion,
+        // 2. an identity conversion (meaning the types are exactly the same), or
+        // 3. an explicit conversion.
+        //
+        // If the conversion exists, and it is one of these types (implicit, identity, or explicit), the analyzer will
+        // skip the diagnostic check, as the callback parameter type is considered compatible with the mocked method's
+        // parameter type.
+        //
+        // There are circumstances where the syntax tree will present an item with an explicit conversion, but the
+        // ITypeSymbol instance passed in here is reduced to the same type. For example, we have a test that has
+        // an explicit conversion operator from a string to a custom type. That is presented here as two instances
+        // of CustomType, which is an implicit identity conversion, not an explicit
+        Conversion conversion = semanticModel.Compilation.ClassifyConversion(source, destination);
+
+        return conversion.Exists && (conversion.IsImplicit || conversion.IsExplicit || conversion.IsIdentity);
+    }
 }

tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodCodeFixTests.cs

@@ -1,9 +1,19 @@
+using Xunit.Abstractions;
 using Verifier = Moq.Analyzers.Test.Helpers.CodeFixVerifier<Moq.Analyzers.CallbackSignatureShouldMatchMockedMethodAnalyzer, Moq.Analyzers.CallbackSignatureShouldMatchMockedMethodCodeFix>;
 
 namespace Moq.Analyzers.Test;
 
+#pragma warning disable SA1204 // Static members should appear before non-static members
+
 public class CallbackSignatureShouldMatchMockedMethodCodeFixTests
 {
+    private readonly ITestOutputHelper _output;
+
+    public CallbackSignatureShouldMatchMockedMethodCodeFixTests(ITestOutputHelper output)
+    {
+        _output = output;
+    }
+
     [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "MA0051:Method is too long", Justification = "Contains test data")]
     public static IEnumerable<object[]> TestData()
     {
@@ -22,7 +32,7 @@ public static IEnumerable<object[]> TestData()
                 """new Mock<IFoo>().Setup(x => x.Do(It.IsAny<List<string>>())).Returns((List<string> l) => { return 0; });""",
             ],
             [
-                """new Mock<IFoo>().Setup(x => x.Do(It.IsAny<string>())).Callback({|Moq1100:(int i)|} => { });""",
+                """new Mock<IFoo>().Setup(x => x.Do(It.IsAny<string>())).Callback(({|Moq1100:int i|}) => { });""",
                 """new Mock<IFoo>().Setup(x => x.Do(It.IsAny<string>())).Callback((string s) => { });""",
             ],
             [
@@ -34,7 +44,7 @@ public static IEnumerable<object[]> TestData()
                 """new Mock<IFoo>().Setup(x => x.Do(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<DateTime>())).Callback((int i, string s, DateTime dt) => { });""",
             ],
             [
-                """new Mock<IFoo>().Setup(x => x.Do(It.IsAny<List<string>>())).Callback({|Moq1100:(int i)|} => { });""",
+                """new Mock<IFoo>().Setup(x => x.Do(It.IsAny<List<string>>())).Callback(({|Moq1100:int i|}) => { });""",
                 """new Mock<IFoo>().Setup(x => x.Do(It.IsAny<List<string>>())).Callback((List<string> l) => { });""",
             ],
             [
@@ -73,6 +83,30 @@ public static IEnumerable<object[]> TestData()
                 """new Mock<IFoo>().Setup(x => x.Do(It.IsAny<List<string>>())).Returns(0).Callback((List<string> l) => { });""",
                 """new Mock<IFoo>().Setup(x => x.Do(It.IsAny<List<string>>())).Returns(0).Callback((List<string> l) => { });""",
             ],
+            [ // Repros for https://github.com/rjmurillo/moq.analyzers/issues/172
+                """new Mock<IFoo>().Setup(m => m.Do(It.IsAny<object?>())).Returns((object? bar) => true);""",
+                """new Mock<IFoo>().Setup(m => m.Do(It.IsAny<object?>())).Returns((object? bar) => true);""",
+            ],
+            [
+                """new Mock<IFoo>().Setup(m => m.Do(42)).Returns((long bar) => true);""",
+                """new Mock<IFoo>().Setup(m => m.Do(42)).Returns((long bar) => true);""",
+            ],
+            [
+                """new Mock<IFoo>().Setup(m => m.Do((long)42)).Returns((long bar) => true);""",
+                """new Mock<IFoo>().Setup(m => m.Do((long)42)).Returns((long bar) => true);""",
+            ],
+            [
+                """new Mock<IFoo>().Setup(m => m.Do((long)42)).Returns((long bar) => true);""",
+                """new Mock<IFoo>().Setup(m => m.Do((long)42)).Returns((long bar) => true);""",
+            ],
+            [
+                """new Mock<IFoo>().Setup(m => m.Do(42)).Returns((object? bar) => true);""",
+                """new Mock<IFoo>().Setup(m => m.Do(42)).Returns((object? bar) => true);""",
+            ],
+            [ // This was also reported as part of 172, but is a different error
+                """new Mock<IFoo>().Setup(m => m.Do(42)).Returns((int bar) => true);""",
+                """new Mock<IFoo>().Setup(m => m.Do(42)).Returns((int bar) => true);""",
+            ],
         }.WithNamespaces().WithMoqReferenceAssemblyGroups();
     }
 
@@ -91,6 +125,10 @@ internal interface IFoo
                 int Do(int i, string s, DateTime dt);
 
                 int Do(List<string> l);
+
+                bool Do(object? bar);
+
+                bool Do(long bar);
             }
 
             internal class UnitTest
@@ -102,6 +140,105 @@ private void Test()
             }
             """;
 
-        await Verifier.VerifyCodeFixAsync(Template(@namespace, original), Template(@namespace, quickFix), referenceAssemblyGroup);
+        string o = Template(@namespace, original);
+        string f = Template(@namespace, quickFix);
+
+        _output.WriteLine("Original:");
+        _output.WriteLine(o);
+        _output.WriteLine(string.Empty);
+        _output.WriteLine("Fixed:");
+        _output.WriteLine(f);
+
+        await Verifier.VerifyCodeFixAsync(o, f, referenceAssemblyGroup);
+    }
+
+    public static IEnumerable<object[]> ConversionTestData()
+    {
+        return new object[][]
+            {
+                [ // This should be allowed because of the implicit conversion from int to CustomType
+                    """new Mock<IFoo>().Setup(x => x.Do(42)).Returns((CustomType i) => true);""",
+                    """new Mock<IFoo>().Setup(x => x.Do(42)).Returns((CustomType i) => true);""",
+                ],
+                [ // This should be allowed because of identity
+                    """new Mock<IFoo>().Setup(x => x.Do(new CustomType(42))).Returns((CustomType i) => true);""",
+                    """new Mock<IFoo>().Setup(x => x.Do(new CustomType(42))).Returns((CustomType i) => true);""",
+                ],
+                [ // This should be allowed because of the explicit conversion from string to CustomType
+                    """new Mock<IFoo>().Setup(x => x.Do((CustomType)"42")).Returns((CustomType i) => true);""",
+                    """new Mock<IFoo>().Setup(x => x.Do((CustomType)"42")).Returns((CustomType i) => true);""",
+                ],
+                [ // This should be allowed because of numeric conversion (explicit)
+                    """new Mock<IFoo>().Setup(x => x.Do((int)42L)).Returns((CustomType i) => true);""",
+                    """new Mock<IFoo>().Setup(x => x.Do((int)42L)).Returns((CustomType i) => true);""",
+                ],
+                [ // This should be allowed because of numeric conversion (explicit)
+                    """new Mock<IFoo>().Setup(x => x.Do((int)42.0)).Returns((CustomType i) => true);""",
+                    """new Mock<IFoo>().Setup(x => x.Do((int)42.0)).Returns((CustomType i) => true);""",
+                ],
+                [
+                    """new Mock<IFoo>().Setup(m => m.Do(It.IsAny<int>())).Returns((CustomType i) => true);""",
+                    """new Mock<IFoo>().Setup(m => m.Do(It.IsAny<int>())).Returns((CustomType i) => true);""",
+                ],
+                [
+                    """new Mock<IFoo>().Setup(m => m.Do(It.IsAny<CustomType>())).Returns((CustomType i) => true);""",
+                    """new Mock<IFoo>().Setup(m => m.Do(It.IsAny<CustomType>())).Returns((CustomType i) => true);""",
+                ],
+            }.WithNamespaces().WithMoqReferenceAssemblyGroups();
+    }
+
+    [Theory]
+    [MemberData(nameof(ConversionTestData))]
+    public async Task ConversionTests(string referenceAssemblyGroup, string @namespace, string original, string quickFix)
+    {
+        static string Template(string ns, string mock) =>
+            $$"""
+            {{ns}}
+
+            internal interface IFoo
+            {
+                bool Do(CustomType custom);
+            }
+
+            public class CustomType
+            {
+                public int Value { get; }
+
+                public CustomType(int value)
+                {
+                    Value = value;
+                }
+
+                // User-defined conversions
+                public static implicit operator CustomType(int value)
+                {
+                    return new CustomType(value);
+                }
+
+                public static explicit operator CustomType(string str)
+                {
+                    return new CustomType(int.Parse(str));
+                }
+            }
+
+            internal class UnitTest
+            {
+                private void Test()
+                {
+                    {{mock}}
+                }
+            }
+            """;
+
+        string o = Template(@namespace, original);
+        string f = Template(@namespace, quickFix);
+
+        _output.WriteLine("Original:");
+        _output.WriteLine(o);
+        _output.WriteLine(string.Empty);
+        _output.WriteLine("Fixed:");
+        _output.WriteLine(f);
+
+        await Verifier.VerifyCodeFixAsync(o, f, referenceAssemblyGroup);
     }
 }