Skip to content

Commit 926d1a4

Browse files
authored
Merge branch 'main' into dependabot/nuget/Microsoft.SourceLink.GitHub-10.0.201
2 parents 3755bdc + 6118325 commit 926d1a4

File tree

7 files changed

+417
-10
lines changed

7 files changed

+417
-10
lines changed

.github/workflows/changelog.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ jobs:
3131
3232
- name: "Commit"
3333
if: ${{ inputs.changelog }}
34-
uses: EndBug/add-and-commit@a94899bca583c204427a224a7af87c02f9b325d5 # v9.1.4
34+
uses: EndBug/add-and-commit@290ea2c423ad77ca9c62ae0f5b224379612c0321 # v10.0.0
3535
with:
3636
message: "chore: updating changelog"
3737
push: true

.github/workflows/codeql-analysis.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ jobs:
1919
- name: Checkout repository
2020
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v5
2121
- name: Initialize CodeQL
22-
uses: github/codeql-action/init@b1bff81932f5cdfc8695c7752dcee935dcd061c8
22+
uses: github/codeql-action/init@c10b8064de6f491fea524254123dbe5e09572f13
2323
with:
2424
languages: ${{ matrix.language }}
2525
# If you wish to specify custom queries, you can do so here or in a config file.
@@ -28,7 +28,7 @@ jobs:
2828
# Prefix the list here with "+" to use these queries and those in the config file.
2929
# Details on CodeQL's query packs refer to : https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs
3030
# queries: security-extended,security-and-quality
31-
uses: github/codeql-action/autobuild@b1bff81932f5cdfc8695c7752dcee935dcd061c8
31+
uses: github/codeql-action/autobuild@c10b8064de6f491fea524254123dbe5e09572f13
3232

3333
- name: Perform CodeQL Analysis
34-
uses: github/codeql-action/analyze@b1bff81932f5cdfc8695c7752dcee935dcd061c8
34+
uses: github/codeql-action/analyze@c10b8064de6f491fea524254123dbe5e09572f13

.github/workflows/pinned-actions.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,6 @@ jobs:
1010
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v5
1111

1212
- name: Ensure SHA pinned actions
13-
uses: zgosalvez/github-actions-ensure-sha-pinned-actions@cc9ffdc62fadb9f83b46dd0979d9ad50408cc6a6 # ratchet:zgosalvez/github-actions-ensure-sha-pinned-actions@v5.0.2
13+
uses: zgosalvez/github-actions-ensure-sha-pinned-actions@ca46236c6ce584ae24bc6283ba8dcf4b3ec8a066 # ratchet:zgosalvez/github-actions-ensure-sha-pinned-actions@v5.0.4
1414
with:
1515
allowlist: philips-software/roslyn-analyzers

Documentation/Diagnostics/PH2160.md

