Ensures property-based conventions only inspect declared properties. (#…

…69) This behaviour change was forced by the dotnet type system. If you have a type, and you want to specify that it's properties have private setters using PropertiesMustHavePrivateSetters, and your type inherits a base type, the type system provides no access or visibility to the setter on the base class regardless of whether it is private set or has no setter Since we cannot determine this, we cannot enforce this convention on the base type when inspecting via the derived type The better approach is to only inspect declared properties If you want to inspect the base type, ensure it is included in the set of types under inspection
andrewabest · Jan 21, 2020 · 1516844 · 1516844
1 parent 8668b41
commit 1516844
Show file tree

Hide file tree

Showing 11 changed files with 238 additions and 17 deletions.
diff --git a/src/Core/Conventional.Tests/Conventional/Conventions/ConventionSpecificationTests.cs b/src/Core/Conventional.Tests/Conventional/Conventions/ConventionSpecificationTests.cs
@@ -58,6 +58,26 @@ public void PropertiesMustHavePublicGetters_Success()
                 .BeTrue();
         }
 
+        private class AllPublicGetterMockBase
+        {
+            public string Private { private get; set; }
+        }
+
+        private class AllPublicGetterMockDerived : AllPublicGetterMockBase
+        {
+            public string Public { get; set; }
+        }
+
+        [Test]
+        public void PropertiesMustHavePublicGetters_IgnoresInheritedProperties()
+        {
+            typeof(AllPublicGetterMockDerived)
+                .MustConformTo(Convention.PropertiesMustHavePublicGetters)
+                .IsSatisfied
+                .Should()
+                .BeTrue();
+        }
+
         private class PrivateGetterMock
         {
             public string PrivateGet { private get; set; }
@@ -88,6 +108,26 @@ public void PropertiesMustHavePublicSetters_Success()
                 .BeTrue();
         }
 
+        private class AllPublicSetterMockBase
+        {
+            public string Private { get; private set; }
+        }
+
+        private class AllPublicSetterMockDerived : AllPublicSetterMockBase
+        {
+            public string Public { get; set; }
+        }
+
+        [Test]
+        public void PropertiesMustHavePublicSetters_IgnoresInheritedProperties()
+        {
+            typeof(AllPublicSetterMockDerived)
+                .MustConformTo(Convention.PropertiesMustHavePublicSetters)
+                .IsSatisfied
+                .Should()
+                .BeTrue();
+        }
+
         private class AllPrivateSetterMock
         {
             public string PrivateSet { get; private set; }
@@ -134,6 +174,26 @@ public void PropertiesMustHaveProtectedSetters_Success()
                 .BeTrue();
         }
 
+        private class AllProtectedSetterMockBase
+        {
+            public string Private { get; private set; }
+        }
+
+        private class AllProtectedSetterMockDerived : AllProtectedSetterMockBase
+        {
+            public string Public { get; protected set; }
+        }
+
+        [Test]
+        public void PropertiesMustHaveProtectedSetters_IgnoresInheritedProperties()
+        {
+            typeof(AllProtectedSetterMockDerived)
+                .MustConformTo(Convention.PropertiesMustHaveProtectedSetters)
+                .IsSatisfied
+                .Should()
+                .BeTrue();
+        }
+
         [Test]
         public void PropertiesMustHaveProtectedSetters_FailsWhenOtherSetterExists()
         {
@@ -164,6 +224,26 @@ public void PropertiesMustHavePrivateSetters_Success()
                 .BeTrue();
         }
 
