Fix crash where lines appended to end of script file causes out of bounds exception (#730)

Author: rjmholt

src/PowerShellEditorServices/Workspace/ScriptFile.cs

@@ -114,7 +114,7 @@ public Token[] ScriptTokens
         }
 
         /// <summary>
-        /// Gets the array of filepaths dot sourced in this ScriptFile 
+        /// Gets the array of filepaths dot sourced in this ScriptFile
         /// </summary>
         public string[] ReferencedFiles
         {
@@ -227,7 +227,7 @@ public static IList<string> GetLines(string text)
         }
 
         /// <summary>
-        /// Deterines whether the supplied path indicates the file is an "untitled:Unitled-X" 
+        /// Deterines whether the supplied path indicates the file is an "untitled:Unitled-X"
         /// which has not been saved to file.
         /// </summary>
         /// <param name="path">The path to check.</param>
@@ -311,12 +311,38 @@ public void ValidatePosition(BufferPosition bufferPosition)
         /// <param name="column">The 1-based column to be validated.</param>
         public void ValidatePosition(int line, int column)
         {
-            int maxLine = this.FileLines.Count;
+            ValidatePosition(line, column, isInsertion: false);
+        }
+
+        /// <summary>
+        /// Throws ArgumentOutOfRangeException if the given position is outside
+        /// of the file's buffer extents. If the position is for an insertion (an applied change)
+        /// the index may be 1 past the end of the file, which is just appended.
+        /// </summary>
+        /// <param name="line">The 1-based line to be validated.</param>
+        /// <param name="column">The 1-based column to be validated.</param>
+        /// <param name="isInsertion">If true, the position to validate is for an applied change.</param>
+        public void ValidatePosition(int line, int column, bool isInsertion)
+        {
+            // If new content is being added, VSCode sometimes likes to add it at (FileLines.Count + 1),
+            // which used to crash EditorServices. Now we append it on to the end of the file.
+            // See https://github.com/PowerShell/vscode-powershell/issues/1283
+            int maxLine = isInsertion ? this.FileLines.Count + 1 : this.FileLines.Count;
             if (line < 1 || line > maxLine)
             {
                 throw new ArgumentOutOfRangeException($"Position {line}:{column} is outside of the line range of 1 to {maxLine}.");
             }
 
+            // If we are inserting at the end of the file, the column should be 1
+            if (isInsertion && line == maxLine)
+            {
+                if (column != 1)
+                {
+                    throw new ArgumentOutOfRangeException($"Insertion at the end of a file must occur at column 1");
+                }
+                return;
+            }
+
             // The maximum column is either **one past** the length of the string
             // or 1 if the string is empty.
             string lineString = this.FileLines[line - 1];
@@ -347,51 +373,65 @@ public void ApplyChange(FileChange fileChange)
             }
             else
             {
-                this.ValidatePosition(fileChange.Line, fileChange.Offset);
-                this.ValidatePosition(fileChange.EndLine, fileChange.EndOffset);
-
-                // Get the first fragment of the first line
-                string firstLineFragment =
-                this.FileLines[fileChange.Line - 1]
-                    .Substring(0, fileChange.Offset - 1);
-
-                // Get the last fragment of the last line
-                string endLine = this.FileLines[fileChange.EndLine - 1];
-                string lastLineFragment =
-                endLine.Substring(
-                    fileChange.EndOffset - 1,
-                    (this.FileLines[fileChange.EndLine - 1].Length - fileChange.EndOffset) + 1);
-
-                // Remove the old lines
-                for (int i = 0; i <= fileChange.EndLine - fileChange.Line; i++)
-                {
-                    this.FileLines.RemoveAt(fileChange.Line - 1);
-                }
+                this.ValidatePosition(fileChange.Line, fileChange.Offset, isInsertion: true);
+                this.ValidatePosition(fileChange.EndLine, fileChange.EndOffset, isInsertion: true);
 
-                // Build and insert the new lines
-                int currentLineNumber = fileChange.Line;
-                for (int changeIndex = 0; changeIndex < changeLines.Length; changeIndex++)
+                // VSCode sometimes likes to give the change start line as (FileLines.Count + 1).
+                // This used to crash EditorServices, but we now treat it as an append.
+                // See https://github.com/PowerShell/vscode-powershell/issues/1283
+                if (fileChange.Line == this.FileLines.Count + 1)
                 {
-                    // Since we split the lines above using \n, make sure to
-                    // trim the ending \r's off as well.
-                    string finalLine = changeLines[changeIndex].TrimEnd('\r');
-
-                    // Should we add first or last line fragments?
-                    if (changeIndex == 0)
+                    foreach (string addedLine in changeLines)
                     {
-                        // Append the first line fragment
-                        finalLine = firstLineFragment + finalLine;
+                        string finalLine = addedLine.TrimEnd('\r');
+                        this.FileLines.Add(finalLine);
                     }
-                    if (changeIndex == changeLines.Length - 1)
+                }
+                // Otherwise, the change needs to go between existing content
+                else
+                {
+                    // Get the first fragment of the first line
+                    string firstLineFragment =
+                    this.FileLines[fileChange.Line - 1]
+                        .Substring(0, fileChange.Offset - 1);
+
+                    // Get the last fragment of the last line
+                    string endLine = this.FileLines[fileChange.EndLine - 1];
+                    string lastLineFragment =
+                    endLine.Substring(
+                        fileChange.EndOffset - 1,
+                        (this.FileLines[fileChange.EndLine - 1].Length - fileChange.EndOffset) + 1);
+
+                    // Remove the old lines
+                    for (int i = 0; i <= fileChange.EndLine - fileChange.Line; i++)
                     {
-                        // Append the last line fragment
-                        finalLine = finalLine + lastLineFragment;
+                        this.FileLines.RemoveAt(fileChange.Line - 1);
                     }
 
-                    this.FileLines.Insert(currentLineNumber - 1, finalLine);
-                    currentLineNumber++;
-                }
+                    // Build and insert the new lines
+                    int currentLineNumber = fileChange.Line;
+                    for (int changeIndex = 0; changeIndex < changeLines.Length; changeIndex++)
+                    {
+                        // Since we split the lines above using \n, make sure to
+                        // trim the ending \r's off as well.
+                        string finalLine = changeLines[changeIndex].TrimEnd('\r');
 
+                        // Should we add first or last line fragments?
+                        if (changeIndex == 0)
+                        {
+                            // Append the first line fragment
+                            finalLine = firstLineFragment + finalLine;
+                        }
+                        if (changeIndex == changeLines.Length - 1)
+                        {
+                            // Append the last line fragment
+                            finalLine = finalLine + lastLineFragment;
+                        }
+
+                        this.FileLines.Insert(currentLineNumber - 1, finalLine);
+                        currentLineNumber++;
+                    }
+                }
             }
 
             // Parse the script again to be up-to-date

