Skip to content

Commit f69e090

Browse files
Add the PosInfoMoq1010 rule to check there is "+= null" when set the event to raise (fixes #52).
1 parent d7ec2d5 commit f69e090

File tree

8 files changed

+159
-4
lines changed

8 files changed

+159
-4
lines changed

PosInformatique.Moq.Analyzers.sln

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Design", "Design", "{815BE8
3838
docs\Design\PosInfoMoq1007.md = docs\Design\PosInfoMoq1007.md
3939
docs\Design\PosInfoMoq1008.md = docs\Design\PosInfoMoq1008.md
4040
docs\Design\PosInfoMoq1009.md = docs\Design\PosInfoMoq1009.md
41+
docs\Design\PosInfoMoq1010.md = docs\Design\PosInfoMoq1010.md
4142
EndProjectSection
4243
EndProject
4344
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Compilation", "Compilation", "{D9C84D36-7F9C-4EFB-BE6F-9F7A05FE957D}"

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ Design rules used to make your unit tests more strongly strict.
4848
| [PosInfoMoq1006: The `It.IsAny<T>()` or `It.Is<T>()` arguments must match the parameters of the mocked method.](docs/Design/PosInfoMoq1006.md) | When setting up a method using `It.IsAny<T>()` or `It.Is<T>()` as arguments, the type `T` must exactly match the parameters of the configured method. |
4949
| [PosInfoMoq1007: The `Verify()` method must specify the `Times` argument.](docs/Design/PosInfoMoq1007.md) | When calling the `Verify()` method, if the `Times` argument is not specified, Moq will assume `Times.AtLeastOnce()` by default. |
5050
| [PosInfoMoq1008: The `Mock.Verify()` and `Mock.VerifyAll()` methods must specify at least one mock.](docs/Design/PosInfoMoq1008.md) | When calling the static methods `Mock.Verify()` or `Mock.VerifyAll()` without providing any `Mock<T>` instances, no verification is performed. |
51-
| [PosInfoMoq1009: Avoid using `Verifiable()` method](docs/Design/PosInfoMoq1008.md) | A `Verify()` of an `Mock<T>` instance has not been called in the *Assert* phase of an unit test for `Verifiable()` setups. |
51+
| [PosInfoMoq1009: Avoid using `Verifiable()` method](docs/Design/PosInfoMoq1009.md) | A `Verify()` of an `Mock<T>` instance has not been called in the *Assert* phase of an unit test for `Verifiable()` setups. |
52+
| [PosInfoMoq1010: Use `+= null` syntax when raising events with `Raise()`/`RaiseAsync()`](docs/Design/PosInfoMoq1010.md) | When using `Mock<T>.Raise()` or `Mock<T>.RaiseAsync()`, the lambda expression used to identify the event should consistently use the `+= null` syntax. |
5253

5354
### Compilation
5455

docs/Design/PosInfoMoq1010.md

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
# PosInfoMoq1010: Use `+= null` syntax when raising events with `Raise()`/`RaiseAsync()`
2+
3+
| Property | Value |
4+
|-----------------------|----------------------------------------------------------------------|
5+
| **Rule ID** | PosInfoMoq1010 |
6+
| **Title** | Use `+= null` syntax when raising events with `Raise()`/`RaiseAsync()` |
7+
| **Category** | Design |
8+
| **Default severity** | Warning |
9+
10+
## Cause
11+
12+
When using `Mock<T>.Raise()` or `Mock<T>.RaiseAsync()`, the lambda expression used to identify the event should consistently use the `+= null` syntax.
13+
14+
## Rule description
15+
16+
While Moq is flexible and can identify the event even with `+= SomeMethod`, using `+= null` is the conventional and clearest way to indicate that you are merely referencing the event for Moq's internal use, not actually subscribing a handler. This improves code readability and intent.
17+
18+
### Example
19+
20+
```csharp
21+
public class Service
22+
{
23+
public event EventHandler Changed;
24+
25+
public void DoSomething()
26+
{
27+
this.Changed?.Invoke(this, EventArgs.Empty);
28+
}
29+
}
30+
```
31+
32+
#### Correct usage (`+= null`)
33+
34+
```csharp
35+
var serviceMock = new Mock<Service>();
36+
37+
// Clear intent: referencing the event for Moq
38+
serviceMock.Raise(s => s.Changed += null, EventArgs.Empty);
39+
```
40+
41+
#### Incorrect usage (`+= SomeMethod`)
42+
43+
```csharp
44+
public class MyTests
45+
{
46+
private void MyEventHandler(object sender, EventArgs e) { /* ... */ }
47+
48+
[Fact]
49+
public void Test()
50+
{
51+
var serviceMock = new Mock<Service>();
52+
53+
// While functional, less clear intent
54+
serviceMock.Raise(s => s.Changed += MyEventHandler, EventArgs.Empty); // ⚠️
55+
}
56+
}
57+
```
58+
59+
## How to fix violations
60+
61+
Change the lambda expression to use `+= null` when identifying the event for `Raise()` or `RaiseAsync()`.
62+
63+
Correct:
64+
65+
```csharp
66+
serviceMock.Raise(s => s.Changed += null, EventArgs.Empty);
67+
```
68+
69+
Incorrect:
70+
71+
```csharp
72+
serviceMock.Raise(s => s.Changed += MyEventHandler, EventArgs.Empty);
73+
```
74+
75+
## When to suppress warnings
76+
77+
This is a design guideline for code readability. While suppressing it won't cause runtime errors, it's generally recommended to follow for consistent and clear code.

src/Moq.Analyzers/AnalyzerReleases.Shipped.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
### New Rules
44
Rule ID | Category | Severity | Notes
55
--------|----------|----------|-------
6+
PosInfoMoq1010 | Design | Warning | RaiseMethodsAnalyzer, [Documentation](https://posinformatique.github.io/PosInformatique.Moq.Analyzers/docs/Design/PosInfoMoq1010.html)
67
PosInfoMoq2017 | Compilation | Error | RaiseMethodsAnalyzer, [Documentation](https://posinformatique.github.io/PosInformatique.Moq.Analyzers/docs/Compilation/PosInfoMoq2017.html)
78
PosInfoMoq2018 | Compilation | Error | RaiseMethodsAnalyzer, [Documentation](https://posinformatique.github.io/PosInformatique.Moq.Analyzers/docs/Compilation/PosInfoMoq2018.html)
89

src/Moq.Analyzers/Analyzers/RaiseMethodsAnalyzer.cs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,20 @@ public class RaiseMethodsAnalyzer : DiagnosticAnalyzer
3535
description: "The first parameter of Raise()/RaiseAsync() must be an event.",
3636
helpLinkUri: "https://posinformatique.github.io/PosInformatique.Moq.Analyzers/docs/Compilation/PosInfoMoq2018.html");
3737

38+
internal static readonly DiagnosticDescriptor EventExpressionMustBeEventAddWithNull = new DiagnosticDescriptor(
39+
"PosInfoMoq1010",
40+
"Use '+= null' syntax when raising events with Raise()/RaiseAsync()",
41+
"Use '+= null' syntax when raising events with Raise()/RaiseAsync()",
42+
"Design",
43+
DiagnosticSeverity.Warning,
44+
isEnabledByDefault: true,
45+
description: "Use '+= null' syntax when raising events with Raise()/RaiseAsync().",
46+
helpLinkUri: "https://posinformatique.github.io/PosInformatique.Moq.Analyzers/docs/Design/PosInfoMoq1010.html");
47+
3848
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(
3949
ParametersMustMatchSignature,
40-
FirstParameterMustBeEvent);
50+
FirstParameterMustBeEvent,
51+
EventExpressionMustBeEventAddWithNull);
4152

4253
public override void Initialize(AnalysisContext context)
4354
{
@@ -74,6 +85,12 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
7485
return;
7586
}
7687

88+
if (invalidEventExpression is not null)
89+
{
90+
// Raise a warning to indicate that the expression after the "+=" is not "null".
91+
context.ReportDiagnostic(EventExpressionMustBeEventAddWithNull, invalidEventExpression.GetLocation());
92+
}
93+
7794
var eventParameters = raiseMethod.EventParameters.ToList();
7895

7996
// Check the overload of the Raise() method called.

src/Moq.Analyzers/Moq.Analyzers.csproj

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,9 @@
1919
<PackageReleaseNotes>
2020
2.0.0
2121
- Add new rules:
22+
- PosInfoMoq1010: Use '+= null' syntax when raising events with Raise()/RaiseAsync()
2223
- PosInfoMoq2017: Mock&lt;T&gt;.Raise()/RaiseAsync() must use parameters matching the event signature.
23-
- PosInfoMoq2018: The first parameter of `Raise()`/`RaiseAsync()` must be an event.
24+
- PosInfoMoq2018: The first parameter of Raise()/RaiseAsync() must be an event.
2425

2526
1.13.0
2627
- Add new rules:

src/Moq.Analyzers/MoqExpressionAnalyzer.cs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
namespace PosInformatique.Moq.Analyzers
88
{
99
using Microsoft.CodeAnalysis;
10+
using Microsoft.CodeAnalysis.CSharp;
1011
using Microsoft.CodeAnalysis.CSharp.Syntax;
1112

1213
internal class MoqExpressionAnalyzer
@@ -544,7 +545,22 @@ public bool IsStrictBehavior(IdentifierNameSyntax localVariableExpression, Cance
544545
parameterSymbols.Add(parameterSymbol.Type);
545546
}
546547

547-
invalidEventExpression = null;
548+
// Check the right assignement expression is "null".
549+
if (assignmentExpressionSyntax.Right is not LiteralExpressionSyntax literalExpressionSyntax)
550+
{
551+
invalidEventExpression = assignmentExpressionSyntax.Right;
552+
}
553+
else
554+
{
555+
if (literalExpressionSyntax.Kind() != SyntaxKind.NullLiteralExpression)
556+
{
557+
invalidEventExpression = assignmentExpressionSyntax.Right;
558+
}
559+
else
560+
{
561+
invalidEventExpression = null;
562+
}
563+
}
548564

549565
return new RaiseMethodCall((IMethodSymbol)methodSymbol.Symbol, parameterSymbols, arguments, eventSymbol);
550566
}

tests/Moq.Analyzers.Tests/Analyzers/RaiseMethodsAnalyzerTest.cs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,47 @@ public interface I
268268
await Verifier.VerifyAnalyzerAsync(source);
269269
}
270270

271+
[Theory]
272+
[InlineData("Raise", "void")]
273+
[InlineData("RaiseAsync", "Task")]
274+
public async Task Raise_WithNotNullDelegate_NoDiagnosticReported(string method, string eventReturnType)
275+
{
276+
var source = @"
277+
namespace ConsoleApplication1
278+
{
279+
using Moq;
280+
using System;
281+
using System.Threading.Tasks;
282+
283+
public class TestClass
284+
{
285+
public void TestMethod()
286+
{
287+
var mock1 = new Mock<I>(MockBehavior.Strict);
288+
289+
mock1." + method + @"(m => m.TheEvent += {|PosInfoMoq1010:MethodMatchEventSignature|}, ""OK"", 1, null);
290+
mock1." + method + @"(m => m.TheEvent += {|PosInfoMoq1010:MethodMatchEventSignature|}, ""OK"", 1, 10);
291+
mock1." + method + @"(m => m.TheEvent += {|PosInfoMoq1010:MethodMatchEventSignature|}, ""OK"", 1, new object());
292+
293+
mock1." + method + @"(m => m.TheEvent += {|PosInfoMoq1010:(a, b, c) => throw new NotSupportedException()|}, ""OK"", 1, null);
294+
mock1." + method + @"(m => m.TheEvent += {|PosInfoMoq1010:(a, b, c) => throw new NotSupportedException()|}, ""OK"", 1, 10);
295+
mock1." + method + @"(m => m.TheEvent += {|PosInfoMoq1010:(a, b, c) => throw new NotSupportedException()|}, ""OK"", 1, new object());
296+
}
297+
298+
public static " + eventReturnType + @" MethodMatchEventSignature(string a, int b, object c) { throw new NotSupportedException(); }
299+
}
300+
301+
public interface I
302+
{
303+
event CustomEventHandler TheEvent;
304+
}
305+
306+
public delegate " + eventReturnType + @" CustomEventHandler(string a, int b, object c);
307+
}";
308+
309+
await Verifier.VerifyAnalyzerAsync(source);
310+
}
311+
271312
[Fact]
272313
public async Task NoMoqLibrary()
273314
{

0 commit comments

Comments
 (0)