Skip to content

Commit 97f885c

Browse files
MattKotsenasMattKotsenas
authored
Add rule to explicitly pick MockBehavior (#226)
Fixes #78 Add rule "Moq1400: Explicitly choose a mocking behavior instead of relying on the default (Loose) behavior" This addresses a top pain point for Moq in large teams: accidentally relying on Moq's default "loose" behavior. Not specifying a behavior or specifying `MockBehavior.Default` in `new Mock()`, `Mock.Of()`, or `new MockRepository()` raises a warning. This change does _not_ enforce `MockBehavior.Strict` everywhere. That will be a separate rule (coming later) as it's more controversial and thus likely requires some configuration.
1 parent 58381b9 commit 97f885c

File tree

8 files changed

+272
-10
lines changed

8 files changed

+272
-10
lines changed

‎docs/rules/Moq1400.md‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# Moq1400: Explicitly choose a mocking behavior instead of relying on the default (Loose) behavior
2+
3+
| Item | Value |
4+
| --- | --- |
5+
| Enabled | True |
6+
| Severity | Warning |
7+
| CodeFix | False |
8+
---
9+
10+
Mocks use the `MockBehavior.Loose` by default. Some people find this default behavior undesirable, as it can lead to
11+
unexpected behavior if the mock is improperly set up. To fix, specify either `MockBehavior.Loose` or
12+
`MockBehavior.Strict` to signify acknowledgement of the mock's behavior.
13+
14+
## Examples of patterns that are flagged by this analyzer
15+
16+
```csharp
17+
interface ISample
18+
{
19+
int Calculate() => 0;
20+
}
21+
22+
var mock = new Mock<ISample>(); // Moq1400: Moq: Explicitly choose a mock behavior
23+
var mock2 = Mock.Of<ISample>(); // Moq1400: Moq: Explicitly choose a mock behavior
24+
```
25+
26+
```csharp
27+
interface ISample
28+
{
29+
int Calculate() => 0;
30+
}
31+
32+
var mock = new Mock<ISample>(MockBehavior.Default); // Moq1400: Moq: Explicitly choose a mock behavior
33+
var mock2 = Mock.Of<ISample>(MockBehavior.Default); // Moq1400: Moq: Explicitly choose a mock behavior
34+
var repo = new MockRepository(MockBehavior.Default); // Moq1400: Moq: Explicitly choose a mock behavior
35+
```
36+
37+
## Solution
38+
39+
```csharp
40+
interface ISample
41+
{
42+
int Calculate() => 0;
43+
}
44+
45+
var mock = new Mock<ISample>(MockBehavior.Strict); // Or `MockBehavior.Loose`
46+
var mock2 = new Mock.Of<ISample>(MockBehavior.Strict); // Or `MockBehavior.Loose`
47+
var repo = new MockRepository(MockBehavior.Strict); // Or `MockBehavior.Loose`
48+
```

‎src/Moq.Analyzers/AnalyzerReleases.Unshipped.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,8 @@
11
; Unshipped analyzer release
22
; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md
3+
4+
### New Rules
5+
6+
Rule ID | Category | Severity | Notes
7+
--------|----------|----------|-------
8+
Moq1400 | Moq | Warning | SetExplicitMockBehaviorAnalyzer, [Documentation](https://github.com/rjmurillo/moq.analyzers/blob/main/docs/rules/Moq1400.md)

‎src/Moq.Analyzers/AsShouldBeUsedOnlyForInterfaceAnalyzer.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,10 @@ private static void Analyze(OperationAnalysisContext context, ImmutableArray<IMe
6868
}
6969

7070
IMethodSymbol targetMethod = invocationOperation.TargetMethod;
71-
#pragma warning disable ECS0900 // Minimize boxing and unboxing
7271
if (!targetMethod.IsInstanceOf(wellKnownAsMethods))
7372
{
7473
return;
7574
}
76-
#pragma warning restore ECS0900 // Minimize boxing and unboxing
7775

7876
ImmutableArray<ITypeSymbol> typeArguments = targetMethod.TypeArguments;
7977
if (typeArguments.Length != 1)

‎src/Moq.Analyzers/Common/DiagnosticIds.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,5 @@ internal static class DiagnosticIds
1212
internal const string SetupOnlyUsedForOverridableMembers = "Moq1200";
1313
internal const string AsyncUsesReturnsAsyncInsteadOfResult = "Moq1201";
1414
internal const string AsShouldOnlyBeUsedForInterfacesRuleId = "Moq1300";
15+
internal const string SetExplicitMockBehavior = "Moq1400";
1516
}

‎src/Moq.Analyzers/Common/ISymbolExtensions.cs‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,33 @@ internal static class ISymbolExtensions
1515
/// <see langword="true"/> if <paramref name="symbol"/> is an instance of <paramref name="other"/>, either as a direct match,
1616
/// or as a specialaization; otherwise, <see langword="false"/>.
1717
/// </returns>
18-
/// <remarks>
19-
/// As an example, <c>Type.Method&lt;int&gt;()</c> is an instance of <c>Type.Method&lt;T&gt;()</c>.
20-
/// </remarks>
18+
/// <example>
19+
/// <c>MyType.MyMethod&lt;int&gt;()</c> is an instance of <c>MyType.MyMethod&lt;T&gt;()</c>.
20+
/// </example>
21+
/// <example>
22+
/// <c>MyType&lt;int&gt;()</c> is an instance of <c>MyType&lt;T&gt;()</c>.
23+
/// </example>
2124
public static bool IsInstanceOf<TSymbol>(this ISymbol symbol, TSymbol other, SymbolEqualityComparer? symbolEqualityComparer = null)
2225
where TSymbol : class, ISymbol
2326
{
2427
symbolEqualityComparer ??= SymbolEqualityComparer.Default;
2528

26-
return symbol switch
29+
if (symbol is IMethodSymbol methodSymbol)
2730
{
28-
IMethodSymbol methodSymbol => symbolEqualityComparer.Equals(methodSymbol.OriginalDefinition, other),
29-
_ => symbolEqualityComparer.Equals(symbol, other),
30-
};
31+
return symbolEqualityComparer.Equals(methodSymbol.OriginalDefinition, other);
32+
}
33+
34+
if (symbol is INamedTypeSymbol namedTypeSymbol)
35+
{
36+
if (namedTypeSymbol.IsGenericType)
37+
{
38+
namedTypeSymbol = namedTypeSymbol.ConstructedFrom;
39+
}
40+
41+
return symbolEqualityComparer.Equals(namedTypeSymbol, other);
42+
}
43+
44+
return symbolEqualityComparer.Equals(symbol, other);
3145
}
3246

