Skip to content

Commit b53adfe

Browse files
committed
Introduce PH2160 v1
1 parent de84bf8 commit b53adfe

4 files changed

Lines changed: 763 additions & 0 deletions

File tree

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
# PH2160: Mock<T> of disposable concrete class should configure protected Dispose
2+
3+
| Property | Value |
4+
|--|--|
5+
| Package | [Philips.CodeAnalysis.MoqAnalyzers](https://www.nuget.org/packages/Philips.CodeAnalysis.MoqAnalyzers) |
6+
| Diagnostic ID | PH2160 |
7+
| Category | [RuntimeFailure](../RuntimeFailure.md) |
8+
| Analyzer | [MockDisposableClassesShouldSetupDisposeAnalyzer](https://github.com/philips-software/roslyn-analyzers/blob/main/Philips.CodeAnalysis.MoqAnalyzers/MockDisposableClassesShouldSetupDisposeAnalyzer.cs) |
9+
| CodeFix | No |
10+
| Severity | Error |
11+
| Enabled By Default | Yes |
12+
13+
## Introduction
14+
15+
This rule identifies uses of `Mock<T>` where `T` is a disposable concrete class that appears to follow the standard virtual `Dispose(bool)` pattern.
16+
17+
## Reason
18+
19+
When Moq mocks a concrete class, virtual members are intercepted. By default, Moq does not call base implementations.
20+
21+
If the mocked type performs its cleanup in a virtual `Dispose(bool)` method, calling `Dispose()` on the mocked object may not execute the real cleanup logic. This can result in leaked resources, background threads not stopping, or other cleanup not occurring during tests.
22+
23+
## How to fix violations
24+
25+
Configure the mock’s protected `Dispose` method explicitly, or use a project-specific helper or wrapper that ensures the base `Dispose(bool)` implementation is executed.
26+
27+
Example:
28+
29+
```csharp
30+
mock.Protected().Setup("Dispose", ItExpr.IsAny<bool>()).CallBase();
31+
```
32+
33+
Any explicit protected setup of `Dispose` is considered acceptable for this rule.
34+
35+
## Examples
36+
37+
### Incorrect
38+
39+
```csharp
40+
using Moq;
41+
42+
public class Worker : IDisposable
43+
{
44+
public void Dispose()
45+
{
46+
Dispose(true);
47+
GC.SuppressFinalize(this);
48+
}
49+
50+
protected virtual void Dispose(bool disposing)
51+
{
52+
// real cleanup
53+
}
54+
}
55+
56+
public class TestClass
57+
{
58+
public void TestMethod()
59+
{
60+
var mock = new Mock<Worker>();
61+
}
62+
}
63+
```
64+
65+
### Correct
66+
67+
```csharp
68+
using Moq;
69+
using Moq.Protected;
70+
71+
public class Worker : IDisposable
72+
{
73+
public void Dispose()
74+
{
75+
Dispose(true);
76+
GC.SuppressFinalize(this);
77+
}
78+
79+
protected virtual void Dispose(bool disposing)
80+
{
81+
// real cleanup
82+
}
83+
}
84+
85+
public class TestClass
86+
{
87+
public void TestMethod()
88+
{
89+
var mock = new Mock<Worker>();
90+
mock.Protected().Setup("Dispose", ItExpr.IsAny<bool>()).CallBase();
91+
}
92+
}
93+
```
94+
95+
## Limitations
96+
97+
This analyzer currently focuses on direct `new Mock<T>(...)` constructions and same-member setup patterns. It may not detect disposal configuration performed indirectly through helper methods, factory abstractions, or other layers of indirection.
98+
99+
## Suppression
100+
101+
```csharp
102+
[SuppressMessage("Philips.CodeAnalysis.MoqAnalyzers", "PH2160:Mock<T> of disposable concrete class should configure protected Dispose", Justification = "Reviewed.")]
103+
```
104+
105+
## References
106+
107+
- [Moq](https://github.com/devlooped/moq)
108+
- [Castle DynamicProxy](https://www.castleproject.org/projects/dynamicproxy/)

Philips.CodeAnalysis.Common/DiagnosticId.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,5 +145,6 @@ public enum DiagnosticId
145145
AvoidUnlicensedPackages = 2155,
146146
AvoidPkcsPaddingWithRsaEncryption = 2158,
147147
AvoidUnnecessaryAttributeParentheses = 2159,
148+
MockDisposableObjectsShouldSetupDispose = 2160
148149
}
149150
}

0 commit comments

Comments
 (0)