+        private class AllPrivateSetterMockBase
+        {
+            public string Public { get; set; }
+        }
+
+        private class AllPrivateSetterMockDerived : AllPrivateSetterMockBase
+        {
+            public string Private { get; private set; }
+        }
+
+        [Test]
+        public void PropertiesMustHavePrivateSetters_IgnoresInheritedProperties()
+        {
+            typeof(AllPrivateSetterMockDerived)
+                .MustConformTo(Convention.PropertiesMustHavePrivateSetters)
+                .IsSatisfied
+                .Should()
+                .BeTrue();
+        }
+
         [Test]
         public void PropertiesMustHavePrivateSetters_FailsWhenOtherSetterExists()
         {
@@ -484,6 +564,17 @@ private class SomeGenericInterfaceImplementation : ISomeGeneric<SomeClassThatReq
         {
         }
 
+        private class MockWithoutPropertyAttributeBase
+        {
+            public string Description { get; set; }
+        }
+
+        private class MockWithPropertyAttributeDerived : MockWithoutPropertyAttributeBase
+        {
+            [MaxLength(255)]
+            public string Name { get; set; }
+        }
+
         private class MockWithPropertyAttribute
         {
             [MaxLength(255)]
@@ -511,6 +602,16 @@ public void PropertiesOfTypeMustHaveAttributeConventionSpecification_Success()
                 .BeTrue();
         }
 
+        [Test]
+        public void PropertiesOfTypeMustHaveAttributeConventionSpecification_IgnoresInheritedProperties()
+        {
+            typeof(MockWithPropertyAttributeDerived)
+                .MustConformTo(Convention.PropertiesOfTypeMustHaveAttribute(typeof(string), typeof(MaxLengthAttribute)))
+                .IsSatisfied
+                .Should()
+                .BeTrue();
+        }
+
         [Test]
         public void PropertiesOfTypeMustHaveAttributeConventionSpecification_WriteOnlySuccess()
         {
@@ -571,6 +672,26 @@ public void EnumerablePropertiesMustBeEagerLoadedConventionSpecification_Success
                  .BeTrue();
         }
 
+        private class HasLazilyLoadedEnumerablesBase
+        {
+            public IEnumerable<string> Descriptions { get; set; }
+        }
+
+        private class HasEagerLoadedEnumerablesDerived : HasLazilyLoadedEnumerablesBase
+        {
+            public string[] Names { get; set; }
+        }
+
+        [Test]
+        public void EnumerablePropertiesMustBeEagerLoadedConventionSpecification_IgnoresInheritedProperties()
+        {
+            typeof(HasEagerLoadedEnumerablesDerived)
+                .MustConformTo(Convention.EnumerablePropertiesMustBeEagerLoaded)
+                .IsSatisfied
+                .Should()
+                .BeTrue();
+        }
+
         private class HasLazilyLoadedEnumerables
         {
             public IEnumerable<string> Names { get; set; } 
@@ -608,6 +729,31 @@ public void CollectionPropertiesMustNotHaveSetters_Success()
                  .BeTrue();
         }
 
+        private class HasMutableCollectionsBase
+        {
+            public string[] Colours { get; set; }
+        }
+
+        private class HasImmutableCollectionsDerived : HasMutableCollectionsBase
+        {
+            public HasImmutableCollectionsDerived(string[] names)
+            {
+                Names = names;
+            }
+
+            public string[] Names { get; }
+        }
+
+        [Test]
+        public void CollectionPropertiesMustNotHaveSetters_IgnoresInheritedProperties()
+        {
+            typeof(HasImmutableCollectionsDerived)
+                .MustConformTo(Convention.CollectionPropertiesMustNotHaveSetters)
+                .IsSatisfied
+                .Should()
+                .BeTrue();
+        }
+
         private class HasMutableCollections
         {
             public string[] Names { get; set; }
@@ -625,18 +771,15 @@ public void CollectionPropertiesMustNotHaveSetters_FailsWhenAMutableCollectionPr
 
         private class HasImmutableProperties
         {
-            private readonly string[] _names;
-            private readonly int _age;
-
             public HasImmutableProperties(string[] names, int age)
             {
-                _names = names;
-                _age = age;
+                Names = names;
+                Age = age;
             }
 
-            public string[] Names => _names;
+            public string[] Names { get; }
 
-            public int Age => _age;
+            public int Age { get; }
         }
 
         [Test]
@@ -649,6 +792,34 @@ public void PropertiesMustNotHaveSetters_Success()
                  .BeTrue();
         }
 
+        private class HasMutablePropertiesBase
+        {
+            public int Age { get; set; }
+        }
+
+        private class HasImmutablePropertiesDerived : HasMutablePropertiesBase
+        {
+            public HasImmutablePropertiesDerived(string[] names, int age)
+            {
+                Names = names;
+                Age = age;
+            }
+
+            public string[] Names { get; }
+
+            public int Age { get; }
+        }
+
+        [Test]
+        public void PropertiesMustNotHaveSetters_IgnoresInheritedProperties()
+        {
+            typeof(HasImmutablePropertiesDerived)
+                .MustConformTo(Convention.PropertiesMustNotHaveSetters)
+                .IsSatisfied
+                .Should()
+                .BeTrue();
+        }
+
         private class HasMutableProperties
         {
             public string[] Names { get; set; }
@@ -666,6 +837,41 @@ public void PropertiesMustNotHaveSetters_FailsWhenMutablePropertiesExists()
             result.Failures.Should().HaveCount(1);
         }
 
+        private class DoesNotHaveEnumerableProperty
+        {
+            public long Ticks { get; set; }
+        }
+
+        [Test]
+        public void MustNotHaveAPropertyOfType_Success()
+        {
+            var result = typeof(DoesNotHaveEnumerableProperty)
+                .MustConformTo(Convention.MustNotHaveAPropertyOfType(typeof(IEnumerable<>), "reason"))
+                .IsSatisfied
+                .Should()
+                .BeTrue();
+        }
+
+        private class HasEnumerablePropertyBase
+        {
+            public IEnumerable<int> Numbers { get; set; }
+        }
+
+        private class DoesNotHaveEnumerablePropertyDerived : HasEnumerablePropertyBase
+        {
+            public long Ticks { get; set; }
+        }
+
+        [Test]
+        public void MustNotHaveAPropertyOfType_IgnoresInheritedProperties()
+        {
+            var result = typeof(DoesNotHaveEnumerablePropertyDerived)
+                .MustConformTo(Convention.MustNotHaveAPropertyOfType(typeof(IEnumerable<>), "reason"))
+                .IsSatisfied
+                .Should()
+                .BeTrue();
+        }
+
         private class HasGenericAndNonGenericProperty
         {
             public IEnumerable<string> Names { get; set; }

diff --git a/...Conventional/Conventions/CollectionPropertiesMustNotHaveSettersConventionSpecification.cs b/...Conventional/Conventions/CollectionPropertiesMustNotHaveSettersConventionSpecification.cs
@@ -2,6 +2,7 @@
 using System.Collections;
 using System.Linq;
 using System.Reflection;
+using Conventional.Extensions;
 
 namespace Conventional.Conventions
 {
@@ -12,7 +13,7 @@ public class CollectionPropertiesMustNotHaveSettersConventionSpecification : Pro
         protected override PropertyInfo[] GetNonConformingProperties(Type type)
         {
             return type
-                .GetProperties(BindingFlags.Instance | BindingFlags.Public)
+                .GetDeclaredProperties()
                 .Where(p => p.PropertyType != typeof (string) && (typeof (IEnumerable).IsAssignableFrom(p.PropertyType)))
                 .Where(p => p.GetSetMethod(true) != null)
                 .ToArray();

diff --git a/.../Conventional/Conventions/EnumerablePropertiesMustBeEagerLoadedConventionSpecification.cs b/.../Conventional/Conventions/EnumerablePropertiesMustBeEagerLoadedConventionSpecification.cs
@@ -2,6 +2,7 @@
 using System.Collections;
 using System.Linq;
 using System.Reflection;
+using Conventional.Extensions;
 
 namespace Conventional.Conventions
 {
@@ -12,7 +13,7 @@ public class EnumerablePropertiesMustBeEagerLoadedConventionSpecification : Conv
         public override ConventionResult IsSatisfiedBy(Type type)
         {
             var enumerables = type
-                .GetProperties(BindingFlags.Instance | BindingFlags.Public)
+                .GetDeclaredProperties()
                 .Select(p => p.PropertyType)
                 .Where(t => t != typeof(string) && (typeof(IEnumerable).IsAssignableFrom(t)))
                 .ToArray();

diff --git a/src/Core/Conventional/Conventions/MustNotHaveAPropertyOfTypeConventionSpecification.cs b/src/Core/Conventional/Conventions/MustNotHaveAPropertyOfTypeConventionSpecification.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Linq;
+using Conventional.Extensions;
 
 namespace Conventional.Conventions
 {
@@ -20,14 +21,14 @@ public override ConventionResult IsSatisfiedBy(Type type)
         {
             if (_propertyType.IsGenericTypeDefinition)
             {
-                return type.GetProperties()
+                return type.GetDeclaredProperties()
                     .Any(p => p.PropertyType.IsGenericType &&
                               p.PropertyType.GetGenericTypeDefinition() == _propertyType)
                     ? NotSatisfied(type)
                     : ConventionResult.Satisfied(type.FullName);
             }
 
-            return type.GetProperties().Any(p => p.PropertyType == _propertyType)
+            return type.GetDeclaredProperties().Any(p => p.PropertyType == _propertyType)
                 ? NotSatisfied(type)
                 : ConventionResult.Satisfied(type.FullName);
         }

diff --git a/src/Core/Conventional/Conventions/PropertiesMustHavePrivateSettersConventionSpecification.cs b/src/Core/Conventional/Conventions/PropertiesMustHavePrivateSettersConventionSpecification.cs
@@ -1,6 +1,7 @@
 using System;
 using System.Linq;
 using System.Reflection;
+using Conventional.Extensions;
 
 namespace Conventional.Conventions
 {
@@ -10,7 +11,7 @@ public class PropertiesMustHavePrivateSettersConventionSpecification : PropertyC
 
         protected override PropertyInfo[] GetNonConformingProperties(Type type)
         {
-            return type.GetProperties()
+            return type.GetDeclaredProperties()
                 .Where(subject => subject.CanWrite == false || subject.GetSetMethod(true).IsPrivate == false)
                 .ToArray();
         }

diff --git a/...ore/Conventional/Conventions/PropertiesMustHaveProtectedSettersConventionSpecification.cs b/...ore/Conventional/Conventions/PropertiesMustHaveProtectedSettersConventionSpecification.cs
@@ -1,6 +1,7 @@
 using System;
 using System.Linq;
 using System.Reflection;
+using Conventional.Extensions;
 
 namespace Conventional.Conventions
 {
@@ -10,7 +11,7 @@ public class PropertiesMustHaveProtectedSettersConventionSpecification : Propert
 
         protected override PropertyInfo[] GetNonConformingProperties(Type type)
         {
-            return type.GetProperties()
+            return type.GetDeclaredProperties()
                 .Where(subject => subject.CanWrite == false || subject.GetSetMethod(true).IsFamily == false)
                 .ToArray();
         }