From fb8adb5bf24701803394e0651dce31ada0e1572b Mon Sep 17 00:00:00 2001 From: Martin Licko Date: Mon, 11 Mar 2024 16:57:27 +0100 Subject: [PATCH 1/4] adding support for NUnit2 for 1. test case and 2. Assert identification --- global.json | 2 +- src/nunit.analyzers/BaseAssertionAnalyzer.cs | 2 +- .../ClassicAssertionAnalyzer.cs | 2 +- .../Constants/NUnitFrameworkConstants.cs | 6 ++++-- .../Extensions/AttributeDataExtensions.cs | 20 +++++++++++++++++-- 5 files changed, 25 insertions(+), 7 deletions(-) diff --git a/global.json b/global.json index dffff818..98054f5f 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,6 @@ { "sdk": { - "version": "6.0.100", + "version": "7.0.401", "rollForward" : "latestFeature", "allowPrerelease": false } diff --git a/src/nunit.analyzers/BaseAssertionAnalyzer.cs b/src/nunit.analyzers/BaseAssertionAnalyzer.cs index 82f0e5b6..19dba46a 100644 --- a/src/nunit.analyzers/BaseAssertionAnalyzer.cs +++ b/src/nunit.analyzers/BaseAssertionAnalyzer.cs @@ -38,7 +38,7 @@ private void AnalyzeInvocation(Version nunitVersion, OperationAnalysisContext co if (context.Operation is not IInvocationOperation invocationOperation) return; - if (!this.IsAssert(nunitVersion.Major >= 4, invocationOperation)) + if (!this.IsAssert(nunitVersion.Major >= 2, invocationOperation)) return; context.CancellationToken.ThrowIfCancellationRequested(); diff --git a/src/nunit.analyzers/ClassicAssertionAnalyzer.cs b/src/nunit.analyzers/ClassicAssertionAnalyzer.cs index 5e164d88..b3d7d735 100644 --- a/src/nunit.analyzers/ClassicAssertionAnalyzer.cs +++ b/src/nunit.analyzers/ClassicAssertionAnalyzer.cs @@ -10,7 +10,7 @@ protected override bool IsAssert(bool hasClassicAssert, IInvocationOperation inv { INamedTypeSymbol containingType = invocationOperation.TargetMethod.ContainingType; - return hasClassicAssert ? containingType.IsClassicAssert() : containingType.IsAssert(); + return containingType.IsAssert(); } } } diff --git a/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs b/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs index b029f4cd..f2e47ca2 100644 --- a/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs +++ b/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs @@ -182,6 +182,8 @@ public static class NUnitFrameworkConstants public const string NameOfOneTimeTearDownAttribute = "OneTimeTearDownAttribute"; public const string NameOfSetUpAttribute = "SetUpAttribute"; public const string NameOfTearDownAttribute = "TearDownAttribute"; + public const string NameOfTestFixtureSetUpAttribute = "TestFixtureSetUpAttribute"; + public const string NameOfTestFixtureTearDownAttribute = "TestFixtureTearDownAttribute"; public const string NameOfCancelAfterAttribute = "CancelAfterAttribute"; @@ -201,9 +203,9 @@ public static class NUnitFrameworkConstants public const string NameOfEqualConstraintWithin = "Within"; public const string NameOfEqualConstraintAsCollection = "AsCollection"; - public const string NUnitFrameworkAssemblyName = "nunit.framework"; + public const string NUnitFrameworkAssemblyName = "NUnit.Framework"; - public const string NUnitFrameworkLegacyAssemblyName = "nunit.framework.legacy"; + public const string NUnitFrameworkLegacyAssemblyName = "NUnit.Framework.Legacy"; public const string NameOfClassicAssert = "ClassicAssert"; } } diff --git a/src/nunit.analyzers/Extensions/AttributeDataExtensions.cs b/src/nunit.analyzers/Extensions/AttributeDataExtensions.cs index 4cea0c0b..bc35a129 100644 --- a/src/nunit.analyzers/Extensions/AttributeDataExtensions.cs +++ b/src/nunit.analyzers/Extensions/AttributeDataExtensions.cs @@ -26,7 +26,8 @@ public static bool DerivesFromIParameterDataSource(this AttributeData @this, Com public static bool IsTestMethodAttribute(this AttributeData @this, Compilation compilation) { - return @this.DerivesFromITestBuilder(compilation) || + return @this.IsTestAttribute() || + @this.DerivesFromITestBuilder(compilation) || @this.DerivesFromISimpleTestBuilder(compilation); } @@ -40,7 +41,9 @@ public static bool IsSetUpOrTearDownMethodAttribute(this AttributeData @this, Co return attributeType.IsType(NUnitFrameworkConstants.FullNameOfTypeOneTimeSetUpAttribute, compilation) || attributeType.IsType(NUnitFrameworkConstants.FullNameOfTypeOneTimeTearDownAttribute, compilation) || attributeType.IsType(NUnitFrameworkConstants.FullNameOfTypeSetUpAttribute, compilation) - || attributeType.IsType(NUnitFrameworkConstants.FullNameOfTypeTearDownAttribute, compilation); + || attributeType.IsType(NUnitFrameworkConstants.FullNameOfTypeTearDownAttribute, compilation) + || attributeType.Name.Equals(NUnitFrameworkConstants.NameOfTestFixtureSetUpAttribute, System.StringComparison.Ordinal) + || attributeType.Name.Equals(NUnitFrameworkConstants.NameOfTestFixtureTearDownAttribute, System.StringComparison.Ordinal); } public static bool IsFixtureLifeCycleAttribute(this AttributeData @this, Compilation compilation) @@ -113,5 +116,18 @@ private static bool DerivesFromInterface(Compilation compilation, AttributeData return attributeData.AttributeClass.AllInterfaces.Any(i => SymbolEqualityComparer.Default.Equals(i, interfaceType)); } + + private static bool IsTestAttribute(this AttributeData @this) + { + var attributeClassString = @this.AttributeClass?.ToString(); + if (attributeClassString is not null) + { + return attributeClassString.Equals(NUnitFrameworkConstants.FullNameOfTypeTestAttribute, System.StringComparison.Ordinal) || + attributeClassString.Equals(NUnitFrameworkConstants.FullNameOfTypeTestCaseAttribute, System.StringComparison.Ordinal) || + attributeClassString.Equals(NUnitFrameworkConstants.FullNameOfTypeTestCaseSourceAttribute, System.StringComparison.Ordinal); + } + + return false; + } } } From 68237f0307246753ac98ddb4799a2302e3e4d541 Mon Sep 17 00:00:00 2001 From: Martin Licko Date: Tue, 26 Mar 2024 20:36:45 +0100 Subject: [PATCH 2/4] Revert "adding support for NUnit2 for 1. test case and 2. Assert identification" This reverts commit fb8adb5bf24701803394e0651dce31ada0e1572b. --- global.json | 2 +- src/nunit.analyzers/BaseAssertionAnalyzer.cs | 2 +- .../ClassicAssertionAnalyzer.cs | 2 +- .../Constants/NUnitFrameworkConstants.cs | 6 ++---- .../Extensions/AttributeDataExtensions.cs | 20 ++----------------- 5 files changed, 7 insertions(+), 25 deletions(-) diff --git a/global.json b/global.json index 98054f5f..dffff818 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,6 @@ { "sdk": { - "version": "7.0.401", + "version": "6.0.100", "rollForward" : "latestFeature", "allowPrerelease": false } diff --git a/src/nunit.analyzers/BaseAssertionAnalyzer.cs b/src/nunit.analyzers/BaseAssertionAnalyzer.cs index 19dba46a..82f0e5b6 100644 --- a/src/nunit.analyzers/BaseAssertionAnalyzer.cs +++ b/src/nunit.analyzers/BaseAssertionAnalyzer.cs @@ -38,7 +38,7 @@ private void AnalyzeInvocation(Version nunitVersion, OperationAnalysisContext co if (context.Operation is not IInvocationOperation invocationOperation) return; - if (!this.IsAssert(nunitVersion.Major >= 2, invocationOperation)) + if (!this.IsAssert(nunitVersion.Major >= 4, invocationOperation)) return; context.CancellationToken.ThrowIfCancellationRequested(); diff --git a/src/nunit.analyzers/ClassicAssertionAnalyzer.cs b/src/nunit.analyzers/ClassicAssertionAnalyzer.cs index b3d7d735..5e164d88 100644 --- a/src/nunit.analyzers/ClassicAssertionAnalyzer.cs +++ b/src/nunit.analyzers/ClassicAssertionAnalyzer.cs @@ -10,7 +10,7 @@ protected override bool IsAssert(bool hasClassicAssert, IInvocationOperation inv { INamedTypeSymbol containingType = invocationOperation.TargetMethod.ContainingType; - return containingType.IsAssert(); + return hasClassicAssert ? containingType.IsClassicAssert() : containingType.IsAssert(); } } } diff --git a/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs b/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs index f2e47ca2..b029f4cd 100644 --- a/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs +++ b/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs @@ -182,8 +182,6 @@ public static class NUnitFrameworkConstants public const string NameOfOneTimeTearDownAttribute = "OneTimeTearDownAttribute"; public const string NameOfSetUpAttribute = "SetUpAttribute"; public const string NameOfTearDownAttribute = "TearDownAttribute"; - public const string NameOfTestFixtureSetUpAttribute = "TestFixtureSetUpAttribute"; - public const string NameOfTestFixtureTearDownAttribute = "TestFixtureTearDownAttribute"; public const string NameOfCancelAfterAttribute = "CancelAfterAttribute"; @@ -203,9 +201,9 @@ public static class NUnitFrameworkConstants public const string NameOfEqualConstraintWithin = "Within"; public const string NameOfEqualConstraintAsCollection = "AsCollection"; - public const string NUnitFrameworkAssemblyName = "NUnit.Framework"; + public const string NUnitFrameworkAssemblyName = "nunit.framework"; - public const string NUnitFrameworkLegacyAssemblyName = "NUnit.Framework.Legacy"; + public const string NUnitFrameworkLegacyAssemblyName = "nunit.framework.legacy"; public const string NameOfClassicAssert = "ClassicAssert"; } } diff --git a/src/nunit.analyzers/Extensions/AttributeDataExtensions.cs b/src/nunit.analyzers/Extensions/AttributeDataExtensions.cs index bc35a129..4cea0c0b 100644 --- a/src/nunit.analyzers/Extensions/AttributeDataExtensions.cs +++ b/src/nunit.analyzers/Extensions/AttributeDataExtensions.cs @@ -26,8 +26,7 @@ public static bool DerivesFromIParameterDataSource(this AttributeData @this, Com public static bool IsTestMethodAttribute(this AttributeData @this, Compilation compilation) { - return @this.IsTestAttribute() || - @this.DerivesFromITestBuilder(compilation) || + return @this.DerivesFromITestBuilder(compilation) || @this.DerivesFromISimpleTestBuilder(compilation); } @@ -41,9 +40,7 @@ public static bool IsSetUpOrTearDownMethodAttribute(this AttributeData @this, Co return attributeType.IsType(NUnitFrameworkConstants.FullNameOfTypeOneTimeSetUpAttribute, compilation) || attributeType.IsType(NUnitFrameworkConstants.FullNameOfTypeOneTimeTearDownAttribute, compilation) || attributeType.IsType(NUnitFrameworkConstants.FullNameOfTypeSetUpAttribute, compilation) - || attributeType.IsType(NUnitFrameworkConstants.FullNameOfTypeTearDownAttribute, compilation) - || attributeType.Name.Equals(NUnitFrameworkConstants.NameOfTestFixtureSetUpAttribute, System.StringComparison.Ordinal) - || attributeType.Name.Equals(NUnitFrameworkConstants.NameOfTestFixtureTearDownAttribute, System.StringComparison.Ordinal); + || attributeType.IsType(NUnitFrameworkConstants.FullNameOfTypeTearDownAttribute, compilation); } public static bool IsFixtureLifeCycleAttribute(this AttributeData @this, Compilation compilation) @@ -116,18 +113,5 @@ private static bool DerivesFromInterface(Compilation compilation, AttributeData return attributeData.AttributeClass.AllInterfaces.Any(i => SymbolEqualityComparer.Default.Equals(i, interfaceType)); } - - private static bool IsTestAttribute(this AttributeData @this) - { - var attributeClassString = @this.AttributeClass?.ToString(); - if (attributeClassString is not null) - { - return attributeClassString.Equals(NUnitFrameworkConstants.FullNameOfTypeTestAttribute, System.StringComparison.Ordinal) || - attributeClassString.Equals(NUnitFrameworkConstants.FullNameOfTypeTestCaseAttribute, System.StringComparison.Ordinal) || - attributeClassString.Equals(NUnitFrameworkConstants.FullNameOfTypeTestCaseSourceAttribute, System.StringComparison.Ordinal); - } - - return false; - } } } From 25215c2b951dafeba2915a42635a39bd566fadb1 Mon Sep 17 00:00:00 2001 From: Martin Licko Date: Tue, 26 Mar 2024 20:35:27 +0100 Subject: [PATCH 3/4] NUnit1032 - considering explicit IDisposable.Dispose implementation --- .../DisposeFieldsAndPropertiesInTearDownAnalyzer.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/nunit.analyzers/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzer.cs b/src/nunit.analyzers/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzer.cs index 3be0f389..d834f9c7 100644 --- a/src/nunit.analyzers/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzer.cs +++ b/src/nunit.analyzers/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzer.cs @@ -554,6 +554,13 @@ private static void DisposedIn(Parameters parameters, HashSet disposals, { return memberAccessExpression.Name.Identifier.Text; } + else if (expression is ParenthesizedExpressionSyntax { Expression: CastExpressionSyntax { Expression: IdentifierNameSyntax castIdentifierNameSyntax, Type: IdentifierNameSyntax + { + Identifier.Text: "IDisposable" + } } }) + { + return castIdentifierNameSyntax.Identifier.Text; + } return null; } From 7573fb65c090faec4cf51674179f56d2d77040d6 Mon Sep 17 00:00:00 2001 From: Martin Licko Date: Tue, 26 Mar 2024 21:26:47 +0100 Subject: [PATCH 4/4] adding tests for NUnit1032 --- ...ldsAndPropertiesInTearDownAnalyzerTests.cs | 40 +++++++++++++++++-- ...seFieldsAndPropertiesInTearDownAnalyzer.cs | 13 +++--- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/src/nunit.analyzers.tests/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzerTests.cs b/src/nunit.analyzers.tests/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzerTests.cs index 76ce0981..3fc1d600 100644 --- a/src/nunit.analyzers.tests/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzerTests.cs +++ b/src/nunit.analyzers.tests/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzerTests.cs @@ -14,6 +14,15 @@ sealed class DummyDisposable : IDisposable public void Dispose() {} }"; + private const string DummyExplicitDisposable = @" + sealed class DummyExplicitDisposable : IAnotherInterface, IDisposable + { + void IDisposable.Dispose() {} + } + public interface IAnotherInterface + { + }"; + // CA2000 allows transfer of ownership using ICollection.Add private const string Disposer = @" private sealed class Disposer : IDisposable @@ -114,6 +123,29 @@ public void TearDownMethod() RoslynAssert.Valid(analyzer, testCode); } + [Test] + public void AnalyzeWhenExplicitlyInterfaceImplementedDisposableFieldSetInConstructorIsDisposedInOneTimeTearDownMethod() + { + var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@" + private IAnotherInterface field; + + public TestClass() + {{ + field = new DummyExplicitDisposable(); + }} + + [OneTimeTearDown] + public void TearDownMethod() + {{ + ((IDisposable)field).Dispose(); + }} + + {DummyExplicitDisposable} + "); + + RoslynAssert.Valid(analyzer, testCode); + } + [TestCase("", "OneTime")] [TestCase("OneTime", "")] public void AnalyzeWhenFieldIsDisposedInWrongMethod(string attributePrefix1, string attributePrefix2) @@ -1005,10 +1037,10 @@ public void TearDown() public partial class TestFixture { [Test] - public void SomeTest() - { - SomeAsserts(); - } + public void SomeTest() + { + SomeAsserts(); + } }"); RoslynAssert.Valid(analyzer, testCodePart1, testCodePart2); diff --git a/src/nunit.analyzers/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzer.cs b/src/nunit.analyzers/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzer.cs index d834f9c7..74f2ce2c 100644 --- a/src/nunit.analyzers/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzer.cs +++ b/src/nunit.analyzers/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzer.cs @@ -554,12 +554,15 @@ private static void DisposedIn(Parameters parameters, HashSet disposals, { return memberAccessExpression.Name.Identifier.Text; } - else if (expression is ParenthesizedExpressionSyntax { Expression: CastExpressionSyntax { Expression: IdentifierNameSyntax castIdentifierNameSyntax, Type: IdentifierNameSyntax - { - Identifier.Text: "IDisposable" - } } }) + + // considering cast to IDisposable, e.g. in case of explicit interface implementation of IDisposable.Dispose() + else if (expression is ParenthesizedExpressionSyntax parenthesizedExpression && + parenthesizedExpression.Expression is CastExpressionSyntax castExpression && + castExpression.Expression is IdentifierNameSyntax castIdentifierName && + castExpression.Type is IdentifierNameSyntax typeIdentifierName && + typeIdentifierName.Identifier.Text.Equals("IDisposable", StringComparison.Ordinal)) { - return castIdentifierNameSyntax.Identifier.Text; + return castIdentifierName.Identifier.Text; } return null;