Skip to content

Commit

Permalink
Add new rule to enforce MockBehavior.Strict (Moq1410) (#302)
Browse files Browse the repository at this point in the history
Introduced a new rule Moq1410, to enforce setting the MockBehavior to
Strict in Mock declarations. This is set as Info and can be elevated in
`.editorconfig`.

Fixes #127 
Related to #296 from @MattKotsenas (Thanks Matt!)

Changes:
- Introduced a new abstract class for analyzing `MockBehavior` so code
can be shared between Matt's implementation and this.
- Added a new diagnostic analyzer (`Moq1410`) to enforce explicit strict
mocking behavior.
- Introduced a code fix provider for handling diagnostics related to
strict mock behavior.
- Enhanced encapsulation by changing public members to internal in the
`MoqKnownSymbols` class.
  • Loading branch information
rjmurillo authored Dec 24, 2024
1 parent 4cf69db commit 93e2f6c
Show file tree
Hide file tree
Showing 13 changed files with 529 additions and 148 deletions.
74 changes: 74 additions & 0 deletions docs/rules/Moq1410.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# Moq1410: Explicitly set the Strict mocking behavior

# Moq1410: Explicitly set the Strict mocking behavior

| Item | Value |
| -------- | ------- |
| Enabled | True |
| Severity | Info |
| CodeFix | True |

---

Mocks use the `MockBehavior.Loose` by default. Some people find this default behavior undesirable, as it can lead to
unexpected behavior if the mock is improperly set up. To fix, specify `MockBehavior.Strict` to cause Moq to always throw
an exception for invocations that don't have a corresponding setup.

## Examples of patterns that are flagged by this analyzer

```csharp
interface ISample
{
int Calculate() => 0;
}

var mock = new Mock<ISample>(); // Moq1410: Moq: Explicitly set the Strict mocking behavior
var mock2 = Mock.Of<ISample>(); // Moq1410: Moq: Explicitly set the Strict mocking behavior
```

```csharp
interface ISample
{
int Calculate() => 0;
}

var mock = new Mock<ISample>(MockBehavior.Default); // Moq1410: Explicitly set the Strict mocking behavior
var mock2 = Mock.Of<ISample>(MockBehavior.Default); // Moq1410: Explicitly set the Strict mocking behavior
var repo = new MockRepository(MockBehavior.Default); // Moq1410: Explicitly set the Strict mocking behavior
```

## Solution

```csharp
interface ISample
{
int Calculate() => 0;
}

var mock = new Mock<ISample>(MockBehavior.Strict);
var mock2 = Mock.Of<ISample>(MockBehavior.Strict);
var repo = new MockRepository(MockBehavior.Strict);
```

## Suppress a warning

If you just want to suppress a single violation, add preprocessor directives to
your source file to disable and then re-enable the rule.

```csharp
#pragma warning disable Moq1410
var mock = new Mock<ISample>(); // Moq1410: Moq: Explicitly set the Strict mocking behavior
#pragma warning restore Moq1410
```

To disable the rule for a file, folder, or project, set its severity to `none`
in the
[configuration file](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/configuration-files).

```ini
[*.{cs,vb}]
dotnet_diagnostic.Moq1410.severity = none
```

For more information, see
[How to suppress code analysis warnings](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/suppress-warnings).
1 change: 1 addition & 0 deletions docs/rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@
| [Moq1201](./Moq1201.md) | Setup of async methods should use `.ReturnsAsync` instance instead of `.Result` |
| [Moq1300](./Moq1300.md) | `Mock.As()` should take interfaces only |
| [Moq1400](./Moq1400.md) | Explicitly choose a mocking behavior instead of relying on the default (Loose) behavior |
| [Moq1400](./Moq1410.md) | Explicitly set the Strict mocking behavior |
3 changes: 2 additions & 1 deletion src/Analyzers/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@

Rule ID | Category | Severity | Notes
--------|----------|----------|-------
Moq1400 | Moq | Warning | SetExplicitMockBehaviorAnalyzer, [Documentation](https://github.com/rjmurillo/moq.analyzers/blob/main/docs/rules/Moq1400.md)
Moq1400 | Moq | Warning | SetExplicitMockBehaviorAnalyzer, [Documentation](https://github.com/rjmurillo/moq.analyzers/blob/main/docs/rules/Moq1400.md)
Moq1410 | Moq | Info | SetStrictMockBehaviorAnalyzer, [Documentation](https://github.com/rjmurillo/moq.analyzers/blob/main/docs/rules/Moq1410.md)
81 changes: 81 additions & 0 deletions src/Analyzers/MockBehaviorDiagnosticAnalyzerBase.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
using Microsoft.CodeAnalysis.Operations;

namespace Moq.Analyzers;

/// <summary>
/// Serves as a base class for diagnostic analyzers that analyze mock behavior in Moq.
/// </summary>
/// <remarks>
/// This abstract class provides common functionality for analyzing Moq's MockBehavior, such as registering
/// compilation start actions and defining the core analysis logic to be implemented by derived classes.
/// </remarks>
public abstract class MockBehaviorDiagnosticAnalyzerBase : DiagnosticAnalyzer
{
/// <inheritdoc />
public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();

context.RegisterCompilationStartAction(RegisterCompilationStartAction);
}

internal abstract void AnalyzeCore(OperationAnalysisContext context, IMethodSymbol target, ImmutableArray<IArgumentOperation> arguments, MoqKnownSymbols knownSymbols);

private void RegisterCompilationStartAction(CompilationStartAnalysisContext context)
{
MoqKnownSymbols knownSymbols = new(context.Compilation);

// Ensure Moq is referenced in the compilation
if (!knownSymbols.IsMockReferenced())
{
return;
}

// Look for the MockBehavior type and provide it to Analyze to avoid looking it up multiple times.
if (knownSymbols.MockBehavior is null)
{
return;
}

context.RegisterOperationAction(context => AnalyzeObjectCreation(context, knownSymbols), OperationKind.ObjectCreation);

context.RegisterOperationAction(context => AnalyzeInvocation(context, knownSymbols), OperationKind.Invocation);
}

private void AnalyzeObjectCreation(OperationAnalysisContext context, MoqKnownSymbols knownSymbols)
{
if (context.Operation is not IObjectCreationOperation creation)
{
return;
}

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;
}

AnalyzeCore(context, creation.Constructor, creation.Arguments, knownSymbols);
}

private void AnalyzeInvocation(OperationAnalysisContext context, MoqKnownSymbols knownSymbols)
{
if (context.Operation is not IInvocationOperation invocation)
{
return;
}

if (!invocation.TargetMethod.IsInstanceOf(knownSymbols.MockOf, out IMethodSymbol? match))
{
// 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;
}

AnalyzeCore(context, match, invocation.Arguments, knownSymbols);
}
}
69 changes: 2 additions & 67 deletions src/Analyzers/SetExplicitMockBehaviorAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Moq.Analyzers;
/// Mock should explicitly specify a behavior and not rely on the default.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class SetExplicitMockBehaviorAnalyzer : DiagnosticAnalyzer
public class SetExplicitMockBehaviorAnalyzer : MockBehaviorDiagnosticAnalyzerBase
{
private static readonly LocalizableString Title = "Moq: Explicitly choose a mock behavior";
private static readonly LocalizableString Message = "Explicitly choose a mocking behavior instead of relying on the default (Loose) behavior";
Expand All @@ -25,73 +25,8 @@ public class SetExplicitMockBehaviorAnalyzer : DiagnosticAnalyzer
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);

/// <inheritdoc />
public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();

context.RegisterCompilationStartAction(RegisterCompilationStartAction);
}

private static void RegisterCompilationStartAction(CompilationStartAnalysisContext context)
{
MoqKnownSymbols knownSymbols = new(context.Compilation);

// Ensure Moq is referenced in the compilation
if (!knownSymbols.IsMockReferenced())
{
return;
}

// Look for the MockBehavior type and provide it to Analyze to avoid looking it up multiple times.
if (knownSymbols.MockBehavior is null)
{
return;
}

context.RegisterOperationAction(context => AnalyzeObjectCreation(context, knownSymbols), OperationKind.ObjectCreation);

context.RegisterOperationAction(context => AnalyzeInvocation(context, knownSymbols), OperationKind.Invocation);
}

private static void AnalyzeObjectCreation(OperationAnalysisContext context, MoqKnownSymbols knownSymbols)
{
if (context.Operation is not IObjectCreationOperation creation)
{
return;
}

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;
}

AnalyzeCore(context, creation.Constructor, creation.Arguments, knownSymbols);
}

