CodeDom creates invalid C# line starting with `////` when a slash follows a newline in CodeComment.Text, causing warnings in Resources.Designer.cs files

[jnm2]

Due to a bug in CodeDom's generation of C# code comments in both .NET Framework and .NET Core/5, any project with <GenerateDocumentationFile>true</GenerateDocumentationFile> that embeds a file that contains a line that starts with / gets invalid C# documentation syntax in .Designer.cs, resulting in warnings:

I asked first at dotnet/roslyn#53729 in case the compiler team thought that documentation warnings should be skipped in generated code, and they decided that the current behavior is by design.

Then I filed an issue at dotnet/msbuild#6677 since the StronglyTypedResourceBuilder which creates Resources.Designer.cs lives there. The same CodeDom problem afflicts the StronglyTypedResourceBuilder in .NET Framework's Microsoft.Build.Tasks.v4.0.dll assembly, file version 4.8.4084.0.

➡ The MSBuild team would like to know where this issue ranks for you.

Details

This is where CodeDom creates the invalid //// XML doc comment line: https://github.com/dotnet/runtime/blob/v5.0.6/src/libraries/System.CodeDom/src/Microsoft/CSharp/CSharpCodeGenerator.cs#L879
.NET Framework's CodeDom has the same bug: https://referencesource.microsoft.com/#System/compmod/microsoft/csharp/csharpcodeprovider.cs,e0b125d92a26ca23

On that line it outputs /// because of string commentLineStart = e.DocComment ? "///" : "//"; outside that loop. Then the next character of value that will be written after that is a single /. This causes invalid C# any time a forward slash follows a newline in CodeComment.Text.

Steps to Reproduce

Full repro: Repro.zip

Resource file being included (real-life example was a SQL file that was truncated by the resource code generator when included in the XML comment):

/

Problematic excerpt from Resources.Designer.cs:

/// <summary>
///   Looks up a localized string similar to 
////.
/// </summary>
internal static string SomeResourceFile {
    get {
        return ResourceManager.GetString("SomeResourceFile", resourceCulture);
    }
}

Project file:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net5.0</TargetFramework>
    <GenerateDocumentationFile>true</GenerateDocumentationFile>
  </PropertyGroup>

  <ItemGroup>
    <Compile Update="Properties\Resources.Designer.cs">
      <DesignTime>True</DesignTime>
      <AutoGen>True</AutoGen>
      <DependentUpon>Resources.resx</DependentUpon>
    </Compile>
  </ItemGroup>

  <ItemGroup>
    <EmbeddedResource Update="Properties\Resources.resx">
      <Generator>ResXFileCodeGenerator</Generator>
      <LastGenOutput>Resources.Designer.cs</LastGenOutput>
    </EmbeddedResource>
  </ItemGroup>

</Project>

Specific errors:

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[danmoseley]

Any interest in offering a PR @jnm2 ?

Tagging subscribers to this area: @buyaa-n, @krwq
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Due to a bug in CodeDom's generation of C# code comments in both .NET Framework and .NET Core/5, any project with <GenerateDocumentationFile>true</GenerateDocumentationFile> that embeds a file that contains a line that starts with / gets invalid C# documentation syntax in .Designer.cs, resulting in warnings:

I asked first at dotnet/roslyn#53729 in case the compiler team thought that documentation warnings should be skipped in generated code, and they decided that the current behavior is by design.

Then I filed an issue at dotnet/msbuild#6677 since the StronglyTypedResourceBuilder which creates Resources.Designer.cs lives there. The same CodeDom problem afflicts the StronglyTypedResourceBuilder in .NET Framework's Microsoft.Build.Tasks.v4.0.dll assembly, file version 4.8.4084.0.

➡ The MSBuild team would like to know where this issue ranks for you.

Details

This is where CodeDom creates the invalid //// XML doc comment line: https://github.com/dotnet/runtime/blob/v5.0.6/src/libraries/System.CodeDom/src/Microsoft/CSharp/CSharpCodeGenerator.cs#L879
.NET Framework's CodeDom has the same bug: https://referencesource.microsoft.com/#System/compmod/microsoft/csharp/csharpcodeprovider.cs,e0b125d92a26ca23

On that line it outputs /// because of string commentLineStart = e.DocComment ? "///" : "//"; outside that loop. Then the next character of value that will be written after that is a single /. This causes invalid C# any time a forward slash follows a newline in CodeComment.Text.

Steps to Reproduce

Full repro: Repro.zip

Resource file being included (real-life example was a SQL file that was truncated by the resource code generator when included in the XML comment):

/

Problematic excerpt from Resources.Designer.cs:

/// <summary>
///   Looks up a localized string similar to 
////.
/// </summary>
internal static string SomeResourceFile {
    get {
        return ResourceManager.GetString("SomeResourceFile", resourceCulture);
    }
}

Project file:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net5.0</TargetFramework>
    <GenerateDocumentationFile>true</GenerateDocumentationFile>
  </PropertyGroup>

  <ItemGroup>
    <Compile Update="Properties\Resources.Designer.cs">
      <DesignTime>True</DesignTime>
      <AutoGen>True</AutoGen>
      <DependentUpon>Resources.resx</DependentUpon>
    </Compile>
  </ItemGroup>

  <ItemGroup>
    <EmbeddedResource Update="Properties\Resources.resx">
      <Generator>ResXFileCodeGenerator</Generator>
      <LastGenOutput>Resources.Designer.cs</LastGenOutput>
    </EmbeddedResource>
  </ItemGroup>

</Project>

Specific errors:

Author:	jnm2
Assignees:	-
Labels:	area-System.CodeDom, untriaged
Milestone:	-

[jnm2]

@danmoseley Sure, what would the fix be? One option is to unconditionally add a space after every ///. I like this option the best because it would look nicer between /// <summary> and /// </summary> etc, but it would cause a diff in all generated files when they get regenerated. Would this also be applied to normal comments? (They are generated in the same code path.)

Another option would be to special-case it so that ///x is still emitted when a line starts with x but /// / gets the extra space. That would have a downside of looking misaligned. Would this also be applied to normal comments?

I think a final option could be to special-case it so that //// is what gets emitted. I like this option the least.

[danmoseley]

This is a library we want to minimize changes in. And certainly don't want to affect existing generated code. So I would special case this on in the most minimal way - probably your second option and just for the broken case.

[jnm2]

There's a VB version of the bug:

''' <summary>
'''   Looks up a localized string similar to 
''''.
''' </summary>
Public Class C

There's also a danger that an ordinary comment in either language could accidentally turn into a doc comment causing warnings, e.g. with content /<a>/ (C#) or '<a>' (VB).

This maximally simplifies the fix, which will be to add a space if the next line of comment content starts with / (C#) or ' (VB), regardless of which kind of comment it is.

[danmoseley]

Seems reasonable to me.