Refactor SetExplicitMockBehaviorAnalyzer to use KnownSymbols and add CodeFix

src/Analyzers/SetExplicitMockBehaviorAnalyzer.cs

@@ -1,4 +1,5 @@
-using Microsoft.CodeAnalysis.Operations;
+using System.Diagnostics.CodeAnalysis;
+using Microsoft.CodeAnalysis.Operations;
 
 namespace Moq.Analyzers;
 
@@ -43,94 +44,114 @@
         }
 
         // Look for the MockBehavior type and provide it to Analyze to avoid looking it up multiple times.
-        INamedTypeSymbol? mockBehaviorSymbol = knownSymbols.MockBehavior;
-        if (mockBehaviorSymbol is null)
+        if (knownSymbols.MockBehavior is null)
         {
             return;
         }
 
-        // Look for the Mock.Of() method and provide it to Analyze to avoid looking it up multiple times.
-        ImmutableArray<IMethodSymbol> ofMethods = knownSymbols.MockOf;
+        context.RegisterOperationAction(context => AnalyzeObjectCreation(context, knownSymbols), OperationKind.ObjectCreation);
 
-        ImmutableArray<INamedTypeSymbol> mockTypes =
-            new INamedTypeSymbol?[] { knownSymbols.Mock1, knownSymbols.MockRepository }
-            .WhereNotNull()
-            .ToImmutableArray();
-
-        context.RegisterOperationAction(
-            context => AnalyzeNewObject(context, mockTypes, mockBehaviorSymbol),
-            OperationKind.ObjectCreation);
-
-        if (!ofMethods.IsEmpty)
-        {
-            context.RegisterOperationAction(
-                context => AnalyzeInvocation(context, ofMethods, mockBehaviorSymbol),
-                OperationKind.Invocation);
-        }
+        context.RegisterOperationAction(context => AnalyzeInvocation(context, knownSymbols), OperationKind.Invocation);
     }
 
-    private static void AnalyzeNewObject(OperationAnalysisContext context, ImmutableArray<INamedTypeSymbol> mockTypes, INamedTypeSymbol mockBehaviorSymbol)
+    private static void AnalyzeObjectCreation(OperationAnalysisContext context, MoqKnownSymbols knownSymbols)
     {
-        if (context.Operation is not IObjectCreationOperation creationOperation)
+        if (context.Operation is not IObjectCreationOperation creation)
         {
             return;
         }
 
-        if (creationOperation.Type is not INamedTypeSymbol namedType)
+        if (creation.Type is null ||
+            creation.Constructor is null ||
+            !(creation.Type.IsInstanceOf(knownSymbols.Mock1) || creation.Type.IsInstanceOf(knownSymbols.MockRepository)))
         {
+            // We could expand this check to include any method that accepts a MockBehavior parameter.
+            // Leaving it narrowly scoped for now to avoid false positives and potential performance problems.
             return;
         }
 
-        if (!namedType.IsInstanceOf(mockTypes))
+        AnalyzeCore(context, creation.Constructor, creation.Arguments, knownSymbols);
+    }
+
+    private static void AnalyzeInvocation(OperationAnalysisContext context, MoqKnownSymbols knownSymbols)
+    {
+        if (context.Operation is not IInvocationOperation invocation)
         {
             return;
         }
 
-        foreach (IArgumentOperation argument in creationOperation.Arguments)
+        if (!invocation.TargetMethod.IsInstanceOf(knownSymbols.MockOf, out IMethodSymbol? match))
         {
-            if (argument.Value is IFieldReferenceOperation fieldReferenceOperation)
-            {
-                ISymbol field = fieldReferenceOperation.Member;
-                if (field.ContainingType.IsInstanceOf(mockBehaviorSymbol) && IsExplicitBehavior(field.Name))
-                {
-                    return;
-                }
-            }
+            // We could expand this check to include any method that accepts a MockBehavior parameter.
+            // Leaving it narrowly scoped for now to avoid false positives and potential performance problems.
+            return;
         }
 
-        context.ReportDiagnostic(creationOperation.CreateDiagnostic(Rule));
+        AnalyzeCore(context, match, invocation.Arguments, knownSymbols);
     }
 
