Fix #45, #63: Completion text replacement bugs

This change resolves a general class of issues around code completions
coming from the language service.  The JSON protocol allows the language
server to specify the range of code that will get replaced by a completion
item.  Previously this range was being computed incorrectly.  We now use
the replacement range given by the PowerShell API.

This also resolves PowerShell/vscode-powershell issues 45 and 12

Author: daviwil

Diff for: src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

@@ -364,38 +364,13 @@ await editorSession.LanguageService.GetCompletionsInFile(
                 int startEditColumn = textDocumentPosition.Position.Character;
                 int endEditColumn = textDocumentPosition.Position.Character;
 
-                // Find the extents of the token under the cursor
-                var completedToken =
-                    scriptFile
-                        .ScriptAst
-                        .FindAll(
-                            ast =>
-                            {
-                                return
-                                    !(ast is PipelineAst) &&
-                                    ast.Extent.StartLineNumber == cursorLine &&
-                                    ast.Extent.StartColumnNumber <= cursorColumn &&
-                                    ast.Extent.EndColumnNumber >= cursorColumn;
-                            },
-                            true)
-                        .LastOrDefault();   // The most relevant AST will be the last
-
-                if (completedToken != null)
-                {
-                    // The edit should replace the token that was found at the cursor position
-                    startEditColumn = completedToken.Extent.StartColumnNumber - 1;
-                    endEditColumn = completedToken.Extent.EndColumnNumber - 1;
-                }
-
                 completionItems =
                     completionResults
                         .Completions
                         .Select(
                             c => CreateCompletionItem(
                                 c,
-                                textDocumentPosition.Position.Line,
-                                startEditColumn,
-                                endEditColumn))
+                                completionResults.ReplacedRange))
                         .ToArray();
             }
             else
