CompilerVisibleProperty fails to escape special characters (# and ;)

[m0sa]

Version Used:
5.0.103

Steps to Reproduce:

1. In a csproj which consumes a configurable analyzer / generator, add

   <PropertyGroup>
     <Foo>Foo;Bar</Foo>
    <!-- I've also tried this, w/o success
      <Foo_>Foo;Bar</Foo>
      <Foo>$([MSBuild]::Escape('$(Foo_)'))</Foo>
     -->
   </PropertyGroup>
   <ItemGroup>
    <CompilerVisibleProperty Include="Foo" />
   </ItemGroup>

2. In the analyzer

   AnalyzerConfigOptions.GlobalOptions.TryGetValue("build_property.Foo", out string value)

Expected Behavior:
value should contain the whole value - "Foo;Bar"

Actual Behavior:
value contains only "Foo", ;Bar is escaped as an .INI comment

---

I've dug around a bit, and the issue seems to be in how the parameters are being passed to WriteAllLines here:

roslyn/src/Compilers/Core/MSBuildTask/Microsoft.Managed.Core.targets

Line 150 in 0aaa2eb

<WriteLinesToFile Lines="$(_GeneratedEditorConfigFileContent)" File="$(GeneratedMSBuildEditorConfigFile)" Overwrite="True" WriteOnlyWhenDifferent="True" />

It unescapes the _GeneratedEditorConfigFileContent value even if it's (manually) escaped:

Binlog:

Generated editorconfig:

Analyzer execution:

[sharwell]

This is effectively a duplicate of #51625, but is occurring under very different circumstances.

[m0sa]

@sharwell indeed, although I stopped looking at this point. Seems like in order for a full round-trip with special characters to work, there would need to be a well defined escaping spec (which seems to be missing now, the value is passed on and parsed as-is).

There are actually 2 bugs here:

1. the msbuild escaped value is unescaped before being written to the editorconfig. In my example above, I'm manually escaping a semicolon to %3b, but the the WriteLinesToFile unescapes it. The editorconfig should contain %3bs instead of a semicolons.
2. There is no escaping happening automatically in msbuild, and there's no unescaping happening when the values are parsed here:
   

   roslyn/src/Compilers/Core/Portable/CommandLine/AnalyzerConfig.cs

   Lines 219 to 232 in 1b6c125

   	var value = propMatches[0].Groups[2].Value;
   	Debug.Assert(!string.IsNullOrEmpty(key));
   	Debug.Assert(key == key.Trim());
   	Debug.Assert(value == value?.Trim());
   	key = CaseInsensitiveComparison.ToLower(key);
   	if (ReservedKeys.Contains(key) || ReservedValues.Contains(value))
   	{
   	value = CaseInsensitiveComparison.ToLower(value);
   	}
   	activeSectionProperties[key] = value ?? "";
   	continue;

   
   e.g. It could get base64 encoded in msbuild and decoded in AnalyzerConfig.cs to support a full roundtrip.

[jaredpar]

@mavasani, @chsienki can u all elaborate on this part?

Seems like in order for a full round-trip with special characters to work, there would need to be a well defined escaping spec (which seems to be missing now, the value is passed on and parsed as-is).

[m0sa]

Here's the workaround I have, so I can pass strings containing arbitrary characters from csproj to analyzers:

Analyzer:

var foo =
    context.AnalyzerConfigOptions.GlobalOptions.TryGetValue("build_property.Foo", out var foo64) 
    && !string.IsNullOrWhiteSpace(foo64)
        ? System.Text.Encoding.UTF8.GetString(System.Convert.FromBase64String(foo64))
        : null;

.csproj consuming (and configuring) the analyzer:

  <ItemGroup>
    <CompilerVisibleProperty Include="Foo" />
  </ItemGroup>
  <UsingTask TaskName="ToBase64" TaskFactory="RoslynCodeTaskFactory" AssemblyFile="$(MSBuildToolsPath)\Microsoft.Build.Tasks.Core.dll">
    <ParameterGroup>
      <In ParameterType="System.String" Required="true" />
      <Out ParameterType="System.String" Output="true" />
    </ParameterGroup>
    <Task>
      <Code Type="Fragment" Language="C#">Out = System.Convert.ToBase64String(System.Text.Encoding.UTF8.GetBytes(In));</Code>
    </Task>
  </UsingTask>
  <Target Name="RoslynIssue51692Workaround" BeforeTargets="BeforeCompile" Condition=" '$(Foo)' != '' ">
    <ToBase64 In="$(Foo)">
      <Output TaskParameter="Out" PropertyName="Foo" />
    </ToBase64>
  </Target>

[chsienki]

@m0sa Funnily enough base64 encoding was one of the options we considered for how to route everything through the various layers correctly.

@jaredpar The problem is that we take an msbuild value -> write that to .globalconfig -> read it into the compiler. There is an impedence mismatch between the way msbuild + .globalconfig handles escaping of values, meaning its possible to loose data. A simple example of this is multi-line values in msbuild. We'll lose everything after the initial line.

Closing this issue as we've seen no reply to the request for more information. If you are able to get the requested information, please add it to the issue and we will retriage it.