-    private static void AnalyzeInvocation(OperationAnalysisContext context, ImmutableArray<IMethodSymbol> wellKnownOfMethods, INamedTypeSymbol mockBehaviorSymbol)
+    [SuppressMessage("Design", "MA0051:Method is too long", Justification = "Should be fixed. Ignoring for now to avoid additional churn as part of larger refactor.")]
+    private static void AnalyzeCore(OperationAnalysisContext context, IMethodSymbol target, ImmutableArray<IArgumentOperation> arguments, MoqKnownSymbols knownSymbols)
     {
-        if (context.Operation is not IInvocationOperation invocationOperation)
+        // Check if the target method has a parameter of type MockBehavior
+        IParameterSymbol? mockParameter = target.Parameters.DefaultIfNotSingle(parameter => parameter.Type.IsInstanceOf(knownSymbols.MockBehavior));
+
+        // If the target method doesn't have a MockBehavior parameter, check if there's an overload that does
+        if (mockParameter is null && target.TryGetOverloadWithParameterOfType(knownSymbols.MockBehavior!, out IMethodSymbol? methodMatch, out _, cancellationToken: context.CancellationToken))
         {
+            if (!methodMatch.TryGetParameterOfType(knownSymbols.MockBehavior!, out IParameterSymbol? parameterMatch, cancellationToken: context.CancellationToken))
+            {
+                return;
+            }
+
+            ImmutableDictionary<string, string?> properties = new DiagnosticEditProperties
+            {
+                TypeOfEdit = DiagnosticEditProperties.EditType.Insert,
+                EditPosition = parameterMatch.Ordinal,
+            }.ToImmutableDictionary();
+
+            // Using a method that doesn't accept a MockBehavior parameter, however there's an overload that does
+            context.ReportDiagnostic(context.Operation.CreateDiagnostic(Rule, properties));
             return;
         }
 
-        IMethodSymbol targetMethod = invocationOperation.TargetMethod;
-        if (!targetMethod.IsInstanceOf(wellKnownOfMethods))
+        IArgumentOperation? mockArgument = arguments.DefaultIfNotSingle(argument => argument.Parameter.IsInstanceOf(mockParameter));
+
+        // Is the behavior set via a default value?
+        if (mockArgument?.ArgumentKind == ArgumentKind.DefaultValue && mockArgument.Value.WalkDownConversion().ConstantValue.Value == knownSymbols.MockBehaviorDefault?.ConstantValue)
         {
-            return;
+            if (!target.TryGetParameterOfType(knownSymbols.MockBehavior!, out IParameterSymbol? parameterMatch, cancellationToken: context.CancellationToken))
+            {
+                return;
+            }
+
+            ImmutableDictionary<string, string?> properties = new DiagnosticEditProperties
+            {
+                TypeOfEdit = DiagnosticEditProperties.EditType.Insert,
+                EditPosition = parameterMatch.Ordinal,
+            }.ToImmutableDictionary();
+
+            context.ReportDiagnostic(context.Operation.CreateDiagnostic(Rule, properties));
         }
 
-        foreach (IArgumentOperation argument in invocationOperation.Arguments)
+        // NOTE: This logic can't handle indirection (e.g. var x = MockBehavior.Default; new Mock(x);). We can't use the constant value either,
+        // as Loose and Default share the same enum value: `1`. Being more accurate I believe requires data flow analysis.
+        //
+        // The operation specifies a MockBehavior; is it MockBehavior.Default?
+        if (mockArgument?.DescendantsAndSelf().OfType<IFieldReferenceOperation>().Any(argument => argument.Member.IsInstanceOf(knownSymbols.MockBehaviorDefault)) == true)
         {
-            if (argument.Value is IFieldReferenceOperation fieldReferenceOperation)
+            if (!target.TryGetParameterOfType(knownSymbols.MockBehavior!, out IParameterSymbol? parameterMatch, cancellationToken: context.CancellationToken))
             {
-                ISymbol field = fieldReferenceOperation.Member;
-                if (field.ContainingType.IsInstanceOf(mockBehaviorSymbol) && IsExplicitBehavior(field.Name))
-                {
-                    return;
-                }
+                return;
             }
-        }
 
-        context.ReportDiagnostic(invocationOperation.CreateDiagnostic(Rule));
-    }
+            ImmutableDictionary<string, string?> properties = new DiagnosticEditProperties
+            {
+                TypeOfEdit = DiagnosticEditProperties.EditType.Replace,
+                EditPosition = parameterMatch.Ordinal,
+            }.ToImmutableDictionary();
 