Lines changed: 90 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
|--|--|
55
| Package | [Philips.CodeAnalysis.MoqAnalyzers](https://www.nuget.org/packages/Philips.CodeAnalysis.MoqAnalyzers) |
66
| Diagnostic ID | PH2160 |
7-
| Category | [RuntimeFailure](../RuntimeFailure.md) |
7+
| Category | [Runtime Failure](../RuntimeFailure.md) |
88
| Analyzer | [MockDisposableClassesShouldSetupDisposeAnalyzer](https://github.com/philips-software/roslyn-analyzers/blob/main/Philips.CodeAnalysis.MoqAnalyzers/MockDisposableClassesShouldSetupDisposeAnalyzer.cs) |
99
| CodeFix | No |
1010
| Severity | Error |
@@ -27,7 +27,7 @@ Configure the mock’s protected `Dispose` method explicitly, or use a project-s
2727
Example:
2828

2929
```csharp
30-
mock.Protected().Setup("Dispose", ItExpr.IsAny<bool>()).CallBase();
30+
mock.Protected().Setup("Dispose", ItExpr.IsAny<bool>()).();
3131
```
3232

3333
Any explicit protected setup of `Dispose` is considered acceptable for this rule.
@@ -87,15 +87,102 @@ public class TestClass
8787
public void TestMethod()
8888
{
8989
var mock = new Mock<Worker>();
90-
mock.Protected().Setup("Dispose", ItExpr.IsAny<bool>()).CallBase();
90+
mock.Protected().Setup("Dispose", ItExpr.IsAny<bool>()).();
9191
}
9292
}
9393
```
94+
## Configuration
95+
96+
This rule supports an optional `.editorconfig` setting to specify a preferred mock type for disposable concrete classes.
97+
98+
```ini
99+
dotnet_code_quality.PH2160.preferred_disposable_mock_type = DisposableObjectMock
100+
```
101+
102+
When this option is configured, the CodeFix replaces `Mock<T>` with the configured generic type.
103+
104+
### Example
105+
106+
```csharp
107+
private readonly Mock<MyDisposable> _mock = new Mock<MyDisposable>();
108+
```
109+
110+
becomes:
111+
112+
```csharp
113+
private readonly DisposableObjectMock<MyDisposable> _mock = new DisposableObjectMock<MyDisposable>();
114+
```
115+
116+
This is especially useful for codebases that already use a custom Moq wrapper to ensure disposable concrete classes execute their real `Dispose(bool)` logic.
117+
118+
The wrapper class could look like this:
119+
```csharp
120+
public class DisposableObjectMock<T> : Mock<T>
121+
where T : class, IDisposable
122+
{
123+
public DisposableObjectMock(params object[] args)
124+
: this(MockBehavior.Default, args)
125+
{ }
126+
127+
public DisposableObjectMock(MockBehavior behavior, params object[] args)
128+
: base(behavior, args)
129+
{
130+
this.Protected().Setup(nameof(IDisposable.Dispose), ItExpr.IsAny<bool>()).CallBase();
131+
}
132+
133+
}
134+
```
135+
136+
## Considerations When Applying Fixes
137+
138+
Enabling proper disposal behavior for `Mock<T>` instances can expose issues in existing unit tests or in the classes being mocked. The following are common scenarios observed when applying this rule.
139+
140+
### Interaction with VerifyAll
141+
142+
Some tests use `mock.VerifyAll()` to ensure that all configured expectations are met. When disposal behavior is enabled (e.g., via `Protected().Setup("Dispose", ...)` or a custom disposable mock wrapper), the `Dispose()` method becomes part of the mock's observable behavior.
143+
144+
As a result, `Dispose()` may be expected to be called before `VerifyAll()` executes. If it is not, tests may fail with unmet expectations.
145+
146+
**Possible resolutions:**
147+
- Explicitly call `Dispose()` on the mocked object before invoking `VerifyAll()`
148+
- Relax verification (e.g., avoid `VerifyAll()` when not strictly necessary)
149+
- Adjust mock setup to exclude `Dispose()` from strict verification scenarios
150+
151+
---
152+
153+
### Non-Idempotent or Fragile Dispose Implementations
154+
155+
In some cases, enabling real disposal logic can cause tests to fail due to issues in the underlying class implementation. For example:
156+
157+
- `Dispose()` may assume certain fields are initialized
158+
- Cleanup logic may not be safe to call in partially constructed states
159+
- Multiple calls to `Dispose()` may not be handled correctly
160+
161+
This can lead to runtime exceptions such as `NullReferenceException`.
162+
163+
**Possible resolutions:**
164+
- Ensure `Dispose()` implementations are **idempotent** (safe to call multiple times)
165+
- Add appropriate null checks or guards in disposal logic
166+
- Review object initialization to ensure required state is established before disposal
167+
168+
---
169+
170+
### Summary
171+
172+
This analyzer helps ensure that mocked disposable objects behave more like their real counterparts. However, enabling disposal may reveal latent issues in tests or implementations that were previously hidden due to Moq's default behavior of not calling base methods.
173+
174+
These issues are typically indicative of real correctness or robustness problems and should be addressed rather than suppressed.
94175

95176
## Limitations
96177

97178
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.
98179

180+
The current CodeFix requires `dotnet_code_quality.PH2160.preferred_disposable_mock_type` to be configured.
181+
182+
The current implementation uses the fully qualified configured type name directly instead of adding a `using` statement and simplifying the type name.
183+
184+
This analyzer does not ensure that the mocked object is actually disposed. It merely ensures that, if disposed, that the protected virtual Dispose method is accounted for. Typically, this approach is appropriate, as mocked objects are often injected into another class, and that class' Dispose will Dispose our mocked object.
185+
99186
## Suppression
100187

101188
```csharp

Philips.CodeAnalysis.MoqAnalyzers/MockDisposableClassesShouldSetupDisposeAnalyzer.cs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// © 2026 Koninklijke Philips N.V. See License.md in the project root for license information.
22

3+
using System.Collections.Immutable;
34
using System.Linq;
45
using Microsoft.CodeAnalysis;
56
using Microsoft.CodeAnalysis.CSharp;
@@ -12,8 +13,8 @@ namespace Philips.CodeAnalysis.MoqAnalyzers
1213
[DiagnosticAnalyzer(LanguageNames.CSharp)]
1314
public class MockDisposableClassesShouldSetupDisposeAnalyzer : SingleDiagnosticAnalyzer
1415
{
16+
public const string PreferredDisposableMockTypeProperty = "PreferredDisposableMockType";
1517
private const string MockName = "Mock";
16-
1718
private const string Title = @"Mock<T> of disposable concrete class should setup virtual Dispose(bool)";
1819
private const string MessageFormat = @"Mock<{0}> may suppress real disposal because Moq does not call base implementations by default";
1920
private const string Description = @"Mocking a disposable concrete class without configuring protected Dispose may prevent cleanup from executing";
@@ -80,7 +81,21 @@ private void AnalyzeObjectCreation(SyntaxNodeAnalysisContext context)
8081
return;
8182
}
8283

83-
context.ReportDiagnostic(Diagnostic.Create(Rule, objectCreationExpressionSyntax.GetLocation(), mockedType.Name));
84+
ReportDiagnostic(context, objectCreationExpressionSyntax, mockedType);
85+
}
86+
87+
private void ReportDiagnostic(SyntaxNodeAnalysisContext context, ObjectCreationExpressionSyntax objectCreationExpressionSyntax, ITypeSymbol mockedType)
88+
{
89+
// Pass the preferred disposable mock type from editorconfig as additional property, so that code fix can use it to determine which type to suggest in the code fix message and when applying the code fix.
90+
var preferredDisposableMockType = Helper.ForAdditionalFiles.GetValueFromEditorConfig(Rule.Id, "preferred_disposable_mock_type");
91+
92+
ImmutableDictionary<string, string> properties = ImmutableDictionary<string, string>.Empty;
93+
if (!string.IsNullOrWhiteSpace(preferredDisposableMockType))
94+
{
95+
properties = properties.Add(PreferredDisposableMockTypeProperty, preferredDisposableMockType.Trim());
96+
}
97+
98+
context.ReportDiagnostic(Diagnostic.Create(Rule, objectCreationExpressionSyntax.GetLocation(), properties, mockedType.Name));
8499
}
85100

