From 7a421cac0b04096e71a5ad09bb82e30065988d13 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 15 Jun 2017 17:45:06 -0700 Subject: [PATCH 1/6] Add TextLines.ReadOnly() method to get read-only collection --- Engine/Strings.resx | 3 +++ Engine/TextLines.cs | 28 +++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/Engine/Strings.resx b/Engine/Strings.resx index 729cd67fb..26af5b4f0 100644 --- a/Engine/Strings.resx +++ b/Engine/Strings.resx @@ -294,6 +294,9 @@ Line element cannot be null. + + Collection is read-only. + Line element cannot be null. diff --git a/Engine/TextLines.cs b/Engine/TextLines.cs index abbe9c9cb..b073249ea 100644 --- a/Engine/TextLines.cs +++ b/Engine/TextLines.cs @@ -30,6 +30,7 @@ public TextLines() { lines = new LinkedList(); Count = 0; + IsReadOnly = false; InvalidateLastAccessed(); } @@ -60,7 +61,7 @@ public TextLines(IEnumerable inputLines) : this() /// /// If the object is ReadOnly or not. /// - public bool IsReadOnly => false; + public bool IsReadOnly { get; private set; } /// /// Sets or gets the element at the given index. @@ -80,6 +81,19 @@ public string this[int index] } } + /// + /// Creates a readonly shallow copy. + /// + /// A readonly shallow copy of the current object. + public IList ReadOnly() + { + var ret = new TextLines(); + ret.IsReadOnly = true; + ret.Count = this.Count; + ret.lines = this.lines; + return ret; + } + /// /// Adds the given string to the end of the list. /// @@ -94,6 +108,7 @@ public void Add(string item) /// public void Clear() { + ValidateReadOnly(); lines.Clear(); } @@ -154,6 +169,7 @@ public int IndexOf(string item) /// public void Insert(int index, string item) { + ValidateReadOnly(); ThrowIfNull(item, nameof(item)); LinkedListNode itemInserted; if (Count == 0 && index == 0) @@ -180,6 +196,7 @@ public void Insert(int index, string item) /// true if removal is successful, otherwise false. public bool Remove(string item) { + ValidateReadOnly(); var itemIndex = IndexOf(item); if (itemIndex == -1) { @@ -195,6 +212,7 @@ public bool Remove(string item) /// public void RemoveAt(int index) { + ValidateReadOnly(); ValidateIndex(index); var node = GetNodeAt(index); if (node.Next != null) @@ -340,5 +358,13 @@ private static void ThrowIfNull(T param, string paramName) throw new ArgumentNullException(paramName); } } + + private void ValidateReadOnly() + { + if (IsReadOnly) + { + throw new NotSupportedException(Strings.TextLinesReadOnlyCollection); + } + } } } From d796058e0639f0a8d6f8189296772179470e6f2f Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 15 Jun 2017 17:46:16 -0700 Subject: [PATCH 2/6] Make EditableText.Lines return a read-only list --- Engine/EditableText.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Engine/EditableText.cs b/Engine/EditableText.cs index 7aae0c28a..df2a07c28 100644 --- a/Engine/EditableText.cs +++ b/Engine/EditableText.cs @@ -24,7 +24,7 @@ public class EditableText /// /// The lines in the Text. /// - public string[] Lines { get { return lines.ToArray(); } } + public IList Lines => lines.ReadOnly(); /// /// The new line character in the Text. @@ -118,8 +118,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; } From a1ec5164879b9456216ef5b585b360035191072f Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 15 Jun 2017 17:50:48 -0700 Subject: [PATCH 3/6] Add a property to return the number of lines in TextLines --- Engine/EditableText.cs | 5 +++++ Engine/ScriptAnalyzer.cs | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Engine/EditableText.cs b/Engine/EditableText.cs index df2a07c28..a90e4e7ce 100644 --- a/Engine/EditableText.cs +++ b/Engine/EditableText.cs @@ -16,6 +16,11 @@ public class EditableText { private TextLines lines { get; set; } + /// + /// Return the number of lines in the text. + /// + public int LineCount => lines.Count; + /// /// The text that is available for editing. /// diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 919ba8df0..1217c30a2 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1569,7 +1569,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 { @@ -1600,7 +1600,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( From f18ccec031ac39962a65ca62bd3f113615e6d412 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 15 Jun 2017 17:59:25 -0700 Subject: [PATCH 4/6] Add tests for EditableText class --- Tests/Engine/EditableText.tests.ps1 | 33 +++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/Tests/Engine/EditableText.tests.ps1 b/Tests/Engine/EditableText.tests.ps1 index ce207a721..c6f888f08 100644 --- a/Tests/Engine/EditableText.tests.ps1 +++ b/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 + } } } From 22015c9f80b8e59298078ca105b78e78df156d8f Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Fri, 16 Jun 2017 11:03:02 -0700 Subject: [PATCH 5/6] Remove todo comments from ScriptAnalyzer.Fix --- Engine/ScriptAnalyzer.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 1217c30a2..5c3a6ec03 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/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)) { @@ -1598,8 +1597,6 @@ 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.LineCount; if (!isRangeNull && lineCount != previousLineCount) { From 4c1521667855300025fef1159cd0102d7161ee42 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Sat, 17 Jun 2017 11:41:39 -0700 Subject: [PATCH 6/6] Make ReadOnly method return a ReadOnlyCollection type --- Engine/EditableText.cs | 3 ++- Engine/Strings.resx | 3 --- Engine/TextLines.cs | 28 ++++++---------------------- 3 files changed, 8 insertions(+), 26 deletions(-) diff --git a/Engine/EditableText.cs b/Engine/EditableText.cs index a90e4e7ce..93f78d66a 100644 --- a/Engine/EditableText.cs +++ b/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; @@ -29,7 +30,7 @@ public class EditableText /// /// The lines in the Text. /// - public IList Lines => lines.ReadOnly(); + public ReadOnlyCollection Lines => lines.ReadOnly(); /// /// The new line character in the Text. diff --git a/Engine/Strings.resx b/Engine/Strings.resx index 26af5b4f0..729cd67fb 100644 --- a/Engine/Strings.resx +++ b/Engine/Strings.resx @@ -294,9 +294,6 @@ Line element cannot be null. - - Collection is read-only. - Line element cannot be null. diff --git a/Engine/TextLines.cs b/Engine/TextLines.cs index b073249ea..722501303 100644 --- a/Engine/TextLines.cs +++ b/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; @@ -30,7 +31,6 @@ public TextLines() { lines = new LinkedList(); Count = 0; - IsReadOnly = false; InvalidateLastAccessed(); } @@ -61,7 +61,7 @@ public TextLines(IEnumerable inputLines) : this() /// /// If the object is ReadOnly or not. /// - public bool IsReadOnly { get; private set; } + public bool IsReadOnly => false; /// /// Sets or gets the element at the given index. @@ -82,16 +82,12 @@ public string this[int index] } /// - /// Creates a readonly shallow copy. + /// Return a readonly collection of the current object. /// - /// A readonly shallow copy of the current object. - public IList ReadOnly() + /// A readonly collection of the current object. + public ReadOnlyCollection ReadOnly() { - var ret = new TextLines(); - ret.IsReadOnly = true; - ret.Count = this.Count; - ret.lines = this.lines; - return ret; + return new ReadOnlyCollection(this); } /// @@ -108,7 +104,6 @@ public void Add(string item) /// public void Clear() { - ValidateReadOnly(); lines.Clear(); } @@ -169,7 +164,6 @@ public int IndexOf(string item) /// public void Insert(int index, string item) { - ValidateReadOnly(); ThrowIfNull(item, nameof(item)); LinkedListNode itemInserted; if (Count == 0 && index == 0) @@ -196,7 +190,6 @@ public void Insert(int index, string item) /// true if removal is successful, otherwise false. public bool Remove(string item) { - ValidateReadOnly(); var itemIndex = IndexOf(item); if (itemIndex == -1) { @@ -212,7 +205,6 @@ public bool Remove(string item) /// public void RemoveAt(int index) { - ValidateReadOnly(); ValidateIndex(index); var node = GetNodeAt(index); if (node.Next != null) @@ -358,13 +350,5 @@ private static void ThrowIfNull(T param, string paramName) throw new ArgumentNullException(paramName); } } - - private void ValidateReadOnly() - { - if (IsReadOnly) - { - throw new NotSupportedException(Strings.TextLinesReadOnlyCollection); - } - } } }