Skip to content

Commit

Permalink
Refactor SetExplicitMockBehaviorAnalyzer to use KnownSymbols and add …
Browse files Browse the repository at this point in the history
…CodeFix (#296)

This PR accomplishes two goals:

1. It refactors the `SetExplicitMockBehaviorAnalyzer` to use
`KnownSymbols` to simplify the analyzer and use the `IOperation`-based
analysis
2. It adds a corresponding code fixer that provides two entries under
the "lightbulb"
  - Set MockBehavior (Loose)
  - Set MockBehavior (Strict)
  • Loading branch information
MattKotsenas authored Dec 23, 2024
1 parent 2cec952 commit f80520f
Show file tree
Hide file tree
Showing 15 changed files with 675 additions and 126 deletions.
125 changes: 73 additions & 52 deletions src/Analyzers/SetExplicitMockBehaviorAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.CodeAnalysis.Operations;
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis.Operations;

namespace Moq.Analyzers;

Expand Down Expand Up @@ -43,94 +44,114 @@ private static void RegisterCompilationStartAction(CompilationStartAnalysisConte
}

// 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));
}
}
}
2 changes: 1 addition & 1 deletion src/Analyzers/SquiggleCop.Baseline.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
14 changes: 14 additions & 0 deletions src/CodeFixes/CodeFixContextExtensions.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}
107 changes: 107 additions & 0 deletions src/CodeFixes/SetExplicitMockBehaviorFixer.cs
Original file line number Diff line number Diff line change
@@ -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();
}
}
}
71 changes: 71 additions & 0 deletions src/CodeFixes/SyntaxGeneratorExtensions.cs
Original file line number Diff line number Diff line change
@@ -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));
}
}
Loading

0 comments on commit f80520f

Please sign in to comment.