chore: Code quality improvements (#2696)

* Use string builder in loop * Use find & handle other platform newlines * Use find * Fix duplicate message * Add more range tests * Improve range code
stryker-mutator · Oct 4, 2023 · b58ebe7 · b58ebe7
1 parent 03f9913
commit b58ebe7
Show file tree

Hide file tree

Showing 4 changed files with 104 additions and 43 deletions.
diff --git a/src/Stryker.Core/Stryker.Core.UnitTest/Helpers/RangeHelperTests.cs b/src/Stryker.Core/Stryker.Core.UnitTest/Helpers/RangeHelperTests.cs
@@ -1,8 +1,11 @@
+using System;
 using System.Linq;
 using FSharp.Compiler.Text;
 using Shouldly;
 using Stryker.Core.Helpers;
 using Xunit;
+using Range = FSharp.Compiler.Text.Range;
+
 
 namespace Stryker.Core.UnitTest.Helpers
 {
@@ -442,37 +445,84 @@ public void GetPosition_OneLine()
         }
 
         [Fact]
-        public void GetPosition_OutOfBounds()
+        public void GetPosition_NoContent()
         {
-            var text = "Line1";
+            var text = "";
+            var index = 0;
+            var position = PositionModule.mkPos(0, 0);
+
+            var result = RangeHelper.GetPosition(text, index);
+
+            result.ShouldBe(position);
+        }
+
+        [Fact]
+        public void GetPosition_NoContent_OutOfBounds()
+        {
+            var text = "";
             var index = 42;
-            var position = PositionModule.mkPos(0, 5);
+            var position = PositionModule.mkPos(0, 0);
+
+            var result = RangeHelper.GetPosition(text, index);
+
+            result.ShouldBe(position);
+        }
+
+        [Theory]
+        [InlineData(1, 0, 1)]
+        [InlineData(4, 0, 4)]
+        [InlineData(7, 1, 2)]
+        [InlineData(9, 1, 4)]
+        public void GetPosition_MultipleLines(int index, int expectedRow, int expectedCol)
+        {
+            var text = $"Line1{Environment.NewLine}Line2";
+            var position = PositionModule.mkPos(expectedRow, expectedCol);
 
             var result = RangeHelper.GetPosition(text, index);
 
             result.ShouldBe(position);
         }
 
         [Fact]
-        public void GetIndex_OneLine()
+        public void GetPosition_OutOfBounds()
         {
             var text = "Line1";
-            var position = PositionModule.mkPos(0, 1);
+            var index = 42;
+            var position = PositionModule.mkPos(0, 5);
 
-            var result = RangeHelper.GetIndex(text, position);
+            var result = RangeHelper.GetPosition(text, index);
 
-            result.ShouldBe(1);
+            result.ShouldBe(position);
         }
 
         [Fact]
-        public void GetIndex_OutOfBounds()
+        public void GetIndex_OneLine() => GetIndex("Line1", expectedIndex: 1, row: 0, col: 1);
+
+        [Theory]
+        [InlineData(1, 0, 1)]
+        [InlineData(4, 0, 4)]
+        [InlineData(7, 1, 2)]
+        [InlineData(9, 1, 4)]
+
+        public void GetIndex_MultipleLines(int expectedIndex, int row, int col) => GetIndex($"Line1{Environment.NewLine}Line2", expectedIndex, row, col);
+
+        [Theory]
+        [InlineData(-1, 0, 42)]
+        [InlineData(-1, 1, 42)]
+        [InlineData(-1, 2, 0)]
+        [InlineData(-1, 2, 42)]
+        public void GetIndex_MultipleLines_OutOfBounds(int expectedIndex, int row, int col) => GetIndex($"Line1{Environment.NewLine}Line2", expectedIndex, row, col);
+
+        [Fact]
+        public void GetIndex_OutOfBounds() => GetIndex("Line1", expectedIndex: -1, row: 42, col: 42);
+
+        private static void GetIndex(string text, int expectedIndex, int row, int col)
         {
-            var text = "Line1";
-            var position = PositionModule.mkPos(42, 42);
+            var position = PositionModule.mkPos(row, col);
 
             var result = RangeHelper.GetIndex(text, position);
 
-            result.ShouldBe(-1);
+            result.ShouldBe(expectedIndex);
         }
 
         private static Range GetRange((int Line, int Column) start, (int Line, int Column) end) =>

diff --git a/src/Stryker.Core/Stryker.Core/Compiling/CsharpCompilingProcess.cs b/src/Stryker.Core/Stryker.Core/Compiling/CsharpCompilingProcess.cs
@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
+using System.Text;
 using Buildalyzer;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
@@ -180,19 +181,21 @@ private void LogEmitResult(EmitResult result)
 
         private void DumpErrorDetails(IEnumerable<Diagnostic> diagnostics)
         {
-            var message = $"An unrecoverable compilation error occurred:{Environment.NewLine}";
+            var messageBuilder = new StringBuilder();
             var materializedDiagnostics = diagnostics.ToArray();
             if (!materializedDiagnostics.Any())
             {
-                message += "Unfortunately there is no more info available, good luck!";
+                messageBuilder.Append("Unfortunately there is no more info available, good luck!");
             }
 
             foreach (var diagnostic in materializedDiagnostics)
             {
-                message += $"{diagnostic.Id}: {diagnostic}{Environment.NewLine}";
+                messageBuilder
+                    .Append(Environment.NewLine)
+                    .Append(diagnostic.Id).Append(": ").AppendLine(diagnostic.ToString());
             }
 
-            _logger.LogTrace(message);
+            _logger.LogTrace("An unrecoverable compilation error occurred: {Diagnostics}", messageBuilder.ToString());
         }
 
         private static string ReadableNumber(int number) => number switch

diff --git a/src/Stryker.Core/Stryker.Core/Helpers/RangeHelper.cs b/src/Stryker.Core/Stryker.Core/Helpers/RangeHelper.cs
@@ -1,4 +1,5 @@
 using System.Collections.Generic;
+using System.IO;
 using System.Linq;
 using FSharp.Compiler.Text;
 
@@ -24,7 +25,7 @@ public static IReadOnlyCollection<Range> Reduce(this IEnumerable<Range> ranges,
                 foreach (var current in rangeList)
                 {
                     // Check if any of the other ranges intersects with the current one
-                    var other = rangeList.FirstOrDefault(s => !RangeModule.equals(s, current) && s.IntersectsWith(current));
+                    var other = rangeList.Find(s => !RangeModule.equals(s, current) && s.IntersectsWith(current));
                     if (!RangeModule.equals(other, Range.Zero))
                     {
                         // Remove the original ranges
@@ -131,47 +132,54 @@ public static bool IntersectsWith(this Range range1, Range range2)
         public static Position GetPosition(string text, int index)
         {
             var line = 0;
-            var col = 0;
 
-            for (var i = 0; i < System.Math.Min(index, text.Length); i++)
+            using var reader = new StringReader(text);
+            var currentIndex = 0;
+            do
             {
-                // TODO: handle x-platform
-                if (text[i] == '\n')
+                string? latestLineContent = reader.ReadLine();
+                if (latestLineContent == null) break;
+
+                var lengthOfThisLine = latestLineContent.Length;
+                var endOfLineIndex = currentIndex + lengthOfThisLine;
+                var indexIsOnThisLine = index <= endOfLineIndex && index >= currentIndex;
+                if (indexIsOnThisLine)
                 {
-                    line++;
-                    col = 0;
+                    var indexOnThisLine = index - currentIndex;
+                    return PositionModule.mkPos(line, indexOnThisLine);
                 }
-                else
+
+                var thisWasTheLastLine = reader.Peek() == -1;
+                if (thisWasTheLastLine)
                 {
-                    col++;
+                    return PositionModule.mkPos(line, lengthOfThisLine);
                 }
-            }
 
-            return PositionModule.mkPos(line, col);
+                line++;
+                currentIndex += lengthOfThisLine;
+            } while (currentIndex < text.Length);
+
+            return PositionModule.mkPos(0, 0);
         }
 
         public static int GetIndex(string text, Position pos)
         {
-            var line = 0;
-            var col = 0;
-
-            for (var i = 0; i < text.Length; i++)
+            using var reader = new StringReader(text);
+            string? latestLineContent = "";
+            var currentIndex = 0;
+            for (var line = 0; line <= pos.Line; ++line)
             {
-                if (line == pos.Line && col == pos.Column)
+                if (latestLineContent == null)
                 {
-                    return i;
+                    break;
                 }
+                currentIndex += latestLineContent.Length;
+                latestLineContent = reader.ReadLine();
+            }
 
-                // TODO: handle x-platform
-                if (text[i] == '\n')
-                {
-                    line++;
-                    col = 0;
-                }
-                else
-                {
-                    col++;
-                }
+            if (latestLineContent != null && pos.Column < latestLineContent.Length)
+            {
+                return currentIndex + pos.Column;
             }
 
             return -1;

diff --git a/src/Stryker.Core/Stryker.Core/Helpers/TextSpanHelper.cs b/src/Stryker.Core/Stryker.Core/Helpers/TextSpanHelper.cs
@@ -25,7 +25,7 @@ public static IReadOnlyCollection<TextSpan> Reduce(this IEnumerable<TextSpan> te
                 foreach (var current in spans)
                 {
                     // Check if any of the other spans intersects with the current one
-                    var other = spans.FirstOrDefault(s => s != current && s.IntersectsWith(current));
+                    var other = spans.Find(s => s != current && s.IntersectsWith(current));
                     if (other != default)
                     {
                         // Remove the original spans