WriteCodeFragment fails in .NET Core MSBuild if input has braces

[nguerrera]

Repro steps

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp3.0</TargetFramework>
    <Description>There's a bug {here}</Description>
  </PropertyGroup>
</Project>

dotnet build

Expected

Builds successfully

Actual

error MSB4018: The "WriteCodeFragment" task failed unexpectedly.
error MSB4018: System.FormatException: Input string was not in a correct format.
error MSB4018:    at System.Text.StringBuilder.FormatError()
error MSB4018:    at System.Text.StringBuilder.AppendFormatHelper(IFormatProvider provider, String format, ParamsArray args)
error MSB4018:    at System.String.FormatHelper(IFormatProvider provider, String format, ParamsArray args)
error MSB4018:    at System.String.Format(String format, Object[] args)
error MSB4018:    at Microsoft.Build.Tasks.WriteCodeFragment.GenerateCodeCoreClr(String& extension)
error MSB4018:    at Microsoft.Build.Tasks.WriteCodeFragment.Execute()

The best fix for this would be to remove the CODEDOM ifdefs and use the same code as .NET Framework in .NET Core msbuild. A regression test should also be added.

Original bug description, focusing more on code inspection than scenario

By code inspection, it appears that the inputs to WriteCodeFragment inputs aren't escaped in the xplat (non-CodeDOM) implementation.

See https://github.com/nguerrera/msbuild/blob/71bf8112fbb1ec2115631c9d2e8db3412004ff5e/src/XMakeTasks/WriteCodeFragment.cs#L360

I believe this will fail if _ParameterN has text that needs to be escaped. (e.g. imagine if there are embedded quotes in the value.) EDIT: Quotes and other things were fixed in #1369, but braces are still a problem as they throw off String.Format.

I also don't understand the non-string side of that ?: expression because the CodeDOM equivalent casts unconditionally to string and why do we expect non-strings to be quoted in their ToString() output?

Spotted while investigating #935

[rainersigwald]

At this point I think we should remove the non-CodeCOM implementation; we should be able to use CodeDOM everywhere now, right?

[nguerrera]

Yes, do we already use the CodeDom implementation everywhere? Should this track removing the dead code or do we have something else tracking that?

[rainersigwald]

Ah, fat-fingered the button. Yeah, we should track removing the ifdef here.

[rainersigwald]

This turns out to not be trivial:

WriteCodeFragment.cs(142,41): error CS0234: The type or namespace name 'ConfigurationException' does not exist in the namespace 'System.Configuration' (are you missing an assembly reference?) [S:\msbuild\src\Tasks\Microsoft.Build.Tasks.csproj]
GenerateResource.cs(3471,35): error CS0103: The name 'StronglyTypedResourceBuilder' does not exist in the current context [S:\msbuild\src\Tasks\Microsoft.Build.Tasks.csproj]

For the latter, I think we might need to resurrect #3819.

I think we should do it. There's joy and security in deleting code. Penciling in for 16.4.