Merge pull request #8795 from DustinCampbell/fix-codewriter-bug

Fix issue in CodeWriter when counting line breaks
dotnet · Jun 5, 2023 · 5072657 · 5072657
2 parents 7d49e8a + a1556bc
commit 5072657
Show file tree

Hide file tree

Showing 13 changed files with 65 additions and 46 deletions.
diff --git a/src/Compiler/Microsoft.AspNetCore.Razor.Language/src/CodeGeneration/CodeWriter.cs b/src/Compiler/Microsoft.AspNetCore.Razor.Language/src/CodeGeneration/CodeWriter.cs
@@ -11,8 +11,6 @@ namespace Microsoft.AspNetCore.Razor.Language.CodeGeneration;
 
 public sealed class CodeWriter
 {
-    private static readonly char[] NewLineCharacters = { '\r', '\n' };
-
     private readonly StringBuilder _builder;
 
     private string _newLine;
@@ -60,7 +58,7 @@ public string NewLine
 
     public int TabSize { get; }
 
-    public SourceLocation Location => new SourceLocation(_absoluteIndex, _currentLineIndex, _currentLineCharacterIndex);
+    public SourceLocation Location => new(_absoluteIndex, _currentLineIndex, _currentLineCharacterIndex);
 
     public char this[int index]
     {
@@ -148,7 +146,7 @@ public CodeWriter Write(string value, int startIndex, int count)
     }
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    internal CodeWriter WriteCore(string value, int startIndex, int count)
+    private CodeWriter WriteCore(string value, int startIndex, int count)
     {
         if (count == 0)
         {
@@ -185,25 +183,31 @@ internal CodeWriter WriteCore(string value, int startIndex, int count)
 
         // Iterate the string, stopping at each occurrence of a newline character. This lets us count the
         // newline occurrences and keep the index of the last one.
-        while ((i = value.IndexOfAny(NewLineCharacters, i)) >= 0)
+        var endIndex = startIndex + count;
+        while (i < endIndex)
         {
-            // Newline found.
-            _currentLineIndex++;
-            _currentLineCharacterIndex = 0;
+            var ch = value[i];
 
             i++;
 
-            // We might have stopped at a \r, so check if it's followed by \n and then advance the index to
-            // start the next search after it.
-            if (count > i &&
-                value[i - 1] == '\r' &&
-                value[i] == '\n')
+            if (ch is '\r' or '\n')
             {
-                i++;
+                // Newline found.
+                _currentLineIndex++;
+                _currentLineCharacterIndex = 0;
+
+                // We might have stopped at a \r, so check if it's followed by \n and then advance the index.
+                // Otherwise, we'll count this line break twice.
+                if (ch == '\r' &&
+                    i < endIndex &&
+                    value[i] == '\n')
+                {
+                    i++;
+                }
+
+                // The 'suffix' of the current line starts after this newline token.
+                trailingPartStart = i;
             }
-
-            // The 'suffix' of the current line starts after this newline token.
-            trailingPartStart = i;
         }
 
         if (trailingPartStart == null)
@@ -214,7 +218,7 @@ internal CodeWriter WriteCore(string value, int startIndex, int count)
         else
         {
             // Newlines found, add the trailing part of 'data'
-            _currentLineCharacterIndex += (count - trailingPartStart.Value);
+            _currentLineCharacterIndex += endIndex - trailingPartStart.Value;
         }
 
         return this;

diff --git a/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/CodeGeneration/CSharpCodeWriterTest.cs b/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/CodeGeneration/CSharpCodeWriterTest.cs
@@ -21,9 +21,9 @@ public static IEnumerable<object[]> NewLines
         {
             return new object[][]
             {
-                    new object[] { "\r" },
-                    new object[] { "\n" },
-                    new object[] { "\r\n" },
+                new object[] { "\r" },
+                new object[] { "\n" },
+                new object[] { "\r\n" },
             };
         }
     }
@@ -265,6 +265,21 @@ public void CSharpCodeWriter_TracksPosition_WithNewline_SplitAcrossWrites_AtBegi
         Assert.Equal(expected2, location2);
     }
 
+    [Fact]
+    public void CSharpCodeWriter_LinesBreaksOutsideOfContentAreNotCounted()
+    {
+        // Arrange
+        var writer = new CodeWriter();
+
+        // Act
+        writer.Write("\r\nHello\r\nWorld\r\n", startIndex: 2, count: 12);
+        var location = writer.Location;
+
+        // Assert
+        var expected = new SourceLocation(absoluteIndex: 12, lineIndex: 1, characterIndex: 5);
+        Assert.Equal(expected, location);
+    }
+
     [Fact]
     public void WriteLineNumberDirective_UsesFilePath_FromSourceLocation()
     {

diff --git a/...ntimeCodeGenerationTest/ComponentWithConstrainedTypeParameters/TestComponent.mappings.txt b/...ntimeCodeGenerationTest/ComponentWithConstrainedTypeParameters/TestComponent.mappings.txt
@@ -10,7 +10,7 @@ Generated Location: (919:27,1 [34] )
 Source Location: (266:11,0 [3] x:\dir\subdir\Test\TestComponent.cshtml)
 |}
 |
-Generated Location: (1397:47,0 [3] )
+Generated Location: (1397:45,0 [3] )
 |}
 |
 