@@ -942,9 +917,7 @@ private static CompletionItemKind MapCompletionKind(CompletionType completionTyp
 
         private static CompletionItem CreateCompletionItem(
             CompletionDetails completionDetails,
-            int lineNumber,
-            int startColumn,
-            int endColumn)
+            BufferRange completionRange)
         {
             string detailString = null;
 
@@ -971,13 +944,13 @@ private static CompletionItem CreateCompletionItem(
                     {
                         Start = new Position
                         {
-                            Line = lineNumber,
-                            Character = startColumn
+                            Line = completionRange.Start.Line - 1,
+                            Character = completionRange.Start.Column - 1
                         },
                         End = new Position
                         {
-                            Line = lineNumber,
-                            Character = endColumn
+                            Line = completionRange.End.Line - 1,
+                            Character = completionRange.End.Column - 1
                         }
                     }
                 }

Diff for: src/PowerShellEditorServices/Language/AstOperations.cs

@@ -38,10 +38,10 @@ internal static class AstOperations
         /// A CommandCompletion instance that contains completions for the
         /// symbol at the given offset.
         /// </returns>
-        static public CompletionResults GetCompletions(
+        static public CommandCompletion GetCompletions(
             Ast scriptAst, 
             Token[] currentTokens, 
-            int fileOffset, 
+            int fileOffset,
             Runspace runspace)
         {
             var type = scriptAst.Extent.StartScriptPosition.GetType();
@@ -75,7 +75,7 @@ static public CompletionResults GetCompletions(
                 }
             }
 
-            return CompletionResults.Create(commandCompletion);
+            return commandCompletion;
         }
 
         /// <summary>

Diff for: src/PowerShellEditorServices/Language/CompletionResults.cs

@@ -24,16 +24,36 @@ public sealed class CompletionResults
         /// </summary>
         public CompletionDetails[] Completions { get; private set; }
 
+        /// <summary>
+        /// Gets the range in the buffer that should be replaced by this
+        /// completion result.
+        /// </summary>
+        public BufferRange ReplacedRange { get; private set; }
+
         #endregion
 
         #region Constructors
 
+        /// <summary>
+        /// Creates an empty CompletionResults instance.
+        /// </summary>
+        public CompletionResults()
+        {
+            this.Completions = new CompletionDetails[0];
+            this.ReplacedRange = new BufferRange();
+        }
+
         internal static CompletionResults Create(
+            ScriptFile scriptFile,
             CommandCompletion commandCompletion)
         {
             return new CompletionResults
             {
                 Completions = GetCompletionsArray(commandCompletion),
+                ReplacedRange = 
+                    scriptFile.GetRangeBetweenOffsets(
+                        commandCompletion.ReplacementIndex,
+                        commandCompletion.ReplacementIndex + commandCompletion.ReplacementLength)
             };
         }
 

Diff for: src/PowerShellEditorServices/Language/LanguageService.cs

@@ -89,22 +89,34 @@ public async Task<CompletionResults> GetCompletionsInFile(
             RunspaceHandle runspaceHandle =
                 await this.powerShellContext.GetRunspaceHandle();
 
-            CompletionResults completionResults =
+            CommandCompletion commandCompletion =
                 AstOperations.GetCompletions(
                     scriptFile.ScriptAst,
                     scriptFile.ScriptTokens,
                     fileOffset,
                     runspaceHandle.Runspace);
 
             runspaceHandle.Dispose();
-                    
-            // save state of most recent completion
-            mostRecentCompletions = completionResults;
-            mostRecentRequestFile = scriptFile.Id;
-            mostRecentRequestLine = lineNumber;
-            mostRecentRequestOffest = columnNumber;
-
-            return completionResults;
+
+            if (commandCompletion != null)
+            {
+                CompletionResults completionResults =
+                    CompletionResults.Create(
+                        scriptFile,
+                        commandCompletion);
+
+                // save state of most recent completion
+                mostRecentCompletions = completionResults;
+                mostRecentRequestFile = scriptFile.Id;
+                mostRecentRequestLine = lineNumber;
+                mostRecentRequestOffest = columnNumber;
+
+                return completionResults;
+            }
+            else
+            {
+                return new CompletionResults();
+            }
         }
 
         /// <summary>

Diff for: src/PowerShellEditorServices/PowerShellEditorServices.csproj

@@ -102,7 +102,9 @@
     <Compile Include="Session\SynchronizingConsoleHostWrapper.cs" />
     <Compile Include="Utility\Logger.cs" />
     <Compile Include="Utility\Validate.cs" />
+    <Compile Include="Workspace\BufferRange.cs" />
     <Compile Include="Workspace\FileChange.cs" />
+    <Compile Include="Workspace\BufferPosition.cs" />
     <Compile Include="Workspace\ScriptFile.cs" />
     <Compile Include="Workspace\ScriptFileMarker.cs" />
     <Compile Include="Workspace\ScriptRegion.cs" />

Diff for: src/PowerShellEditorServices/Workspace/BufferPosition.cs

@@ -0,0 +1,35 @@
+//
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+//
+
+namespace Microsoft.PowerShell.EditorServices
+{
+    /// <summary>
+    /// Provides details about a position in a file buffer.
+    /// </summary>
+    public struct BufferPosition
+    {
+        /// <summary>
+        /// Gets the line number of the position in the buffer.
+        /// </summary>
+        public int Line { get; private set; }
+
+        /// <summary>
+        /// Gets the column number of the position in the buffer.
+        /// </summary>
+        public int Column { get; private set; }
+
+        /// <summary>
+        /// Creates a new instance of the BufferPosition class.
+        /// </summary>
+        /// <param name="line">The line number of the position.</param>
+        /// <param name="column">The column number of the position.</param>
+        public BufferPosition(int line, int column)
+        {
+            this.Line = line;
+            this.Column = column;
+        }
+    }
+}
+

Diff for: src/PowerShellEditorServices/Workspace/BufferRange.cs

@@ -0,0 +1,36 @@
+//
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+//
+
+namespace Microsoft.PowerShell.EditorServices
+{
+    /// <summary>
+    /// Provides details about a range between two positions in
+    /// a file buffer.
+    /// </summary>
+    public struct BufferRange
+    {
+        /// <summary>
+        /// Gets the start position of the range in the buffer.
+        /// </summary>
+        public BufferPosition Start { get; private set; }
+
+        /// <summary>
+        /// Gets the end position of the range in the buffer.
+        /// </summary>
+        public BufferPosition End { get; private set; }
+
+        /// <summary>
+        /// Creates a new instance of the BufferRange class.
+        /// </summary>
+        /// <param name="start">The start position of the range.</param>
+        /// <param name="end">The end position of the range.</param>
+        public BufferRange(BufferPosition start, BufferPosition end)
+        {
+            this.Start = start;
+            this.End = end;
+        }
+    }
+}
+

Diff for: src/PowerShellEditorServices/Workspace/ScriptFile.cs

@@ -255,6 +255,81 @@ public int GetOffsetAtPosition(int lineNumber, int columnNumber)
             return offset;
         }
 
