From a997e13e1f8c210ada22588f35d34f4aa2e42944 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 5 Mar 2020 09:39:47 -0800 Subject: [PATCH 1/6] Add DiagnosticId UrlFormat propertites for Obsolete attribute --- .../src/System/ObsoleteAttribute.cs | 4 ++ .../System.Runtime/ref/System.Runtime.cs | 2 + .../tests/System/ObsoleteAttributeTests.cs | 64 ++++++++++++++++--- 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/ObsoleteAttribute.cs b/src/libraries/System.Private.CoreLib/src/System/ObsoleteAttribute.cs index 94a26dd95078c5..604ad78c594148 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ObsoleteAttribute.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ObsoleteAttribute.cs @@ -48,5 +48,9 @@ public ObsoleteAttribute(string? message, bool error) public string? Message => _message; public bool IsError => _error; + + public string? DiagnosticId { get; set; } + + public string? UrlFormat { get; set; } } } diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index efd42520dc7261..d21f950b648eb7 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -2899,6 +2899,8 @@ public ObsoleteAttribute(string? message) { } public ObsoleteAttribute(string? message, bool error) { } public bool IsError { get { throw null; } } public string? Message { get { throw null; } } + public string? DiagnosticId { get { throw null; } set { } } + public string? UrlFormat { get { throw null; } set { } } } public sealed partial class OperatingSystem : System.ICloneable, System.Runtime.Serialization.ISerializable { diff --git a/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs b/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs index 99f6c31831fb96..f9007ec216ae59 100644 --- a/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs +++ b/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs @@ -2,6 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Collections.Generic; +using System.Linq; +using System.Runtime.InteropServices; using Xunit; namespace System.Tests @@ -14,28 +17,69 @@ public static void Ctor_Default() var attribute = new ObsoleteAttribute(); Assert.Null(attribute.Message); Assert.False(attribute.IsError); + Assert.Null(attribute.DiagnosticId); + Assert.Null(attribute.UrlFormat); } [Theory] - [InlineData(null)] - [InlineData("")] - [InlineData("message")] - public void Ctor_String(string message) + [InlineData(null, null)] + [InlineData("", "BCL0006")] + [InlineData("message", "")] + public void Ctor_String_Id(string message, string id) { - var attribute = new ObsoleteAttribute(message); + var attribute = new ObsoleteAttribute(message) { DiagnosticId = id}; Assert.Equal(message, attribute.Message); Assert.False(attribute.IsError); + Assert.Equal(id, attribute.DiagnosticId); + Assert.Null(attribute.UrlFormat); } [Theory] - [InlineData(null, true)] - [InlineData("", false)] - [InlineData("message", true)] - public void Ctor_String_Bool(string message, bool error) + [InlineData(null, true, "")] + [InlineData("", false, null)] + [InlineData("message", true, "https://aka.ms/obsolete/{0}")] + public void Ctor_String_Bool_Url(string message, bool error, string url) { - var attribute = new ObsoleteAttribute(message, error); + var attribute = new ObsoleteAttribute(message, error) { UrlFormat = url}; Assert.Equal(message, attribute.Message); Assert.Equal(error, attribute.IsError); + Assert.Null(attribute.DiagnosticId); + Assert.Equal(url, attribute.UrlFormat); } + + [Theory] + [MemberData(nameof(TestData_Attributes))] + public void TestAttributeValuesWithReflection(Type testType, string message, bool error, string id, string url) { + + Attribute attribute = Attribute.GetCustomAttributes(testType, typeof(Attribute)).Where((e) => e.GetType() == typeof(ObsoleteAttribute)).SingleOrDefault(); + + Assert.NotNull(attribute); + Assert.Equal(message, attribute.GetType().GetProperty("Message").GetValue(attribute)); + Assert.Equal(error, attribute.GetType().GetProperty("IsError").GetValue(attribute)); + Assert.Equal(id, attribute.GetType().GetProperty("DiagnosticId").GetValue(attribute)); + Assert.Equal(url, attribute.GetType().GetProperty("UrlFormat").GetValue(attribute)); + } + + public static IEnumerable TestData_Attributes() + { +#pragma warning disable + yield return new object[] { typeof(ClassMessageAndDiagnosticIdPropertySet), "Don't use", false, "BCL0006", null }; + yield return new object[] { typeof(ClassEmptyConstructorOptionalPropertiesSet), null, false, "BCL0003", "https://aka.ms/obsolete3/{0}" }; + yield return new object[] { typeof(ClassMessageAndUrlFormatPropertySet), "Obsolete", false, null, "https://aka.ms/obsolete2/{0}" }; + yield return new object[] { typeof(ClassAllFieldPropertiesSet), "Deprecated", false, "BCL0001", "https://aka.ms/obsolete1/{0}" }; +#pragma warning restore + } + + [Obsolete("Don't use", DiagnosticId = "BCL0006")] + private class ClassMessageAndDiagnosticIdPropertySet { } + + [Obsolete( DiagnosticId = "BCL0003", UrlFormat = "https://aka.ms/obsolete3/{0}")] + private class ClassEmptyConstructorOptionalPropertiesSet { } + + [Obsolete("Obsolete", UrlFormat = "https://aka.ms/obsolete2/{0}")] + private class ClassMessageAndUrlFormatPropertySet { } + + [Obsolete("Deprecated", false, UrlFormat = "https://aka.ms/obsolete1/{0}", DiagnosticId = "BCL0001")] + private class ClassAllFieldPropertiesSet { } } } From 69a2fbe4c2f51a2fba46aedd4a4ae7787f70bd80 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 5 Mar 2020 11:16:49 -0800 Subject: [PATCH 2/6] Applying feedback --- .../src/System/ObsoleteAttribute.cs | 24 +++++++++---------- .../System.Runtime/ref/System.Runtime.cs | 2 +- .../tests/System/ObsoleteAttributeTests.cs | 17 ++++++------- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/ObsoleteAttribute.cs b/src/libraries/System.Private.CoreLib/src/System/ObsoleteAttribute.cs index 604ad78c594148..ac4d5d2a27ed5a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ObsoleteAttribute.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ObsoleteAttribute.cs @@ -18,36 +18,36 @@ namespace System // Error indicates if the compiler should treat usage of such a method as an // error. (this would be used if the actual implementation of the obsolete // method's implementation had changed). + // DiagnosticId. Represents the ID the compiler will use when reporting a use of the API. + // UrlFormat.The URL that should be used by an IDE for navigating to corresponding documentation. Instead of taking the URL directly, + // the API takes a format string. This allows having a generic URL that includes the diagnostic ID. // - [AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Enum | + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Enum | AttributeTargets.Interface | AttributeTargets.Constructor | AttributeTargets.Method | AttributeTargets.Property | AttributeTargets.Field | AttributeTargets.Event | AttributeTargets.Delegate, Inherited = false)] public sealed class ObsoleteAttribute : Attribute { - private readonly string? _message; - private readonly bool _error; - public ObsoleteAttribute() { - _message = null; - _error = false; + Message = null; + IsError = false; } public ObsoleteAttribute(string? message) { - _message = message; - _error = false; + Message = message; + IsError = false; } public ObsoleteAttribute(string? message, bool error) { - _message = message; - _error = error; + Message = message; + IsError = error; } - public string? Message => _message; + public string? Message { get; } - public bool IsError => _error; + public bool IsError { get; } public string? DiagnosticId { get; set; } diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index d21f950b648eb7..467d544051fa53 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -2898,8 +2898,8 @@ public ObsoleteAttribute() { } public ObsoleteAttribute(string? message) { } public ObsoleteAttribute(string? message, bool error) { } public bool IsError { get { throw null; } } - public string? Message { get { throw null; } } public string? DiagnosticId { get { throw null; } set { } } + public string? Message { get { throw null; } } public string? UrlFormat { get { throw null; } set { } } } public sealed partial class OperatingSystem : System.ICloneable, System.Runtime.Serialization.ISerializable diff --git a/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs b/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs index f9007ec216ae59..025d6a65fb7348 100644 --- a/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs +++ b/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs @@ -27,7 +27,7 @@ public static void Ctor_Default() [InlineData("message", "")] public void Ctor_String_Id(string message, string id) { - var attribute = new ObsoleteAttribute(message) { DiagnosticId = id}; + var attribute = new ObsoleteAttribute(message) { DiagnosticId = id }; Assert.Equal(message, attribute.Message); Assert.False(attribute.IsError); Assert.Equal(id, attribute.DiagnosticId); @@ -40,7 +40,7 @@ public void Ctor_String_Id(string message, string id) [InlineData("message", true, "https://aka.ms/obsolete/{0}")] public void Ctor_String_Bool_Url(string message, bool error, string url) { - var attribute = new ObsoleteAttribute(message, error) { UrlFormat = url}; + var attribute = new ObsoleteAttribute(message, error) { UrlFormat = url }; Assert.Equal(message, attribute.Message); Assert.Equal(error, attribute.IsError); Assert.Null(attribute.DiagnosticId); @@ -49,7 +49,8 @@ public void Ctor_String_Bool_Url(string message, bool error, string url) [Theory] [MemberData(nameof(TestData_Attributes))] - public void TestAttributeValuesWithReflection(Type testType, string message, bool error, string id, string url) { + public void TestAttributeValuesWithReflection(Type testType, string message, bool error, string id, string url) + { Attribute attribute = Attribute.GetCustomAttributes(testType, typeof(Attribute)).Where((e) => e.GetType() == typeof(ObsoleteAttribute)).SingleOrDefault(); @@ -73,13 +74,13 @@ public static IEnumerable TestData_Attributes() [Obsolete("Don't use", DiagnosticId = "BCL0006")] private class ClassMessageAndDiagnosticIdPropertySet { } - [Obsolete( DiagnosticId = "BCL0003", UrlFormat = "https://aka.ms/obsolete3/{0}")] - private class ClassEmptyConstructorOptionalPropertiesSet { } + [Obsolete(DiagnosticId = "BCL0003", UrlFormat = "https://aka.ms/obsolete3/{0}")] + private interface ClassEmptyConstructorOptionalPropertiesSet { } [Obsolete("Obsolete", UrlFormat = "https://aka.ms/obsolete2/{0}")] - private class ClassMessageAndUrlFormatPropertySet { } + private delegate void ClassMessageAndUrlFormatPropertySet(); [Obsolete("Deprecated", false, UrlFormat = "https://aka.ms/obsolete1/{0}", DiagnosticId = "BCL0001")] - private class ClassAllFieldPropertiesSet { } - } + private struct ClassAllFieldPropertiesSet { } + } } From f59d0bf9b7af3c5425acc27acb9121f568a372c3 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 5 Mar 2020 11:26:29 -0800 Subject: [PATCH 3/6] Formatting --- .../System.Private.CoreLib/src/System/ObsoleteAttribute.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/ObsoleteAttribute.cs b/src/libraries/System.Private.CoreLib/src/System/ObsoleteAttribute.cs index ac4d5d2a27ed5a..0bd3af61b8a211 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ObsoleteAttribute.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ObsoleteAttribute.cs @@ -22,7 +22,7 @@ namespace System // UrlFormat.The URL that should be used by an IDE for navigating to corresponding documentation. Instead of taking the URL directly, // the API takes a format string. This allows having a generic URL that includes the diagnostic ID. // - [AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Enum | + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Enum | AttributeTargets.Interface | AttributeTargets.Constructor | AttributeTargets.Method | AttributeTargets.Property | AttributeTargets.Field | AttributeTargets.Event | AttributeTargets.Delegate, Inherited = false)] public sealed class ObsoleteAttribute : Attribute From 06af5538fc651eeac52e40164d86da851fd80938 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 5 Mar 2020 11:44:18 -0800 Subject: [PATCH 4/6] Removed reflection test --- .../tests/System/ObsoleteAttributeTests.cs | 36 ------------------- 1 file changed, 36 deletions(-) diff --git a/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs b/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs index 025d6a65fb7348..a5a7ef60d71e5c 100644 --- a/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs +++ b/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs @@ -46,41 +46,5 @@ public void Ctor_String_Bool_Url(string message, bool error, string url) Assert.Null(attribute.DiagnosticId); Assert.Equal(url, attribute.UrlFormat); } - - [Theory] - [MemberData(nameof(TestData_Attributes))] - public void TestAttributeValuesWithReflection(Type testType, string message, bool error, string id, string url) - { - - Attribute attribute = Attribute.GetCustomAttributes(testType, typeof(Attribute)).Where((e) => e.GetType() == typeof(ObsoleteAttribute)).SingleOrDefault(); - - Assert.NotNull(attribute); - Assert.Equal(message, attribute.GetType().GetProperty("Message").GetValue(attribute)); - Assert.Equal(error, attribute.GetType().GetProperty("IsError").GetValue(attribute)); - Assert.Equal(id, attribute.GetType().GetProperty("DiagnosticId").GetValue(attribute)); - Assert.Equal(url, attribute.GetType().GetProperty("UrlFormat").GetValue(attribute)); - } - - public static IEnumerable TestData_Attributes() - { -#pragma warning disable - yield return new object[] { typeof(ClassMessageAndDiagnosticIdPropertySet), "Don't use", false, "BCL0006", null }; - yield return new object[] { typeof(ClassEmptyConstructorOptionalPropertiesSet), null, false, "BCL0003", "https://aka.ms/obsolete3/{0}" }; - yield return new object[] { typeof(ClassMessageAndUrlFormatPropertySet), "Obsolete", false, null, "https://aka.ms/obsolete2/{0}" }; - yield return new object[] { typeof(ClassAllFieldPropertiesSet), "Deprecated", false, "BCL0001", "https://aka.ms/obsolete1/{0}" }; -#pragma warning restore - } - - [Obsolete("Don't use", DiagnosticId = "BCL0006")] - private class ClassMessageAndDiagnosticIdPropertySet { } - - [Obsolete(DiagnosticId = "BCL0003", UrlFormat = "https://aka.ms/obsolete3/{0}")] - private interface ClassEmptyConstructorOptionalPropertiesSet { } - - [Obsolete("Obsolete", UrlFormat = "https://aka.ms/obsolete2/{0}")] - private delegate void ClassMessageAndUrlFormatPropertySet(); - - [Obsolete("Deprecated", false, UrlFormat = "https://aka.ms/obsolete1/{0}", DiagnosticId = "BCL0001")] - private struct ClassAllFieldPropertiesSet { } } } From aa82701d2c0e533f44e2fd47fd66a2731cb4d7b3 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 5 Mar 2020 13:00:44 -0800 Subject: [PATCH 5/6] Remove unneeded import --- .../System.Runtime/tests/System/ObsoleteAttributeTests.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs b/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs index a5a7ef60d71e5c..1bda0b57528acf 100644 --- a/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs +++ b/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs @@ -2,9 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.Collections.Generic; -using System.Linq; -using System.Runtime.InteropServices; using Xunit; namespace System.Tests From 01b7641d43d5cd6d4c8f3181569f3b1c3a7e1c6c Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 5 Mar 2020 13:05:38 -0800 Subject: [PATCH 6/6] Remove spaces --- .../System.Runtime/tests/System/ObsoleteAttributeTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs b/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs index 1bda0b57528acf..af3bb7cd1f7a4c 100644 --- a/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs +++ b/src/libraries/System.Runtime/tests/System/ObsoleteAttributeTests.cs @@ -43,5 +43,5 @@ public void Ctor_String_Bool_Url(string message, bool error, string url) Assert.Null(attribute.DiagnosticId); Assert.Equal(url, attribute.UrlFormat); } - } + } }