86101
private bool TryGetMockedType(SyntaxNodeAnalysisContext context, ObjectCreationExpressionSyntax objectCreationExpressionSyntax, out ITypeSymbol mockedType)
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
// © 2026 Koninklijke Philips N.V. See License.md in the project root for license information.
2+
3+
using System.Collections.Immutable;
4+
using System.Composition;
5+
using System.Threading;
6+
using System.Threading.Tasks;
7+
using Microsoft.CodeAnalysis;
8+
using Microsoft.CodeAnalysis.CodeFixes;
9+
using Microsoft.CodeAnalysis.CSharp;
10+
using Microsoft.CodeAnalysis.CSharp.Syntax;
11+
using Microsoft.CodeAnalysis.Formatting;
12+
using Philips.CodeAnalysis.Common;
13+
14+
namespace Philips.CodeAnalysis.MoqAnalyzers
15+
{
16+
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(MockDisposableClassesShouldSetupDisposeCodeFixProvider)), Shared]
17+
public class MockDisposableClassesShouldSetupDisposeCodeFixProvider : SingleDiagnosticCodeFixProvider<ObjectCreationExpressionSyntax>
18+
{
19+
protected override string Title => "Use configured disposable mock type";
20+
21+
protected override DiagnosticId DiagnosticId => DiagnosticId.MockDisposableObjectsShouldSetupDispose;
22+
23+
protected override async Task<Document> ApplyFix(Document document, ObjectCreationExpressionSyntax node, ImmutableDictionary<string, string> properties, CancellationToken cancellationToken)
24+
{
25+
var configuredTypeName = GetPreferredDisposableMockType(properties);
26+
if (string.IsNullOrWhiteSpace(configuredTypeName))
27+
{
28+
return document;
29+
}
30+
31+
SyntaxNode rootNode = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
32+
if (rootNode == null)
33+
{
34+
return document;
35+
}
36+
37+
if (node.Type is not GenericNameSyntax objectCreationGenericName)
38+
{
39+
return document;
40+
}
41+
42+
TypeSyntax replacementTypeSyntax = CreateReplacementTypeSyntax(configuredTypeName, objectCreationGenericName.TypeArgumentList);
43+
44+
TypeSyntax replacementType = replacementTypeSyntax.WithAdditionalAnnotations(Formatter.Annotation);
45+
46+
SyntaxNode newRoot;
47+
TypeSyntax declaredTypeToReplace = GetDeclaredTypeToReplace(node, objectCreationGenericName);
48+
49+
if (declaredTypeToReplace != null)
50+
{
51+
newRoot = rootNode.ReplaceNodes(
52+
new SyntaxNode[] { node.Type, declaredTypeToReplace },
53+
(original, _) => replacementType.WithTriviaFrom(original));
54+
}
55+
else
56+
{
57+
newRoot = rootNode.ReplaceNode(
58+
node.Type,
59+
replacementType.WithTriviaFrom(node.Type));
60+
}
61+
62+
return document.WithSyntaxRoot(newRoot);
63+
}
64+
65+
private static TypeSyntax GetDeclaredTypeToReplace(ObjectCreationExpressionSyntax objectCreationSyntax, GenericNameSyntax originalObjectCreationType)
66+
{
67+
VariableDeclarationSyntax variableDeclarationSyntax = objectCreationSyntax.FirstAncestorOrSelf<VariableDeclarationSyntax>();
68+
if (variableDeclarationSyntax != null &&
69+
variableDeclarationSyntax.Type is GenericNameSyntax declaredGenericName &&
70+
declaredGenericName.Identifier.ValueText == originalObjectCreationType.Identifier.ValueText)
71+
{
72+
return variableDeclarationSyntax.Type;
73+
}
74+
75+
PropertyDeclarationSyntax propertyDeclarationSyntax = objectCreationSyntax.FirstAncestorOrSelf<PropertyDeclarationSyntax>();
76+
if (propertyDeclarationSyntax != null &&
77+
propertyDeclarationSyntax.Type is GenericNameSyntax propertyGenericName &&
78+
propertyGenericName.Identifier.ValueText == originalObjectCreationType.Identifier.ValueText)
79+
{
80+
return propertyDeclarationSyntax.Type;
81+
}
82+
83+
return null;
84+
}
85+
86+
private static string GetPreferredDisposableMockType(ImmutableDictionary<string, string> properties)
87+
{
88+
if (properties != null &&
89+
properties.TryGetValue(MockDisposableClassesShouldSetupDisposeAnalyzer.PreferredDisposableMockTypeProperty, out var configuredTypeName) &&
90+
!string.IsNullOrWhiteSpace(configuredTypeName))
91+
{
92+
return configuredTypeName.Trim();
93+
}
94+
95+
return null;
96+
}
97+
98+
private static TypeSyntax CreateReplacementTypeSyntax(string configuredTypeName, TypeArgumentListSyntax originalTypeArguments)
99+
{
100+
NameSyntax configuredNameSyntax = SyntaxFactory.ParseName(configuredTypeName);
101+
102+
SimpleNameSyntax genericNameSyntax;
103+
if (configuredNameSyntax is QualifiedNameSyntax qualifiedNameSyntax)
104+
{
105+
genericNameSyntax = qualifiedNameSyntax.Right;
106+
}
107+
else
108+
{
109+
genericNameSyntax = configuredNameSyntax as SimpleNameSyntax;
110+
}
111+
112+
GenericNameSyntax appendedGenericName = SyntaxFactory.GenericName(genericNameSyntax.Identifier, originalTypeArguments);
113+
114+
if (configuredNameSyntax is QualifiedNameSyntax qualifiedConfiguredName)
115+
{
116+
return qualifiedConfiguredName.WithRight(appendedGenericName);
117+
}
118+
119+
return appendedGenericName;
120+
}
121+
}
122+
}

0 commit comments

Comments
 (0)