Merge pull request #658 from manfred-brands/issue635-PartialClasses

Issue635 partial classes

src/nunit.analyzers.tests/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzerTests.cs

@@ -225,6 +225,26 @@ public void TearDownMethod()
             RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
         }
 
+        [Test]
+        public void AnalyzeWhenReadOnlyFieldSetInConstructorIsNotDisposed()
+        {
+            var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
+        private readonly object? ↓field;
+
+        public TestClass() => field = new DummyDisposable();
+
+        [TearDown]
+        public void TearDownMethod()
+        {{
+            Assert.That(field, Is.Not.Null);
+        }}
+
+        {DummyDisposable}
+        ");
+
+            RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
+        }
+
         [Test]
         public void FieldConditionallyAssignedInCalledLocalMethod()
         {
@@ -528,7 +548,25 @@ public void TearDownMethod()
         }
 
         [Test]
-        public void AnalyzeWhenPropertydSetInConstructorIsNotDisposed()
+        public void AnalyzeWhenReadOnlyPropertyWithInitializerIsNotDisposed()
+        {
+            var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
+        ↓protected object? Property {{ get; }} = new DummyDisposable();
+
+        [TearDown]
+        public void TearDownMethod()
+        {{
+            Assert.That(Property, Is.Not.Null);
+        }}
+
+        {DummyDisposable}
+        ");
+
+            RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
+        }
+
+        [Test]
+        public void AnalyzeWhenPropertySetInConstructorIsNotDisposed()
         {
             var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
         ↓protected object? Property {{ get; private set; }}
@@ -547,6 +585,26 @@ public void TearDownMethod()
             RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
         }
 
+        [Test]
+        public void AnalyzeWhenReadOnlyPropertySetInConstructorIsNotDisposed()
+        {
+            var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
+        ↓protected object? Property {{ get; }}
+
+        public TestClass() => Property = new DummyDisposable();
+
+        [TearDown]
+        public void TearDownMethod()
+        {{
+            Assert.That(Property, Is.Not.Null);
+        }}
+
+        {DummyDisposable}
+        ");
+
+            RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
+        }
+
         [TestCase("")]
         [TestCase("OneTime")]
         public void AnalyzeWhenFieldIsDisposedUsingDisposer(string attributePrefix)
@@ -879,5 +937,119 @@ public void StaticFieldsAreDisposed(string modifier)
 
             RoslynAssert.Valid(analyzer, testCode);
         }
+
+        [Test]
+        public void EarlyOutForNonSetableFieldsAndProperties()
+        {
+            var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
+        public partial class TestFixture
+        {
+            public const double ToleranceConstant = 1E-10;
+
+            public static readonly double ToleranceReadOnlyField = 1E-10;
+
+            public static double ToleranceExpressionProperty => 1E-10;
+
+            public static double ToleranceReadOnlyProperty { get; } = 1E-10;
+
+            public static object ToleranceGetOnlyExpressionProperty
+            {
+                get => 1E-10;
+            }
+
+            public static object ToleranceGetOnlyProperty
+            {
+                get
+                {
+                    return 1E-10;
+                }
+            }
+
+            [Test]
+            public void SomeTest() => throw new NotImplementedException();
+        }");
+
+            int earlyOutCount = DisposeFieldsAndPropertiesInTearDownAnalyzer.EarlyOutCount;
+            RoslynAssert.Valid(analyzer, testCode);
+            Assert.That(DisposeFieldsAndPropertiesInTearDownAnalyzer.EarlyOutCount, Is.GreaterThan(earlyOutCount));
+        }
+
+        [Test]
+        public void DoesNotThrowExceptionsWhenUsingPartialClasses()
+        {
+            var testCodePart1 = TestUtility.WrapClassInNamespaceAndAddUsing($@"
+        public partial class TestFixture
+        {{
+            private const double Tolerance = 1E-10;
+
+            private static IDisposable? Property {{ get; set; }}
+
+            [SetUp]
+            public void SetUp()
+            {{
+                Property = new DummyDisposable();
+            }}
+
+            [TearDown]
+            public void TearDown()
+            {{
+                Property?.Dispose();
+            }}
+
+            private static void SomeAsserts() => throw new InvalidOperationException();
+
+            {DummyDisposable}
+        }}");
+
+            var testCodePart2 = TestUtility.WrapClassInNamespaceAndAddUsing(@"
+        public partial class TestFixture
+        {
+            [Test]
+	        public void SomeTest()
+	        {
+		        SomeAsserts();
+	        }
+        }");
+
+            RoslynAssert.Valid(analyzer, testCodePart1, testCodePart2);
+        }
+
+        [Test]
+        public void DoesNotThrowExceptionsWhenUsingExplicitProperties()
+        {
+            var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
+        [TestFixture]
+        public abstract class Test : IDataProvider
+        {
+            private DataReader DataReader;
+
+            DataReader IDataProvider.DataReader
+            {
+                get => DataReader;
+                set => DataReader = value;
+            }
+
+            protected Test() => DataReader = new DataReader(this);
+
+            [OneTimeSetUp]
+            public void SetListener() => throw new NotImplementedException();
+        }
+
+        public interface IDataProvider
+        {
+            DataReader DataReader
+            {
+                get;
+                set;
+            }
+        }
+
+        public sealed class DataReader
+        {
+            public DataReader(IDataProvider provider) => throw new NotImplementedException();
+        }");
+
+            RoslynAssert.Valid(analyzer, testCode);
+        }
     }
 }

