Fix invalid CodeDom comment generation when a new line starts with a slash (#56640)

jnm2 · danmoseley · web-flow · commit 7a799ce4d21b · 2021-08-08T21:19:06.000-06:00
* Failing tests where CodeDom creates invalid comment types

* Prevent CodeDom from creating invalid comment types

* Make C# generator impl more consistent with VB generator

* Switch attribute from ActiveIssue to SkipOnTargetFramework

* Be even more conservative about when the space is needed

* Attempt to improve readability

* Update VBCodeGenerationTests.cs

Co-authored-by: Dan Moseley &lt;danmose@microsoft.com&gt;
diff --git a/src/libraries/System.CodeDom/src/Microsoft/CSharp/CSharpCodeGenerator.cs b/src/libraries/System.CodeDom/src/Microsoft/CSharp/CSharpCodeGenerator.cs
@@ -851,10 +851,20 @@ private void GenerateComment(CodeComment e)
             string commentLineStart = e.DocComment ? "///" : "//";
             Output.Write(commentLineStart);
             Output.Write(' ');
+            bool isAfterCommentLineStart = false;
 
             string value = e.Text;
             for (int i = 0; i < value.Length; i++)
             {
+                if (isAfterCommentLineStart)
+                {
+                    if (value[i] == '/' && (e.DocComment || !value.HasCharAt(i + 1, '/')))
+                    {
+                        Output.Write(' ');
+                    }
+                    isAfterCommentLineStart = false;
+                }
+
                 if (value[i] == '\u0000')
                 {
                     continue;
@@ -863,22 +873,25 @@ private void GenerateComment(CodeComment e)
 
                 if (value[i] == '\r')
                 {
-                    if (i < value.Length - 1 && value[i + 1] == '\n')
+                    if (value.HasCharAt(i + 1, '\n'))
                     { // if next char is '\n', skip it
                         Output.Write('\n');
                         i++;
                     }
                     _output.InternalOutputTabs();
                     Output.Write(commentLineStart);
+                    isAfterCommentLineStart = true;
                 }
                 else if (value[i] == '\n')
                 {
                     _output.InternalOutputTabs();
                     Output.Write(commentLineStart);
+                    isAfterCommentLineStart = true;
                 }
                 else if (value[i] == '\u2028' || value[i] == '\u2029' || value[i] == '\u0085')
                 {
                     Output.Write(commentLineStart);
+                    isAfterCommentLineStart = true;
                 }
             }
             Output.WriteLine();
diff --git a/src/libraries/System.CodeDom/src/Microsoft/VisualBasic/VBCodeGenerator.cs b/src/libraries/System.CodeDom/src/Microsoft/VisualBasic/VBCodeGenerator.cs
@@ -1224,29 +1224,43 @@ protected override void GenerateComment(CodeComment e)
         {
             string commentLineStart = e.DocComment ? "'''" : "'";
             Output.Write(commentLineStart);
+            bool isAfterCommentLineStart = true;
+
             string value = e.Text;
             for (int i = 0; i < value.Length; i++)
             {
+                if (isAfterCommentLineStart)
+                {
+                    if (value[i] == '\'' && (e.DocComment || (value.HasCharAt(i + 1, '\'') && !value.HasCharAt(i + 2, '\''))))
+                    {
+                        Output.Write(' ');
+                    }
+                    isAfterCommentLineStart = false;
+                }
+
                 Output.Write(value[i]);
 
                 if (value[i] == '\r')
                 {
-                    if (i < value.Length - 1 && value[i + 1] == '\n')
+                    if (value.HasCharAt(i + 1, '\n'))
                     { // if next char is '\n', skip it
                         Output.Write('\n');
                         i++;
                     }
                     ((ExposedTabStringIndentedTextWriter)Output).InternalOutputTabs();
                     Output.Write(commentLineStart);
+                    isAfterCommentLineStart = true;
                 }
                 else if (value[i] == '\n')
                 {
                     ((ExposedTabStringIndentedTextWriter)Output).InternalOutputTabs();
                     Output.Write(commentLineStart);
+                    isAfterCommentLineStart = true;
                 }
                 else if (value[i] == '\u2028' || value[i] == '\u2029' || value[i] == '\u0085')
                 {
                     Output.Write(commentLineStart);
+                    isAfterCommentLineStart = true;
                 }
             }
             Output.WriteLine();
diff --git a/src/libraries/System.CodeDom/src/StringExtensions.cs b/src/libraries/System.CodeDom/src/StringExtensions.cs
@@ -0,0 +1,13 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace System.CodeDom
+{
+    internal static class StringExtensions
+    {
+        internal static bool HasCharAt(this string value, int index, char character)
+        {
+            return index < value.Length && value[index] == character;
+        }
+    }
+}
diff --git a/src/libraries/System.CodeDom/src/System.CodeDom.csproj b/src/libraries/System.CodeDom/src/System.CodeDom.csproj
@@ -136,6 +136,7 @@ Microsoft.VisualBasic.VBCodeProvider</PackageDescription>
     <Compile Include="System\CodeDom\MemberAttributes.cs" />
     <Compile Include="System\Collections\Specialized\FixedStringLookup.cs" />
     <Compile Include="$(CommonPath)System\CSharpHelpers.cs" />
+    <Compile Include="StringExtensions.cs" />
   </ItemGroup>
   <ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
     <Reference Include="System.Collections" />
diff --git a/src/libraries/System.CodeDom/tests/System/CodeDom/Compiler/CSharpCodeGenerationTests.cs b/src/libraries/System.CodeDom/tests/System/CodeDom/Compiler/CSharpCodeGenerationTests.cs
@@ -3516,5 +3516,61 @@ private void Test<T1, T2, T3, T4>()
                   }
                 ");
         }
+
+        [Fact]
+        [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "The bug was present on .NET Framework: https://github.com/dotnet/runtime/issues/56267")]
+        public void OrdinaryCommentsDoNotAccidentallyBecomeDocumentationComments()
+        {
+            var codeTypeDeclaration = new CodeTypeDeclaration("ClassWithCommment")
+            {
+                IsClass = true,
+                Comments =
+                {
+                    new CodeCommentStatement(
+                        "/ Lines starting with exactly one slash" + Environment.NewLine +
+                        "/ each get a separating space," + Environment.NewLine +
+                        "but other lines do not get a space. This way generated files only change on tool upgrade where there were generation bugs."+ Environment.NewLine +
+                        "// This includes lines starting with more than one slash.",
+                        docComment: false),
+                },
+            };
+
+            AssertEqualPreserveLineBreaks(codeTypeDeclaration,
+                @"
+                  // / Lines starting with exactly one slash
+                  // / each get a separating space,
+                  //but other lines do not get a space. This way generated files only change on tool upgrade where there were generation bugs.
+                  //// This includes lines starting with more than one slash.
+                  public class ClassWithCommment {
+                  }
+                ");
+        }
+
+        [Fact]
+        [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "The bug was present on .NET Framework: https://github.com/dotnet/runtime/issues/56267")]
+        public void DocumentationCommentsDoNotAccidentallyBecomeOrdinaryComments()
+        {
+            var codeTypeDeclaration = new CodeTypeDeclaration("ClassWithCommment")
+            {
+                IsClass = true,
+                Comments =
+                {
+                    new CodeCommentStatement(
+                        "/ Lines starting with a slash each get a separating space," + Environment.NewLine +
+                        "// including lines starting with more than one slash," + Environment.NewLine +
+                        "but other lines do not get a space. This way generated files only change on tool upgrade where there were generation bugs.",
+                        docComment: true),
+                },
+            };
+
+            AssertEqualPreserveLineBreaks(codeTypeDeclaration,
+                @"
+                  /// / Lines starting with a slash each get a separating space,
+                  /// // including lines starting with more than one slash,
+                  ///but other lines do not get a space. This way generated files only change on tool upgrade where there were generation bugs.
+                  public class ClassWithCommment {
+                  }
+                ");
+        }
     }
 }
