Do not suppress CA1001 if Fixture uses LifeCycle.InstancePerTestCase

nunit · Sep 24, 2023 · c6175b3 · c6175b3
1 parent 3a43fae
commit c6175b3
Show file tree

Hide file tree

Showing 10 changed files with 132 additions and 20 deletions.
diff --git a/documentation/NUnit3003.md b/documentation/NUnit3003.md
@@ -8,7 +8,7 @@
 | Severity | Info
 | Enabled  | True
 | Category | Suppressor
-| Code     | [TestFixtureSuppressor](https://github.com/nunit/nunit.analyzers/blob/master/src/nunit.analyzers/DiagnosticSuppressors/TestFixtureSuppressor.cs)
+| Code     | [AvoidUninstantiatedInternalClassSuppressor](https://github.com/nunit/nunit.analyzers/blob/master/src/nunit.analyzers/DiagnosticSuppressors/AvoidUninstantiatedInternalClassSuppressor.cs)
 
 ## Description
 

diff --git a/documentation/NUnit3004.md b/documentation/NUnit3004.md
@@ -8,7 +8,7 @@
 | Severity | Info
 | Enabled  | True
 | Category | Suppressor
-| Code     | [TestFixtureSuppressor](https://github.com/nunit/nunit.analyzers/blob/master/src/nunit.analyzers/DiagnosticSuppressors/TestFixtureSuppressor.cs)
+| Code     | [TypesThatOwnDisposableFieldsShouldBeDisposableSuppressor](https://github.com/nunit/nunit.analyzers/blob/master/src/nunit.analyzers/DiagnosticSuppressors/TypesThatOwnDisposableFieldsShouldBeDisposableSuppressor.cs)
 
 ## Description
 

diff --git a/src/nunit.analyzers.tests/Constants/NUnitFrameworkConstantsTests.cs b/src/nunit.analyzers.tests/Constants/NUnitFrameworkConstantsTests.cs
@@ -155,6 +155,9 @@ public sealed class NUnitFrameworkConstantsTests
             (nameof(NUnitFrameworkConstants.FullNameOfTypeSetUpAttribute), typeof(SetUpAttribute)),
             (nameof(NUnitFrameworkConstants.FullNameOfTypeTearDownAttribute), typeof(TearDownAttribute)),
 
+            (nameof(NUnitFrameworkConstants.FullNameOfFixtureLifeCycleAttribute), typeof(FixtureLifeCycleAttribute)),
+            (nameof(NUnitFrameworkConstants.FullNameOfLifeCycle), typeof(LifeCycle)),
+
             (nameof(NUnitFrameworkConstants.FullNameOfSameAsConstraint), typeof(SameAsConstraint)),
             (nameof(NUnitFrameworkConstants.FullNameOfSomeItemsConstraint), typeof(SomeItemsConstraint)),
             (nameof(NUnitFrameworkConstants.FullNameOfEqualToConstraint), typeof(EqualConstraint)),

diff --git a/...nalyzers.tests/DiagnosticSuppressors/AvoidUninstantiatedInternalClassesSuppressorTests.cs b/...nalyzers.tests/DiagnosticSuppressors/AvoidUninstantiatedInternalClassesSuppressorTests.cs
@@ -51,7 +51,7 @@ public void TestSubtract()
             }
         ";
 
-        private static readonly DiagnosticSuppressor suppressor = new TestFixtureSuppressor();
+        private static readonly DiagnosticSuppressor suppressor = new AvoidUninstantiatedInternalClassSuppressor();
         private DiagnosticAnalyzer analyzer;
 
         [OneTimeSetUp]

diff --git a/src/nunit.analyzers.tests/DiagnosticSuppressors/DisposableFieldsSuppressorTests.cs b/src/nunit.analyzers.tests/DiagnosticSuppressors/DisposableFieldsSuppressorTests.cs
@@ -11,7 +11,7 @@ namespace NUnit.Analyzers.Tests.DiagnosticSuppressors
 {
     public class DisposableFieldsSuppressorTests
     {
-        private static readonly DiagnosticSuppressor suppressor = new TestFixtureSuppressor();
+        private static readonly DiagnosticSuppressor suppressor = new TypesThatOwnDisposableFieldsShouldBeDisposableSuppressor();
         private DiagnosticAnalyzer analyzer;
 
         [OneTimeSetUp]
@@ -90,5 +90,36 @@ public void Dispose() {{}}
 
             await TestHelpers.Suppressed(this.analyzer, suppressor, testCode).ConfigureAwait(true);
         }
+
+        [Test]
+        public async Task ShouldNotSuppressWhenInstancePerTestCase()
+        {
+            var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
+            [FixtureLifeCycle(LifeCycle.InstancePerTestCase)]
+            public class TestClass
+            {
+                private readonly IDisposable? field;
+
+                public TestClass()
+                {
+                    field = new DummyDisposable();
+                }
+
+                [Test]
+                public void Test()
+                {
+                    Assert.That(field, Is.Not.Null);
+                }
+
+                private sealed class DummyDisposable : IDisposable
+                {
+                    public void Dispose() {}
+                }
+            }
+            ");
+
+            // InstancePerTestCase mean test should use IDisposable
+            await TestHelpers.NotSuppressed(this.analyzer, suppressor, testCode).ConfigureAwait(true);
+        }
     }
 }
diff --git a/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs b/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs
@@ -117,6 +117,9 @@ public static class NUnitFrameworkConstants
         public const string FullNameOfTypeSetUpAttribute = "NUnit.Framework.SetUpAttribute";
         public const string FullNameOfTypeTearDownAttribute = "NUnit.Framework.TearDownAttribute";
 
+        public const string FullNameOfFixtureLifeCycleAttribute = "NUnit.Framework.FixtureLifeCycleAttribute";
+        public const string FullNameOfLifeCycle = "NUnit.Framework.LifeCycle";
+
         public const string NameOfConstraint = "Constraint";
 
         public const string FullNameOfSameAsConstraint = "NUnit.Framework.Constraints.SameAsConstraint";

diff --git a/...osticSuppressors/TestFixtureSuppressor.cs → ...dUninstantiatedInternalClassSuppressor.cs b/...osticSuppressors/TestFixtureSuppressor.cs → ...dUninstantiatedInternalClassSuppressor.cs
@@ -11,20 +11,15 @@
 namespace NUnit.Analyzers.DiagnosticSuppressors
 {
     [DiagnosticAnalyzer(LanguageNames.CSharp)]
-    public sealed class TestFixtureSuppressor : DiagnosticSuppressor
+    public sealed class AvoidUninstantiatedInternalClassSuppressor : DiagnosticSuppressor
     {
         internal static readonly SuppressionDescriptor AvoidUninstantiatedInternalTestFixtureClasses = new(
             id: AnalyzerIdentifiers.AvoidUninstantiatedInternalClasses,
             suppressedDiagnosticId: "CA1812",
             justification: "Class is an NUnit TestFixture and is instantiated using reflection");
 
-        internal static readonly SuppressionDescriptor TypesThatOwnDisposableFieldsShouldHaveATearDown = new(
-            id: AnalyzerIdentifiers.TypesThatOwnDisposableFieldsShouldBeDisposable,
-            suppressedDiagnosticId: "CA1001",
-            justification: "Field should be Disposed in TearDown or OneTimeTearDown method");
-
         public override ImmutableArray<SuppressionDescriptor> SupportedSuppressions { get; } =
-            ImmutableArray.Create(AvoidUninstantiatedInternalTestFixtureClasses, TypesThatOwnDisposableFieldsShouldHaveATearDown);
+            ImmutableArray.Create(AvoidUninstantiatedInternalTestFixtureClasses);
 
         public override void ReportSuppressions(SuppressionAnalysisContext context)
         {
@@ -48,17 +43,10 @@ public override void ReportSuppressions(SuppressionAnalysisContext context)
                 SemanticModel semanticModel = context.GetSemanticModel(sourceTree);
                 INamedTypeSymbol? typeSymbol = (INamedTypeSymbol?)semanticModel.GetDeclaredSymbol(classDeclaration, context.CancellationToken);
 
-                // Does the class have any Test related methods
                 if (typeSymbol is not null &&
-                    typeSymbol.GetMembers().OfType<IMethodSymbol>().Any(m => m.IsTestRelatedMethod(context.Compilation)))
+                    typeSymbol.IsTestFixture(context.Compilation))
                 {
-                    foreach (var suppression in this.SupportedSuppressions)
-                    {
-                        if (diagnostic.Descriptor.Id == suppression.SuppressedDiagnosticId)
-                        {
-                            context.ReportSuppression(Suppression.Create(suppression, diagnostic));
-                        }
-                    }
+                    context.ReportSuppression(Suppression.Create(AvoidUninstantiatedInternalTestFixtureClasses, diagnostic));
                 }
             }
         }

diff --git a/...alyzers/DiagnosticSuppressors/TypesThatOwnDisposableFieldsShouldBeDisposableSuppressor.cs b/...alyzers/DiagnosticSuppressors/TypesThatOwnDisposableFieldsShouldBeDisposableSuppressor.cs
@@ -0,0 +1,72 @@
+#if !NETSTANDARD1_6
+
+using System.Collections.Immutable;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using NUnit.Analyzers.Constants;
+using NUnit.Analyzers.Extensions;
+
+namespace NUnit.Analyzers.DiagnosticSuppressors
+{
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public sealed class TypesThatOwnDisposableFieldsShouldBeDisposableSuppressor : DiagnosticSuppressor
+    {
+        internal static readonly SuppressionDescriptor TypesThatOwnDisposableFieldsShouldHaveATearDown = new(
+            id: AnalyzerIdentifiers.TypesThatOwnDisposableFieldsShouldBeDisposable,
+            suppressedDiagnosticId: "CA1001",
+            justification: "Field should be Disposed in TearDown or OneTimeTearDown method");
+
+        public override ImmutableArray<SuppressionDescriptor> SupportedSuppressions { get; } =
+            ImmutableArray.Create(TypesThatOwnDisposableFieldsShouldHaveATearDown);
+
+        public override void ReportSuppressions(SuppressionAnalysisContext context)
+        {
+            foreach (var diagnostic in context.ReportedDiagnostics)
+            {
+                SyntaxTree? sourceTree = diagnostic.Location.SourceTree;
+
+                if (sourceTree is null)
+                {
+                    continue;
+                }
+
+                SyntaxNode node = sourceTree.GetRoot(context.CancellationToken)
+                                            .FindNode(diagnostic.Location.SourceSpan);
+
+                if (node is not ClassDeclarationSyntax classDeclaration)
+                {
+                    continue;
+                }
+
+                SemanticModel semanticModel = context.GetSemanticModel(sourceTree);
+                INamedTypeSymbol? typeSymbol = (INamedTypeSymbol?)semanticModel.GetDeclaredSymbol(classDeclaration, context.CancellationToken);
+
+                if (typeSymbol is not null)
+                {
+                    // Is the class set up for a InstancePerTestCase
+                    AttributeData? fixtureLifeCycleAttribute = typeSymbol.GetAllAttributes().FirstOrDefault(x => x.IsFixtureLifeCycleAttribute(context.Compilation));
+                    if (fixtureLifeCycleAttribute is not null &&
+                        fixtureLifeCycleAttribute.ConstructorArguments.Length == 1 &&
+                        fixtureLifeCycleAttribute.ConstructorArguments[0] is TypedConstant typeConstant &&
+                        typeConstant.Kind == TypedConstantKind.Enum &&
+                        typeConstant.Type.IsType(NUnitFrameworkConstants.FullNameOfLifeCycle, context.Compilation) &&
+                        typeConstant.Value is 1 /* LifeCycle.InstancePerTestCase */)
+                    {
+                        // If a TestFixture used InstancePerTestCase, it should be IDisposable
+                        if (diagnostic.Descriptor.Id == TypesThatOwnDisposableFieldsShouldHaveATearDown.SuppressedDiagnosticId)
+                            continue;
+                    }
+
+                    if (typeSymbol.IsTestFixture(context.Compilation))
+                    {
+                        context.ReportSuppression(Suppression.Create(TypesThatOwnDisposableFieldsShouldHaveATearDown, diagnostic));
+                    }
+                }
+            }
+        }
+    }
+}
+
+#endif
diff --git a/src/nunit.analyzers/Extensions/AttributeDataExtensions.cs b/src/nunit.analyzers/Extensions/AttributeDataExtensions.cs
@@ -43,6 +43,16 @@ public static bool IsSetUpOrTearDownMethodAttribute(this AttributeData @this, Co
                 || attributeType.IsType(NUnitFrameworkConstants.FullNameOfTypeTearDownAttribute, compilation);
         }
 
+        public static bool IsFixtureLifeCycleAttribute(this AttributeData @this, Compilation compilation)
+        {
+            var attributeType = @this.AttributeClass;
+
+            if (attributeType is null)
+                return false;
+
+            return attributeType.IsType(NUnitFrameworkConstants.FullNameOfFixtureLifeCycleAttribute, compilation);
+        }
+
         public static AttributeArgumentSyntax? GetConstructorArgumentSyntax(this AttributeData @this, int position,
             CancellationToken cancellationToken = default)
         {

diff --git a/src/nunit.analyzers/Extensions/IMethodSymbolExtensions.cs b/src/nunit.analyzers/Extensions/IMethodSymbolExtensions.cs
@@ -67,5 +67,10 @@ internal static bool HasTestRelatedAttributes(this IMethodSymbol methodSymbol, C
             return methodSymbol.GetAttributes().Any(
                 a => a.IsTestMethodAttribute(compilation) || a.IsSetUpOrTearDownMethodAttribute(compilation));
         }
+
+        internal static bool IsTestFixture(this ITypeSymbol typeSymbol, Compilation compilation)
+        {
+            return typeSymbol.GetMembers().OfType<IMethodSymbol>().Any(m => m.IsTestRelatedMethod(compilation));
+        }
     }
 }