private static void AnalyzeInvocation(OperationAnalysisContext context, MoqKnownSymbols knownSymbols)
{
if (context.Operation is not IInvocationOperation invocation)
{
return;
}

if (!invocation.TargetMethod.IsInstanceOf(knownSymbols.MockOf, out IMethodSymbol? match))
{
// 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;
}

AnalyzeCore(context, match, invocation.Arguments, knownSymbols);
}

[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)
internal override void AnalyzeCore(OperationAnalysisContext context, IMethodSymbol target, ImmutableArray<IArgumentOperation> arguments, MoqKnownSymbols knownSymbols)
{
// Check if the target method has a parameter of type MockBehavior
IParameterSymbol? mockParameter = target.Parameters.DefaultIfNotSingle(parameter => parameter.Type.IsInstanceOf(knownSymbols.MockBehavior));
Expand Down
93 changes: 93 additions & 0 deletions src/Analyzers/SetStrictMockBehaviorAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis.Operations;

namespace Moq.Analyzers;

/// <summary>
/// Mock should explicitly specify Strict behavior.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class SetStrictMockBehaviorAnalyzer : MockBehaviorDiagnosticAnalyzerBase
{
private static readonly LocalizableString Title = "Moq: Set MockBehavior to Strict";
private static readonly LocalizableString Message = "Explicitly set the Strict mocking behavior";

private static readonly DiagnosticDescriptor Rule = new(
DiagnosticIds.SetStrictMockBehavior,
Title,
Message,
DiagnosticCategory.Moq,
DiagnosticSeverity.Info,
isEnabledByDefault: true,
helpLinkUri: $"https://github.com/rjmurillo/moq.analyzers/blob/{ThisAssembly.GitCommitId}/docs/rules/{DiagnosticIds.SetStrictMockBehavior}.md");

/// <inheritdoc />
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);

/// <inheritdoc />
[SuppressMessage("Design", "MA0051:Method is too long", Justification = "Should be fixed. Ignoring for now to avoid additional churn as part of larger refactor.")]
internal override void AnalyzeCore(OperationAnalysisContext context, IMethodSymbol target, ImmutableArray<IArgumentOperation> arguments, MoqKnownSymbols knownSymbols)
{
// 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;
}

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)
{
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));
return;
}