3347
/// <inheritdoc cref="IsInstanceOf{TSymbol}(ISymbol, TSymbol, SymbolEqualityComparer?)"/>
@@ -36,7 +50,7 @@ public static bool IsInstanceOf<TSymbol>(this ISymbol symbol, TSymbol other, Sym
3650
/// The symbols to compare to. Returns <see langword="true"/> if <paramref name="symbol"/> matches any of others.
3751
/// </param>
3852
/// <param name="symbolEqualityComparer">The <see cref="SymbolEqualityComparer"/> to use for equality.</param>
39-
public static bool IsInstanceOf<TSymbol>(this ISymbol symbol, IEnumerable<TSymbol> others, SymbolEqualityComparer? symbolEqualityComparer = null)
53+
public static bool IsInstanceOf<TSymbol>(this ISymbol symbol, ImmutableArray<TSymbol> others, SymbolEqualityComparer? symbolEqualityComparer = null)
4054
where TSymbol : class, ISymbol
4155
{
4256
symbolEqualityComparer ??= SymbolEqualityComparer.Default;

‎src/Moq.Analyzers/SetExplicitMockBehaviorAnalyzer.cs‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
using Microsoft.CodeAnalysis.Operations;
2+
3+
namespace Moq.Analyzers;
4+
5+
/// <summary>
6+
/// Mock should explicitly specify a behavior and not rely on the default.
7+
/// </summary>
8+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
9+
public class SetExplicitMockBehaviorAnalyzer : DiagnosticAnalyzer
10+
{
11+
private static readonly LocalizableString Title = "Moq: Explicitly choose a mock behavior";
12+
private static readonly LocalizableString Message = "Explicitly choose a mocking behavior instead of relying on the default (Loose) behavior";
13+
14+
private static readonly DiagnosticDescriptor Rule = new(
15+
DiagnosticIds.SetExplicitMockBehavior,
16+
Title,
17+
Message,
18+
DiagnosticCategory.Moq,
19+
DiagnosticSeverity.Warning,
20+
isEnabledByDefault: true,
21+
helpLinkUri: $"https://github.com/rjmurillo/moq.analyzers/blob/{ThisAssembly.GitCommitId}/docs/rules/{DiagnosticIds.SetExplicitMockBehavior}.md");
22+
23+
/// <inheritdoc />
24+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
25+
26+
/// <inheritdoc />
27+
public override void Initialize(AnalysisContext context)
28+
{
29+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
30+
context.EnableConcurrentExecution();
31+
32+
context.RegisterCompilationStartAction(RegisterCompilationStartAction);
33+
}
34+
35+
private static void RegisterCompilationStartAction(CompilationStartAnalysisContext context)
36+
{
37+
// Ensure Moq is referenced in the compilation
38+
ImmutableArray<INamedTypeSymbol> mockTypes = context.Compilation.GetMoqMock();
39+
if (mockTypes.IsEmpty)
40+
{
41+
return;
42+
}
43+
44+
// Look for the MockBehavior type and provide it to Analyze to avoid looking it up multiple times.
45+
INamedTypeSymbol? mockBehaviorSymbol = context.Compilation.GetTypeByMetadataName(WellKnownTypeNames.MoqBehavior);
46+
if (mockBehaviorSymbol is null)
47+
{
48+
return;
49+
}
50+
51+
// Look for the Mock.Of() method and provide it to Analyze to avoid looking it up multiple times.
52+
#pragma warning disable ECS0900 // Minimize boxing and unboxing
53+
ImmutableArray<IMethodSymbol> ofMethods = mockTypes
54+
.SelectMany(mockType => mockType.GetMembers(WellKnownTypeNames.Of))
55+
.OfType<IMethodSymbol>()
56+
.Where(method => method.IsGenericMethod)
57+
.ToImmutableArray();
58+
#pragma warning restore ECS0900 // Minimize boxing and unboxing
59+
60+
context.RegisterOperationAction(
61+
context => AnalyzeNewObject(context, mockTypes, mockBehaviorSymbol),
62+
OperationKind.ObjectCreation);
63+
64+
if (!ofMethods.IsEmpty)
65+
{
66+
context.RegisterOperationAction(
67+
context => AnalyzeInvocation(context, ofMethods, mockBehaviorSymbol),
68+
OperationKind.Invocation);
69+
}
70+
}
71+
72+
private static void AnalyzeNewObject(OperationAnalysisContext context, ImmutableArray<INamedTypeSymbol> mockTypes, INamedTypeSymbol mockBehaviorSymbol)
73+
{
74+
if (context.Operation is not IObjectCreationOperation creationOperation)
75+
{
76+
return;
77+
}
78+
79+
if (creationOperation.Type is not INamedTypeSymbol namedType)
80+
{
81+
return;
82+
}
83+
84+
if (!namedType.IsInstanceOf(mockTypes))
85+
{
86+
return;
87+
}
88+
89+
foreach (IArgumentOperation argument in creationOperation.Arguments)
90+
{
91+
if (argument.Value is IFieldReferenceOperation fieldReferenceOperation)
92+
{
93+
ISymbol field = fieldReferenceOperation.Member;
94+
if (field.ContainingType.IsInstanceOf(mockBehaviorSymbol) && IsExplicitBehavior(field.Name))
95+
{
96+
return;
97+
}
98+
}
99+
}
100+
101+
context.ReportDiagnostic(creationOperation.Syntax.GetLocation().CreateDiagnostic(Rule));
102+
}
103+
104+
private static void AnalyzeInvocation(OperationAnalysisContext context, ImmutableArray<IMethodSymbol> wellKnownOfMethods, INamedTypeSymbol mockBehaviorSymbol)
105+
{
106+
if (context.Operation is not IInvocationOperation invocationOperation)
107+
{
108+
return;
109+
}
110+
111+
IMethodSymbol targetMethod = invocationOperation.TargetMethod;
112+
if (!targetMethod.IsInstanceOf(wellKnownOfMethods))
113+
{
114+
return;
115+
}
116+
117+
foreach (IArgumentOperation argument in invocationOperation.Arguments)
118+
{
119+
if (argument.Value is IFieldReferenceOperation fieldReferenceOperation)
120+
{
121+
ISymbol field = fieldReferenceOperation.Member;
122+
if (field.ContainingType.IsInstanceOf(mockBehaviorSymbol) && IsExplicitBehavior(field.Name))
123+
{
124+
return;
125+
}
126+
}
127+
}
128+
129+
context.ReportDiagnostic(invocationOperation.Syntax.GetLocation().CreateDiagnostic(Rule));
130+
}
131+
132+
private static bool IsExplicitBehavior(string symbolName)
133+
{
134+
return string.Equals(symbolName, "Loose", StringComparison.Ordinal) || string.Equals(symbolName, "Strict", StringComparison.Ordinal);
135+
}
136+
}

‎tests/Moq.Analyzers.Test/Helpers/ReferenceAssemblyCatalog.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ internal static class ReferenceAssemblyCatalog
2424
// implementation of `.As<T>()` (see https://github.com/devlooped/moq/commit/b552aeddd82090ee0f4743a1ab70a16f3e6d2d11).
2525
{ nameof(Net80WithOldMoq), ReferenceAssemblies.Net.Net80.AddPackages([new PackageIdentity("Moq", "4.8.2")]) },
2626

27+
// This must be 4.12.0 or later in order to have the new `Mock.Of<T>(MockBehavior)` method (see https://github.com/devlooped/moq/commit/1561c006c87a0894c5257a1e541da44e40e33dd3).
2728
// 4.18.4 is currently the most downloaded version of Moq.
2829
{ nameof(Net80WithNewMoq), ReferenceAssemblies.Net.Net80.AddPackages([new PackageIdentity("Moq", "4.18.4")]) },
2930
};

‎tests/Moq.Analyzers.Test/SetExplicitMockBehaviorAnalyzerTests.cs‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier<Moq.Analyzers.SetExplicitMockBehaviorAnalyzer>;
2+
3+
namespace Moq.Analyzers.Test;
4+
5+
public class SetExplicitMockBehaviorAnalyzerTests
6+
{
7+
public static IEnumerable<object[]> TestData()
8+
{
9+
IEnumerable<object[]> mockConstructors = new object[][]
10+
{
11+
["""{|Moq1400:new Mock<ISample>()|};"""],
12+
["""{|Moq1400:new Mock<ISample>(MockBehavior.Default)|};"""],
13+
["""new Mock<ISample>(MockBehavior.Loose);"""],
14+
["""new Mock<ISample>(MockBehavior.Strict);"""],
15+
}.WithNamespaces().WithMoqReferenceAssemblyGroups();
16+
17+
IEnumerable<object[]> fluentBuilders = new object[][]
18+
{
19+
["""{|Moq1400:Mock.Of<ISample>()|};"""],
20+
["""{|Moq1400:Mock.Of<ISample>(MockBehavior.Default)|};"""],
21+
["""Mock.Of<ISample>(MockBehavior.Loose);"""],
22+
["""Mock.Of<ISample>(MockBehavior.Strict);"""],
23+
}.WithNamespaces().WithNewMoqReferenceAssemblyGroups();
24+
25+
IEnumerable<object[]> mockRepositories = new object[][]
26+
{
27+
["""{|Moq1400:new MockRepository(MockBehavior.Default)|};"""],
28+
["""new MockRepository(MockBehavior.Loose);"""],
29+
["""new MockRepository(MockBehavior.Strict);"""],
30+
}.WithNamespaces().WithNewMoqReferenceAssemblyGroups();
31+
32+
return mockConstructors.Union(fluentBuilders).Union(mockRepositories);
33+
}
34+
35+
[Theory]
36+
[MemberData(nameof(TestData))]
37+
public async Task ShouldAnalyzeMocksWithoutExplictMockBehavior(string referenceAssemblyGroup, string @namespace, string mock)
38+
{
39+
await Verifier.VerifyAnalyzerAsync(
40+
$$"""
41+
{{@namespace}}
42+
43+
public interface ISample
44+
{
45+
int Calculate(int a, int b);
46+
}
47+
48+
internal class UnitTest
49+
{
50+
private void Test()
51+
{
52+
{{mock}}
53+
}
54+
}
55+
""",
56+
referenceAssemblyGroup);
57+
}
58+
}

0 commit comments

Comments
 (0)