Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: suppress CA1001 when Dispose is called in the TearDown method #568

Closed
jhinder opened this issue Aug 7, 2023 · 2 comments · Fixed by #576
Closed

Feature request: suppress CA1001 when Dispose is called in the TearDown method #568

jhinder opened this issue Aug 7, 2023 · 2 comments · Fixed by #576

Comments

@jhinder
Copy link

jhinder commented Aug 7, 2023

The .NET analyzers (from the SDK) report CA1001 when the object under test is IDisposable. We're calling Dispose in the [TearDown] method, but we still have to suppress CA1001 (pragma directive, editorconfig, etc) because the .NET analyzers don't recognize the NUnit patterns and because we have TreatWarningsAsErrors enabled in our codebase.

Could you use the DiagnosticSuppressor API to suppress CA1001 in that case?

public sealed class C : IDisposable
{
    public void Dispose()
    {
    }
}

[TestFixture]
public class Test // CA1001: Type 'Test' owns disposable field(s) '_objectUnderTest' but is not disposable
{
    [SetUp]
    public void SetUp()
    {
        _objectUnderTest = new C();
    }

    [TearDown]
    public void TearDown()
    {
        _objectUnderTest.Dispose();
    }

    private C _objectUnderTest;
}
@manfred-brands
Copy link
Member

@jhinder I know where you are coming from.

If the TearDown is simple, like your example, this might be doable, but as soon as there are conditional statements or indirect calls it will become virtually impossible.
If you look at the code for the DisposeAnalysis in the Roslyn Analyzers you see what I mean.

At work we currently add the below to our .editorconfig to solve this for unit tests:

[*Fixture.cs]
# Types that own disposable fields should be disposable
# TODO: We agree with this rule, except for NUnit
# where fields initialized in SetUp must be disposed in TearDown.
dotnet_diagnostic.CA1001.severity = none

But we want to get rid of it as well because we had test not cleaning up, so it would be good to detect this.

Watch this space.

@manfred-brands
Copy link
Member

Ok, suppression is possible.
I can get the list of offending field names. Now I have to find out of they are disposed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants