diff --git a/src/Moq.Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs b/src/Moq.Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs index ead2b8735..fc8b21f68 100644 --- a/src/Moq.Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs +++ b/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 mockedMethodArguments, + SeparatedSyntaxList 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); + } } diff --git a/tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodCodeFixTests.cs b/tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodCodeFixTests.cs index 60a3e9d6c..462bf89fa 100644 --- a/tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodCodeFixTests.cs +++ b/tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodCodeFixTests.cs @@ -1,9 +1,19 @@ +using Xunit.Abstractions; using Verifier = Moq.Analyzers.Test.Helpers.CodeFixVerifier; 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 TestData() { @@ -22,7 +32,7 @@ public static IEnumerable TestData() """new Mock().Setup(x => x.Do(It.IsAny>())).Returns((List l) => { return 0; });""", ], [ - """new Mock().Setup(x => x.Do(It.IsAny())).Callback({|Moq1100:(int i)|} => { });""", + """new Mock().Setup(x => x.Do(It.IsAny())).Callback(({|Moq1100:int i|}) => { });""", """new Mock().Setup(x => x.Do(It.IsAny())).Callback((string s) => { });""", ], [ @@ -34,7 +44,7 @@ public static IEnumerable TestData() """new Mock().Setup(x => x.Do(It.IsAny(), It.IsAny(), It.IsAny())).Callback((int i, string s, DateTime dt) => { });""", ], [ - """new Mock().Setup(x => x.Do(It.IsAny>())).Callback({|Moq1100:(int i)|} => { });""", + """new Mock().Setup(x => x.Do(It.IsAny>())).Callback(({|Moq1100:int i|}) => { });""", """new Mock().Setup(x => x.Do(It.IsAny>())).Callback((List l) => { });""", ], [ @@ -73,6 +83,30 @@ public static IEnumerable TestData() """new Mock().Setup(x => x.Do(It.IsAny>())).Returns(0).Callback((List l) => { });""", """new Mock().Setup(x => x.Do(It.IsAny>())).Returns(0).Callback((List l) => { });""", ], + [ // Repros for https://github.com/rjmurillo/moq.analyzers/issues/172 + """new Mock().Setup(m => m.Do(It.IsAny())).Returns((object? bar) => true);""", + """new Mock().Setup(m => m.Do(It.IsAny())).Returns((object? bar) => true);""", + ], + [ + """new Mock().Setup(m => m.Do(42)).Returns((long bar) => true);""", + """new Mock().Setup(m => m.Do(42)).Returns((long bar) => true);""", + ], + [ + """new Mock().Setup(m => m.Do((long)42)).Returns((long bar) => true);""", + """new Mock().Setup(m => m.Do((long)42)).Returns((long bar) => true);""", + ], + [ + """new Mock().Setup(m => m.Do((long)42)).Returns((long bar) => true);""", + """new Mock().Setup(m => m.Do((long)42)).Returns((long bar) => true);""", + ], + [ + """new Mock().Setup(m => m.Do(42)).Returns((object? bar) => true);""", + """new Mock().Setup(m => m.Do(42)).Returns((object? bar) => true);""", + ], + [ // This was also reported as part of 172, but is a different error + """new Mock().Setup(m => m.Do(42)).Returns((int bar) => true);""", + """new Mock().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 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 ConversionTestData() + { + return new object[][] + { + [ // This should be allowed because of the implicit conversion from int to CustomType + """new Mock().Setup(x => x.Do(42)).Returns((CustomType i) => true);""", + """new Mock().Setup(x => x.Do(42)).Returns((CustomType i) => true);""", + ], + [ // This should be allowed because of identity + """new Mock().Setup(x => x.Do(new CustomType(42))).Returns((CustomType i) => true);""", + """new Mock().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().Setup(x => x.Do((CustomType)"42")).Returns((CustomType i) => true);""", + """new Mock().Setup(x => x.Do((CustomType)"42")).Returns((CustomType i) => true);""", + ], + [ // This should be allowed because of numeric conversion (explicit) + """new Mock().Setup(x => x.Do((int)42L)).Returns((CustomType i) => true);""", + """new Mock().Setup(x => x.Do((int)42L)).Returns((CustomType i) => true);""", + ], + [ // This should be allowed because of numeric conversion (explicit) + """new Mock().Setup(x => x.Do((int)42.0)).Returns((CustomType i) => true);""", + """new Mock().Setup(x => x.Do((int)42.0)).Returns((CustomType i) => true);""", + ], + [ + """new Mock().Setup(m => m.Do(It.IsAny())).Returns((CustomType i) => true);""", + """new Mock().Setup(m => m.Do(It.IsAny())).Returns((CustomType i) => true);""", + ], + [ + """new Mock().Setup(m => m.Do(It.IsAny())).Returns((CustomType i) => true);""", + """new Mock().Setup(m => m.Do(It.IsAny())).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); } }