dotnet / sdk

Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI
https://dot.net/core
MIT License
2.71k stars 1.07k forks source link

Getting NULL when using ResourceManager.GetString() with strings using placeholders #10467

Closed danielValdezR closed 4 years ago

danielValdezR commented 5 years ago

I have a embedded Resource.resx file and a Resource.Designer.cs autogenerated containing many string resources, some of them have placeholders (i.e. "Hello, '{0}'") and these are the ones that are returning null when calling from code, i.e. Resource.KeyName. This Resource.resx file exist in a library project with TargetFrameworks net452, netstandard2.0 and netstandard2.1. I'm dealing with this issue when I run my Test Project (net452, netcoreapp3.0, netcoreapp2.2) and make use of that Resource.

This issue is only happening when the tests are ran using net452 (I have tried with net472 also and behaves the same) with resources containing placeholders and compiling/running tests from CLI (dotnet test), when using VS it's working fine. I had to add the following to my .csproj of the library because it was failing at compiling:

<GenerateResourceUsePreserializedResources>**true**</GenerateResourceUsePreserializedResources>

<PackageReference Include="System.Resources.Extensions">
      <Version>4.6.0-preview8.19405.3</Version>
</PackageReference>

SDK used: 3.0.100-preview8-013656

danmoseley commented 5 years ago

This issue is only happening when the tests are ran using net452

@ericstj thoughts about where this issue might lie? The above suggests it's in .NET Framework right?

tarekgh commented 5 years ago

@danmosemsft this looks to me the problem is with the CLI/msbuild. it looks VS compilation is succeeding because using full framework msbuild targets I guess.

danmoseley commented 5 years ago

OK, moving. CLI folks, thoughts? This seems CLI build specific.

ericstj commented 5 years ago

This is most likely related to changes in ResGen. /cc @rainersigwald

What's interesting here is GenerateResourceUsePreserializedResources + net452. System.Resources.Extensions only supports netstandard2.0, so the net452 compile would not be seeing System.Resources.Extensions.dll. I would expect the net452 compile of your library from the dotnet CLI to fail if S.R.E was actually required. If you target net461 or higher, does it still fail?

We would need more information to know for sure. @danielValdezR can you please attach an isolated repro project that demonstrates the behavior?

rainersigwald commented 5 years ago

Possibly a failure to compile on net452 was being masked by https://github.com/microsoft/msbuild/issues/4553.

I would also like to see a repro project.

tarekgh commented 5 years ago

having a repro project will help here as I am even not sure what calling Resource.KeyName means. I tried a small console project calling a library with a resources and I am not experiencing any problems reading the resources even when targeting net45 and using CLI.

danielValdezR commented 5 years ago

repro project: https://github.com/danielValdezR/ResourcesTest

Steps to reproduce: cd ..\ResourcesTest\ResourcesTest.Tests

dotnet clean
dotnet build -- MSB3822: Non-string resources require the System.Resources.Extensions assembly at runtime, but it was not found in this project's references.
dotnet build
dotnet test -f netcoreapp3.0 -- should PASS
dotnet test -f net452 -- should FAIL

Log: https://github.com/danielValdezR/ResourcesTest/blob/master/log.txt

rainersigwald commented 5 years ago

Thanks @danielValdezR! The fact that the second build is required is indicative of microsoft/msbuild#4553. I tried your repro on my machine with a preview9 build (3.0.100-preview9-014004) and all three TargetFrameworks pass tests. Another change means that System.Resources.Extensions is not required as often, so this specific repro project doesn't require it, though that won't be true of all .resx files.

I'm going to close this based on my results. If you see further bad behavior after updating to preview 9, please comment again and we can reopen and investigate.