test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs

@@ -13,7 +13,7 @@ namespace PSLanguageService.Test
 {
     public class ScriptFileChangeTests
     {
-        private static readonly Version PowerShellVersion = new Version("5.0"); 
+        private static readonly Version PowerShellVersion = new Version("5.0");
 
         [Fact]
         public void CanApplySingleLineInsert()
@@ -127,6 +127,22 @@ public void CanApplyMultiLineDelete()
                 });
         }
 
+        [Fact]
+        public void CanApplyEditsToEndOfFile()
+        {
+            this.AssertFileChange(
+                "line1\r\nline2\r\nline3\r\n\r\n",
+                "line1\r\nline2\r\nline3\r\n\r\n\r\n\r\n",
+                new FileChange
+                {
+                    Line = 5,
+                    EndLine = 5,
+                    Offset = 1,
+                    EndOffset = 1,
+                    InsertString = "\r\n\r\n"
+                });
+        }
+
         [Fact]
         public void FindsDotSourcedFiles()
         {
@@ -139,7 +155,7 @@ public void FindsDotSourcedFiles()
 
             using (StringReader stringReader = new StringReader(exampleScriptContents))
             {
-                ScriptFile scriptFile = 
+                ScriptFile scriptFile =
                     new ScriptFile(
                         "DotSourceTestFile.ps1",
                         "DotSourceTestFile.ps1",
@@ -178,7 +194,7 @@ internal static ScriptFile CreateScriptFile(string initialString)
             using (StringReader stringReader = new StringReader(initialString))
             {
                 // Create an in-memory file from the StringReader
-                ScriptFile fileToChange = 
+                ScriptFile fileToChange =
                     new ScriptFile(
                         "TestFile.ps1",
                         "TestFile.ps1",