-    private static bool IsExplicitBehavior(string symbolName)
-    {
-        return string.Equals(symbolName, "Loose", StringComparison.Ordinal) || string.Equals(symbolName, "Strict", StringComparison.Ordinal);
+            context.ReportDiagnostic(context.Operation.CreateDiagnostic(Rule, properties));
+        }
     }
 }

src/Analyzers/SquiggleCop.Baseline.yaml

@@ -359,7 +359,7 @@
 - {Id: HAA0603, Title: Delegate allocation from a method group, Category: Performance, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false}
 - {Id: HAA0604, Title: Delegate allocation from a method group, Category: Performance, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false}
 - {Id: IDE0004, Title: Remove Unnecessary Cast, Category: Style, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: true}
-- {Id: IDE0005, Title: Using directive is unnecessary., Category: Style, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false}
+- {Id: IDE0005, Title: Using directive is unnecessary., Category: Style, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: true}
 - {Id: IDE0005_gen, Title: Using directive is unnecessary., Category: Style, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: true}
 - {Id: IDE0007, Title: Use implicit type, Category: Style, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false}
 - {Id: IDE0008, Title: Use explicit type, Category: Style, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false}

src/CodeFixes/CallbackSignatureShouldMatchMockedMethodCodeFix.cs → src/CodeFixes/CallbackSignatureShouldMatchMockedMethodFixer.cs

@@ -9,9 +9,9 @@ namespace Moq.CodeFixes;
 /// <summary>
 /// Fixes for CallbackSignatureShouldMatchMockedMethodAnalyzer (Moq1100).
 /// </summary>
-[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(CallbackSignatureShouldMatchMockedMethodCodeFix))]
+[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(CallbackSignatureShouldMatchMockedMethodFixer))]
 [Shared]
