Merge pull request #68081 from beccamc/dev/beccam/rangeToText

LSP.Range may request beyond length of file

src/Compilers/Core/Portable/CodeAnalysisResources.resx

@@ -756,4 +756,7 @@
   <data name="InvalidInstrumentationKind" xml:space="preserve">
     <value>Invalid instrumentation kind: {0}</value>
   </data>
+  <data name="LineCannotBeGreaterThanEnd" xml:space="preserve">
+    <value>The requested line number {0} must be less than the number of lines {1}.</value>
+  </data>
 </root>

src/Compilers/Core/Portable/Text/TextLineCollection.cs

@@ -62,6 +62,11 @@ public LinePositionSpan GetLinePositionSpan(TextSpan span)
         /// </summary>
         public int GetPosition(LinePosition position)
         {
+            if (position.Line >= this.Count)
+            {
+                throw new ArgumentOutOfRangeException(nameof(position.Line), string.Format(CodeAnalysisResources.LineCannotBeGreaterThanEnd, position.Line, this.Count));
+            }
+
             return this[position.Line].Start + position.Character;
         }
 

src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs

@@ -194,23 +194,20 @@ public static LSP.VersionedTextDocumentIdentifier DocumentToVersionedTextDocumen
         public static LinePosition PositionToLinePosition(LSP.Position position)
             => new LinePosition(position.Line, position.Character);
 
