diff --git a/src/Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs b/src/Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs index 3fc3475ab..06a1f8458 100644 --- a/src/Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs +++ b/src/Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs @@ -80,27 +80,31 @@ private static void Analyze(OperationAnalysisContext context, MoqKnownSymbols kn return; } - ParenthesizedLambdaExpressionSyntax? callbackLambda = TryGetCallbackLambda(callbackOrReturnsInvocation); + LambdaExpressionSyntax? callbackLambda = TryGetCallbackLambda(callbackOrReturnsInvocation); if (callbackLambda == null) { return; } // Ignoring calls with no arguments because those are valid in Moq - SeparatedSyntaxList lambdaParameters = callbackLambda.ParameterList.Parameters; + SeparatedSyntaxList lambdaParameters = GetLambdaParameters(callbackLambda); if (lambdaParameters.Count == 0) { return; } InvocationExpressionSyntax? setupInvocation = semanticModel.FindSetupMethodFromCallbackInvocation(knownSymbols, callbackOrReturnsInvocation, context.CancellationToken); + if (setupInvocation is null) + { + return; + } ValidateCallbackAgainstSetup(context, semanticModel, setupInvocation, callbackLambda, lambdaParameters); } - private static ParenthesizedLambdaExpressionSyntax? TryGetCallbackLambda(InvocationExpressionSyntax callbackOrReturnsInvocation) + private static LambdaExpressionSyntax? TryGetCallbackLambda(InvocationExpressionSyntax callbackOrReturnsInvocation) { - if (callbackOrReturnsInvocation.ArgumentList.Arguments[0]?.Expression is ParenthesizedLambdaExpressionSyntax directLambda) + if (callbackOrReturnsInvocation.ArgumentList.Arguments[0]?.Expression is LambdaExpressionSyntax directLambda) { return directLambda; } @@ -113,23 +117,34 @@ private static void Analyze(OperationAnalysisContext context, MoqKnownSymbols kn } // Extract the lambda from the delegate constructor (support both parenthesized and simple lambdas) - LambdaExpressionSyntax? lambdaExpression = delegateConstructor.ArgumentList!.Arguments[0]?.Expression as LambdaExpressionSyntax; + return delegateConstructor.ArgumentList!.Arguments[0]?.Expression as LambdaExpressionSyntax; + } - // Simple lambdas are currently skipped to avoid handling edge cases and maintain simplicity. - // TODO(#1012): Implement support for SimpleLambdaExpressionSyntax in delegate constructors. - if (lambdaExpression is SimpleLambdaExpressionSyntax) + private static SeparatedSyntaxList GetLambdaParameters(LambdaExpressionSyntax lambda) + { + return lambda switch { - return null; - } + ParenthesizedLambdaExpressionSyntax parenthesized => parenthesized.ParameterList.Parameters, + SimpleLambdaExpressionSyntax simple => SyntaxFactory.SingletonSeparatedList(simple.Parameter), + _ => default, + }; + } - return lambdaExpression as ParenthesizedLambdaExpressionSyntax; + private static SyntaxNode GetDiagnosticNode(LambdaExpressionSyntax lambda) + { + return lambda switch + { + ParenthesizedLambdaExpressionSyntax parenthesized => parenthesized.ParameterList, + SimpleLambdaExpressionSyntax simple => simple.Parameter, + _ => lambda, + }; } private static void ValidateCallbackAgainstSetup( OperationAnalysisContext context, SemanticModel semanticModel, - InvocationExpressionSyntax? setupInvocation, - ParenthesizedLambdaExpressionSyntax callbackLambda, + InvocationExpressionSyntax setupInvocation, + LambdaExpressionSyntax callbackLambda, SeparatedSyntaxList lambdaParameters) { InvocationExpressionSyntax? mockedMethodInvocation = setupInvocation.FindMockedMethodInvocationFromSetupMethod(); @@ -143,7 +158,7 @@ private static void ValidateCallbackAgainstSetup( if (mockedMethodArguments.Count != lambdaParameters.Count) { - Diagnostic diagnostic = callbackLambda.ParameterList.CreateDiagnostic(Rule, methodName); + Diagnostic diagnostic = GetDiagnosticNode(callbackLambda).CreateDiagnostic(Rule, methodName); context.ReportDiagnostic(diagnostic); } else diff --git a/src/CodeFixes/CallbackSignatureShouldMatchMockedMethodFixer.cs b/src/CodeFixes/CallbackSignatureShouldMatchMockedMethodFixer.cs index 9201c7c32..25d61968a 100644 --- a/src/CodeFixes/CallbackSignatureShouldMatchMockedMethodFixer.cs +++ b/src/CodeFixes/CallbackSignatureShouldMatchMockedMethodFixer.cs @@ -1,4 +1,4 @@ -using System.Composition; +using System.Composition; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.Text; @@ -12,6 +12,8 @@ namespace Moq.CodeFixes; [Shared] public class CallbackSignatureShouldMatchMockedMethodFixer : CodeFixProvider { + private static readonly string FixTitle = "Fix Moq callback signature"; + /// public sealed override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(DiagnosticIds.BadCallbackParameters); @@ -31,66 +33,166 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) Diagnostic diagnostic = context.Diagnostics.First(); TextSpan diagnosticSpan = diagnostic.Location.SourceSpan; - // Find the type declaration identified by the diagnostic. - ParameterListSyntax? badArgumentListSyntax = root.FindToken(diagnosticSpan.Start) - .Parent? + SyntaxNode? node = root.FindToken(diagnosticSpan.Start).Parent; + + // Try parenthesized lambda path first (diagnostic on ParameterListSyntax). + ParameterListSyntax? badArgumentListSyntax = node? .AncestorsAndSelf() .OfType() .FirstOrDefault(); - if (badArgumentListSyntax is null) + if (badArgumentListSyntax is not null) { + ParenthesizedLambdaExpressionSyntax? parenthesizedLambda = badArgumentListSyntax + .FirstAncestorOrSelf(); + + context.RegisterCodeFix( + CodeAction.Create( + FixTitle, + cancellationToken => FixParenthesizedCallbackSignatureAsync(root, context.Document, badArgumentListSyntax, parenthesizedLambda, cancellationToken), + FixTitle), + diagnostic); return; } - // Register a code action that will invoke the fix. - context.RegisterCodeFix( - CodeAction.Create( - "Fix Moq callback signature", - cancellationToken => FixCallbackSignatureAsync(root, context.Document, badArgumentListSyntax, cancellationToken), - "Fix Moq callback signature"), - diagnostic); + // SimpleLambdaExpressionSyntax nodes never contain a ParameterListSyntax, + // so the ancestor search above intentionally falls through to this path. + SimpleLambdaExpressionSyntax? simpleLambda = node? + .AncestorsAndSelf() + .OfType() + .FirstOrDefault(); + + if (simpleLambda is not null && !IsInsideDelegateConstructor(simpleLambda)) + { + context.RegisterCodeFix( + CodeAction.Create( + FixTitle, + cancellationToken => FixSimpleLambdaCallbackSignatureAsync(root, context.Document, simpleLambda, cancellationToken), + FixTitle), + diagnostic); + } } - private static async Task FixCallbackSignatureAsync(SyntaxNode root, Document document, ParameterListSyntax? oldParameters, CancellationToken cancellationToken) + private static async Task FixParenthesizedCallbackSignatureAsync(SyntaxNode root, Document document, ParameterListSyntax oldParameters, ParenthesizedLambdaExpressionSyntax? parentLambda, CancellationToken cancellationToken) { - SemanticModel? semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + if (parentLambda is not null && IsInsideDelegateConstructor(parentLambda)) + { + return document; + } - if (semanticModel is null) + ParameterListSyntax? newParameters = await ResolveNewParameterListAsync(document, oldParameters, oldParameters.SpanStart, oldParameters.Parameters, cancellationToken).ConfigureAwait(false); + if (newParameters is null) { return document; } - MoqKnownSymbols knownSymbols = new(semanticModel.Compilation); + SyntaxNode newRoot = root.ReplaceNode(oldParameters, newParameters); + return document.WithSyntaxRoot(newRoot); + } - if (oldParameters?.Parent?.Parent?.Parent?.Parent is not InvocationExpressionSyntax callbackInvocation) + private static async Task FixSimpleLambdaCallbackSignatureAsync(SyntaxNode root, Document document, SimpleLambdaExpressionSyntax simpleLambda, CancellationToken cancellationToken) + { + // No delegate-constructor guard needed here; RegisterCodeFixesAsync + // already filters out simple lambdas inside delegate constructors. + SeparatedSyntaxList originalParams = SyntaxFactory.SingletonSeparatedList(simpleLambda.Parameter); + ParameterListSyntax? newParameters = await ResolveNewParameterListAsync(document, simpleLambda, simpleLambda.SpanStart, originalParams, cancellationToken).ConfigureAwait(false); + if (newParameters is null) { return document; } + ParenthesizedLambdaExpressionSyntax parenthesizedLambda = SyntaxFactory.ParenthesizedLambdaExpression( + simpleLambda.AsyncKeyword, + newParameters, + simpleLambda.ArrowToken, + simpleLambda.Block, + simpleLambda.ExpressionBody) + .WithTriviaFrom(simpleLambda); + + SyntaxNode newRoot = root.ReplaceNode(simpleLambda, parenthesizedLambda); + return document.WithSyntaxRoot(newRoot); + } + + private static async Task ResolveNewParameterListAsync(Document document, SyntaxNode lambdaNode, int position, SeparatedSyntaxList originalParameters, CancellationToken cancellationToken) + { + SemanticModel? semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + if (semanticModel is null) + { + return null; + } + + MoqKnownSymbols knownSymbols = new(semanticModel.Compilation); + + InvocationExpressionSyntax? callbackInvocation = lambdaNode + .Ancestors() + .OfType() + .FirstOrDefault(); + + if (callbackInvocation is null) + { + return null; + } + + IMethodSymbol? mockedMethod = FindSingleMockedMethod(semanticModel, knownSymbols, callbackInvocation, cancellationToken); + if (mockedMethod is null) + { + return null; + } + + return BuildParameterList(semanticModel, mockedMethod, position, originalParameters); + } + + private static bool IsInsideDelegateConstructor(LambdaExpressionSyntax lambda) + { + return lambda.Parent is ArgumentSyntax + { + Parent: ArgumentListSyntax + { + Parent: BaseObjectCreationExpressionSyntax + } + }; + } + + private static IMethodSymbol? FindSingleMockedMethod(SemanticModel semanticModel, MoqKnownSymbols knownSymbols, InvocationExpressionSyntax callbackInvocation, CancellationToken cancellationToken) + { InvocationExpressionSyntax? setupMethodInvocation = semanticModel.FindSetupMethodFromCallbackInvocation(knownSymbols, callbackInvocation, cancellationToken); if (setupMethodInvocation is null) { - return document; + return null; } - IMethodSymbol[] matchingMockedMethods = semanticModel.GetAllMatchingMockedMethodSymbolsFromSetupMethodInvocation(setupMethodInvocation).ToArray(); + // Short-circuit: we only need to know if there is exactly one match. + IMethodSymbol[] matchingMockedMethods = semanticModel.GetAllMatchingMockedMethodSymbolsFromSetupMethodInvocation(setupMethodInvocation).Take(2).ToArray(); if (matchingMockedMethods.Length != 1) { - return document; + return null; } - ParameterListSyntax newParameters = SyntaxFactory.ParameterList(SyntaxFactory.SeparatedList(matchingMockedMethods[0].Parameters.Select( - parameterSymbol => - { - TypeSyntax type = SyntaxFactory.ParseTypeName(parameterSymbol.Type.ToMinimalDisplayString(semanticModel, oldParameters.SpanStart)); - SyntaxTokenList modifiers = GetParameterModifiers(parameterSymbol.RefKind); - return SyntaxFactory.Parameter(default, modifiers, type, SyntaxFactory.Identifier(parameterSymbol.Name), null); - }))); + return matchingMockedMethods[0]; + } - SyntaxNode newRoot = root.ReplaceNode(oldParameters, newParameters); - return document.WithSyntaxRoot(newRoot); + private static ParameterListSyntax BuildParameterList(SemanticModel semanticModel, IMethodSymbol mockedMethod, int position, SeparatedSyntaxList originalParameters) + { + ImmutableArray parameters = mockedMethod.Parameters; + ParameterSyntax[] result = new ParameterSyntax[parameters.Length]; + + for (int index = 0; index < parameters.Length; index++) + { + IParameterSymbol parameterSymbol = parameters[index]; + TypeSyntax type = SyntaxFactory.ParseTypeName(parameterSymbol.Type.ToMinimalDisplayString(semanticModel, position)); + SyntaxTokenList modifiers = GetParameterModifiers(parameterSymbol.RefKind); + + string name = index < originalParameters.Count + ? originalParameters[index].Identifier.ValueText + : parameterSymbol.Name; + + SyntaxToken identifier = SyntaxFactory.Identifier(name); + + result[index] = SyntaxFactory.Parameter(default, modifiers, type, identifier, null); + } + + return SyntaxFactory.ParameterList(SyntaxFactory.SeparatedList(result)); } private static SyntaxTokenList GetParameterModifiers(RefKind refKind) @@ -100,7 +202,6 @@ private static SyntaxTokenList GetParameterModifiers(RefKind refKind) RefKind.Ref => SyntaxFactory.TokenList(SyntaxFactory.Token(SyntaxKind.RefKeyword)), RefKind.Out => SyntaxFactory.TokenList(SyntaxFactory.Token(SyntaxKind.OutKeyword)), RefKind.In => SyntaxFactory.TokenList(SyntaxFactory.Token(SyntaxKind.InKeyword)), - RefKind.None => SyntaxFactory.TokenList(), _ => SyntaxFactory.TokenList(), }; } diff --git a/src/Common/SemanticModelExtensions.cs b/src/Common/SemanticModelExtensions.cs index cc0fb39a2..c898e941d 100644 --- a/src/Common/SemanticModelExtensions.cs +++ b/src/Common/SemanticModelExtensions.cs @@ -23,12 +23,25 @@ internal static class SemanticModelExtensions } SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(method, cancellationToken); - if (symbolInfo.Symbol is null) + ISymbol? resolvedSymbol = symbolInfo.Symbol; + + // When overload resolution fails (e.g. untyped simple lambda argument), + // fall back to candidate symbols so the walk can continue up the chain. + if (resolvedSymbol is null) { - return null; + // CandidateSymbols suffice here because downstream consumers + // re-resolve the mocked method from the Setup lambda body independently. + if (symbolInfo.CandidateReason == CandidateReason.OverloadResolutionFailure + && symbolInfo.CandidateSymbols.Any(s => s.IsMoqSetupMethod(knownSymbols))) + { + return invocation; + } + + expression = method.Expression; + continue; } - if (symbolInfo.Symbol.IsMoqSetupMethod(knownSymbols)) + if (resolvedSymbol.IsMoqSetupMethod(knownSymbols)) { return invocation; } diff --git a/tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodAnalyzerTests.cs b/tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodAnalyzerTests.cs index 38f449bd2..2dc220835 100644 --- a/tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodAnalyzerTests.cs @@ -57,6 +57,15 @@ public static IEnumerable CallbackValidationData() // Explicitly typed lambda with correct parameter type (exercises GetDeclaredSymbol path) ["""new Mock().Setup(x => x.DoWork("test")).Callback((string x) => { });"""], + + // Simple lambda in delegate constructor with correct type (issue #1012) + ["""new Mock().Setup(x => x.DoWork("test")).Callback(new Action(x => { }));"""], + + // Simple lambda in delegate constructor with correct type using Returns (issue #1012) + ["""new Mock().Setup(x => x.DoWork("test")).Returns(new Func(x => 42));"""], + + // Parenthesized lambda in delegate constructor with correct type (issue #1012) + ["""new Mock().Setup(x => x.DoWork("test")).Callback(new Action((string x) => { }));"""], }.WithNamespaces().WithMoqReferenceAssemblyGroups(); // Invalid patterns that SHOULD trigger the analyzer @@ -82,6 +91,18 @@ public static IEnumerable CallbackValidationData() // Explicitly typed lambda with wrong parameter type (exercises semantic resolution mismatch) ["""new Mock().Setup(x => x.DoWork("test")).Callback(({|Moq1100:DateTime wrongParam|}) => { });"""], + + // Simple lambda in delegate constructor with wrong type (issue #1012) + ["""new Mock().Setup(x => x.DoWork("test")).Callback(new Action({|Moq1100:x|} => { }));"""], + + // Simple lambda in delegate constructor with argument count mismatch (issue #1012) + ["""new Mock().Setup(x => x.ProcessMultiple(It.IsAny(), It.IsAny(), It.IsAny())).Callback(new Action({|Moq1100:x|} => { }));"""], + + // Parenthesized lambda in delegate constructor with wrong type (issue #1012) + ["""new Mock().Setup(x => x.DoWork("test")).Callback(new Action(({|Moq1100:int x|}) => { }));"""], + + // Parenthesized lambda in delegate constructor with wrong argument count (issue #1012) + ["""new Mock().Setup(x => x.ProcessMultiple(It.IsAny(), It.IsAny(), It.IsAny())).Callback(new Action({|Moq1100:(int x)|} => { }));"""], }.WithNamespaces().WithMoqReferenceAssemblyGroups(); return validPatterns.Concat(invalidPatterns); diff --git a/tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodCodeFixTests.cs b/tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodCodeFixTests.cs index 395211f86..849429a90 100644 --- a/tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodCodeFixTests.cs +++ b/tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodCodeFixTests.cs @@ -1,5 +1,6 @@ using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Testing; using Microsoft.CodeAnalysis.Text; using AnalyzerVerifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier; using Verifier = Moq.Analyzers.Test.Helpers.CodeFixVerifier; @@ -36,19 +37,19 @@ public static IEnumerable TestData() ], [ """new Mock().Setup(x => x.Do(It.IsAny())).Callback(({|Moq1100:int i|}) => { });""", - """new Mock().Setup(x => x.Do(It.IsAny())).Callback((string s) => { });""", + """new Mock().Setup(x => x.Do(It.IsAny())).Callback((string i) => { });""", ], [ """new Mock().Setup(x => x.Do(It.IsAny())).Callback({|Moq1100:(string s1, string s2)|} => { });""", - """new Mock().Setup(x => x.Do(It.IsAny())).Callback((string s) => { });""", + """new Mock().Setup(x => x.Do(It.IsAny())).Callback((string s1) => { });""", ], [ """new Mock().Setup(x => x.Do(It.IsAny(), It.IsAny(), It.IsAny())).Callback({|Moq1100:(string s1, int i1)|} => { });""", - """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(), It.IsAny(), It.IsAny())).Callback((int s1, string i1, DateTime dt) => { });""", ], [ """new Mock().Setup(x => x.Do(It.IsAny>())).Callback(({|Moq1100:int i|}) => { });""", - """new Mock().Setup(x => x.Do(It.IsAny>())).Callback((List l) => { });""", + """new Mock().Setup(x => x.Do(It.IsAny>())).Callback((List i) => { });""", ], [ """new Mock().Setup(x => x.Do(It.IsAny())).Callback((string s) => { });""", @@ -112,11 +113,11 @@ public static IEnumerable TestData() ], [ // Parenthesized Setup with wrong callback type """(new Mock().Setup(x => x.Do(It.IsAny()))).Callback(({|Moq1100:int i|}) => { });""", - """(new Mock().Setup(x => x.Do(It.IsAny()))).Callback((string s) => { });""", + """(new Mock().Setup(x => x.Do(It.IsAny()))).Callback((string i) => { });""", ], [ // Double-parenthesized Setup with wrong callback type """((new Mock().Setup(x => x.Do(It.IsAny())))).Callback(({|Moq1100:int i|}) => { });""", - """((new Mock().Setup(x => x.Do(It.IsAny())))).Callback((string s) => { });""", + """((new Mock().Setup(x => x.Do(It.IsAny())))).Callback((string i) => { });""", ], }.WithNamespaces().WithMoqReferenceAssemblyGroups(); } @@ -125,36 +126,8 @@ public static IEnumerable TestData() [MemberData(nameof(TestData))] public async Task ShouldSuggestQuickFixWhenIncorrectCallbacks(string referenceAssemblyGroup, string @namespace, string original, string quickFix) { - static string Template(string ns, string mock) => - $$""" - {{ns}} - - internal interface IFoo - { - int Do(string s); - - int Do(int i, string s, DateTime dt); - - int Do(List l); - - bool Do(object? bar); - - bool Do(long bar); - } - - internal delegate void StringDelegate(string s); - - internal class UnitTest - { - private void Test() - { - {{mock}} - } - } - """; - - string o = Template(@namespace, original); - string f = Template(@namespace, quickFix); + string o = CallbackTemplate(@namespace, original); + string f = CallbackTemplate(@namespace, quickFix); _output.WriteLine("Original:"); _output.WriteLine(o); @@ -255,6 +228,209 @@ private void Test() await Verifier.VerifyCodeFixAsync(o, f, referenceAssemblyGroup); } + public static IEnumerable SimpleLambdaTestData() + { + return new object[][] + { + [ // Simple lambda in delegate constructor with wrong type bails out (issue #1012) + """new Mock().Setup(x => x.Do(It.IsAny())).Callback(new Action({|Moq1100:x|} => { }));""", + """new Mock().Setup(x => x.Do(It.IsAny())).Callback(new Action({|Moq1100:x|} => { }));""", + ], + [ // Simple lambda in delegate constructor with wrong parameter count bails out (issue #1012) + """new Mock().Setup(x => x.Do(It.IsAny(), It.IsAny(), It.IsAny())).Callback(new Action({|Moq1100:x|} => { }));""", + """new Mock().Setup(x => x.Do(It.IsAny(), It.IsAny(), It.IsAny())).Callback(new Action({|Moq1100:x|} => { }));""", + ], + [ // Expression-bodied simple lambda in delegate constructor bails out + """new Mock().Setup(x => x.Do(It.IsAny())).Callback(new Action({|Moq1100:x|} => x.ToString()));""", + """new Mock().Setup(x => x.Do(It.IsAny())).Callback(new Action({|Moq1100:x|} => x.ToString()));""", + ], + }.WithNamespaces().WithMoqReferenceAssemblyGroups(); + } + + // Direct simple lambda fixer tests (FixerConvertsDirectSimpleLambda*ToParenthesized) + // verify the happy path. They use synthetic diagnostics because the compiler cannot + // infer the delegate type for untyped simple lambdas, preventing the analyzer from + // reporting Moq1100. The compilable source provides a valid semantic model while + // the fixer input uses the simple lambda form. + [Theory] + [MemberData(nameof(SimpleLambdaTestData))] + public async Task ShouldFixSimpleLambdaCallbackSignature(string referenceAssemblyGroup, string @namespace, string original, string quickFix) + { + string o = CallbackTemplate(@namespace, original); + string f = CallbackTemplate(@namespace, quickFix); + + _output.WriteLine("Original:"); + _output.WriteLine(o); + _output.WriteLine(string.Empty); + _output.WriteLine("Fixed:"); + _output.WriteLine(f); + + // The fixer bails out for simple lambdas inside delegate constructors, + // so no fix iterations occur. Compiler diagnostics are suppressed because + // the delegate constructor type intentionally mismatches the lambda signature. + await Verifier.VerifyCodeFixAsync(o, f, referenceAssemblyGroup, numberOfIncrementalIterations: 0, numberOfFixAllIterations: 0, CompilerDiagnostics.None); + } + + [Fact] + public async Task FixerConvertsDirectSimpleLambdaBlockToParenthesized() + { + const string compilableSource = + """ + using System; + using System.Collections.Generic; + using Moq; + + internal interface IFoo + { + int Do(string s); + } + + internal class UnitTest + { + private void Test() + { + new Mock().Setup(x => x.Do(It.IsAny())).Callback((string x) => { }); + } + } + """; + + await VerifySimpleLambdaConversionAsync(compilableSource, "(string x) => { }", "Callback(x => { })"); + } + + [Fact] + public async Task FixerConvertsDirectSimpleLambdaExpressionToParenthesized() + { + const string compilableSource = + """ + using System; + using System.Collections.Generic; + using Moq; + + internal interface IFoo + { + int Do(string s); + } + + internal class UnitTest + { + private void Test() + { + new Mock().Setup(x => x.Do(It.IsAny())).Callback((string x) => x.ToString()); + } + } + """; + + await VerifySimpleLambdaConversionAsync(compilableSource, "(string x) => x.ToString()", "Callback(x => x.ToString())"); + } + + [Fact] + public async Task FixerSkipsSimpleLambdaInsideDelegateConstructor() + { + // When a simple lambda is inside a delegate constructor, the fixer + // should NOT register a code action (it bails out). + const string source = + """ + using System; + using System.Collections.Generic; + using Moq; + + internal interface IFoo + { + int Do(string s); + } + + internal class UnitTest + { + private void Test() + { + new Mock().Setup(x => x.Do(It.IsAny())).Callback(new Action(x => { })); + } + } + """; + + (SemanticModel model, SyntaxTree tree) = await CompilationHelper.CreateMoqCompilationAsync(source); + SyntaxNode root = await tree.GetRootAsync(); + + SimpleLambdaExpressionSyntax simpleLambda = root + .DescendantNodes() + .OfType() + .Last(); + + Diagnostic diagnostic = CreateSyntheticDiagnosticAtSpan(tree, simpleLambda.Parameter.Span); + using AdhocWorkspace workspace = new(); + Document document = CreateTestDocument(workspace, model, source); + List actions = await InvokeFixerAsync(document, diagnostic); + + // Fixer should NOT register a code action for simple lambdas in delegate constructors. + Assert.Empty(actions); + } + + [Fact] + public async Task FixerSkipsParenthesizedLambdaInsideDelegateConstructor() + { + const string source = + """ + using System; + using System.Collections.Generic; + using Moq; + + internal interface IFoo + { + int Do(string s); + } + + internal class UnitTest + { + private void Test() + { + new Mock().Setup(x => x.Do(It.IsAny())).Callback(new Action((int x) => { })); + } + } + """; + + (SemanticModel model, SyntaxTree tree) = await CompilationHelper.CreateMoqCompilationAsync(source); + SyntaxNode root = await tree.GetRootAsync(); + + ParenthesizedLambdaExpressionSyntax parenthesizedLambda = root + .DescendantNodes() + .OfType() + .Last(); + + Diagnostic diagnostic = CreateSyntheticDiagnosticAtSpan(tree, parenthesizedLambda.ParameterList.Span); + using AdhocWorkspace workspace = new(); + Document document = CreateTestDocument(workspace, model, source); + List actions = await InvokeFixerAsync(document, diagnostic); + + Assert.Single(actions); + await AssertDocumentUnchangedAsync(actions[0], document); + } + + [Fact] + public async Task FixerConvertsDirectSimpleLambdaForReturns() + { + const string compilableSource = + """ + using System; + using System.Collections.Generic; + using Moq; + + internal interface IFoo + { + int Do(string s); + } + + internal class UnitTest + { + private void Test() + { + new Mock().Setup(x => x.Do(It.IsAny())).Returns((string x) => 42); + } + } + """; + + await VerifySimpleLambdaConversionAsync(compilableSource, "(string x) => 42", "Returns(x => 42)"); + } + [Theory] [MemberData(nameof(DoppelgangerTestHelper.GetAllCustomMockData), MemberType = typeof(DoppelgangerTestHelper))] public async Task ShouldPassIfCustomMockClassIsUsed(string mockCode) @@ -303,6 +479,77 @@ private void Test() await AssertDocumentUnchangedAsync(actions[0], document); } + private static string CallbackTemplate(string ns, string mock) => + $$""" + {{ns}} + + internal interface IFoo + { + int Do(string s); + + int Do(int i, string s, DateTime dt); + + int Do(List l); + + bool Do(object? bar); + + bool Do(long bar); + } + + internal delegate void StringDelegate(string s); + + internal class UnitTest + { + private void Test() + { + {{mock}} + } + } + """; + + /// + /// Verifies that the fixer converts a simple lambda to a parenthesized lambda + /// with correct types from the mocked method signature. + /// + private static async Task VerifySimpleLambdaConversionAsync(string compilableSource, string expectedFragment, string unexpectedFragment) + { + (SemanticModel model, SyntaxTree _) = await CompilationHelper.CreateMoqCompilationAsync(compilableSource).ConfigureAwait(false); + using AdhocWorkspace workspace = new(); + Document compilableDoc = CreateTestDocument(workspace, model, compilableSource); + + SyntaxNode compilableRoot = (await compilableDoc.GetSyntaxRootAsync().ConfigureAwait(false))!; + ParenthesizedLambdaExpressionSyntax parenthesizedLambda = compilableRoot + .DescendantNodes() + .OfType() + .Last(); + + SimpleLambdaExpressionSyntax simpleLambda = SyntaxFactory.SimpleLambdaExpression( + SyntaxFactory.Parameter(SyntaxFactory.Identifier("x")), + parenthesizedLambda.Block, + parenthesizedLambda.ExpressionBody) + .WithArrowToken(parenthesizedLambda.ArrowToken) + .WithTriviaFrom(parenthesizedLambda); + + SyntaxNode modifiedRoot = compilableRoot.ReplaceNode(parenthesizedLambda, simpleLambda); + Document modifiedDoc = compilableDoc.WithSyntaxRoot(modifiedRoot); + + SyntaxNode modifiedSyntaxRoot = (await modifiedDoc.GetSyntaxRootAsync().ConfigureAwait(false))!; + SimpleLambdaExpressionSyntax targetLambda = modifiedSyntaxRoot + .DescendantNodes() + .OfType() + .Last(); + + Diagnostic diagnostic = CreateSyntheticDiagnosticAtSpan(modifiedSyntaxRoot.SyntaxTree, targetLambda.Parameter.Span); + List actions = await InvokeFixerAsync(modifiedDoc, diagnostic).ConfigureAwait(false); + + Assert.Single(actions); + + string changedText = await GetChangedTextAsync(actions[0], modifiedDoc).ConfigureAwait(false); + + Assert.Contains(expectedFragment, changedText, StringComparison.Ordinal); + Assert.DoesNotContain(unexpectedFragment, changedText, StringComparison.Ordinal); + } + private static Diagnostic CreateSyntheticDiagnostic(SyntaxNode root, SyntaxTree tree) { ParameterListSyntax parameterList = root @@ -311,6 +558,11 @@ private static Diagnostic CreateSyntheticDiagnostic(SyntaxNode root, SyntaxTree .Last() .ParameterList; + return CreateSyntheticDiagnosticAtSpan(tree, parameterList.Span); + } + + private static Diagnostic CreateSyntheticDiagnosticAtSpan(SyntaxTree tree, TextSpan span) + { DiagnosticDescriptor descriptor = new( DiagnosticIds.BadCallbackParameters, "Bad callback parameters", @@ -319,7 +571,7 @@ private static Diagnostic CreateSyntheticDiagnostic(SyntaxNode root, SyntaxTree DiagnosticSeverity.Warning, isEnabledByDefault: true); - return Diagnostic.Create(descriptor, Location.Create(tree, parameterList.Span)); + return Diagnostic.Create(descriptor, Location.Create(tree, span)); } private static Document CreateTestDocument(AdhocWorkspace workspace, SemanticModel model, string source) @@ -346,17 +598,16 @@ private static async Task> InvokeFixerAsync(Document document, private static async Task AssertDocumentUnchangedAsync(CodeAction action, Document document) { - ImmutableArray operations = await action.GetOperationsAsync(CancellationToken.None).ConfigureAwait(false); - ApplyChangesOperation? applyChanges = operations.OfType().FirstOrDefault(); - - if (applyChanges is null) - { - return; - } - - Document changedDocument = applyChanges.ChangedSolution.GetDocument(document.Id)!; string originalText = (await document.GetTextAsync(CancellationToken.None).ConfigureAwait(false)).ToString(); - string changedText = (await changedDocument.GetTextAsync(CancellationToken.None).ConfigureAwait(false)).ToString(); + string changedText = await GetChangedTextAsync(action, document).ConfigureAwait(false); Assert.Equal(originalText, changedText); } + + private static async Task GetChangedTextAsync(CodeAction action, Document document) + { + ImmutableArray operations = await action.GetOperationsAsync(CancellationToken.None).ConfigureAwait(false); + ApplyChangesOperation applyChanges = Assert.Single(operations.OfType()); + Document changedDocument = applyChanges.ChangedSolution.GetDocument(document.Id)!; + return (await changedDocument.GetTextAsync(CancellationToken.None).ConfigureAwait(false)).ToString(); + } } diff --git a/tests/Moq.Analyzers.Test/Common/SemanticModelExtensionsTests.cs b/tests/Moq.Analyzers.Test/Common/SemanticModelExtensionsTests.cs index 11fd59c67..f64d51893 100644 --- a/tests/Moq.Analyzers.Test/Common/SemanticModelExtensionsTests.cs +++ b/tests/Moq.Analyzers.Test/Common/SemanticModelExtensionsTests.cs @@ -751,6 +751,42 @@ public void M() Assert.False(result); } + [Fact] + public async Task FindSetupMethodFromCallbackInvocation_WithOverloadResolutionFailure_FallsBackToCandidate() + { + // An untyped simple lambda parameter makes overload resolution ambiguous, + // causing GetSymbolInfo to return CandidateSymbols instead of Symbol. + // The method should fall back to candidates and still find the Setup invocation. + const string code = @" +using System; +using Moq; +public interface IFoo { int Bar(string s); } +public class C +{ + public void M() + { + new Mock().Setup(x => x.Bar(It.IsAny())).Callback(x => { }); + } +}"; + (SemanticModel model, SyntaxTree tree) = await CompilationHelper.CreateMoqCompilationAsync(code); + MoqKnownSymbols knownSymbols = new MoqKnownSymbols(model.Compilation); + SyntaxNode root = await tree.GetRootAsync(); + + // Find the Callback invocation. With an untyped simple lambda, overload + // resolution for .Callback(x => { }) fails, producing CandidateSymbols. + InvocationExpressionSyntax callbackInvocation = root + .DescendantNodes().OfType() + .First(i => i.Expression is MemberAccessExpressionSyntax ma + && string.Equals(ma.Name.Identifier.Text, "Callback", StringComparison.Ordinal)); + + InvocationExpressionSyntax? setupInvocation = model.FindSetupMethodFromCallbackInvocation( + knownSymbols, callbackInvocation, CancellationToken.None); + + Assert.NotNull(setupInvocation); + MemberAccessExpressionSyntax setupAccess = (MemberAccessExpressionSyntax)setupInvocation!.Expression; + Assert.Equal("Setup", setupAccess.Name.Identifier.Text); + } + private static (SemanticModel Model, ITypeSymbol FirstType, ITypeSymbol SecondType) GetTwoVariableTypes( string code, string firstName, diff --git a/tests/Moq.Analyzers.Test/Helpers/CodeFixVerifier.cs b/tests/Moq.Analyzers.Test/Helpers/CodeFixVerifier.cs index 9004adb1c..f52490090 100644 --- a/tests/Moq.Analyzers.Test/Helpers/CodeFixVerifier.cs +++ b/tests/Moq.Analyzers.Test/Helpers/CodeFixVerifier.cs @@ -1,4 +1,4 @@ -using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.Testing; namespace Moq.Analyzers.Test.Helpers; @@ -7,7 +7,7 @@ internal static class CodeFixVerifier where TAnalyzer : DiagnosticAnalyzer, new() where TCodeFixProvider : CodeFixProvider, new() { - public static async Task VerifyCodeFixAsync(string originalSource, string fixedSource, string referenceAssemblyGroup, CompilerDiagnostics? compilerDiagnostics = null) + public static async Task VerifyCodeFixAsync(string originalSource, string fixedSource, string referenceAssemblyGroup, int? numberOfIncrementalIterations = null, int? numberOfFixAllIterations = null, CompilerDiagnostics? compilerDiagnostics = null) { ReferenceAssemblies referenceAssemblies = ReferenceAssemblyCatalog.Catalog[referenceAssemblyGroup]; @@ -18,6 +18,16 @@ public static async Task VerifyCodeFixAsync(string originalSource, string fixedS ReferenceAssemblies = referenceAssemblies, }; + if (numberOfIncrementalIterations.HasValue) + { + test.NumberOfIncrementalIterations = numberOfIncrementalIterations.Value; + } + + if (numberOfFixAllIterations.HasValue) + { + test.NumberOfFixAllIterations = numberOfFixAllIterations.Value; + } + if (compilerDiagnostics.HasValue) { test.CompilerDiagnostics = compilerDiagnostics.Value; diff --git a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs index 0f5a61a1a..7db84aa0b 100644 --- a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs +++ b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs @@ -124,6 +124,6 @@ private async Task VerifyAsync(string referenceAssemblyGroup, string @namespace, output.WriteLine("Fixed:"); output.WriteLine(f); - await Verifier.VerifyCodeFixAsync(o, f, referenceAssemblyGroup, compilerDiagnostics).ConfigureAwait(false); + await Verifier.VerifyCodeFixAsync(o, f, referenceAssemblyGroup, compilerDiagnostics: compilerDiagnostics).ConfigureAwait(false); } }