From 4dcb885758aed1e8df022479f306ab1f559046ed Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Thu, 28 Apr 2022 16:41:06 -0700 Subject: [PATCH 1/3] Fix TypeConverter when enabling UseSystemResourceKeys --- .../src/System/ComponentModel/ArrayConverter.cs | 2 +- .../src/System/ComponentModel/CollectionConverter.cs | 2 +- .../System/ComponentModel/CultureInfoConverter.cs | 2 +- .../ComponentModel/Design/DesignerOptionService.cs | 2 +- .../ComponentModel/ExtendedPropertyDescriptor.cs | 2 +- .../System/ComponentModel/InstanceCreationEditor.cs | 2 +- .../ComponentModel/MultilineStringConverter.cs | 2 +- .../src/System/ComponentModel/ReferenceConverter.cs | 2 +- .../src/System/ComponentModel/TypeConverter.cs | 4 ++-- .../src/System/ComponentModel/TypeListConverter.cs | 2 +- .../src/System/Timers/TimersDescriptionAttribute.cs | 2 +- .../tests/ContainerTests.cs | 12 ++++++------ .../tests/CultureInfoConverterTests.cs | 10 ++++++++++ 13 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ArrayConverter.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ArrayConverter.cs index f71ed58a86cd6..5ee2d43496f51 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ArrayConverter.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ArrayConverter.cs @@ -19,7 +19,7 @@ public class ArrayConverter : CollectionConverter { if (destinationType == typeof(string) && value is Array) { - return SR.Format(SR.Array, value.GetType().Name); + return string.Format(SR.GetResourceString(nameof(SR.Array), "{0} Array"), value.GetType().Name); } return base.ConvertTo(context, culture, value, destinationType); diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/CollectionConverter.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/CollectionConverter.cs index 52b332bc803df..4f0f1dcc5b9e6 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/CollectionConverter.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/CollectionConverter.cs @@ -20,7 +20,7 @@ public class CollectionConverter : TypeConverter { if (destinationType == typeof(string) && value is ICollection) { - return SR.Collection; + return SR.GetResourceString(nameof(SR.Collection), "(Collection)"); } return base.ConvertTo(context, culture, value, destinationType); diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/CultureInfoConverter.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/CultureInfoConverter.cs index 20787a19e4b1b..fc9d63999632c 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/CultureInfoConverter.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/CultureInfoConverter.cs @@ -22,7 +22,7 @@ public class CultureInfoConverter : TypeConverter /// /// Retrieves the "default" name for our culture. /// - private static string DefaultCultureString => SR.CultureInfoConverterDefaultCultureString; + private static string DefaultCultureString => SR.GetResourceString(nameof(SR.CultureInfoConverterDefaultCultureString), "(Default)"); private const string DefaultInvariantCultureString = "(Default)"; diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/Design/DesignerOptionService.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/Design/DesignerOptionService.cs index ba0efec8f371c..444c0601c394a 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/Design/DesignerOptionService.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/Design/DesignerOptionService.cs @@ -467,7 +467,7 @@ public override PropertyDescriptorCollection GetProperties(ITypeDescriptorContex { if (destinationType == typeof(string)) { - return SR.CollectionConverterText; + return SR.GetResourceString(nameof(SR.CollectionConverterText), "(Collection)"); } return base.ConvertTo(cxt, culture, value, destinationType); } diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ExtendedPropertyDescriptor.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ExtendedPropertyDescriptor.cs index 959ce0dcb3e97..645f469ebed3b 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ExtendedPropertyDescriptor.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ExtendedPropertyDescriptor.cs @@ -99,7 +99,7 @@ public override string DisplayName string? providerName = site?.Name; if (providerName != null && providerName.Length > 0) { - name = SR.Format(SR.MetaExtenderName, name, providerName); + name = string.Format(SR.GetResourceString(nameof(SR.MetaExtenderName), "{0} on {1}"), name, providerName); } } return name; diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/InstanceCreationEditor.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/InstanceCreationEditor.cs index 68a566544d01b..5f0ab1fe809e8 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/InstanceCreationEditor.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/InstanceCreationEditor.cs @@ -11,7 +11,7 @@ namespace System.ComponentModel /// public abstract class InstanceCreationEditor { - public virtual string Text => SR.InstanceCreationEditorDefaultText; + public virtual string Text => SR.GetResourceString(nameof(SR.InstanceCreationEditorDefaultText), "(New...)"); /// /// This method is invoked when you user chooses the link displayed by the PropertyGrid for the InstanceCreationEditor. diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/MultilineStringConverter.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/MultilineStringConverter.cs index 5a632e69e8600..151085de3cefa 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/MultilineStringConverter.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/MultilineStringConverter.cs @@ -20,7 +20,7 @@ public class MultilineStringConverter : TypeConverter if (destinationType == typeof(string) && value is string) { - return SR.Text; + return SR.GetResourceString(nameof(SR.Text), "(Text)"); } return base.ConvertTo(context, culture, value, destinationType); diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReferenceConverter.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReferenceConverter.cs index 5f011122588a1..5d23a90fb78a0 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReferenceConverter.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReferenceConverter.cs @@ -15,7 +15,7 @@ namespace System.ComponentModel /// public class ReferenceConverter : TypeConverter { - private static readonly string s_none = SR.toStringNone; + private static readonly string s_none = SR.GetResourceString(nameof(SR.toStringNone), "(none)"); private readonly Type _type; /// diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeConverter.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeConverter.cs index e3ffc2fbd063a..b22befb2ba6bf 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeConverter.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeConverter.cs @@ -191,7 +191,7 @@ public virtual bool CanConvertTo(ITypeDescriptorContext? context, [NotNullWhen(t /// protected Exception GetConvertFromException(object? value) { - string? valueTypeName = value == null ? SR.Null : value.GetType().FullName; + string? valueTypeName = value == null ? SR.GetResourceString(nameof(SR.Null), "(null)") : value.GetType().FullName; throw new NotSupportedException(SR.Format(SR.ConvertFromException, GetType().Name, valueTypeName)); } @@ -201,7 +201,7 @@ protected Exception GetConvertFromException(object? value) /// protected Exception GetConvertToException(object? value, Type destinationType) { - string? valueTypeName = value == null ? SR.Null : value.GetType().FullName; + string? valueTypeName = value == null ? SR.GetResourceString(nameof(SR.Null), "(null)") : value.GetType().FullName; throw new NotSupportedException(SR.Format(SR.ConvertToException, GetType().Name, valueTypeName, destinationType.FullName)); } diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeListConverter.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeListConverter.cs index 154cca5ae7c6b..3df91fba7d725 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeListConverter.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeListConverter.cs @@ -73,7 +73,7 @@ public override bool CanConvertTo(ITypeDescriptorContext? context, [NotNullWhen( { if (value == null) { - return SR.none; + return SR.GetResourceString(nameof(SR.none), "(none)"); } else { diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/Timers/TimersDescriptionAttribute.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/Timers/TimersDescriptionAttribute.cs index 8660f3b5658ea..54b1c8cc112f3 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/Timers/TimersDescriptionAttribute.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/Timers/TimersDescriptionAttribute.cs @@ -40,7 +40,7 @@ public override string Description if (!_replaced) { _replaced = true; - DescriptionValue = SR.Format(base.Description); + DescriptionValue = string.Format(base.Description); } return base.Description; } diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/ContainerTests.cs b/src/libraries/System.ComponentModel.TypeConverter/tests/ContainerTests.cs index f1f331994a934..1dde0eb727597 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/tests/ContainerTests.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/ContainerTests.cs @@ -322,7 +322,7 @@ public void Add2_Name_Duplicate() Assert.Equal(typeof(ArgumentException), ex.GetType()); Assert.Null(ex.InnerException); Assert.NotNull(ex.Message); - Assert.True(ex.Message.IndexOf("'dup'") != -1); + Assert.True(ex.Message.IndexOf("'dup'") != -1 || ex.Message.IndexOf(" dup") != -1); Assert.Null(ex.ParamName); Assert.Equal(1, container.Components.Count); @@ -334,7 +334,7 @@ public void Add2_Name_Duplicate() Assert.Equal(typeof(ArgumentException), ex.GetType()); Assert.Null(ex.InnerException); Assert.NotNull(ex.Message); - Assert.True(ex.Message.IndexOf("'duP'") != -1); + Assert.True(ex.Message.IndexOf("'duP'") != -1 || ex.Message.IndexOf(" duP") != -1); Assert.Null(ex.ParamName); Assert.Equal(1, container.Components.Count); @@ -356,7 +356,7 @@ public void Add2_Name_Duplicate() Assert.Equal(typeof(ArgumentException), ex.GetType()); Assert.Null(ex.InnerException); Assert.NotNull(ex.Message); - Assert.True(ex.Message.IndexOf("'dup'") != -1); + Assert.True(ex.Message.IndexOf("'dup'") != -1 || ex.Message.IndexOf(" dup") != -1); Assert.Null(ex.ParamName); Assert.Equal(2, container.Components.Count); Assert.Equal(1, container2.Components.Count); @@ -729,7 +729,7 @@ public void ValidateName_Name_Duplicate() Assert.Equal(typeof(ArgumentException), ex.GetType()); Assert.Null(ex.InnerException); Assert.NotNull(ex.Message); - Assert.True(ex.Message.IndexOf("'dup'") != -1); + Assert.True(ex.Message.IndexOf("'dup'") != -1 || ex.Message.IndexOf(" dup") != -1); Assert.Null(ex.ParamName); Assert.Equal(2, _container.Components.Count); _container.InvokeValidateName(compB, "whatever"); @@ -742,7 +742,7 @@ public void ValidateName_Name_Duplicate() Assert.Equal(typeof(ArgumentException), ex.GetType()); Assert.Null(ex.InnerException); Assert.NotNull(ex.Message); - Assert.True(ex.Message.IndexOf("'dup'") != -1); + Assert.True(ex.Message.IndexOf("'dup'") != -1 || ex.Message.IndexOf(" dup") != -1); Assert.Null(ex.ParamName); Assert.Equal(2, _container.Components.Count); _container.InvokeValidateName(compC, "whatever"); @@ -757,7 +757,7 @@ public void ValidateName_Name_Duplicate() Assert.Equal(typeof(ArgumentException), ex.GetType()); Assert.Null(ex.InnerException); Assert.NotNull(ex.Message); - Assert.True(ex.Message.IndexOf("'dup'") != -1); + Assert.True(ex.Message.IndexOf("'dup'") != -1 || ex.Message.IndexOf(" dup") != -1); Assert.Null(ex.ParamName); Assert.Equal(2, _container.Components.Count); _container.InvokeValidateName(compD, "whatever"); diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/CultureInfoConverterTests.cs b/src/libraries/System.ComponentModel.TypeConverter/tests/CultureInfoConverterTests.cs index bbe51686b1ec8..16b7841127e6f 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/tests/CultureInfoConverterTests.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/CultureInfoConverterTests.cs @@ -6,6 +6,7 @@ using System.Globalization; using System.Linq; using System.Reflection; +using System.Tests; using Xunit; namespace System.ComponentModel.Tests @@ -152,6 +153,15 @@ public void GetCultureName_Overriden_ConversionsReturnsExpected() Assert.Equal("Fixed", converter.ConvertTo(new CultureInfo("en-US"), typeof(string))); } + [Fact] + public void CultureInfoConverterForDefaultValue() + { + using (new ThreadCultureChange(null, CultureInfo.InvariantCulture)) + { + Assert.Equal("", ((CultureInfo)TypeDescriptor.GetConverter(typeof(System.Globalization.CultureInfo)).ConvertFrom(null, null, "(Default)")).Name); + } + } + private class SubCultureInfoConverter : CultureInfoConverter { public new string GetCultureName(CultureInfo culture) From 48e84a306c8149cd4788cfb1d1b4ad759d35ec92 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Fri, 29 Apr 2022 09:54:17 -0700 Subject: [PATCH 2/3] Address the feedback --- .../src/System/Timers/TimersDescriptionAttribute.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/Timers/TimersDescriptionAttribute.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/Timers/TimersDescriptionAttribute.cs index 54b1c8cc112f3..4ec216bca3113 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/Timers/TimersDescriptionAttribute.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/Timers/TimersDescriptionAttribute.cs @@ -40,7 +40,7 @@ public override string Description if (!_replaced) { _replaced = true; - DescriptionValue = string.Format(base.Description); + DescriptionValue = base.Description; } return base.Description; } From d977a034ea947bcdfd6895b21c919b827603e3f8 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Fri, 29 Apr 2022 18:04:01 -0700 Subject: [PATCH 3/3] Keep original behavior and enable running the added test with UseSystemResourceKeys on --- .../Timers/TimersDescriptionAttribute.cs | 5 ++++- .../tests/CultureInfoConverterTests.cs | 19 ++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/Timers/TimersDescriptionAttribute.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/Timers/TimersDescriptionAttribute.cs index 4ec216bca3113..66ee30ec18372 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/Timers/TimersDescriptionAttribute.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/Timers/TimersDescriptionAttribute.cs @@ -40,7 +40,10 @@ public override string Description if (!_replaced) { _replaced = true; - DescriptionValue = base.Description; + + // We call string.Format here only to keep the original behavior which throws when having null description. + // That will keep the exception is thrown from same original place with the exact parameters. + DescriptionValue = string.Format(base.Description); } return base.Description; } diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/CultureInfoConverterTests.cs b/src/libraries/System.ComponentModel.TypeConverter/tests/CultureInfoConverterTests.cs index 16b7841127e6f..466d92d6eee2a 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/tests/CultureInfoConverterTests.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/CultureInfoConverterTests.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.DotNet.RemoteExecutor; using System.Collections.Generic; using System.ComponentModel.Design.Serialization; using System.Globalization; @@ -153,13 +154,21 @@ public void GetCultureName_Overriden_ConversionsReturnsExpected() Assert.Equal("Fixed", converter.ConvertTo(new CultureInfo("en-US"), typeof(string))); } - [Fact] - public void CultureInfoConverterForDefaultValue() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public void CultureInfoConverterForDefaultValue(bool useSystemResourceKeys) { - using (new ThreadCultureChange(null, CultureInfo.InvariantCulture)) + RemoteInvokeOptions options = new RemoteInvokeOptions(); + options.RuntimeConfigurationOptions.Add("System.Resources.UseSystemResourceKeys", useSystemResourceKeys); + + RemoteExecutor.Invoke(() => { - Assert.Equal("", ((CultureInfo)TypeDescriptor.GetConverter(typeof(System.Globalization.CultureInfo)).ConvertFrom(null, null, "(Default)")).Name); - } + using (new ThreadCultureChange(null, CultureInfo.InvariantCulture)) + { + Assert.Equal("", ((CultureInfo)TypeDescriptor.GetConverter(typeof(System.Globalization.CultureInfo)).ConvertFrom(null, null, "(Default)")).Name); + } + }, options).Dispose(); } private class SubCultureInfoConverter : CultureInfoConverter