-        public static LinePositionSpan RangeToLinePositionSpan(LSP.Range range)
-            => new LinePositionSpan(PositionToLinePosition(range.Start), PositionToLinePosition(range.End));
-
         public static TextSpan RangeToTextSpan(LSP.Range range, SourceText text)
         {
-            var linePositionSpan = RangeToLinePositionSpan(range);
+            var linePositionSpan = new LinePositionSpan(PositionToLinePosition(range.Start), PositionToLinePosition(range.End));
 
             try
             {
                 try
                 {
                     return text.Lines.GetTextSpan(linePositionSpan);
                 }
-                catch (ArgumentException)
+                catch (ArgumentException ex)
                 {
                     // Create a custom error for this so we can examine the data we're getting.
-                    throw new ArgumentException($"Range={RangeToString(range)}. text.Length={text.Length}. text.Lines.Count={text.Lines.Count}");
+                    throw new ArgumentException($"Range={RangeToString(range)}. text.Length={text.Length}. text.Lines.Count={text.Lines.Count}", ex);
                 }
             }
             // Temporary exception reporting to investigate https://github.com/dotnet/roslyn/issues/66258.

src/Features/LanguageServer/Protocol/Microsoft.CodeAnalysis.LanguageServer.Protocol.csproj

@@ -92,4 +92,19 @@
     <PublicAPI Include="PublicAPI.Unshipped.txt" />
   </ItemGroup>
 
+  <ItemGroup>
+    <Compile Update="LanguageServerResources.Designer.cs">
+      <DesignTime>True</DesignTime>
+      <AutoGen>True</AutoGen>
+      <DependentUpon>LanguageServerResources.resx</DependentUpon>
+    </Compile>
+  </ItemGroup>
+
+  <ItemGroup>
+    <EmbeddedResource Update="LanguageServerResources.resx" GenerateSource="true" Namespace="Microsoft.CodeAnalysis.LanguageServer">
+      <Generator>ResXFileCodeGenerator</Generator>
+      <LastGenOutput>LanguageServerResources.Designer.cs</LastGenOutput>
+    </EmbeddedResource>
+  </ItemGroup>
+
 </Project>

src/Features/LanguageServer/ProtocolUnitTests/ProtocolConversionsTests.cs

@@ -2,9 +2,12 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System;
 using System.Linq;
+using Microsoft.CodeAnalysis.Text;
 using Microsoft.VisualStudio.LanguageServer.Protocol;
 using Xunit;
+using Range = Microsoft.VisualStudio.LanguageServer.Protocol.Range;
 
 namespace Microsoft.CodeAnalysis.LanguageServer.UnitTests
 {
@@ -19,5 +22,105 @@ public void CompletionItemKind_DoNotUseMethodAndFunction()
 
             Assert.False(containsFunction && containsMethod, "Don't use Method and Function completion item kinds as it causes user confusion.");
         }
+
+        [Fact]
+        public void RangeToTextSpanStartWithNextLine()
+        {
+            var markup = GetTestMarkup();
+
+            var sourceText = SourceText.From(markup);
+            var range = new Range() { Start = new Position(0, 0), End = new Position(1, 0) };
+            var textSpan = ProtocolConversions.RangeToTextSpan(range, sourceText);
+
+            // End should be start of the second line
+            Assert.Equal(0, textSpan.Start);
+            Assert.Equal(10, textSpan.End);
+        }
+
+        [Fact]
+        public void RangeToTextSpanMidLine()
+        {
+            var markup = GetTestMarkup();
+            var sourceText = SourceText.From(markup);
+
+            // Take just "x = 5"
+            var range = new Range() { Start = new Position(2, 8), End = new Position(2, 12) };
+            var textSpan = ProtocolConversions.RangeToTextSpan(range, sourceText);
+
+            Assert.Equal(21, textSpan.Start);
+            Assert.Equal(25, textSpan.End);
+        }
+
+        [Fact]
+        public void RangeToTextSpanLineEndOfDocument()
+        {
+            var markup = GetTestMarkup();
+            var sourceText = SourceText.From(markup);
+
+            var range = new Range() { Start = new Position(0, 0), End = new Position(3, 1) };
+            var textSpan = ProtocolConversions.RangeToTextSpan(range, sourceText);
+
+            Assert.Equal(0, textSpan.Start);
+            Assert.Equal(30, textSpan.End);
+        }
+
+        [Fact]
+        public void RangeToTextSpanLineEndOfDocumentWithEndOfLineChars()
+        {
+            var markup =
+@"void M()
+{
+    var x = 5;
+}
+"; // add additional end line 
+
+            var sourceText = SourceText.From(markup);
+
+            var range = new Range() { Start = new Position(0, 0), End = new Position(4, 0) };
+            var textSpan = ProtocolConversions.RangeToTextSpan(range, sourceText);
+
+            // Result now includes end of line characters for line 3
+            Assert.Equal(0, textSpan.Start);
+            Assert.Equal(32, textSpan.End);
+        }
+
+        [Fact]
+        public void RangeToTextSpanLineOutOfRangeError()
+        {
+            var markup = GetTestMarkup();
+            var sourceText = SourceText.From(markup);
+
+            var range = new Range() { Start = new Position(0, 0), End = new Position(sourceText.Lines.Count, 0) };
+            Assert.Throws<ArgumentException>(() => ProtocolConversions.RangeToTextSpan(range, sourceText));
+        }
+
+        [Fact]
+        public void RangeToTextSpanEndAfterStartError()
+        {
+            var markup = GetTestMarkup();
+            var sourceText = SourceText.From(markup);
+
+            // This start position will be beyond the end position
+            var range = new Range() { Start = new Position(2, 20), End = new Position(3, 0) };
+            Assert.Throws<ArgumentException>(() => ProtocolConversions.RangeToTextSpan(range, sourceText));
+        }
+
+        private static string GetTestMarkup()
+        {
+            // Markup is 31 characters long. Line break (\n) is 2 characters 
+            /*
+            void M()        [Line = 0; Start = 0; End = 8; End including line break = 10]
+            {               [Line = 1; Start = 10; End = 11; End including line break = 13]
+                var x = 5;  [Line = 2; Start = 13; End = 27; End including line break = 29]
+            }               [Line = 3; Start = 29; End = 30; End including line break = 30]
+             */
+
+            var markup =
+@"void M()
+{
+    var x = 5;
+}";
+            return markup;
+        }
     }
 }