From c6175b397e083c82b68b8d6bc77df832154d2738 Mon Sep 17 00:00:00 2001 From: Manfred Brands Date: Sun, 24 Sep 2023 09:06:48 +0800 Subject: [PATCH] Do not suppress CA1001 if Fixture uses LifeCycle.InstancePerTestCase --- documentation/NUnit3003.md | 2 +- documentation/NUnit3004.md | 2 +- .../Constants/NUnitFrameworkConstantsTests.cs | 3 + ...tantiatedInternalClassesSuppressorTests.cs | 2 +- .../DisposableFieldsSuppressorTests.cs | 33 ++++++++- .../Constants/NUnitFrameworkConstants.cs | 3 + ...dUninstantiatedInternalClassSuppressor.cs} | 20 ++---- ...sableFieldsShouldBeDisposableSuppressor.cs | 72 +++++++++++++++++++ .../Extensions/AttributeDataExtensions.cs | 10 +++ .../Extensions/IMethodSymbolExtensions.cs | 5 ++ 10 files changed, 132 insertions(+), 20 deletions(-) rename src/nunit.analyzers/DiagnosticSuppressors/{TestFixtureSuppressor.cs => AvoidUninstantiatedInternalClassSuppressor.cs} (64%) create mode 100644 src/nunit.analyzers/DiagnosticSuppressors/TypesThatOwnDisposableFieldsShouldBeDisposableSuppressor.cs diff --git a/documentation/NUnit3003.md b/documentation/NUnit3003.md index 0e7fb822..62b79aa5 100644 --- 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 index c152a646..3be4640b 100644 --- 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 index 60279f5a..208db289 100644 --- 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/src/nunit.analyzers.tests/DiagnosticSuppressors/AvoidUninstantiatedInternalClassesSuppressorTests.cs b/src/nunit.analyzers.tests/DiagnosticSuppressors/AvoidUninstantiatedInternalClassesSuppressorTests.cs index 54cd00af..8dbba997 100644 --- a/src/nunit.analyzers.tests/DiagnosticSuppressors/AvoidUninstantiatedInternalClassesSuppressorTests.cs +++ b/src/nunit.analyzers.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 index 93c68ea0..6b1899f3 100644 --- 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 index 6be82ff4..4f1e9e29 100644 --- 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/src/nunit.analyzers/DiagnosticSuppressors/TestFixtureSuppressor.cs b/src/nunit.analyzers/DiagnosticSuppressors/AvoidUninstantiatedInternalClassSuppressor.cs similarity index 64% rename from src/nunit.analyzers/DiagnosticSuppressors/TestFixtureSuppressor.cs rename to src/nunit.analyzers/DiagnosticSuppressors/AvoidUninstantiatedInternalClassSuppressor.cs index 05a44ee1..6f99ac45 100644 --- a/src/nunit.analyzers/DiagnosticSuppressors/TestFixtureSuppressor.cs +++ b/src/nunit.analyzers/DiagnosticSuppressors/AvoidUninstantiatedInternalClassSuppressor.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 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().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/src/nunit.analyzers/DiagnosticSuppressors/TypesThatOwnDisposableFieldsShouldBeDisposableSuppressor.cs b/src/nunit.analyzers/DiagnosticSuppressors/TypesThatOwnDisposableFieldsShouldBeDisposableSuppressor.cs new file mode 100644 index 00000000..b6dae83c --- /dev/null +++ b/src/nunit.analyzers/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 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 index 495c2ee0..4cea0c0b 100644 --- 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 index 8652c91b..f6fd9c2c 100644 --- 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().Any(m => m.IsTestRelatedMethod(compilation)); + } } }