-
Notifications
You must be signed in to change notification settings - Fork 32
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add DisposableFieldsSuppressor and DisposeFieldsInTearDownAnalyzer
- Loading branch information
1 parent
e9c7ab2
commit d67a14d
Showing
18 changed files
with
855 additions
and
10 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
# NUnit1032 | ||
|
||
## An IDisposable field should be Disposed in a TearDown method | ||
|
||
| Topic | Value | ||
| :-- | :-- | ||
| Id | NUnit1032 | ||
| Severity | Error | ||
| Enabled | True | ||
| Category | Structure | ||
| Code | [DisposeFieldsInTearDownAnalyzer](https://github.com/nunit/nunit.analyzers/blob/master/src/nunit.analyzers/DisposeFieldsInTearDown/DisposeFieldsInTearDownAnalyzer.cs) | ||
|
||
## Description | ||
|
||
An IDisposable field should be Disposed in a TearDown method. | ||
|
||
## Motivation | ||
|
||
Not Diposing fields can cause memory leaks or failing tests. | ||
|
||
## How to fix violations | ||
|
||
Dispose any fields that are initialized in `SetUp` or `Test` methods in a `TearDown` method. | ||
Fields that are initialized in `OneTimeSetUp` must be disposed in `OneTimeTearDown`. | ||
|
||
<!-- start generated config severity --> | ||
## Configure severity | ||
|
||
### Via ruleset file | ||
|
||
Configure the severity per project, for more info see [MSDN](https://learn.microsoft.com/en-us/visualstudio/code-quality/using-rule-sets-to-group-code-analysis-rules?view=vs-2022). | ||
|
||
### Via .editorconfig file | ||
|
||
```ini | ||
# NUnit1032: An IDisposable field should be Disposed in a TearDown method | ||
dotnet_diagnostic.NUnit1032.severity = chosenSeverity | ||
``` | ||
|
||
where `chosenSeverity` can be one of `none`, `silent`, `suggestion`, `warning`, or `error`. | ||
|
||
### Via #pragma directive | ||
|
||
```csharp | ||
#pragma warning disable NUnit1032 // An IDisposable field should be Disposed in a TearDown method | ||
Code violating the rule here | ||
#pragma warning restore NUnit1032 // An IDisposable field should be Disposed in a TearDown method | ||
``` | ||
|
||
Or put this at the top of the file to disable all instances. | ||
|
||
```csharp | ||
#pragma warning disable NUnit1032 // An IDisposable field should be Disposed in a TearDown method | ||
``` | ||
|
||
### Via attribute `[SuppressMessage]` | ||
|
||
```csharp | ||
[System.Diagnostics.CodeAnalysis.SuppressMessage("Structure", | ||
"NUnit1032:An IDisposable field should be Disposed in a TearDown method", | ||
Justification = "Reason...")] | ||
``` | ||
<!-- end generated config severity --> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
# NUnit3004 | ||
|
||
## Field should be Disposed in TearDown or OneTimeTearDown method | ||
|
||
| Topic | Value | ||
| :-- | :-- | ||
| Id | NUnit3004 | ||
| Severity | Info | ||
| Enabled | True | ||
| Category | Suppressor | ||
| Code | [TestFixtureSuppressor](https://github.com/nunit/nunit.analyzers/blob/master/src/nunit.analyzers/DiagnosticSuppressors/TestFixtureSuppressor.cs) | ||
|
||
## Description | ||
|
||
Field/Property is Disposed in TearDown or OneTimeTearDown method | ||
|
||
## Motivation | ||
|
||
The Roslyn analyzer fires [CA1001](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1001) | ||
for classes that have [`IDisposable`](https://learn.microsoft.com/en-us/dotnet/api/system.idisposable) members, but itself is not `IDisposable`. | ||
|
||
Many NUnit tests initialize fields in tests or a `SetUp` method and then `Dispose` them in the `TearDown` method. | ||
|
||
## How to fix violations | ||
|
||
Ensure that all `IDisposable` fields have a `Dispose` call in the `TearDown` method. | ||
|
||
<!-- start generated config severity --> | ||
## Configure severity | ||
|
||
The rule has no severity, but can be disabled. | ||
|
||
### Via ruleset file | ||
|
||
To disable the rule for a project, you need to add a | ||
[ruleset file](https://github.com/nunit/nunit.analyzers/blob/master/src/nunit.analyzers/DiagnosticSuppressors/NUnit.Analyzers.Suppressions.ruleset) | ||
|
||
```xml | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<RuleSet Name="NUnit.Analyzer Suppressions" Description="DiagnosticSuppression Rules" ToolsVersion="12.0"> | ||
<Rules AnalyzerId="DiagnosticSuppressors" RuleNamespace="NUnit.NUnitAnalyzers"> | ||
<Rule Id="NUnit3001" Action="Info" /> <!-- Possible Null Reference --> | ||
<Rule Id="NUnit3002" Action="Info" /> <!-- NonNullableField/Property is Uninitialized --> | ||
<Rule Id="NUnit3003" Action="Info" /> <!-- Avoid Uninstantiated Internal Classes --> | ||
<Rule Id="NUnit3004" Action="Info" /> <!-- Types that own disposable fields should be disposable --> | ||
</Rules> | ||
</RuleSet> | ||
``` | ||
|
||
and add it to the project like: | ||
|
||
```xml | ||
<PropertyGroup> | ||
<CodeAnalysisRuleSet>NUnit.Analyzers.Suppressions.ruleset</CodeAnalysisRuleSet> | ||
</PropertyGroup> | ||
``` | ||
|
||
For more info about rulesets see [MSDN](https://learn.microsoft.com/en-us/visualstudio/code-quality/using-rule-sets-to-group-code-analysis-rules?view=vs-2022). | ||
|
||
### Via .editorconfig file | ||
|
||
This is currently not working. Waiting for [Roslyn](https://github.com/dotnet/roslyn/issues/49727) | ||
|
||
```ini | ||
# NUnit3004: Field should be Disposed in TearDown or OneTimeTearDown method | ||
dotnet_diagnostic.NUnit3004.severity = none | ||
``` | ||
<!-- end generated config severity --> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
93 changes: 93 additions & 0 deletions
93
src/nunit.analyzers.tests/DiagnosticSuppressors/DisposableFieldsSuppressorTests.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
using System; | ||
using System.IO; | ||
using System.Reflection; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis.Diagnostics; | ||
using NUnit.Analyzers.DiagnosticSuppressors; | ||
using NUnit.Framework; | ||
|
||
namespace NUnit.Analyzers.Tests.DiagnosticSuppressors | ||
{ | ||
public class DisposableFieldsSuppressorTests | ||
{ | ||
private static readonly DiagnosticSuppressor suppressor = new TestFixtureSuppressor(); | ||
private DiagnosticAnalyzer analyzer; | ||
|
||
[OneTimeSetUp] | ||
public void OneTimeSetUp() | ||
{ | ||
// Find the NetAnalyzers assembly (note version should match the one referenced) | ||
string netAnalyzersPath = Path.Combine(PathHelper.GetNuGetPackageDirectory(), | ||
"microsoft.codeanalysis.netanalyzers/7.0.4/analyzers/dotnet/cs/Microsoft.CodeAnalysis.CSharp.NetAnalyzers.dll"); | ||
Assembly netAnalyzerAssembly = Assembly.LoadFrom(netAnalyzersPath); | ||
Type analyzerType = netAnalyzerAssembly.GetType("Microsoft.CodeQuality.CSharp.Analyzers.ApiDesignGuidelines.CSharpTypesThatOwnDisposableFieldsShouldBeDisposableAnalyzer", true)!; | ||
this.analyzer = (DiagnosticAnalyzer)Activator.CreateInstance(analyzerType)!; | ||
|
||
this.analyzer = new DefaultEnabledAnalyzer(this.analyzer); | ||
} | ||
|
||
[Test] | ||
public async Task FieldNotDisposed() | ||
{ | ||
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@" | ||
private IDisposable? field; | ||
[Test] | ||
public void Test() | ||
{ | ||
field = new DummyDisposable(); | ||
Assert.That(field, Is.Not.Null); | ||
} | ||
private sealed class DummyDisposable : IDisposable | ||
{ | ||
public void Dispose() {} | ||
} | ||
"); | ||
|
||
// This rule doesn't care. Actual checking is done in DisposeFieldsInTearDownAnalyzer | ||
await TestHelpers.Suppressed(this.analyzer, suppressor, testCode).ConfigureAwait(true); | ||
} | ||
|
||
[TestCase("TearDown", "")] | ||
[TestCase("OneTimeTearDown", "this.")] | ||
public async Task FieldDisposed(string attribute, string prefix) | ||
{ | ||
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@$" | ||
private IDisposable? field1; | ||
private IDisposable? field2; | ||
[{attribute}] | ||
public void CleanUp() | ||
{{ | ||
{prefix}field1?.Dispose(); | ||
if ({prefix}field2 is not null) | ||
{{ | ||
{prefix}field2.Dispose(); | ||
}} | ||
}} | ||
[Test] | ||
public void Test1() | ||
{{ | ||
{prefix}field1 = new DummyDisposable(); | ||
Assert.That({prefix}field1, Is.Not.Null); | ||
}} | ||
[Test] | ||
public void Test2() | ||
{{ | ||
{prefix}field2 = new DummyDisposable(); | ||
Assert.That({prefix}field2, Is.Not.Null); | ||
}} | ||
private sealed class DummyDisposable : IDisposable | ||
{{ | ||
public void Dispose() {{}} | ||
}} | ||
"); | ||
|
||
await TestHelpers.Suppressed(this.analyzer, suppressor, testCode).ConfigureAwait(true); | ||
} | ||
} | ||
} |
Oops, something went wrong.