// NOTE: This logic can't handle indirection (e.g. var x = MockBehavior.Default; new Mock(x);)
//
// The operation specifies a MockBehavior; is it MockBehavior.Strict?
if (mockArgument?.Value.WalkDownConversion().ConstantValue.Value != knownSymbols.MockBehaviorStrict?.ConstantValue
&& mockArgument?.DescendantsAndSelf().OfType<IFieldReferenceOperation>().Any(argument => argument.Member.IsInstanceOf(knownSymbols.MockBehaviorStrict)) != true)
{
if (!target.TryGetParameterOfType(knownSymbols.MockBehavior!, out IParameterSymbol? parameterMatch, cancellationToken: context.CancellationToken))
{
return;
}

ImmutableDictionary<string, string?> properties = new DiagnosticEditProperties
{
TypeOfEdit = DiagnosticEditProperties.EditType.Replace,
EditPosition = parameterMatch.Ordinal,
}.ToImmutableDictionary();

context.ReportDiagnostic(context.Operation.CreateDiagnostic(Rule, properties));
}
}
}
23 changes: 23 additions & 0 deletions src/CodeFixes/BehaviorType.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
namespace Moq.CodeFixes;

/// <summary>
/// Options to customize the behavior of Moq.
/// </summary>
/// <remarks>
/// Local copy of Moq's MockBehavior enum to avoid dependency on Moq library.
/// </remarks>
internal enum BehaviorType
{
/// <summary>
/// Will never throw exceptions, returning default values when necessary
/// (<see langword="null" /> for reference types, zero for value types,
/// or empty for enumerables and arrays).
/// </summary>
Loose,

/// <summary>
/// Causes Moq to always throw an exception for invocations that don't have
/// a corresponding Setup.
/// </summary>
Strict,
}
Loading

0 comments on commit 93e2f6c

Please sign in to comment.