Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor SetExplicitMockBehaviorAnalyzer to use KnownSymbols and add CodeFix #296

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 70 additions & 51 deletions src/Analyzers/SetExplicitMockBehaviorAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,94 +43,113 @@
}

// 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)
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved
{
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);
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved
}

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.
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved
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)
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved
private static void AnalyzeCore(OperationAnalysisContext context, IMethodSymbol target, ImmutableArray<IArgumentOperation> arguments, MoqKnownSymbols knownSymbols)

Check failure on line 92 in src/Analyzers/SetExplicitMockBehaviorAnalyzer.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Analyzers/SetExplicitMockBehaviorAnalyzer.cs#L92

The Cyclomatic Complexity of this method is 12 which is greater than 10 authorized.
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved
{
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 Dictionary<string, string?>(StringComparer.Ordinal)
{
{ DiagnosticEditProperties.EditTypeKey, DiagnosticEditProperties.EditType.Insert.ToString() },
{ DiagnosticEditProperties.EditPositionKey, parameterMatch.Ordinal.ToString() },
}.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 Dictionary<string, string?>(StringComparer.Ordinal)
{
{ DiagnosticEditProperties.EditTypeKey, DiagnosticEditProperties.EditType.Insert.ToString() },
{ DiagnosticEditProperties.EditPositionKey, parameterMatch.Ordinal.ToString() },
}.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)
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved
{
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 Dictionary<string, string?>(StringComparer.Ordinal)
{
{ DiagnosticEditProperties.EditTypeKey, DiagnosticEditProperties.EditType.Replace.ToString() },
{ DiagnosticEditProperties.EditPositionKey, parameterMatch.Ordinal.ToString() },
}.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));
}
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved
}
}
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
42 changes: 42 additions & 0 deletions src/CodeFixes/CodeFixContextExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
using System.Diagnostics.CodeAnalysis;

Check failure on line 1 in src/CodeFixes/CodeFixContextExtensions.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/CodeFixes/CodeFixContextExtensions.cs#L1

Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved
using Microsoft.CodeAnalysis.CodeFixes;

namespace Moq.CodeFixes;

internal static class CodeFixContextExtensions

Check warning on line 6 in src/CodeFixes/CodeFixContextExtensions.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/CodeFixes/CodeFixContextExtensions.cs#L6

Types should not have members with visibility set higher than the type's visibility
{
public static bool TryGetEditProperties(this CodeFixContext context, [NotNullWhen(true)] out DiagnosticEditProperties? editProperties)
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved
{
ImmutableDictionary<string, string?> properties = context.Diagnostics[0].Properties;
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved

// Try parsing; if anything fails return false
editProperties = null;
if (!properties.TryGetValue(DiagnosticEditProperties.EditTypeKey, out string? editTypeString))
{
return false;
}

if (!properties.TryGetValue(DiagnosticEditProperties.EditPositionKey, out string? editPositionString))
{
return false;
}

if (!Enum.TryParse(editTypeString, out DiagnosticEditProperties.EditType editType))
{
return false;
}

if (!int.TryParse(editPositionString, out int editPosition))

Check failure on line 29 in src/CodeFixes/CodeFixContextExtensions.cs

View workflow job for this annotation

GitHub Actions / build (windows-latest)

Use an overload of 'TryParse' that has a 'System.IFormatProvider' parameter (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0011.md)

Check failure on line 29 in src/CodeFixes/CodeFixContextExtensions.cs

View workflow job for this annotation

GitHub Actions / build (windows-latest)

Use an overload of 'TryParse' that has a 'System.IFormatProvider' parameter (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0011.md)

Check failure on line 29 in src/CodeFixes/CodeFixContextExtensions.cs

View workflow job for this annotation

GitHub Actions / build (ubuntu-latest)

Use an overload of 'TryParse' that has a 'System.IFormatProvider' parameter (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0011.md)

Check failure on line 29 in src/CodeFixes/CodeFixContextExtensions.cs

View workflow job for this annotation

GitHub Actions / build (ubuntu-latest)

Use an overload of 'TryParse' that has a 'System.IFormatProvider' parameter (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0011.md)
{
return false;
}
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved

editProperties = new DiagnosticEditProperties
{
TypeOfEdit = editType,
EditPosition = editPosition,
};

return true;
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved
}
}
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;

Check failure on line 1 in src/CodeFixes/SetExplicitMockBehaviorFixer.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/CodeFixes/SetExplicitMockBehaviorFixer.cs#L1

Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved
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);
}
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved

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)

Check failure on line 70 in src/CodeFixes/SetExplicitMockBehaviorFixer.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/CodeFixes/SetExplicitMockBehaviorFixer.cs#L70

The Cyclomatic Complexity of this method is 13 which is greater than 10 authorized.
{
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

Check failure on line 78 in src/CodeFixes/SetExplicitMockBehaviorFixer.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/CodeFixes/SetExplicitMockBehaviorFixer.cs#L78

Reduce the number of conditional operators (4) used in the expression (maximum allowed 3).
|| 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;

Check failure on line 1 in src/CodeFixes/SyntaxGeneratorExtensions.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/CodeFixes/SyntaxGeneratorExtensions.cs#L1

Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved

namespace Moq.CodeFixes;

internal static class SyntaxGeneratorExtensions

Check warning on line 5 in src/CodeFixes/SyntaxGeneratorExtensions.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/CodeFixes/SyntaxGeneratorExtensions.cs#L5

Types should not have members with visibility set higher than the type's visibility
{
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));
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved
}

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));
MattKotsenas marked this conversation as resolved.
Show resolved Hide resolved
}

throw new ArgumentException($"Must be of type {nameof(InvocationExpressionSyntax)} or {nameof(ObjectCreationExpressionSyntax)} but is of type {syntax.GetType().Name}", nameof(syntax));
}
}
Loading
Loading