Fix invalid CodeDom comment generation when a new line starts with a slash (#56640)
authorJoseph Musser <me@jnm2.com>
Mon, 9 Aug 2021 03:19:06 +0000 (23:19 -0400)
committerGitHub <noreply@github.com>
Mon, 9 Aug 2021 03:19:06 +0000 (21:19 -0600)
* 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 <danmose@microsoft.com>
src/libraries/System.CodeDom/src/Microsoft/CSharp/CSharpCodeGenerator.cs
src/libraries/System.CodeDom/src/Microsoft/VisualBasic/VBCodeGenerator.cs
src/libraries/System.CodeDom/src/StringExtensions.cs [new file with mode: 0644]
src/libraries/System.CodeDom/src/System.CodeDom.csproj
src/libraries/System.CodeDom/tests/System/CodeDom/Compiler/CSharpCodeGenerationTests.cs
src/libraries/System.CodeDom/tests/System/CodeDom/Compiler/VBCodeGenerationTests.cs

index 356529d..ef64456 100644 (file)
@@ -851,10 +851,20 @@ namespace Microsoft.CSharp
             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 @@ namespace Microsoft.CSharp
 
                 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();
index 5c55dfc..68b7ba7 100644 (file)
@@ -1224,29 +1224,43 @@ namespace Microsoft.VisualBasic
         {
             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
new file mode 100644 (file)
index 0000000..285d985
--- /dev/null
@@ -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;
+        }
+    }
+}
index 03c92a3..1c2713a 100644 (file)
@@ -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" />
index b727fca..fd84431 100644 (file)
@@ -3516,5 +3516,61 @@ namespace System.CodeDom.Compiler.Tests
                   }
                 ");
         }
+
+        [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 {
+                  }
+                ");
+        }
     }
 }
index 2abddb4..89c4b40 100644 (file)
@@ -3264,5 +3264,63 @@ namespace System.CodeDom.Compiler.Tests
                       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
+                ");
+        }
     }
 }