Skip to content

Commit 8db5abc

Browse files
Copilotbcollamore
Copilot
and
bcollamore
authored
feat: Add analyzer to avoid problematic using statement patterns (PH2149) (#855)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bcollamore <57269455+bcollamore@users.noreply.github.com>
1 parent d4e915a commit 8db5abc

4 files changed

Lines changed: 368 additions & 0 deletions

File tree

‎Documentation/Diagnostics/PH2149.md‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
# PH2149: Avoid problematic using statement patterns
2+
3+
| Property | Value |
4+
|--|--|
5+
| Package | [Philips.CodeAnalysis.MaintainabilityAnalyzers](https://www.nuget.org/packages/Philips.CodeAnalysis.MaintainabilityAnalyzers) |
6+
| Diagnostic ID | PH2149 |
7+
| Category | [Maintainability](../Maintainability.md) |
8+
| Analyzer | [AvoidProblematicUsingPatternsAnalyzer](https://github.com/philips-software/roslyn-analyzers/blob/main/Philips.CodeAnalysis.MaintainabilityAnalyzers/Maintainability/AvoidProblematicUsingPatternsAnalyzer.cs)
9+
| CodeFix | No |
10+
| Severity | Warning |
11+
| Enabled By Default | No |
12+
13+
## Introduction
14+
15+
This rule flags `using` statements that use fields, member access expressions, or local variables, which can lead to double disposal or ownership issues.
16+
17+
## Reason
18+
19+
Using statements with fields or variables can be problematic because they may dispose objects that are already being managed elsewhere, potentially causing:
20+
21+
- Double disposal exceptions when the object is disposed both by the using statement and by other code
22+
- Access to disposed objects if the field/variable is used after the using block
23+
- Unclear ownership semantics where multiple parts of code think they own the object
24+
25+
## How to fix violations
26+
27+
Replace problematic using patterns with safer alternatives:
28+
29+
- Create new objects directly in the using statement
30+
- Call methods that return disposable objects
31+
- Use parameters instead of fields when possible
32+
- Consider refactoring the design to have clearer ownership semantics
33+
34+
## Examples
35+
36+
### Incorrect
37+
38+
```csharp
39+
class BadExample
40+
{
41+
private Stream _stream = new MemoryStream();
42+
43+
public void BadMethod()
44+
{
45+
// Direct field usage - may lead to double disposal
46+
using (_stream)
47+
{
48+
// Do something
49+
}
50+
51+
// Field assignment to using variable
52+
using (var localStream = _stream)
53+
{
54+
// Do something
55+
}
56+
57+
// Member access expression
58+
using (this.Stream)
59+
{
60+
// Do something
61+
}
62+
63+
// Local variable reuse
64+
Stream existingStream = new MemoryStream();
65+
using (var localStream = existingStream)
66+
{
67+
// Do something
68+
}
69+
}
70+
}
71+
```
72+
73+
### Correct
74+
75+
```csharp
76+
class GoodExample
77+
{
78+
public void GoodMethod(Stream inputParameter)
79+
{
80+
// Create new object directly - safe
81+
using (var stream = new MemoryStream())
82+
{
83+
// Do something
84+
}
85+
86+
// Method call that returns disposable - safe
87+
using (var stream = CreateStream())
88+
{
89+
// Do something
90+
}
91+
92+
// Using parameter - safe (caller owns the object)
93+
using (var stream = inputParameter)
94+
{
95+
// Do something
96+
}
97+
}
98+
99+
private Stream CreateStream()
100+
{
101+
return new MemoryStream();
102+
}
103+
}
104+
```

‎Philips.CodeAnalysis.Common/DiagnosticId.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ public enum DiagnosticId
139139
AvoidStringFormatInInterpolatedString = 2145,
140140
AvoidToStringOnString = 2146,
141141
AvoidVariableNamedUnderscore = 2147,
142+
AvoidProblematicUsingPatterns = 2149,
142143
AvoidTodoComments = 2151,
143144
AvoidUnusedToString = 2153,
144145
AvoidUnlicensedPackages = 2155,

‎Philips.CodeAnalysis.MaintainabilityAnalyzers/Maintainability/AvoidProblematicUsingPatternsAnalyzer.cs‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// © 2025 Koninklijke Philips N.V. See License.md in the project root for license information.
2+
3+
using Microsoft.CodeAnalysis;
4+
using Microsoft.CodeAnalysis.CSharp.Syntax;
5+
using Microsoft.CodeAnalysis.Diagnostics;
6+
using Philips.CodeAnalysis.Common;
7+
8+
namespace Philips.CodeAnalysis.MaintainabilityAnalyzers.Maintainability
9+
{
10+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
11+
public class AvoidProblematicUsingPatternsAnalyzer : SingleDiagnosticAnalyzer<UsingStatementSyntax, AvoidProblematicUsingPatternsSyntaxNodeAction>
12+
{
13+
private const string Title = @"Avoid problematic using statement patterns";
14+
public const string MessageFormat = @"Avoid using statement that may lead to double disposal.";
15+
private const string Description = @"Avoid using statements with fields or variables.";
16+
17+
public AvoidProblematicUsingPatternsAnalyzer()
18+
: base(DiagnosticId.AvoidProblematicUsingPatterns, Title, MessageFormat, Description, Categories.Maintainability, isEnabled: false)
19+
{ }
20+
}
21+
public class AvoidProblematicUsingPatternsSyntaxNodeAction : SyntaxNodeAction<UsingStatementSyntax>
22+
{
23+
public override void Analyze()
24+
{
25+
if (Node.Declaration != null)
26+
{
27+
AnalyzeUsingDeclaration(Node.Declaration);
28+
}
29+
else if (Node.Expression != null)
30+
{
31+
AnalyzeUsingExpression(Node.Expression);
32+
}
33+
}
34+
35+
private void AnalyzeUsingDeclaration(VariableDeclarationSyntax declaration)
36+
{
37+
foreach (VariableDeclaratorSyntax variable in declaration.Variables)
38+
{
39+
if (variable.Initializer?.Value != null)
40+
{
41+
ExpressionSyntax initializerValue = variable.Initializer.Value;
42+
if (IsProblematicExpression(initializerValue))
43+
{
44+
Location location = variable.GetLocation();
45+
ReportDiagnostic(location);
46+
}
47+
}
48+
}
49+
}
50+
51+
private void AnalyzeUsingExpression(ExpressionSyntax expression)
52+
{
53+
if (IsProblematicExpression(expression))
54+
{
55+
Location location = expression.GetLocation();
56+
ReportDiagnostic(location);
57+
}
58+
}
59+
60+
private bool IsProblematicExpression(ExpressionSyntax expression)
61+
{
62+
if (expression is MemberAccessExpressionSyntax)
63+
{
64+
return true;
65+
}
66+
67+
if (expression is IdentifierNameSyntax identifier)
68+
{
69+
SymbolInfo symbolInfo = Context.SemanticModel.GetSymbolInfo(identifier);
70+
if (symbolInfo.Symbol is IFieldSymbol)
71+
{
72+
return true;
73+
}
74+
75+
if (symbolInfo.Symbol is ILocalSymbol)
76+
{
77+
return true;
78+
}
79+
}
80+
81+
return false;
82+
}
83+
}
84+
}

‎Philips.CodeAnalysis.Test/Maintainability/Maintainability/AvoidProblematicUsingPatternsAnalyzerTest.cs‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
// © 2024 Koninklijke Philips N.V. See License.md in the project root for license information.
2+
3+
using System.Threading.Tasks;
4+
using Microsoft.CodeAnalysis.Diagnostics;
5+
using Microsoft.VisualStudio.TestTools.UnitTesting;
6+
using Philips.CodeAnalysis.MaintainabilityAnalyzers.Maintainability;
7+
using Philips.CodeAnalysis.Test.Helpers;
8+
using Philips.CodeAnalysis.Test.Verifiers;
9+
10+
namespace Philips.CodeAnalysis.Test.Maintainability.Maintainability
11+
{
12+
[TestClass]
13+
public class AvoidProblematicUsingPatternsAnalyzerTest : DiagnosticVerifier
14+
{
15+
[TestMethod]
16+
[TestCategory(TestDefinitions.UnitTests)]
17+
public async Task WhenUsingFieldDirectlyDiagnosticIsTriggeredAsync()
18+
{
19+
var source = @"
20+
using System;
21+
using System.IO;
22+
23+
class TestClass
24+
{
25+
private Stream _stream;
26+
27+
public void TestMethod()
28+
{
29+
using (_stream)
30+
{
31+
// Do something
32+
}
33+
}
34+
}";
35+
await VerifyDiagnostic(source).ConfigureAwait(false);
36+
}
37+
38+
[TestMethod]
39+
[TestCategory(TestDefinitions.UnitTests)]
40+
public async Task WhenUsingVariableAssignmentFromFieldDiagnosticIsTriggeredAsync()
41+
{
42+
var source = @"
43+
using System;
44+
using System.IO;
45+
46+
class TestClass
47+
{
48+
private Stream _stream;
49+
50+
public void TestMethod()
51+
{
52+
using (var localStream = _stream)
53+
{
54+
// Do something
55+
}
56+
}
57+
}";
58+
await VerifyDiagnostic(source).ConfigureAwait(false);
59+
}
60+
61+
[TestMethod]
62+
[TestCategory(TestDefinitions.UnitTests)]
63+
public async Task WhenUsingVariableAssignmentFromVariableDiagnosticIsTriggeredAsync()
64+
{
65+
var source = @"
66+
using System;
67+
using System.IO;
68+
69+
class TestClass
70+
{
71+
public void TestMethod()
72+
{
73+
Stream existingStream = new MemoryStream();
74+
using (var localStream = existingStream)
75+
{
76+
// Do something
77+
}
78+
}
79+
}";
80+
await VerifyDiagnostic(source).ConfigureAwait(false);
81+
}
82+
83+
[TestMethod]
84+
[TestCategory(TestDefinitions.UnitTests)]
85+
public async Task WhenUsingMemberAccessDiagnosticIsTriggeredAsync()
86+
{
87+
var source = @"
88+
using System;
89+
using System.IO;
90+
91+
class TestClass
92+
{
93+
public Stream Stream { get; set; }
94+
95+
public void TestMethod()
96+
{
97+
using (this.Stream)
98+
{
99+
// Do something
100+
}
101+
}
102+
}";
103+
await VerifyDiagnostic(source).ConfigureAwait(false);
104+
}
105+
106+
[TestMethod]
107+
[TestCategory(TestDefinitions.UnitTests)]
108+
public async Task WhenUsingNewObjectNoDiagnosticIsTriggeredAsync()
109+
{
110+
var source = @"
111+
using System;
112+
using System.IO;
113+
114+
class TestClass
115+
{
116+
public void TestMethod()
117+
{
118+
using (var stream = new MemoryStream())
119+
{
120+
// Do something
121+
}
122+
}
123+
}";
124+
await VerifySuccessfulCompilation(source).ConfigureAwait(false);
125+
}
126+
127+
[TestMethod]
128+
[TestCategory(TestDefinitions.UnitTests)]
129+
public async Task WhenUsingMethodCallNoDiagnosticIsTriggeredAsync()
130+
{
131+
var source = @"
132+
using System;
133+
using System.IO;
134+
135+
class TestClass
136+
{
137+
public void TestMethod()
138+
{
139+
using (var stream = CreateStream())
140+
{
141+
// Do something
142+
}
143+
}
144+
145+
private Stream CreateStream()
146+
{
147+
return new MemoryStream();
148+
}
149+
}";
150+
await VerifySuccessfulCompilation(source).ConfigureAwait(false);
151+
}
152+
153+
[TestMethod]
154+
[TestCategory(TestDefinitions.UnitTests)]
155+
public async Task WhenUsingParameterNoDiagnosticIsTriggeredAsync()
156+
{
157+
var source = @"
158+
using System;
159+
using System.IO;
160+
161+
class TestClass
162+
{
163+
public void TestMethod(Stream inputStream)
164+
{
165+
using (var stream = inputStream)
166+
{
167+
// Do something
168+
}
169+
}
170+
}";
171+
await VerifySuccessfulCompilation(source).ConfigureAwait(false);
172+
}
173+
174+
protected override DiagnosticAnalyzer GetDiagnosticAnalyzer()
175+
{
176+
return new AvoidProblematicUsingPatternsAnalyzer();
177+
}
178+
}
179+
}

0 commit comments

Comments
 (0)