@@ -21,7 +21,7 @@ Source Location: (294:15,7 [236] x:\dir\subdir\Test\TestComponent.cshtml)
     [Parameter] public TItem3 Item3 { get; set; }
     [Parameter] public RenderFragment<TItem2> ChildContent { get; set; }
 |
-Generated Location: (1637:57,7 [236] )
+Generated Location: (1637:55,7 [236] )
 |
     [Parameter] public TItem1 Item1 { get; set; }
     [Parameter] public List<TItem2> Items2 { get; set; }

diff --git a/...ationTest/ComponentWithConstrainedTypeParameters_WithSemicolon/TestComponent.mappings.txt b/...ationTest/ComponentWithConstrainedTypeParameters_WithSemicolon/TestComponent.mappings.txt
@@ -10,7 +10,7 @@ Generated Location: (919:27,1 [34] )
 Source Location: (269:11,0 [3] x:\dir\subdir\Test\TestComponent.cshtml)
 |}
 |
-Generated Location: (1397:47,0 [3] )
+Generated Location: (1397:45,0 [3] )
 |}
 |
 
@@ -21,7 +21,7 @@ Source Location: (297:15,7 [236] x:\dir\subdir\Test\TestComponent.cshtml)
     [Parameter] public TItem3 Item3 { get; set; }
     [Parameter] public RenderFragment<TItem2> ChildContent { get; set; }
 |
-Generated Location: (1637:57,7 [236] )
+Generated Location: (1637:55,7 [236] )
 |
     [Parameter] public TItem1 Item1 { get; set; }
     [Parameter] public List<TItem2> Items2 { get; set; }

diff --git a/...onentRuntimeCodeGenerationTest/ComponentWithTypeParameterArray/TestComponent.mappings.txt b/...onentRuntimeCodeGenerationTest/ComponentWithTypeParameterArray/TestComponent.mappings.txt
@@ -2,15 +2,15 @@
 |foreach (var item in Items2)
 {
 |
-Generated Location: (1090:37,1 [33] )
+Generated Location: (1090:33,1 [33] )
 |foreach (var item in Items2)
 {
 |
 
 Source Location: (176:10,0 [3] x:\dir\subdir\Test\TestComponent.cshtml)
 |}
 |
-Generated Location: (1509:54,0 [3] )
+Generated Location: (1509:50,0 [3] )
 |}
 |
 
@@ -21,7 +21,7 @@ Source Location: (222:14,7 [248] x:\dir\subdir\Test\TestComponent.cshtml)
     [Parameter] public Func<TItem[]> Items3 { get; set; }
     [Parameter] public RenderFragment<TItem[]> ChildContent { get; set; }
 |
-Generated Location: (1958:72,7 [248] )
+Generated Location: (1958:68,7 [248] )
 |
     [Parameter] public TItem[] Items1 { get; set; }
     [Parameter] public List<TItem[]> Items2 { get; set; }

