Moq1100 incorrectly firing on nullable parameters

src/Moq.Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs

@@ -58,31 +58,101 @@
 
         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++)
+            ValidateParameters(context, mockedMethodArguments, lambdaParameters);
+        }
+    }
+
+    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)
             {
-                TypeSyntax? lambdaParameterTypeSyntax = lambdaParameters[argumentIndex].Type;
+                continue;
+            }
 
-                // TODO: Don't know if continue or break is the right thing to do here
-                if (lambdaParameterTypeSyntax is null) continue;
+            TypeInfo lambdaParameterType = context.SemanticModel.GetTypeInfo(
+                lambdaParameterTypeSyntax,
+                context.CancellationToken);
+            TypeInfo mockedMethodArgumentType = context.SemanticModel.GetTypeInfo(
+                mockedMethodArguments[argumentIndex].Expression,
+                context.CancellationToken);
 
-                TypeInfo lambdaParameterType = context.SemanticModel.GetTypeInfo(lambdaParameterTypeSyntax, context.CancellationToken);
+            ITypeSymbol? lambdaParameterTypeSymbol = lambdaParameterType.Type;
+            ITypeSymbol? mockedMethodTypeSymbol = mockedMethodArgumentType.Type;
+
+            if (lambdaParameterTypeSymbol is null || mockedMethodTypeSymbol is null)
+            {
+                continue;
+            }
 
-                TypeInfo mockedMethodArgumentType = context.SemanticModel.GetTypeInfo(mockedMethodArguments[argumentIndex].Expression, context.CancellationToken);
+            if (!HasUserDefinedConversion(context.SemanticModel, mockedMethodTypeSymbol, lambdaParameterTypeSymbol))
+            {
+                Diagnostic diagnostic = lambdaParameters[argumentIndex].GetLocation().CreateDiagnostic(Rule);
+                context.ReportDiagnostic(diagnostic);
+            }
+        }
+    }
 
-                string? mockedMethodTypeName = mockedMethodArgumentType.ConvertedType?.ToString();
-                string? lambdaParameterTypeName = lambdaParameterType.ConvertedType?.ToString();
+    private static bool HasUserDefinedConversion(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.
+        Conversion conversion = semanticModel.Compilation.ClassifyConversion(source, destination);
+        bool conversionExists = conversion.Exists && (conversion.IsImplicit || conversion.IsExplicit || conversion.IsIdentity);
+
+        switch (conversionExists)
+        {
+            case true:
+                return true;
 
-                if (!string.Equals(mockedMethodTypeName, lambdaParameterTypeName, StringComparison.Ordinal))
+            // Fall back to seeking an implicit conversion operator
+            // The Conversion type will not show a user defined implicit conversion, so we have to seek it out explicitly
+            case false:
                 {
-                    Diagnostic diagnostic = callbackLambda.ParameterList.CreateDiagnostic(Rule);
-                    context.ReportDiagnostic(diagnostic);
+                    foreach (ISymbol? member in destination.GetMembers())
+                    {
+                        // Check for implicit conversion operator
+                        if (member is not IMethodSymbol { MethodKind: MethodKind.Conversion } method
+                            || !method.ReturnType.Equals(destination, SymbolEqualityComparer.IncludeNullability))
+                        {
+                            continue;
+                        }
+
+                        // Implicit conversion
+                        if (method is { IsImplicitlyDeclared: true, Parameters.Length: 1 } && method.Parameters[0].Type.Equals(source, SymbolEqualityComparer.IncludeNullability))
+                        {
+                            return true;
+                        }
+                    }
+
+                    break;
                 }
-            }
         }
+
+        return false;
     }
 }

src/Moq.Analyzers/SquiggleCop.Baseline.yaml

@@ -699,8 +699,8 @@
 - {Id: RCS1114FadeOut, Title: Remove redundant delegate creation, Category: Roslynator, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false}
 - {Id: RCS1118, Title: Mark local variable as const, Category: Roslynator, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false}
 - {Id: RCS1123, Title: Add parentheses when necessary, Category: Roslynator, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false}
-- {Id: RCS1124, Title: Inline local variable, Category: Roslynator, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false}
-- {Id: RCS1124FadeOut, Title: Inline local variable, Category: Roslynator, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false}
+- {Id: RCS1124, Title: Inline local variable, Category: Roslynator, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: true}
+- {Id: RCS1124FadeOut, Title: Inline local variable, Category: Roslynator, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: true}
 - {Id: RCS1126, Title: Add braces to if-else, Category: Roslynator, DefaultSeverity: Note, IsEnabledByDefault: false, EffectiveSeverities: [None], IsEverSuppressed: true}
 - {Id: RCS1128, Title: Use coalesce expression, Category: Roslynator, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false}
 - {Id: RCS1129, Title: Remove redundant field initialization, Category: Roslynator, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: true}

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,85 @@ 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[]> TestData2()
+    {
+        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 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);""",
+                ],
+            }.WithNamespaces().WithMoqReferenceAssemblyGroups();
+    }
+
+    [Theory]
+    [MemberData(nameof(TestData2))]
+    public async Task UserDefinedConversionOperator(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);
     }
 }