Fix out-of-range failure when formatting a stale range

src/Microsoft.SqlTools.LanguageService/Formatter/TSqlFormatterService.cs

@@ -159,8 +159,17 @@ private Task<FormatOperationResult> FormatRangeAndReturnEdits(DocumentRangeForma
                 {
                     return CreateSkippedResult(stopwatch);
                 }
+                BufferRange bufferRange = range.ToBufferRange();
+                if (!scriptFile.IsRangeValid(bufferRange))
+                {
+                    // The buffer here can lag the editor's copy, so a requested range may
+                    // outrun it. Formatting what this copy holds would rewrite the wrong
+                    // span of the client's document, so do nothing instead.
+                    Logger.Verbose($"Skipping range format for {docFormatParams.TextDocument.Uri}; range is outside the current buffer");
+                    return CreateSkippedResult(stopwatch);
+                }
                 TextEdit textEdit = new TextEdit { Range = range };
-                string text = scriptFile.GetTextInRange(range.ToBufferRange());
+                string text = scriptFile.GetTextInRange(bufferRange);
                 return DoFormat(docFormatParams, textEdit, text, stopwatch);
             });
         }

src/Microsoft.SqlTools.LanguageService/Workspace/Contracts/ScriptFile.cs

@@ -159,7 +159,7 @@ public string GetLine(int lineNumber)
         {
             Validate.IsWithinRange(
                 "lineNumber", lineNumber,
-                1, FileLines.Count + 1);
+                1, FileLines.Count);
 
             return FileLines[lineNumber - 1];
         }
@@ -231,22 +231,49 @@ public void ValidatePosition(BufferPosition bufferPosition)
         /// <param name="column">The 1-based column to be validated.</param>
         public void ValidatePosition(int line, int column)
         {
-            if (line < 1 || line > FileLines.Count + 1)
+            if (line < 1 || line > FileLines.Count)
             {
                 throw new ArgumentOutOfRangeException(nameof(line), SR.WorkspaceServicePositionLineOutOfRange);
             }
 
-            // The maximum column is either one past the length of the string
-            // or 1 if the string is empty.
-            string lineString = FileLines[line - 1];
-            int maxColumn = lineString.Length > 0 ? lineString.Length + 1 : 1;
-
-            if (column < 1 || column > maxColumn)
+            if (column < 1 || column > GetMaxColumn(line))
             {
                 throw new ArgumentOutOfRangeException(nameof(column), SR.WorkspaceServicePositionColumnOutOfRange(line));
             }
         }
 
+        /// <summary>
+        /// Determines whether the given range falls entirely within this file's extents.
+        /// A client can send a range built against a newer revision of the document than
+        /// the one held here, so callers that must not fail should test the range first.
+        /// </summary>
+        /// <param name="bufferRange">The buffer range to test.</param>
+        public bool IsRangeValid(BufferRange bufferRange)
+        {
+            return bufferRange != null
+                && IsPositionValid(bufferRange.Start)
+                && IsPositionValid(bufferRange.End);
+        }
+
+        private bool IsPositionValid(BufferPosition position)
+        {
+            return position != null
+                && position.Line >= 1
+                && position.Line <= FileLines.Count
+                && position.Column >= 1
+                && position.Column <= GetMaxColumn(position.Line);
+        }
+
+        /// <summary>
+        /// Gets the highest valid 1-based column for the given 1-based line, which is
+        /// one past the length of the line, or 1 when the line is empty.
+        /// </summary>
+        private int GetMaxColumn(int line)
+        {
+            string lineString = FileLines[line - 1];
+            return lineString.Length > 0 ? lineString.Length + 1 : 1;
+        }
+
         /// <summary>
         /// Applies the provided FileChange to the file's contents
         /// </summary>

test/Microsoft.SqlTools.ServiceLayer.UnitTests/Formatter/TSqlFormatterServiceTests.cs

@@ -318,6 +318,30 @@ await TestUtils.RunAndVerify<TextEdit[]>(
         }
 
 
+        [Test]
+        public async Task FormatRangeShouldSkipRangeBeyondBuffer()
+        {
+            // Given a request whose range runs one line past this copy of the document,
+            // as happens when the service's buffer lags behind the editor's buffer
+            SetupLanguageService();
+            SetupScriptFile(defaultSqlContents);
+            rangeFormatParams.Range = new Range
+            {
+                Start = new Position { Line = 0, Character = 0 },
+                End = new Position { Line = 1, Character = 0 }
+            };
+
+            // When format range is called
+            await TestUtils.RunAndVerify<TextEdit[]>(
+                test: (requestContext) => FormatterService.HandleDocRangeFormatRequest(rangeFormatParams, requestContext),
+                verify: (edits =>
+                {
+                    // Then the request succeeds with no edits rather than failing
+                    Assert.AreEqual(0, edits.Length);
+                }));
+        }
+
+
         [Test]
         public async Task FormatDocumentTelemetryShouldIncludeFormatTypeProperty()
         {

test/Microsoft.SqlTools.ServiceLayer.UnitTests/ServiceHost/ScriptFileTests.cs

@@ -249,6 +249,62 @@ public void ThrowsExceptionWithEditOutsideOfRange()
                 });
         }
 
+        /// <summary>
+        /// A position on the line just past the end of the file is out of range, and must be
+        /// reported as such instead of surfacing the underlying list indexer failure.
+        /// </summary>
+        [Test]
+        public void ReportsLineOutOfRangeJustPastEndOfFile()
+        {
+            ScriptFile scriptFile = GetTestScriptFile("first\r\nsecond");
+
+            ArgumentOutOfRangeException ex = Assert.Throws<ArgumentOutOfRangeException>(() =>
+                scriptFile.GetLinesInRange(
+                    new BufferRange(
+                        new BufferPosition(1, 1),
+                        new BufferPosition(3, 1))));
+
+            Assert.AreEqual("line", ex.ParamName);
+        }
+
+        [Test]
+        public void GetLineThrowsJustPastEndOfFile()
+        {
+            ScriptFile scriptFile = GetTestScriptFile("first\r\nsecond");
+
+            ArgumentOutOfRangeException ex = Assert.Throws<ArgumentOutOfRangeException>(
+                () => scriptFile.GetLine(3));
+
+            Assert.AreEqual("lineNumber", ex.ParamName);
+        }
+
+        [Test]
+        public void IsRangeValidDetectsRangesOutsideTheFile()
+        {
+            ScriptFile scriptFile = GetTestScriptFile("first\r\nsecond");
+
+            Assert.True(
+                scriptFile.IsRangeValid(new BufferRange(1, 1, 2, 7)),
+                "Range within the file is valid");
+            Assert.False(
+                scriptFile.IsRangeValid(new BufferRange(1, 1, 3, 1)),
+                "Range ending one line past the end of the file is not valid");
+            Assert.False(
+                scriptFile.IsRangeValid(new BufferRange(1, 1, 2, 20)),
+                "Range ending past the end of the last line is not valid");
+        }
+
+        [Test]
+        public void TrailingNewlineCreatesValidEmptyFinalLine()
+        {
+            ScriptFile scriptFile = GetTestScriptFile("first\r\nsecond\r\n");
+
+            Assert.AreEqual(string.Empty, scriptFile.GetLine(3));
+            Assert.True(
+                scriptFile.IsRangeValid(new BufferRange(3, 1, 3, 1)),
+                "The empty line after a trailing newline is within the file");
+        }
+
         private void AssertFileChange(
             string initialString,
             string expectedString,