diff --git a/...RuntimeCodeGenerationTest/ComponentWithTypeParameterValueTuple/TestComponent.mappings.txt b/...RuntimeCodeGenerationTest/ComponentWithTypeParameterValueTuple/TestComponent.mappings.txt
@@ -2,15 +2,15 @@
 |foreach (var item in Items2)
 {
 |
-Generated Location: (1098:37,1 [33] )
+Generated Location: (1098:33,1 [33] )
 |foreach (var item in Items2)
 {
 |
 
 Source Location: (195:11,0 [3] x:\dir\subdir\Test\TestComponent.cshtml)
 |}
 |
-Generated Location: (1517:54,0 [3] )
+Generated Location: (1517:50,0 [3] )
 |}
 |
 
@@ -20,7 +20,7 @@ Source Location: (207:13,7 [215] x:\dir\subdir\Test\TestComponent.cshtml)
     [Parameter] public List<(TItem1, TItem2)> Items2 { get; set; }
     [Parameter] public RenderFragment<(TItem1, TItem2)> ChildContent { get; set; }
 |
-Generated Location: (1697:63,7 [215] )
+Generated Location: (1697:59,7 [215] )
 |
     [Parameter] public (TItem1, TItem2) Item1 { get; set; }
     [Parameter] public List<(TItem1, TItem2)> Items2 { get; set; }

diff --git a/...ComponentRuntimeCodeGenerationTest/ComponentWithTypeParameters/TestComponent.mappings.txt b/...ComponentRuntimeCodeGenerationTest/ComponentWithTypeParameters/TestComponent.mappings.txt
@@ -10,7 +10,7 @@ Generated Location: (827:24,1 [34] )
 Source Location: (178:10,0 [3] x:\dir\subdir\Test\TestComponent.cshtml)
 |}
 |
-Generated Location: (1303:44,0 [3] )
+Generated Location: (1303:42,0 [3] )
 |}
 |
 
@@ -20,7 +20,7 @@ Source Location: (188:11,7 [185] x:\dir\subdir\Test\TestComponent.cshtml)
     [Parameter] public List<TItem2> Items2 { get; set; }
     [Parameter] public RenderFragment<TItem2> ChildContent { get; set; }
 |
-Generated Location: (1483:53,7 [185] )
+Generated Location: (1483:51,7 [185] )
 |
     [Parameter] public TItem1 Item1 { get; set; }
     [Parameter] public List<TItem2> Items2 { get; set; }

diff --git a/...meCodeGenerationTest/ComponentWithTypeParameters_WithSemicolon/TestComponent.mappings.txt b/...meCodeGenerationTest/ComponentWithTypeParameters_WithSemicolon/TestComponent.mappings.txt
@@ -10,7 +10,7 @@ Generated Location: (827:24,1 [34] )
 Source Location: (180:10,0 [3] x:\dir\subdir\Test\TestComponent.cshtml)
 |}
 |
-Generated Location: (1303:44,0 [3] )
+Generated Location: (1303:42,0 [3] )
 |}
 |
 
@@ -20,7 +20,7 @@ Source Location: (190:11,7 [185] x:\dir\subdir\Test\TestComponent.cshtml)
     [Parameter] public List<TItem2> Items2 { get; set; }
     [Parameter] public RenderFragment<TItem2> ChildContent { get; set; }
 |
-Generated Location: (1483:53,7 [185] )
+Generated Location: (1483:51,7 [185] )
 |
     [Parameter] public TItem1 Item1 { get; set; }
     [Parameter] public List<TItem2> Items2 { get; set; }

diff --git a/...ests/ComponentRuntimeCodeGenerationTest/Component_WithCssScope/TestComponent.mappings.txt b/...ests/ComponentRuntimeCodeGenerationTest/Component_WithCssScope/TestComponent.mappings.txt
@@ -1,26 +1,26 @@
 Source Location: (318:6,30 [20] x:\dir\subdir\Test\TestComponent.cshtml)
 |myComponentReference|
