SR.GetResourceString doesn't return resource key when UsingResourceKeys is true

[vitek-karas]

SR.UsingResourceKeys is a property meant to be set to true if the resources should not report full resource strings, but instead just the resource key itself. This is used for example in .NET Native but we now plan to potentially use it when trimming apps in .NET Core well. The advantage of doing this is that the tooling can remove the relatively large string resources and thus reduce the size of an app.

The intent of this feature was to use the resource key value instead of the resource string for an error message. So for example instead of: "Value cannot be null" it should return "ArgumentNull_Generic".

Unfortunately the code currently returns empty strings only. This is because the System.SR.GetResourceString(string) overload calls GetResourceString(resourceKey, string.Empty); but the implementation of that will only return the resourceKey if the second parameter is null - which in this case pretty much never is. Otherwise it returns he second parameter which is always null.

Tagging subscribers to this area: @tarekgh, @buyaa-n, @krwq
Notify danmosemsft if you want to be subscribed.

[vitek-karas]

/cc @eerhardt

[eerhardt]

I think this issue only applies to System.Private.CoreLib, right?

runtime/src/libraries/System.Private.CoreLib/src/System/SR.cs

Lines 21 to 25 in bd6cbe3

	// Needed for debugger integration
	internal static string GetResourceString(string resourceKey)
	{
	return GetResourceString(resourceKey, string.Empty);
	}

That method is only complied in for CoreLib, all the other libraries don't have it.

Is the fix here simply to change string.Empty to null? Normally I would think we could just remove the method altogether, but the comment Needed for debugger integration makes me hesitant to do that. I wonder what "debugger integration" is happening with this method?

[vitek-karas]

I'm hesitant about the change to null. The default value is used even when we're not in the trimmed scenario - in that case it's used as a fallback if the resource key can't be found. So it's probably better to leave it an empty string (so that the API never returns null) and instead detect empty string as the "special" value.

I don't know anything about the debugger comment.

/cc @MichalStrehovsky

[MichalStrehovsky]

It looks like the tooling to generate SR classes changed from how it used to be in buildtools.

CoreRT repo still builds with buildtools. The generated SR classes look like this:

    internal static partial class SR
    {

#if !DEBUGRESOURCES
        // SNIP

        internal static string Argument_OffsetOfFieldNotFound {
            get { return System.SR.GetResourceString("Argument_OffsetOfFieldNotFound", null); }
        }
#else
        // SNIP

        internal static string Argument_OffsetOfFieldNotFound {
            get { return System.SR.GetResourceString("Argument_OffsetOfFieldNotFound", @"Field passed in is not a marshaled member of the type '{0}'."); }
        }
#endif
    }

Whereas the arcade tooling generates:

    internal static partial class SR
    {
        /// <summary>Compile instantiations from reference modules used in the current module</summary>
        internal static string @BubbleGenericsOption => GetResourceString("BubbleGenericsOption");

I don't know about the debugger comment either.

[MichalStrehovsky]

Maybe @tarekgh would know about resources and #if !DEBUGRESOURCES?

[tarekgh]

Our build tools (when compiling a library in .NET Native) used to include all resource strings as literal strings inside the code and have DEBUGRESOURCES def to enable these strings. This is done in debug builds only. So if you use the debug build of the library the method GetResourceString(resourceKey, string) will pass the second parameter the resource literal string. This was done to help to get the full resource strings when using the debug builds. I don't think our build tools still producing this in debug builds in our runtime repo.

[tarekgh]

Looking at the whole things here, in .NET Native we used to override UsingResourceKeys value through the IL transformers and turn it true during the library compilation. I don't think we are doing any of that in runtime anyway. so all logic can be removed if the linker work is not going to use it. Please advise how you think the linker will work and we can decide what needs to be done here.

[vitek-karas]

@tarekgh The goal is for linker to do basically exactly what .NET Native did - we're going to overwrite the UsingResourceKeys to return true. That's how I found about the weird behavior - I made linker to overwrite the method and tried to run a simple app which printed out an error message for ArgumentNullException.

Before change it prints out:

System.ArgumentNullException: Value cannot be null. (Parameter 'args')

After change:

System.ArgumentNullException:  , args

The expected output based on the intent of the code should look like:

System.ArgumentNullException: ArgumentNull_Generic Arg_ParamName_Name, args

[tarekgh]

I looked at the difference between the corelib and the rest of the libraries, in corelib, we have GetString implemented as @vitek-karas mentioned

	internal static string GetResourceString(string resourceKey)
	{
		return GetResourceString(resourceKey, string.Empty);
	}

but in the rest of the libraries, we have the method defined with the default parameter

	internal static string GetResourceString(string resourceKey, string defaultString = null)
	{
		if (UsingResourceKeys())
		{
		}
		string text = null;
		try
		{
			text = ResourceManager.GetString(resourceKey);
		}
		catch (MissingManifestResourceException)
		{
		}
		if (defaultString != null && resourceKey.Equals(text))
		{
			return defaultString;
		}
		return text;
	}

runtime/src/libraries/Common/src/System/SR.cs

Line 22 in fc15e38

internal static string GetResourceString(string resourceKey, string? defaultString = null)

So it looks the issue is specific to corelib. @vitek-karas may confirm if seeing the same with any other library.

Another point, the code difference between the corelib and other libraries was originally introduced because corelib was not including the ResourceManager code in the first place (in .NET Native). but this is not the case anymore as corelib now carries the ResourceManager code. I believe this code can be unified now by removing the SR.cs file from corelib and let it link to the SR.cs which placed under the common source.

I don't recall anything about the debugger integration comment, but I'll try to search more to know if I can find anything that can help there.