From 881805c6941e3da5ec9ad8ee044de425488d2a51 Mon Sep 17 00:00:00 2001 From: Eric StJohn Date: Fri, 22 Apr 2022 13:41:39 -0700 Subject: [PATCH] Fix APICompat on properties and events --- .../Rules/Compat/AttributeDifference.cs | 31 ++++++++++--------- .../tests/AttributeDifferenceTests.cs | 4 ++- .../Contract/AttributeDifference.cs | 2 ++ .../AttributeDifferenceClass1.cs | 4 +++ 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.DotNet.ApiCompat/src/Microsoft.DotNet.ApiCompat.Core/Rules/Compat/AttributeDifference.cs b/src/Microsoft.DotNet.ApiCompat/src/Microsoft.DotNet.ApiCompat.Core/Rules/Compat/AttributeDifference.cs index f3a5d13dab3..8f5c2668aac 100644 --- a/src/Microsoft.DotNet.ApiCompat/src/Microsoft.DotNet.ApiCompat.Core/Rules/Compat/AttributeDifference.cs +++ b/src/Microsoft.DotNet.ApiCompat/src/Microsoft.DotNet.ApiCompat.Core/Rules/Compat/AttributeDifference.cs @@ -78,24 +78,27 @@ public override DifferenceType Diff(IDifferences differences, ITypeDefinition im public override DifferenceType Diff(IDifferences differences, ITypeDefinitionMember impl, ITypeDefinitionMember contract) { - var implMethod = impl as IMethodDefinition; - var contractMethod = contract as IMethodDefinition; - if (implMethod == null || contractMethod == null) + if (impl == null || contract == null) return DifferenceType.Unknown; - bool changed = CheckAttributeDifferences(differences, implMethod, implMethod.Attributes, contractMethod.Attributes); - - IParameterDefinition[] method1Params = implMethod.Parameters.ToArray(); - IParameterDefinition[] method2Params = contractMethod.Parameters.ToArray(); - for (int i = 0; i < implMethod.ParameterCount; i++) - changed |= CheckAttributeDifferences(differences, method1Params[i], method1Params[i].Attributes, method2Params[i].Attributes, member: implMethod); + bool changed = CheckAttributeDifferences(differences, impl, impl.Attributes, contract.Attributes); - if (implMethod.IsGeneric) + var implMethod = impl as IMethodDefinition; + var contractMethod = contract as IMethodDefinition; + if (implMethod != null && contractMethod != null) { - IGenericParameter[] method1GenParams = implMethod.GenericParameters.ToArray(); - IGenericParameter[] method2GenParam = contractMethod.GenericParameters.ToArray(); - for (int i = 0; i < implMethod.GenericParameterCount; i++) - changed |= CheckAttributeDifferences(differences, method1GenParams[i], method1GenParams[i].Attributes, method2GenParam[i].Attributes, member: implMethod); + IParameterDefinition[] method1Params = implMethod.Parameters.ToArray(); + IParameterDefinition[] method2Params = contractMethod.Parameters.ToArray(); + for (int i = 0; i < implMethod.ParameterCount; i++) + changed |= CheckAttributeDifferences(differences, method1Params[i], method1Params[i].Attributes, method2Params[i].Attributes, member: implMethod); + + if (implMethod.IsGeneric) + { + IGenericParameter[] method1GenParams = implMethod.GenericParameters.ToArray(); + IGenericParameter[] method2GenParam = contractMethod.GenericParameters.ToArray(); + for (int i = 0; i < implMethod.GenericParameterCount; i++) + changed |= CheckAttributeDifferences(differences, method1GenParams[i], method1GenParams[i].Attributes, method2GenParam[i].Attributes, member: implMethod); + } } return changed ? DifferenceType.Changed : DifferenceType.Unchanged; diff --git a/src/Microsoft.DotNet.ApiCompat/tests/AttributeDifferenceTests.cs b/src/Microsoft.DotNet.ApiCompat/tests/AttributeDifferenceTests.cs index 168b2f02721..dc9ca6a4faa 100644 --- a/src/Microsoft.DotNet.ApiCompat/tests/AttributeDifferenceTests.cs +++ b/src/Microsoft.DotNet.ApiCompat/tests/AttributeDifferenceTests.cs @@ -21,12 +21,14 @@ public void AttributeDifferenceIsFound(ApiCompatFrontend frontend) Assert.Contains("CannotRemoveAttribute : Attribute 'System.ComponentModel.DesignerAttribute' exists on 'AttributeDifference.AttributeDifferenceClass1' in the implementation but not the contract.", runOutput); Assert.Contains("CannotRemoveAttribute : Attribute 'AttributeDifference.FooAttribute' exists on 'AttributeDifference.AttributeDifferenceClass1' in the implementation but not the contract.", runOutput); + Assert.Contains("CannotRemoveAttribute : Attribute 'AttributeDifference.FooAttribute' exists on 'AttributeDifference.AttributeDifferenceClass1.PropertyWithAttribute' in the implementation but not the contract.", runOutput); + Assert.Contains("CannotRemoveAttribute : Attribute 'AttributeDifference.FooAttribute' exists on 'AttributeDifference.AttributeDifferenceClass1.EventWithAttribute' in the implementation but not the contract.", runOutput); Assert.Contains("CannotRemoveAttribute : Attribute 'System.ComponentModel.DefaultValueAttribute' exists on generic param 'T' on member 'AttributeDifference.AttributeDifferenceClass1.GenericMethodWithAttribute()' in the implementation but not the contract.", runOutput); Assert.Contains("CannotRemoveAttribute : Attribute 'System.ComponentModel.DefaultValueAttribute' exists on generic param 'T' on member 'AttributeDifference.AttributeDifferenceClass1.GenericMethodWithAttribute()' in the implementation but not the contract.", runOutput); Assert.Contains("CannotRemoveAttribute : Attribute 'AttributeDifference.FooAttribute' exists on 'AttributeDifference.AttributeDifferenceClass1.MethodWithAttribute()' in the implementation but not the contract.", runOutput); Assert.Contains("CannotRemoveAttribute : Attribute 'AttributeDifference.FooAttribute' exists on parameter 'myParameter' on member 'AttributeDifference.AttributeDifferenceClass1.MethodWithAttribute(System.String, System.Object)' in the implementation but not the contract.", runOutput); Assert.Contains("CannotRemoveAttribute : Attribute 'System.ComponentModel.DefaultValueAttribute' exists on generic param 'TOne' on member 'AttributeDifference.AttributeDifferenceGenericCLass' in the implementation but not the contract.", runOutput); - Assert.Contains("Total Issues: 6", runOutput); + Assert.Contains("Total Issues: 8", runOutput); } [Theory] diff --git a/src/Microsoft.DotNet.ApiCompat/tests/TestProjects/AttributeDifference/Contract/AttributeDifference.cs b/src/Microsoft.DotNet.ApiCompat/tests/TestProjects/AttributeDifference/Contract/AttributeDifference.cs index 1344760bc70..0f0a4af3ac6 100644 --- a/src/Microsoft.DotNet.ApiCompat/tests/TestProjects/AttributeDifference/Contract/AttributeDifference.cs +++ b/src/Microsoft.DotNet.ApiCompat/tests/TestProjects/AttributeDifference/Contract/AttributeDifference.cs @@ -11,6 +11,8 @@ public class AttributeDifferenceClass1 public string MethodWithAttribute(string myParameter, [DefaultValue("myObject")] object myObject) => throw null; public T GenericMethodWithAttribute() => throw null; public void MethodWithAttribute() { } + public string PropertyWithAttribute { get; set; } + public event System.EventHandler EventWithAttribute { add { } remove { } } } public class AttributeDifferenceGenericCLass { diff --git a/src/Microsoft.DotNet.ApiCompat/tests/TestProjects/AttributeDifference/Implementation/AttributeDifferenceClass1.cs b/src/Microsoft.DotNet.ApiCompat/tests/TestProjects/AttributeDifference/Implementation/AttributeDifferenceClass1.cs index 2e9020dd8f1..3000785e406 100644 --- a/src/Microsoft.DotNet.ApiCompat/tests/TestProjects/AttributeDifference/Implementation/AttributeDifferenceClass1.cs +++ b/src/Microsoft.DotNet.ApiCompat/tests/TestProjects/AttributeDifference/Implementation/AttributeDifferenceClass1.cs @@ -15,6 +15,10 @@ public class AttributeDifferenceClass1 public T GenericMethodWithAttribute<[DefaultValue("T")] T>() => default(T); [Foo] public void MethodWithAttribute() { } + [Foo] + public string PropertyWithAttribute { get; set; } + [Foo] + public event System.EventHandler EventWithAttribute { add { } remove { } } } public class AttributeDifferenceGenericCLass<[DefaultValue("TOne")] TOne, [DefaultValue("TTwo")] TTwo>