src/nunit.analyzers/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzer.cs

@@ -44,6 +44,8 @@ public sealed class DisposeFieldsAndPropertiesInTearDownAnalyzer : DiagnosticAna
         public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
             ImmutableArray.Create(fieldIsNotDisposedInTearDown);
 
+        internal static int EarlyOutCount { get; private set; }
+
         public override void Initialize(AnalysisContext context)
         {
             context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
@@ -72,41 +74,70 @@ private static void AnalyzeDisposableFields(SyntaxNodeAnalysisContext context)
             if (typeSymbol.IsInstancePerTestCaseFixture(context.Compilation) ||
                 !typeSymbol.IsTestFixture(context.Compilation))
             {
-                // CA1001 should picked this up. Assuming it is enabled.
+                // CA1001 should pick this up. Assuming it is enabled.
                 return;
             }
 
             var fieldDeclarations = classDeclaration.Members
-                                                    .OfType<FieldDeclarationSyntax>()
-                                                    .Select(x => x.Declaration)
-                                                    .SelectMany(x => x.Variables);
+                                                    .OfType<FieldDeclarationSyntax>();
 
             var propertyDeclarations = classDeclaration.Members
                                                        .OfType<PropertyDeclarationSyntax>()
                                                        .Where(x => x.AccessorList is not null);
 
+            bool hasConstructors = classDeclaration.Members
+                                                   .OfType<ConstructorDeclarationSyntax>()
+                                                   .Any();
+
             HashSet<string> symbolsWithDisposableInitializers = new();
 
             Dictionary<string, SyntaxNode> symbols = new();
-            foreach (var field in fieldDeclarations)
+            foreach (var fieldDeclaration in fieldDeclarations)
             {
-                symbols.Add(field.Identifier.Text, field);
-                if (field.Initializer is not null && NeedsDisposal(model, field.Initializer.Value))
+                foreach (var field in fieldDeclaration.Declaration.Variables)
                 {
-                    symbolsWithDisposableInitializers.Add(field.Identifier.Text);
+                    if (field.Initializer is not null)
+                    {
+                        if (NeedsDisposal(model, field.Initializer.Value))
+                        {
+                            symbolsWithDisposableInitializers.Add(field.Identifier.Text);
+                            symbols.Add(field.Identifier.Text, field);
+                            continue;
+                        }
+                    }
+
+                    if (CanBeAssignedTo(fieldDeclaration, hasConstructors))
+                    {
+                        symbols.Add(field.Identifier.Text, field);
+                    }
                 }
             }
 
             foreach (var property in propertyDeclarations)
             {
-                symbols.Add(property.Identifier.Text, property);
-                if (property.Initializer is not null && NeedsDisposal(model, property.Initializer.Value))
-                    symbolsWithDisposableInitializers.Add(property.Identifier.Text);
+                if (property.ExplicitInterfaceSpecifier is not null)
+                    continue;
+
+                if (property.Initializer is not null)
+                {
+                    if (NeedsDisposal(model, property.Initializer.Value))
+                    {
+                        symbolsWithDisposableInitializers.Add(property.Identifier.Text);
+                        symbols.Add(property.Identifier.Text, property);
+                        continue;
+                    }
+                }
+
+                if (CanBeAssignedTo(property, hasConstructors))
+                {
+                    symbols.Add(property.Identifier.Text, property);
+                }
             }
 
             if (symbols.Count == 0)
             {
                 // No fields or properties to consider.
+                EarlyOutCount++;
                 return;
             }
 
@@ -150,6 +181,21 @@ private static void AnalyzeDisposableFields(SyntaxNodeAnalysisContext context)
                 NUnitFrameworkConstants.NameOfTearDownAttribute, otherMethods, tearDownMethods);
         }
 
+        private static bool CanBeAssignedTo(FieldDeclarationSyntax declaration, bool hasConstructors)
+        {
+            return !declaration.Modifiers.Any(modifier => modifier.IsKind(SyntaxKind.ConstKeyword)) &&
+                (!declaration.Modifiers.Any(modifier => modifier.IsKind(SyntaxKind.ReadOnlyKeyword)) || hasConstructors);
+        }
+
+        private static bool CanBeAssignedTo(PropertyDeclarationSyntax declaration, bool hasConstructors)
+        {
+            AccessorListSyntax? accessorList = declaration.AccessorList;
+            if (accessorList is null)
+                return false;           // Expression body property
+
+            return hasConstructors || accessorList.Accessors.Any(accessor => accessor.IsKind(SyntaxKind.SetAccessorDeclaration));
+        }
+
         private static bool HasAttribute(IMethodSymbol method, string attributeName)
         {
             // Look for attribute not only on the method itself but
@@ -539,6 +585,12 @@ public bool IsLocalMethodCall(
                 InvocationExpressionSyntax invocationExpression,
                 [NotNullWhen(true)] out IMethodSymbol? calledMethod)
             {
+                if (this.Model.SyntaxTree != invocationExpression.SyntaxTree)
+                {
+                    calledMethod = null;
+                    return false;
+                }
+
                 calledMethod = this.Model.GetSymbolInfo(invocationExpression).Symbol as IMethodSymbol;
                 return calledMethod is not null &&
                     SymbolEqualityComparer.Default.Equals(calledMethod.ContainingType, this.type);