+        /// <summary>
+        /// Calculates the 1-based line and column number position based
+        /// on the given buffer offset.
+        /// </summary>
+        /// <param name="bufferOffset">The buffer offset to convert.</param>
+        /// <returns>A new BufferPosition containing the position of the offset.</returns>
+        public BufferPosition GetPositionAtOffset(int bufferOffset)
+        {
+            BufferRange bufferRange = 
+                GetRangeBetweenOffsets(
+                    bufferOffset, bufferOffset);
+
+            return bufferRange.Start;
+        }
+
+        /// <summary>
+        /// Calculates the 1-based line and column number range based on
+        /// the given start and end buffer offsets.
+        /// </summary>
+        /// <param name="startOffset">The start offset of the range.</param>
+        /// <param name="endOffset">The end offset of the range.</param>
+        /// <returns>A new BufferRange containing the positions in the offset range.</returns>
+        public BufferRange GetRangeBetweenOffsets(int startOffset, int endOffset)
+        {
+            bool foundStart = false;
+            int currentOffset = 0;
+            int searchedOffset = startOffset;
+
+            BufferPosition startPosition = new BufferPosition();
+            BufferPosition endPosition = startPosition;
+
+            int line = 0;
+            while (line < this.FileLines.Count)
+            {
+                if (searchedOffset <= currentOffset + this.FileLines[line].Length)
+                {
+                    int column = searchedOffset - currentOffset;
+
+                    // Have we already found the start position?
+                    if (foundStart)
+                    {
+                        // Assign the end position and end the search
+                        endPosition = new BufferPosition(line + 1, column + 1);
+                        break;
+                    }
+                    else
+                    {
+                        startPosition = new BufferPosition(line + 1, column + 1);
+
+                        // Do we only need to find the start position?
+                        if (startOffset == endOffset)
+                        {
+                            endPosition = startPosition;
+                            break;
+                        }
+                        else
+                        {
+                            // Since the end offset can be on the same line,
+                            // skip the line increment and continue searching
+                            // for the end position
+                            foundStart = true;
+                            searchedOffset = endOffset;
+                            continue;
+                        }
+                    }
+                }
+
+                // Increase the current offset and include newline length
+                currentOffset += this.FileLines[line].Length + Environment.NewLine.Length;
+                line++;
+            }
+
+            return new BufferRange(startPosition, endPosition);
+        }
+
         #endregion
 
         #region Private Methods

Diff for: test/PowerShellEditorServices.Test.Shared/Completion/CompleteAttributeValue.cs

@@ -0,0 +1,24 @@
+//
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+//
+
+namespace Microsoft.PowerShell.EditorServices.Test.Shared.Completion
+{
+    public class CompleteAttributeValue
+    {
+        public static readonly ScriptRegion SourceDetails = 
+            new ScriptRegion
+            {
+                File = @"Completion\CompletionExamples.psm1",
+                StartLineNumber = 16,
+                StartColumnNumber = 38
+            };
+
+        public static readonly BufferRange ExpectedRange =
+            new BufferRange(
+                new BufferPosition(16, 33),
+                new BufferPosition(16, 38));
+    }
+}
+

Diff for: test/PowerShellEditorServices.Test.Shared/Completion/CompleteFilePath.cs

@@ -0,0 +1,30 @@
+//
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+//
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Microsoft.PowerShell.EditorServices.Test.Shared.Completion
+{
+    public class CompleteFilePath
+    {
+        public static readonly ScriptRegion SourceDetails = 
+            new ScriptRegion
+            {
+                File = @"Completion\CompletionExamples.psm1",
+                StartLineNumber = 19,
+                StartColumnNumber = 25
+            };
+
+        public static readonly BufferRange ExpectedRange =
+            new BufferRange(
+                new BufferPosition(19, 15),
+                new BufferPosition(19, 25));
+    }
+}
+