-Generated Location: (2193:62,30 [20] )
+Generated Location: (2193:52,30 [20] )
 |myComponentReference|
 
 Source Location: (439:10,1 [34] x:\dir\subdir\Test\TestComponent.cshtml)
 |if (DateTime.Now.Year > 1950)
 {
 |
-Generated Location: (2484:73,1 [34] )
+Generated Location: (2484:63,1 [34] )
 |if (DateTime.Now.Year > 1950)
 {
 |
 
 Source Location: (511:12,38 [18] x:\dir\subdir\Test\TestComponent.cshtml)
 |myElementReference|
-Generated Location: (2917:85,38 [18] )
+Generated Location: (2917:75,38 [18] )
 |myElementReference|
 
 Source Location: (639:14,0 [3] x:\dir\subdir\Test\TestComponent.cshtml)
 |}
 |
-Generated Location: (4060:113,0 [3] )
+Generated Location: (4060:103,0 [3] )
 |}
 |
 
@@ -35,7 +35,7 @@ Source Location: (651:16,7 [233] x:\dir\subdir\Test\TestComponent.cshtml)
         for (var i = 0; i < 10; i++)
         {
 |
-Generated Location: (4240:122,7 [233] )
+Generated Location: (4240:112,7 [233] )
 |
     ElementReference myElementReference;
     TemplatedComponent myComponentReference;
@@ -55,7 +55,7 @@ Source Location: (933:26,0 [164] x:\dir\subdir\Test\TestComponent.cshtml)
         System.GC.KeepAlive(myVariable);
     }
 |
-Generated Location: (5155:157,0 [164] )
+Generated Location: (5155:147,0 [164] )
 |        }
 
         System.GC.KeepAlive(myElementReference);

diff --git a/...ntRuntimeCodeGenerationTest/Component_WithKey_WithChildContent/TestComponent.mappings.txt b/...ntRuntimeCodeGenerationTest/Component_WithKey_WithChildContent/TestComponent.mappings.txt
@@ -1,5 +1,5 @@
 Source Location: (19:0,19 [9] x:\dir\subdir\Test\TestComponent.cshtml)
 |123 + 456|
-Generated Location: (1134:27,19 [9] )
+Generated Location: (1134:25,19 [9] )
 |123 + 456|
 
diff --git a/...ntRuntimeCodeGenerationTest/Component_WithRef_WithChildContent/TestComponent.mappings.txt b/...ntRuntimeCodeGenerationTest/Component_WithRef_WithChildContent/TestComponent.mappings.txt
@@ -1,14 +1,14 @@
 Source Location: (19:0,19 [10] x:\dir\subdir\Test\TestComponent.cshtml)
 |myInstance|
-Generated Location: (1173:27,19 [10] )
+Generated Location: (1173:25,19 [10] )
 |myInstance|
 
 Source Location: (108:4,7 [104] x:\dir\subdir\Test\TestComponent.cshtml)
 |
     private Test.MyComponent myInstance;
     public void Foo() { System.GC.KeepAlive(myInstance); }
 |
-Generated Location: (1462:39,7 [104] )
+Generated Location: (1462:37,7 [104] )
 |
     private Test.MyComponent myInstance;
     public void Foo() { System.GC.KeepAlive(myInstance); }

diff --git a/...timeCodeGenerationTest/ProducesEnhancedLinePragmaWhenNecessary/TestComponent.mappings.txt b/...timeCodeGenerationTest/ProducesEnhancedLinePragmaWhenNecessary/TestComponent.mappings.txt
@@ -5,7 +5,7 @@
         return foo;
     }
 |
-Generated Location: (1330:47,7 [79] )
+Generated Location: (1330:39,7 [79] )
 |
     public string JsonToHtml(string foo)
     {

diff --git a/...entRuntimeCodeGenerationTest/WhiteSpace_WithPreserveWhitespace/TestComponent.mappings.txt b/...entRuntimeCodeGenerationTest/WhiteSpace_WithPreserveWhitespace/TestComponent.mappings.txt
@@ -2,7 +2,7 @@
 |
         int Foo = 18;
     |
-Generated Location: (1190:35,11 [29] )
+Generated Location: (1190:33,11 [29] )
 |
         int Foo = 18;
     |