diff --git a/src/libraries/System.CodeDom/tests/System/CodeDom/Compiler/VBCodeGenerationTests.cs b/src/libraries/System.CodeDom/tests/System/CodeDom/Compiler/VBCodeGenerationTests.cs
@@ -3264,5 +3264,63 @@ End Sub
                       End Class
                   End Namespace");
         }
+
+        [Fact]
+        [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "The bug was present on .NET Framework: https://github.com/dotnet/runtime/issues/56267")]
+        public void OrdinaryCommentsDoNotAccidentallyBecomeDocumentationComments()
+        {
+            var codeTypeDeclaration = new CodeTypeDeclaration("ClassWithCommment")
+            {
+                IsClass = true,
+                Comments =
+                {
+                    new CodeCommentStatement(
+                        "'' Lines starting with exactly two single quotes" + Environment.NewLine +
+                        "'' each get a separating space," + Environment.NewLine +
+                        "but other lines do not get a space. This way generated files only change on tool upgrade where there were generation bugs." + Environment.NewLine +
+                        "' Not even lines starting with only one single quote" + Environment.NewLine +
+                        "''' or three single quotes.",
+                        docComment: false),
+                },
+            };
+
+            AssertEqualPreserveLineBreaks(codeTypeDeclaration,
+                @"
+                  ' '' Lines starting with exactly two single quotes
+                  ' '' each get a separating space,
+                  'but other lines do not get a space. This way generated files only change on tool upgrade where there were generation bugs.
+                  '' Not even lines starting with only one single quote
+                  '''' or three single quotes.
+                  Public Class ClassWithCommment
+                  End Class
+                ");
+        }
+
+        [Fact]
+        [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "The bug was present on .NET Framework: https://github.com/dotnet/runtime/issues/56267")]
+        public void DocumentationCommentsDoNotAccidentallyBecomeOrdinaryComments()
+        {
+            var codeTypeDeclaration = new CodeTypeDeclaration("ClassWithCommment")
+            {
+                IsClass = true,
+                Comments =
+                {
+                    new CodeCommentStatement(
+                        "' Lines starting with a single quote" + Environment.NewLine +
+                        "'' or more than one quote, each get a separating space," + Environment.NewLine +
+                        "but other lines do not get a space. This way generated files only change on tool upgrade where there were generation bugs.",
+                        docComment: true),
+                },
+            };
+
+            AssertEqualPreserveLineBreaks(codeTypeDeclaration,
+                @"
+                  ''' ' Lines starting with a single quote
+                  ''' '' or more than one quote, each get a separating space,
+                  '''but other lines do not get a space. This way generated files only change on tool upgrade where there were generation bugs.
+                  Public Class ClassWithCommment
+                  End Class
+                ");
+        }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -851,10 +851,20 @@ private void GenerateComment(CodeComment e)`
`851`	`851`	`string commentLineStart = e.DocComment ? "///" : "//";`
`852`	`852`	`Output.Write(commentLineStart);`
`853`	`853`	`Output.Write(' ');`
	`854`	`+ bool isAfterCommentLineStart = false;`
`854`	`855`
`855`	`856`	`string value = e.Text;`
`856`	`857`	`for (int i = 0; i < value.Length; i++)`
`857`	`858`	`{`
	`859`	`+ if (isAfterCommentLineStart)`
	`860`	`+ {`
	`861`	`+ if (value[i] == '/' && (e.DocComment \|\| !value.HasCharAt(i + 1, '/')))`
	`862`	`+ {`
	`863`	`+ Output.Write(' ');`
	`864`	`+ }`
	`865`	`+ isAfterCommentLineStart = false;`
	`866`	`+ }`
	`867`	`+`
`858`	`868`	`if (value[i] == '\u0000')`
`859`	`869`	`{`
`860`	`870`	`continue;`
`@@ -863,22 +873,25 @@ private void GenerateComment(CodeComment e)`
`863`	`873`
`864`	`874`	`if (value[i] == '\r')`
`865`	`875`	`{`
`866`		`- if (i < value.Length - 1 && value[i + 1] == '\n')`
	`876`	`+ if (value.HasCharAt(i + 1, '\n'))`
`867`	`877`	`{ // if next char is '\n', skip it`
`868`	`878`	`Output.Write('\n');`
`869`	`879`	`i++;`
`870`	`880`	`}`
`871`	`881`	`_output.InternalOutputTabs();`
`872`	`882`	`Output.Write(commentLineStart);`
	`883`	`+ isAfterCommentLineStart = true;`
`873`	`884`	`}`
`874`	`885`	`else if (value[i] == '\n')`
`875`	`886`	`{`
`876`	`887`	`_output.InternalOutputTabs();`
`877`	`888`	`Output.Write(commentLineStart);`
	`889`	`+ isAfterCommentLineStart = true;`
`878`	`890`	`}`
`879`	`891`	`else if (value[i] == '\u2028' \|\| value[i] == '\u2029' \|\| value[i] == '\u0085')`
`880`	`892`	`{`
`881`	`893`	`Output.Write(commentLineStart);`
	`894`	`+ isAfterCommentLineStart = true;`
`882`	`895`	`}`
`883`	`896`	`}`
`884`	`897`	`Output.WriteLine();`
Original file line number	Diff line number	Diff line change
`@@ -1224,29 +1224,43 @@ protected override void GenerateComment(CodeComment e)`
`1224`	`1224`	`{`
`1225`	`1225`	`string commentLineStart = e.DocComment ? "'''" : "'";`
`1226`	`1226`	`Output.Write(commentLineStart);`
	`1227`	`+ bool isAfterCommentLineStart = true;`
	`1228`	`+`
`1227`	`1229`	`string value = e.Text;`
`1228`	`1230`	`for (int i = 0; i < value.Length; i++)`
`1229`	`1231`	`{`
	`1232`	`+ if (isAfterCommentLineStart)`
	`1233`	`+ {`
	`1234`	`+ if (value[i] == '\'' && (e.DocComment \|\| (value.HasCharAt(i + 1, '\'') && !value.HasCharAt(i + 2, '\''))))`
	`1235`	`+ {`
	`1236`	`+ Output.Write(' ');`
	`1237`	`+ }`
	`1238`	`+ isAfterCommentLineStart = false;`
	`1239`	`+ }`
	`1240`	`+`
`1230`	`1241`	`Output.Write(value[i]);`
`1231`	`1242`
`1232`	`1243`	`if (value[i] == '\r')`
`1233`	`1244`	`{`
`1234`		`- if (i < value.Length - 1 && value[i + 1] == '\n')`
	`1245`	`+ if (value.HasCharAt(i + 1, '\n'))`
`1235`	`1246`	`{ // if next char is '\n', skip it`
`1236`	`1247`	`Output.Write('\n');`
`1237`	`1248`	`i++;`
`1238`	`1249`	`}`
`1239`	`1250`	`((ExposedTabStringIndentedTextWriter)Output).InternalOutputTabs();`
`1240`	`1251`	`Output.Write(commentLineStart);`
	`1252`	`+ isAfterCommentLineStart = true;`
`1241`	`1253`	`}`
`1242`	`1254`	`else if (value[i] == '\n')`
`1243`	`1255`	`{`
`1244`	`1256`	`((ExposedTabStringIndentedTextWriter)Output).InternalOutputTabs();`
`1245`	`1257`	`Output.Write(commentLineStart);`
	`1258`	`+ isAfterCommentLineStart = true;`
`1246`	`1259`	`}`
`1247`	`1260`	`else if (value[i] == '\u2028' \|\| value[i] == '\u2029' \|\| value[i] == '\u0085')`
`1248`	`1261`	`{`
`1249`	`1262`	`Output.Write(commentLineStart);`
	`1263`	`+ isAfterCommentLineStart = true;`
`1250`	`1264`	`}`
`1251`	`1265`	`}`
`1252`	`1266`	`Output.WriteLine();`