Remove uses of SR.GetResourceString throughout the libs (#87912)

MichalStrehovsky · web-flow · commit a0e670a631c0 · 2023-06-23T11:51:08.000+09:00
Fixes #81338.
diff --git a/src/libraries/Common/src/System/SR.cs b/src/libraries/Common/src/System/SR.cs
@@ -15,7 +15,17 @@ internal static partial class SR
         // The trimming tools are also capable of replacing the value of this method when the application is being trimmed.
         internal static bool UsingResourceKeys() => s_usingResourceKeys;
 
-        internal static string GetResourceString(string resourceKey)
+        // We can optimize out the resource string blob if we can see all accesses to it happening
+        // through the generated SR.XXX properties.
+        // If a call to GetResourceString is left, the optimization gets defeated and we need to keep
+        // the whole resource blob. It's important to keep this private. CoreCLR's CoreLib gets a free
+        // pass because the VM needs to be able to call into this, but that's a known set of constants.
+#if CORECLR || LEGACY_GETRESOURCESTRING_USER
+        internal
+#else
+        private
+#endif
+        static string GetResourceString(string resourceKey)
         {
             if (UsingResourceKeys())
             {
@@ -37,7 +47,12 @@ internal static string GetResourceString(string resourceKey)
             return resourceString!; // only null if missing resources
         }
 
-        internal static string GetResourceString(string resourceKey, string defaultString)
+#if LEGACY_GETRESOURCESTRING_USER
+        internal
+#else
+        private
+#endif
+        static string GetResourceString(string resourceKey, string defaultString)
         {
             string resourceString = GetResourceString(resourceKey);
 
diff --git a/src/libraries/Microsoft.CSharp/src/Microsoft.CSharp.csproj b/src/libraries/Microsoft.CSharp/src/Microsoft.CSharp.csproj
@@ -9,6 +9,7 @@
     <EnableComBinder Condition="'$(TargetPlatformIdentifier)' == 'windows'">true</EnableComBinder>
     <DefineConstants Condition="'$(EnableComBinder)' == 'true'">$(DefineConstants);ENABLECOMBINDER</DefineConstants>
     <AllowUnsafeBlocks Condition="'$(EnableComBinder)' == 'true'">true</AllowUnsafeBlocks>
+    <DefineConstants>$(DefineConstants);LEGACY_GETRESOURCESTRING_USER</DefineConstants>
     <EnableAOTAnalyzer>false</EnableAOTAnalyzer>
   </PropertyGroup>
   <ItemGroup>
diff --git a/src/libraries/Microsoft.Extensions.Primitives/src/ThrowHelper.cs b/src/libraries/Microsoft.Extensions.Primitives/src/ThrowHelper.cs
@@ -58,7 +58,20 @@ internal static ArgumentException GetArgumentException(ExceptionResource resourc
 
         private static string GetResourceText(ExceptionResource resource)
         {
-            return SR.GetResourceString(GetResourceName(resource));
+            Debug.Assert(Enum.IsDefined(typeof(ExceptionResource), resource),
+                "The enum value is not defined, please check the ExceptionResource Enum.");
+
+            switch (resource)
+            {
+                case ExceptionResource.Argument_InvalidOffsetLength: return SR.Argument_InvalidOffsetLength;
+                case ExceptionResource.Argument_InvalidOffsetLengthStringSegment: return SR.Argument_InvalidOffsetLengthStringSegment;
+                case ExceptionResource.Capacity_CannotChangeAfterWriteStarted: return SR.Capacity_CannotChangeAfterWriteStarted;
+                case ExceptionResource.Capacity_NotEnough: return SR.Capacity_NotEnough;
+                case ExceptionResource.Capacity_NotUsedEntirely: return SR.Capacity_NotUsedEntirely;
+                default:
+                    Debug.Fail($"Unexpected resource {resource}");
+                    return "";
+            }
         }
 
         private static string GetArgumentName(ExceptionArgument argument)
@@ -68,14 +81,6 @@ private static string GetArgumentName(ExceptionArgument argument)
 
             return argument.ToString();
         }
-
-        private static string GetResourceName(ExceptionResource resource)
-        {
-            Debug.Assert(Enum.IsDefined(typeof(ExceptionResource), resource),
-                "The enum value is not defined, please check the ExceptionResource Enum.");
-
-            return resource.ToString();
-        }
     }
 
     internal enum ExceptionArgument
diff --git 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.GetResourceString(nameof(SR.Array), "{0} Array"), value.GetType().Name);
+                return SR.Format(SR.UsingResourceKeys() ? "{0} Array" : SR.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
@@ -20,7 +20,7 @@ public class CollectionConverter : TypeConverter
         {
             if (destinationType == typeof(string) && value is ICollection)
             {
-                return SR.GetResourceString(nameof(SR.Collection), "(Collection)");
+                return SR.UsingResourceKeys() ? "(Collection)" : SR.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
@@ -22,7 +22,7 @@ public class CultureInfoConverter : TypeConverter
         /// <summary>
         /// Retrieves the "default" name for our culture.
         /// </summary>
-        private static string DefaultCultureString => SR.GetResourceString(nameof(SR.CultureInfoConverterDefaultCultureString), "(Default)");
+        private static string DefaultCultureString => SR.UsingResourceKeys() ? "(Default)" : SR.CultureInfoConverterDefaultCultureString;
 
         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
@@ -461,7 +461,7 @@ public override PropertyDescriptorCollection GetProperties(ITypeDescriptorContex
             {
                 if (destinationType == typeof(string))
                 {
-                    return SR.GetResourceString(nameof(SR.CollectionConverterText), "(Collection)");
+                    return SR.UsingResourceKeys() ? "(Collection)" : SR.CollectionConverterText;
                 }
                 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
@@ -99,7 +99,7 @@ public override string DisplayName
                     string? providerName = site?.Name;
                     if (providerName != null && providerName.Length > 0)
                     {
-                        name = SR.Format(SR.GetResourceString(nameof(SR.MetaExtenderName), "{0} on {1}"), name, providerName);
+                        name = SR.Format(SR.UsingResourceKeys() ? "{0} on {1}" : SR.MetaExtenderName, 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
@@ -11,7 +11,7 @@ namespace System.ComponentModel
     /// </summary>
     public abstract class InstanceCreationEditor
     {
-        public virtual string Text => SR.GetResourceString(nameof(SR.InstanceCreationEditorDefaultText), "(New...)");
+        public virtual string Text => SR.UsingResourceKeys() ? "(New...)" : SR.InstanceCreationEditorDefaultText;
 
         /// <summary>
         /// 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
@@ -20,7 +20,7 @@ public class MultilineStringConverter : TypeConverter
 
             if (destinationType == typeof(string) && value is string)
             {
-                return SR.GetResourceString(nameof(SR.Text), "(Text)");
+                return SR.UsingResourceKeys() ? "(Text)" : SR.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
@@ -15,7 +15,7 @@ namespace System.ComponentModel
     /// </summary>
     public class ReferenceConverter : TypeConverter
     {
-        private static readonly string s_none = SR.GetResourceString(nameof(SR.toStringNone), "(none)");
+        private static readonly string s_none = SR.UsingResourceKeys() ? "(none)" : SR.toStringNone;
         private readonly Type _type;
 
         /// <summary>
diff --git 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
         /// </summary>
         protected Exception GetConvertFromException(object? value)
         {
-            string? valueTypeName = value == null ? SR.GetResourceString(nameof(SR.Null), "(null)") : value.GetType().FullName;
+            string? valueTypeName = value == null ? (SR.UsingResourceKeys() ? "(null)" : SR.Null) : value.GetType().FullName;
             throw new NotSupportedException(SR.Format(SR.ConvertFromException, GetType().Name, valueTypeName));
         }
 
@@ -201,7 +201,7 @@ protected Exception GetConvertFromException(object? value)
         /// </summary>
         protected Exception GetConvertToException(object? value, Type destinationType)
         {
-            string? valueTypeName = value == null ? SR.GetResourceString(nameof(SR.Null), "(null)") : value.GetType().FullName;
+            string? valueTypeName = value == null ? (SR.UsingResourceKeys() ? "(null)" : SR.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
@@ -73,7 +73,7 @@ public override bool CanConvertTo(ITypeDescriptorContext? context, [NotNullWhen(
             {
                 if (value == null)
                 {
-                    return SR.GetResourceString(nameof(SR.none), "(none)");
+                    return SR.UsingResourceKeys() ? "(none)" : SR.none;
                 }
                 else
                 {
diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/Timers/Timer.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/Timers/Timer.cs
@@ -76,7 +76,7 @@ public Timer(TimeSpan interval) : this(interval.TotalMilliseconds)
         /// Gets or sets a value indicating whether the Timer raises the Tick event each time the specified
         /// Interval has elapsed, when Enabled is set to true.
         /// </summary>
-        [TimersDescription(nameof(SR.TimerAutoReset), null), DefaultValue(true)]
+        [TimersDescription(TimersDescriptionStringId.TimerAutoReset), DefaultValue(true)]
         public bool AutoReset
         {
             get => _autoReset;
@@ -102,7 +102,7 @@ public bool AutoReset
         /// is able to raise events at a defined interval.
         /// The default value by design is false, don't change it.
         /// </summary>
-        [TimersDescription(nameof(SR.TimerEnabled), null), DefaultValue(false)]
+        [TimersDescription(TimersDescriptionStringId.TimerEnabled), DefaultValue(false)]
         public bool Enabled
         {
             get => _enabled;
@@ -160,7 +160,7 @@ private void UpdateTimer()
         /// <summary>
         /// Gets or sets the interval on which to raise events.
         /// </summary>
-        [TimersDescription(nameof(SR.TimerInterval), null), DefaultValue(100d)]
+        [TimersDescription(TimersDescriptionStringId.TimerInterval), DefaultValue(100d)]
         public double Interval
         {
             get => _interval;
@@ -184,7 +184,7 @@ public double Interval
         /// Occurs when the <see cref='System.Timers.Timer.Interval'/> has
         /// elapsed.
         /// </summary>
-        [TimersDescription(nameof(SR.TimerIntervalElapsed), null)]
+        [TimersDescription(TimersDescriptionStringId.TimerIntervalElapsed)]
         public event ElapsedEventHandler Elapsed
         {
             add => _onIntervalElapsed += value;
@@ -212,7 +212,7 @@ public override ISite? Site
         /// Gets or sets the object used to marshal event-handler calls that are issued when
         /// an interval has elapsed.
         /// </summary>
-        [DefaultValue(null), TimersDescription(nameof(SR.TimerSynchronizingObject), null)]
+        [DefaultValue(null), TimersDescription(TimersDescriptionStringId.TimerSynchronizingObject)]
         public ISynchronizeInvoke? SynchronizingObject
         {
             get
diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/Timers/TimersDescriptionAttribute.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/Timers/TimersDescriptionAttribute.cs
@@ -21,13 +21,21 @@ public class TimersDescriptionAttribute : DescriptionAttribute
         /// </summary>
         public TimersDescriptionAttribute(string description) : base(description) { }
 
-        /// <summary>
-        /// Constructs a new localized sys description.
-        /// </summary>
-        internal TimersDescriptionAttribute(string description, string? unused) : base(SR.GetResourceString(description))
+        internal TimersDescriptionAttribute(TimersDescriptionStringId id) : base(GetResourceString(id)) { }
+
+        private static string GetResourceString(TimersDescriptionStringId id)
         {
-            // Needed for overload resolution
-            Debug.Assert(unused == null);
+            switch (id)
+            {
+                case TimersDescriptionStringId.TimerAutoReset: return SR.TimerAutoReset;
+                case TimersDescriptionStringId.TimerEnabled: return SR.TimerEnabled;
+                case TimersDescriptionStringId.TimerInterval: return SR.TimerInterval;
+                case TimersDescriptionStringId.TimerIntervalElapsed: return SR.TimerIntervalElapsed;
+                case TimersDescriptionStringId.TimerSynchronizingObject: return SR.TimerSynchronizingObject;
+                default:
+                    Debug.Fail($"Unexpected resource {id}");
+                    return "";
+            }
         }
 
         /// <summary>
@@ -49,4 +57,13 @@ public override string Description
             }
         }
     }
+
+    internal enum TimersDescriptionStringId
+    {
+        TimerAutoReset,
+        TimerEnabled,
+        TimerInterval,
+        TimerIntervalElapsed,
+        TimerSynchronizingObject
+    }
 }
diff --git a/src/libraries/System.Configuration.ConfigurationManager/tests/System.Configuration.ConfigurationManager.Tests.csproj b/src/libraries/System.Configuration.ConfigurationManager/tests/System.Configuration.ConfigurationManager.Tests.csproj
@@ -3,6 +3,7 @@
     <IncludeRemoteExecutor>true</IncludeRemoteExecutor>
     <TargetFrameworks>$(NetCoreAppCurrent);$(NetFrameworkMinimum)</TargetFrameworks>
     <IgnoreForCI Condition="'$(TargetOS)' == 'browser'">true</IgnoreForCI>
+    <DefineConstants>$(DefineConstants);LEGACY_GETRESOURCESTRING_USER</DefineConstants>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="$(CommonTestPath)System\IO\TempDirectory.cs" Link="Common\System\IO\TempDirectory.cs" />
diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs
@@ -197,10 +197,10 @@ internal void ToString(TraceFormat traceFormat, StringBuilder sb)
         {
             // Passing a default string for "at" in case SR.UsingResourceKeys() is true
             // as this is a special case and we don't want to have "Word_At" on stack traces.
-            string word_At = SR.GetResourceString(nameof(SR.Word_At), defaultString: "at");
+            string word_At = SR.UsingResourceKeys() ? "at" : SR.Word_At;
             // We also want to pass in a default for inFileLineNumber.
-            string inFileLineNum = SR.GetResourceString(nameof(SR.StackTrace_InFileLineNumber), defaultString: "in {0}:line {1}");
-            string inFileILOffset = SR.GetResourceString(nameof(SR.StackTrace_InFileILOffset), defaultString: "in {0}:token 0x{1:x}+0x{2:x}");
+            string inFileLineNum = SR.UsingResourceKeys() ? "in {0}:line {1}" : SR.StackTrace_InFileLineNumber;
+            string inFileILOffset = SR.UsingResourceKeys() ? "in {0}:token 0x{1:x}+0x{2:x}" : SR.StackTrace_InFileILOffset;
             bool fFirstFrame = true;
             for (int iFrameIndex = 0; iFrameIndex < _numOfFrames; iFrameIndex++)
             {
@@ -340,8 +340,7 @@ internal void ToString(TraceFormat traceFormat, StringBuilder sb)
                     {
                         sb.AppendLine();
                         // Passing default for Exception_EndStackTraceFromPreviousThrow in case SR.UsingResourceKeys is set.
-                        sb.Append(SR.GetResourceString(nameof(SR.Exception_EndStackTraceFromPreviousThrow),
-                            defaultString: "--- End of stack trace from previous location ---"));
+                        sb.Append(SR.UsingResourceKeys() ? "--- End of stack trace from previous location ---" : SR.Exception_EndStackTraceFromPreviousThrow);
                     }
                 }
             }
diff --git a/src/libraries/System.Text.Encoding.CodePages/src/System/Text/BaseCodePageEncoding.netcoreapp.cs b/src/libraries/System.Text.Encoding.CodePages/src/System/Text/BaseCodePageEncoding.netcoreapp.cs
@@ -66,12 +66,10 @@ internal static unsafe EncodingInfo [] GetEncodings(CodePagesEncodingProvider pr
                         default:    codePageName = new string(&codePageIndex.CodePageName); break;
                     }
 
-                    string? resourceName = EncodingNLS.GetLocalizedEncodingNameResource(codePageIndex.CodePage);
                     string? displayName = null;
-
-                    if (resourceName != null && resourceName.StartsWith("Globalization_cp_", StringComparison.OrdinalIgnoreCase))
+                    if (!SR.UsingResourceKeys())
                     {
-                        displayName = SR.GetResourceString(resourceName);
+                        displayName = EncodingNLS.GetLocalizedEncodingNameResource(codePageIndex.CodePage);
                     }
 
                     encodingInfoList[i] = new EncodingInfo(provider, codePageIndex.CodePage, codePageName, displayName ?? codePageName);
diff --git a/src/libraries/System.Transactions.Local/src/System/Transactions/Oletx/OletxTransaction.cs b/src/libraries/System.Transactions.Local/src/System/Transactions/Oletx/OletxTransaction.cs
@@ -552,7 +552,7 @@ internal Guid Identifier
                 // for COM+ interop purposes, but we can't get the guid or the status of the transaction.
                 if (TxGuid.Equals(Guid.Empty))
                 {
-                    throw TransactionException.Create(SR.GetResourceString(SR.CannotGetTransactionIdentifier), null);
+                    throw TransactionException.Create(SR.CannotGetTransactionIdentifier, null);
                 }
 
                 return TxGuid;

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ public class ArrayConverter : CollectionConverter`
`19`	`19`	`{`
`20`	`20`	`if (destinationType == typeof(string) && value is Array)`
`21`	`21`	`{`
`22`		`- return SR.Format(SR.GetResourceString(nameof(SR.Array), "{0} Array"), value.GetType().Name);`
	`22`	`+ return SR.Format(SR.UsingResourceKeys() ? "{0} Array" : SR.Array, value.GetType().Name);`
`23`	`23`	`}`
`24`	`24`
`25`	`25`	`return base.ConvertTo(context, culture, value, destinationType);`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ public class CollectionConverter : TypeConverter`
`20`	`20`	`{`
`21`	`21`	`if (destinationType == typeof(string) && value is ICollection)`
`22`	`22`	`{`
`23`		`- return SR.GetResourceString(nameof(SR.Collection), "(Collection)");`
	`23`	`+ return SR.UsingResourceKeys() ? "(Collection)" : SR.Collection;`
`24`	`24`	`}`
`25`	`25`
`26`	`26`	`return base.ConvertTo(context, culture, value, destinationType);`
Original file line number	Diff line number	Diff line change
`@@ -461,7 +461,7 @@ public override PropertyDescriptorCollection GetProperties(ITypeDescriptorContex`
`461`	`461`	`{`
`462`	`462`	`if (destinationType == typeof(string))`
`463`	`463`	`{`
`464`		`- return SR.GetResourceString(nameof(SR.CollectionConverterText), "(Collection)");`
	`464`	`+ return SR.UsingResourceKeys() ? "(Collection)" : SR.CollectionConverterText;`
`465`	`465`	`}`
`466`	`466`	`return base.ConvertTo(cxt, culture, value, destinationType);`
`467`	`467`	`}`
Original file line number	Diff line number	Diff line change
`@@ -99,7 +99,7 @@ public override string DisplayName`
`99`	`99`	`string? providerName = site?.Name;`
`100`	`100`	`if (providerName != null && providerName.Length > 0)`
`101`	`101`	`{`
`102`		`- name = SR.Format(SR.GetResourceString(nameof(SR.MetaExtenderName), "{0} on {1}"), name, providerName);`
	`102`	`+ name = SR.Format(SR.UsingResourceKeys() ? "{0} on {1}" : SR.MetaExtenderName, name, providerName);`
`103`	`103`	`}`
`104`	`104`	`}`
`105`	`105`	`return name;`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ namespace System.ComponentModel`
`11`	`11`	`/// </summary>`
`12`	`12`	`public abstract class InstanceCreationEditor`
`13`	`13`	`{`
`14`		`- public virtual string Text => SR.GetResourceString(nameof(SR.InstanceCreationEditorDefaultText), "(New...)");`
	`14`	`+ public virtual string Text => SR.UsingResourceKeys() ? "(New...)" : SR.InstanceCreationEditorDefaultText;`
`15`	`15`
`16`	`16`	`/// <summary>`
`17`	`17`	`/// This method is invoked when you user chooses the link displayed by the PropertyGrid for the InstanceCreationEditor.`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ public class MultilineStringConverter : TypeConverter`
`20`	`20`
`21`	`21`	`if (destinationType == typeof(string) && value is string)`
`22`	`22`	`{`
`23`		`- return SR.GetResourceString(nameof(SR.Text), "(Text)");`
	`23`	`+ return SR.UsingResourceKeys() ? "(Text)" : SR.Text;`
`24`	`24`	`}`
`25`	`25`
`26`	`26`	`return base.ConvertTo(context, culture, value, destinationType);`