-public class CallbackSignatureShouldMatchMockedMethodCodeFix : CodeFixProvider
+public class CallbackSignatureShouldMatchMockedMethodFixer : CodeFixProvider
 {
     /// <inheritdoc />
     public sealed override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(DiagnosticIds.BadCallbackParameters);

src/CodeFixes/CodeFixContextExtensions.cs

@@ -0,0 +1,14 @@
+using System.Diagnostics.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeFixes;
+
+namespace Moq.CodeFixes;
+
+internal static class CodeFixContextExtensions
+{
+    public static bool TryGetEditProperties(this CodeFixContext context, [NotNullWhen(true)] out DiagnosticEditProperties? editProperties)
+    {
+        ImmutableDictionary<string, string?> properties = context.Diagnostics[0].Properties;
+
+        return DiagnosticEditProperties.TryGetFromImmutableDictionary(properties, out editProperties);
+    }
+}

src/CodeFixes/SetExplicitMockBehaviorFixer.cs

@@ -0,0 +1,107 @@
+using System.Composition;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.Editing;
+using Microsoft.CodeAnalysis.Simplification;
+
+namespace Moq.CodeFixes;
+
+/// <summary>
+/// Fixes for <see cref="DiagnosticIds.SetExplicitMockBehavior"/>.
+/// </summary>
+[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(SetExplicitMockBehaviorFixer))]
+[Shared]
+public class SetExplicitMockBehaviorFixer : CodeFixProvider
+{
+    private enum BehaviorType
+    {
+        Loose,
+        Strict,
+    }
+
+    /// <inheritdoc />
+    public override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(DiagnosticIds.SetExplicitMockBehavior);
+
+    /// <inheritdoc />
+    public override FixAllProvider? GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
+
+    /// <inheritdoc />
+    public override async Task RegisterCodeFixesAsync(CodeFixContext context)
+    {
+        SyntaxNode? root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
+        SyntaxNode? nodeToFix = root?.FindNode(context.Span, getInnermostNodeForTie: true);
+
+        if (!context.TryGetEditProperties(out DiagnosticEditProperties? editProperties))
+        {
+            return;
+        }
+
+        if (nodeToFix is null)
+        {
+            return;
+        }
+
+        context.RegisterCodeFix(new SetExplicitMockBehaviorCodeAction("Set MockBehavior (Loose)", context.Document, nodeToFix, BehaviorType.Loose, editProperties.TypeOfEdit, editProperties.EditPosition), context.Diagnostics);
+        context.RegisterCodeFix(new SetExplicitMockBehaviorCodeAction("Set MockBehavior (Strict)", context.Document, nodeToFix, BehaviorType.Strict, editProperties.TypeOfEdit, editProperties.EditPosition), context.Diagnostics);
+    }
+
+    private sealed class SetExplicitMockBehaviorCodeAction : CodeAction
+    {
+        private readonly Document _document;
+        private readonly SyntaxNode _nodeToFix;
+        private readonly BehaviorType _behaviorType;
+        private readonly DiagnosticEditProperties.EditType _editType;
+        private readonly int _position;
+
+        public SetExplicitMockBehaviorCodeAction(string title, Document document, SyntaxNode nodeToFix, BehaviorType behaviorType, DiagnosticEditProperties.EditType editType, int position)
+        {
+            Title = title;
+            _document = document;
+            _nodeToFix = nodeToFix;
+            _behaviorType = behaviorType;
+            _editType = editType;
+            _position = position;
+        }
+
+        public override string Title { get; }
+
+        public override string? EquivalenceKey => Title;
+
+        protected override async Task<Document> GetChangedDocumentAsync(CancellationToken cancellationToken)
+        {
+            DocumentEditor editor = await DocumentEditor.CreateAsync(_document, cancellationToken).ConfigureAwait(false);
+            SemanticModel? model = await _document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
+            IOperation? operation = model?.GetOperation(_nodeToFix, cancellationToken);
+
+            MoqKnownSymbols knownSymbols = new(editor.SemanticModel.Compilation);
+
+            if (knownSymbols.MockBehavior is null
+                || knownSymbols.MockBehaviorDefault is null
+                || knownSymbols.MockBehaviorLoose is null
+                || knownSymbols.MockBehaviorStrict is null
+                || operation is null)
+            {
+                return _document;
+            }
+
+            SyntaxNode behavior = _behaviorType switch
+            {
+                BehaviorType.Loose => editor.Generator.MemberAccessExpression(knownSymbols.MockBehaviorLoose),
+                BehaviorType.Strict => editor.Generator.MemberAccessExpression(knownSymbols.MockBehaviorStrict),
+                _ => throw new InvalidOperationException(),
+            };
+
+            SyntaxNode argument = editor.Generator.Argument(behavior);
+
+            SyntaxNode newNode = _editType switch
+            {
+                DiagnosticEditProperties.EditType.Insert => editor.Generator.InsertArguments(operation, _position, argument),
+                DiagnosticEditProperties.EditType.Replace => editor.Generator.ReplaceArgument(operation, _position, argument),
+                _ => throw new InvalidOperationException(),
+            };
+
+            editor.ReplaceNode(_nodeToFix, newNode.WithAdditionalAnnotations(Simplifier.Annotation));
+            return editor.GetChangedDocument();
+        }
+    }
+}

