Merge pull request #785 from PowerShell/kapilmb/update-formatter

Make minor improvements to fixing mechanism

Author: Kapil Borle

Engine/EditableText.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Globalization;
 using System.Linq;
 using System.Management.Automation.Language;
@@ -16,6 +17,11 @@ public class EditableText
     {
         private TextLines lines { get; set; }
 
+        /// <summary>
+        /// Return the number of lines in the text.
+        /// </summary>
+        public int LineCount => lines.Count;
+
         /// <summary>
         /// The text that is available for editing.
         /// </summary>
@@ -24,7 +30,7 @@ public class EditableText
         /// <summary>
         /// The lines in the Text.
         /// </summary>
-        public string[] Lines { get { return lines.ToArray(); } }
+        public ReadOnlyCollection<string> Lines => lines.ReadOnly();
 
         /// <summary>
         /// The new line character in the Text.
@@ -118,8 +124,8 @@ public bool IsValidRange(Range range)
                 throw new ArgumentNullException(nameof(range));
             }
 
-            return range.Start.Line <= Lines.Length
-                && range.End.Line <= Lines.Length
+            return range.Start.Line <= Lines.Count
+                && range.End.Line <= Lines.Count
                 && range.Start.Column <= Lines[range.Start.Line - 1].Length
                 && range.End.Column <= Lines[range.End.Line - 1].Length + 1;
         }

Engine/ScriptAnalyzer.cs

@@ -1557,7 +1557,6 @@ public EditableText Fix(EditableText text, Range range, out Range updatedRange)
                 throw new ArgumentNullException(nameof(text));
             }
 
-            // todo validate range
             var isRangeNull = range == null;
             if (!isRangeNull && !text.IsValidRange(range))
             {
@@ -1569,7 +1568,7 @@ public EditableText Fix(EditableText text, Range range, out Range updatedRange)
             }
 
             range = isRangeNull ? null : SnapToEdges(text, range);
-            var previousLineCount = text.Lines.Length;
+            var previousLineCount = text.LineCount;
             var previousUnusedCorrections = 0;
             do
             {
@@ -1598,9 +1597,7 @@ public EditableText Fix(EditableText text, Range range, out Range updatedRange)
                 }
 
                 previousUnusedCorrections = unusedCorrections;
-
-                // todo add a TextLines.NumLines property because accessing TextLines.Lines is expensive
-                var lineCount = text.Lines.Length;
+                var lineCount = text.LineCount;
                 if (!isRangeNull && lineCount != previousLineCount)
                 {
                     range = new Range(

Engine/TextLines.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Globalization;
 using System.Linq;
 
@@ -80,6 +81,15 @@ public string this[int index]
             }
         }
 
+        /// <summary>
+        /// Return a readonly collection of the current object.
+        /// </summary>
+        /// <returns>A readonly collection of the current object.</returns>
+        public ReadOnlyCollection<string> ReadOnly()
+        {
+            return new ReadOnlyCollection<string>(this);
+        }
+
         /// <summary>
         /// Adds the given string to the end of the list.
         /// </summary>

Tests/Engine/EditableText.tests.ps1

@@ -104,5 +104,38 @@ function foo {
             $result = $editableText.ApplyEdit($edit)
             $result.ToString() | Should Be $expected
         }
+
+        It "Should return a read-only collection of lines in the text" {
+            $def = @'
+function foo {
+    param(
+        [bool] $param1
+    )
+}
+'@
+            $text = New-Object `
+                -TypeName "Microsoft.Windows.PowerShell.ScriptAnalyzer.EditableText" `
+                -ArgumentList @($def)
+
+            {$text.Lines.Add("abc")} | Should Throw
+        }
+
+        It "Should return the correct number of lines in the text" {
+            $def = @'
+function foo
+{
+get-childitem
+$x=1+2
+$hashtable = @{
+property1 = "value"
+    anotherProperty = "another value"
+}
+}
+'@
+            $text = New-Object `
+                -TypeName "Microsoft.Windows.PowerShell.ScriptAnalyzer.EditableText" `
+                -ArgumentList @($def)
+            $text.LineCount | Should Be 9
+        }
      }
 }