src/CodeFixes/SyntaxGeneratorExtensions.cs

@@ -0,0 +1,71 @@
+using Microsoft.CodeAnalysis.Editing;
+
+namespace Moq.CodeFixes;
+
+internal static class SyntaxGeneratorExtensions
+{
+    public static SyntaxNode MemberAccessExpression(this SyntaxGenerator generator, IFieldSymbol fieldSymbol)
+    {
+        return generator.MemberAccessExpression(generator.TypeExpression(fieldSymbol.Type), generator.IdentifierName(fieldSymbol.Name));
+    }
+
+    public static SyntaxNode InsertArguments(this SyntaxGenerator generator, IOperation operation, int index, params SyntaxNode[] items)
+    {
+        // Ideally we could modify argument lists only using the IOperation APIs, but I haven't figured out a way to do that yet.
+        return generator.InsertArguments(operation.Syntax, index, items);
+    }
+
+    public static SyntaxNode InsertArguments(this SyntaxGenerator generator, SyntaxNode syntax, int index, params SyntaxNode[] items)
+    {
+        if (Array.Exists(items, item => item is not ArgumentSyntax))
+        {
+            throw new ArgumentException("Must all be of type ArgumentSyntax", nameof(items));
+        }
+
+        if (syntax is InvocationExpressionSyntax invocation)
+        {
+            SeparatedSyntaxList<ArgumentSyntax> arguments = invocation.ArgumentList.Arguments;
+            arguments = arguments.InsertRange(index, items.OfType<ArgumentSyntax>());
+            return invocation.WithArgumentList(SyntaxFactory.ArgumentList(arguments));
+        }
+
+        if (syntax is ObjectCreationExpressionSyntax creation)
+        {
+            SeparatedSyntaxList<ArgumentSyntax> arguments = creation.ArgumentList?.Arguments ?? [];
+            arguments = arguments.InsertRange(index, items.OfType<ArgumentSyntax>());
+            return creation.WithArgumentList(SyntaxFactory.ArgumentList(arguments));
+        }
+
+        throw new ArgumentException($"Must be of type {nameof(InvocationExpressionSyntax)} or {nameof(ObjectCreationExpressionSyntax)} but is of type {syntax.GetType().Name}", nameof(syntax));
+    }
+
+    public static SyntaxNode ReplaceArgument(this SyntaxGenerator generator, IOperation operation, int index, SyntaxNode item)
+    {
+        // Ideally we could modify argument lists only using the IOperation APIs, but I haven't figured out a way to do that yet.
+        return generator.ReplaceArgument(operation.Syntax, index, item);
+    }
+
+    public static SyntaxNode ReplaceArgument(this SyntaxGenerator generator, SyntaxNode syntax, int index, SyntaxNode item)
+    {
+        if (item is not ArgumentSyntax argument)
+        {
+            throw new ArgumentException("Must be of type ArgumentSyntax", nameof(item));
+        }
+
+        if (syntax is InvocationExpressionSyntax invocation)
+        {
+            SeparatedSyntaxList<ArgumentSyntax> arguments = invocation.ArgumentList.Arguments;
+            arguments = arguments.RemoveAt(index).Insert(index, argument);
+            return invocation.WithArgumentList(SyntaxFactory.ArgumentList(arguments));
+        }
+
+        if (syntax is ObjectCreationExpressionSyntax creation)
+        {
+            SeparatedSyntaxList<ArgumentSyntax> arguments = creation.ArgumentList?.Arguments ?? [];
+            arguments = arguments.RemoveAt(index).Insert(index, argument);
+            return creation.WithArgumentList(SyntaxFactory.ArgumentList(arguments));
+        }
+
+        throw new ArgumentException($"Must be of type {nameof(InvocationExpressionSyntax)} or {nameof(ObjectCreationExpressionSyntax)} but is of type {syntax.GetType().Name}", nameof(syntax));
+    }
+}