Skip to content

Commit

Permalink
Merge pull request #1957 from 333fred/property-completion
Browse files Browse the repository at this point in the history 
Property completion
  • Loading branch information
@filipw
filipw authored Sep 28, 2020
2 parents ed3d417 + 74a8cc3 commit 2b5d02f
Show file tree
Hide file tree
Showing 5 changed files with 257 additions and 81 deletions.
12 changes: 7 additions & 5 deletions src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#nullable enable

using System;
using System.Collections.Generic;

namespace OmniSharp.Models.v1.Completion
Expand Down Expand Up @@ -55,15 +56,16 @@ public class CompletionItem
public string? FilterText { get; set; }

/// <summary>
/// A string that should be inserted into a document when selecting
/// this completion.When null or empty the label is used.
/// The format of <see cref="InsertText"/>. This applies to both <see cref="InsertText"/> and
/// <see cref="TextEdit"/>.<see cref="LinePositionSpanTextChange.NewText"/>.
/// </summary>
public string? InsertText { get; set; }
public InsertTextFormat? InsertTextFormat { get; set; }

/// <summary>
/// The format of <see cref="InsertText"/>.
/// An edit which is applied to a document when selecting this completion. When an edit is provided the value of
/// <see cref="InsertText"/> is ignored.
/// </summary>
public InsertTextFormat? InsertTextFormat { get; set; }
public LinePositionSpanTextChange? TextEdit { get; set; }

/// <summary>
/// An optional set of characters that when pressed while this completion is active will accept it first and
Expand Down
119 changes: 82 additions & 37 deletions src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public async Task<CompletionResponse> Handle(CompletionRequest request)
}

var triggerCharactersBuilder = ImmutableArray.CreateBuilder<char>(completions.Rules.DefaultCommitCharacters.Length);
var completionsBuilder = ImmutableArray.CreateBuilder<CompletionItem>();
var completionsBuilder = ImmutableArray.CreateBuilder<CompletionItem>(completions.Items.Length);

// If we don't encounter any unimported types, and the completion context thinks that some would be available, then
// that completion provider is still creating the cache. We'll mark this completion list as not completed, and the
Expand All @@ -162,6 +162,9 @@ public async Task<CompletionResponse> Handle(CompletionRequest request)
bool expectingImportedItems = expandedItemsAvailable && _workspace.Options.GetOption(CompletionItemExtensions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp) == true;
var syntax = await document.GetSyntaxTreeAsync();

var replacingSpanStartPosition = sourceText.Lines.GetLinePosition(typedSpan.Start);
var replacingSpanEndPosition = sourceText.Lines.GetLinePosition(typedSpan.End);

for (int i = 0; i < completions.Items.Length; i++)
{
var completion = completions.Items[i];
Expand Down Expand Up @@ -243,17 +246,44 @@ public async Task<CompletionResponse> Handle(CompletionRequest request)

var change = await completionService.GetChangeAsync(document, completion);

// If the span we're using to key the completion off is the same as the replacement
// span, then we don't need to do anything special, just snippitize the text and
// exit
if (typedSpan == change.TextChange.Span)
{
// If the span we're using to key the completion off is the same as the replacement
// span, then we don't need to do anything special, just snippitize the text and
// exit
(insertText, insertTextFormat) = getAdjustedInsertTextWithPosition(change, position, newOffset: 0);
break;
}

if (change.TextChange.Span.Start > typedSpan.Start)
{
// If the span we're using to key the replacement span is within the original typed span
// span, we want to prepend the missing text from the original typed text to here. The
// reason is that some lsp clients, such as vscode, use the range from the text edit as
// the selector for what filter text to use. This can lead to odd scenarios where invoking
// completion and typing `EQ` will bring up the Equals override, but then dismissing and
// reinvoking completion will have a range that just replaces the Q. Vscode will then consider
// that capital Q to be the start of the filter text, and filter out the Equals overload
// leaving the user with no completion and no explanation
Debug.Assert(change.TextChange.Span.End == typedSpan.End);
var prefix = typedText.Substring(0, change.TextChange.Span.Start - typedSpan.Start);

(insertText, insertTextFormat) = getAdjustedInsertTextWithPosition(change, position, newOffset: 0, prefix);
break;
}

int additionalEditEndOffset;
(additionalTextEdits, additionalEditEndOffset) = await GetAdditionalTextEdits(change, sourceText, (CSharpParseOptions)syntax!.Options, typedSpan, completion.DisplayText, providerName);
(additionalTextEdits, additionalEditEndOffset) = GetAdditionalTextEdits(change, sourceText, (CSharpParseOptions)syntax!.Options, typedSpan, completion.DisplayText, providerName);

if (additionalEditEndOffset < 0)
{
// We couldn't find the position of the change in the new text. This shouldn't happen in normal cases,
// but handle as best we can in release
Debug.Fail("Couldn't find the new cursor position in the replacement text!");
additionalTextEdits = null;
insertText = completion.DisplayText;
break;
}

// Now that we have the additional edit, adjust the rest of the new text
(insertText, insertTextFormat) = getAdjustedInsertTextWithPosition(change, position, additionalEditEndOffset);
Expand Down Expand Up @@ -281,7 +311,14 @@ public async Task<CompletionResponse> Handle(CompletionRequest request)
completionsBuilder.Add(new CompletionItem
{
Label = completion.DisplayTextPrefix + completion.DisplayText + completion.DisplayTextSuffix,
InsertText = insertText,
TextEdit = new LinePositionSpanTextChange
{
NewText = insertText,
StartLine = replacingSpanStartPosition.Line,
StartColumn = replacingSpanStartPosition.Character,
EndLine = replacingSpanEndPosition.Line,
EndColumn = replacingSpanEndPosition.Character
},
InsertTextFormat = insertTextFormat,
AdditionalTextEdits = additionalTextEdits,
// Ensure that unimported items are sorted after things already imported.
Expand All @@ -298,7 +335,7 @@ public async Task<CompletionResponse> Handle(CompletionRequest request)
return new CompletionResponse
{
IsIncomplete = !seenUnimportedCompletions && expectingImportedItems,
Items = completionsBuilder.ToImmutableArray()
Items = completionsBuilder.Capacity == completionsBuilder.Count ? completionsBuilder.MoveToImmutable() : completionsBuilder.ToImmutable()
};

CompletionTrigger getCompletionTrigger(bool includeTriggerCharacter)
Expand Down Expand Up @@ -369,7 +406,8 @@ static ImmutableArray<char> buildCommitCharacters(CSharpCompletionList completio
static (string, InsertTextFormat) getAdjustedInsertTextWithPosition(
CompletionChange change,
int originalPosition,
int newOffset)
int newOffset,
string? prependText = null)
{
// We often have to trim part of the given change off the front, but we
// still want to turn the resulting change into a snippet and control
Expand All @@ -385,21 +423,14 @@ static ImmutableArray<char> buildCommitCharacters(CSharpCompletionList completio
if (!(change.NewPosition is int newPosition)
|| newPosition >= (change.TextChange.Span.Start + newText.Length))
{
return (newText.Substring(newOffset), InsertTextFormat.PlainText);
}

if (newPosition < (originalPosition + newOffset))
{
Debug.Fail($"Unknown case of attempting to move cursor before the text that needs to be cut off. Requested cutoff: {newOffset}. New Position: {newPosition}");
// Gracefully handle as best we can in release
return (newText.Substring(newOffset), InsertTextFormat.PlainText);
return (prependText + newText.Substring(newOffset), InsertTextFormat.PlainText);
}

// Roslyn wants to move the cursor somewhere inside the result. Substring from the
// requested start to the new position, and from the new position to the end of the
// string.
int midpoint = newPosition - change.TextChange.Span.Start;
var beforeText = LspSnippetHelpers.Escape(newText.Substring(newOffset, midpoint - newOffset));
var beforeText = LspSnippetHelpers.Escape(prependText + newText.Substring(newOffset, midpoint - newOffset));
var afterText = LspSnippetHelpers.Escape(newText.Substring(midpoint));

return (beforeText + "$0" + afterText, InsertTextFormat.Snippet);
Expand Down Expand Up @@ -457,7 +488,12 @@ public async Task<CompletionResolveResponse> Handle(CompletionResolveRequest req
var sourceText = await document.GetTextAsync();
var typedSpan = completionService.GetDefaultCompletionListSpan(sourceText, position);
var change = await completionService.GetChangeAsync(document, lastCompletionItem, typedSpan);
(request.Item.AdditionalTextEdits, _) = await GetAdditionalTextEdits(change, sourceText, (CSharpParseOptions)syntax!.Options, typedSpan, lastCompletionItem.DisplayText, providerName);
var (additionalTextEdits, offset) = GetAdditionalTextEdits(change, sourceText, (CSharpParseOptions)syntax!.Options, typedSpan, lastCompletionItem.DisplayText, providerName);
if (offset > 0)
{
Debug.Assert(additionalTextEdits is object);
request.Item.AdditionalTextEdits = additionalTextEdits;
}
break;
}

Expand All @@ -467,7 +503,7 @@ public async Task<CompletionResolveResponse> Handle(CompletionResolveRequest req
};
}

private async ValueTask<(IReadOnlyList<LinePositionSpanTextChange> edits, int endOffset)> GetAdditionalTextEdits(
private (IReadOnlyList<LinePositionSpanTextChange>? edits, int endOffset) GetAdditionalTextEdits(
CompletionChange change,
SourceText sourceText,
CSharpParseOptions parseOptions,
Expand All @@ -480,15 +516,15 @@ public async Task<CompletionResolveResponse> Handle(CompletionResolveRequest req
// as the additional edit is not allowed to overlap with the insertion point.
var additionalEditStartPosition = sourceText.Lines.GetLinePosition(change.TextChange.Span.Start);
var additionalEditEndPosition = sourceText.Lines.GetLinePosition(typedSpan.Start - 1);
int additionalEditEndOffset = await getAdditionalTextEditEndOffset(change, sourceText, parseOptions, typedSpan, completionDisplayText, providerName);
int additionalEditEndOffset = getAdditionalTextEditEndOffset(change, sourceText, parseOptions, completionDisplayText, providerName);
if (additionalEditEndOffset < 1)
{
// The first index of this was either 0 and the edit span was wrong,
// or it wasn't found at all. In this case, just do the best we can:
// send the whole string wtih no additional edits and log a warning.
_logger.LogWarning("Could not find the first index of the display text.\nDisplay text: {0}.\nCompletion Text: {1}",
completionDisplayText, change.TextChange.NewText);
return default;
return (null, -1);
}

return (ImmutableArray.Create(new LinePositionSpanTextChange
Expand All @@ -501,20 +537,12 @@ public async Task<CompletionResolveResponse> Handle(CompletionResolveRequest req
EndColumn = additionalEditEndPosition.Character,
}), additionalEditEndOffset);

static async ValueTask<int> getAdditionalTextEditEndOffset(CompletionChange change, SourceText sourceText, CSharpParseOptions parseOptions, TextSpan typedSpan, string completionDisplayText, string providerName)
static int getAdditionalTextEditEndOffset(CompletionChange change, SourceText sourceText, CSharpParseOptions parseOptions, string completionDisplayText, string providerName)
{
// For many simple cases, we can just find the first or last index of the completionDisplayText and that's good
// enough
int endOffset = (providerName == CompletionItemExtensions.ExtensionMethodImportCompletionProvider ||
providerName == CompletionItemExtensions.TypeImportCompletionProvider)
// Import completion will put the displaytext at the end of the line, override completion will
// put it at the front.
? change.TextChange.NewText!.LastIndexOf(completionDisplayText)
: change.TextChange.NewText!.IndexOf(completionDisplayText);

if (endOffset > -1)
if (providerName == CompletionItemExtensions.ExtensionMethodImportCompletionProvider ||
providerName == CompletionItemExtensions.TypeImportCompletionProvider)
{
return endOffset;
return change.TextChange.NewText!.LastIndexOf(completionDisplayText);
}

// The DisplayText wasn't in the final string. This can happen in a few cases:
Expand All @@ -530,13 +558,30 @@ static async ValueTask<int> getAdditionalTextEditEndOffset(CompletionChange chan
// to adjust the API here.
//
// In order to find the correct location here, we parse the change. The location
// of the name of the last method is the correct location in the string.
// of the method or property that contains the new cursor position is the location
// of the new changes
Debug.Assert(providerName == CompletionItemExtensions.OverrideCompletionProvider ||
providerName == CompletionItemExtensions.PartialMethodCompletionProvider);
Debug.Assert(change.NewPosition.HasValue);

var parsedTree = CSharpSyntaxTree.ParseText(sourceText.WithChanges(change.TextChange).ToString(), parseOptions);

var tokenOfNewPosition = parsedTree.GetRoot().FindToken(change.NewPosition!.Value);
var finalNode = tokenOfNewPosition.Parent;
while (finalNode != null)
{
switch (finalNode)
{
case MethodDeclarationSyntax decl:
return decl.Identifier.SpanStart - change.TextChange.Span.Start;
case PropertyDeclarationSyntax prop:
return prop.Identifier.SpanStart - change.TextChange.Span.Start;
}

finalNode = finalNode.Parent;
}

var parsedTree = CSharpSyntaxTree.ParseText(change.TextChange.NewText, parseOptions);
var lastMethodDecl = (await parsedTree.GetRootAsync()).DescendantNodes().OfType<MethodDeclarationSyntax>().Last();
return lastMethodDecl.Identifier.SpanStart;
return -1;
}
}
}
Expand Down
Loading

0 comments on commit 2b5d02f

Please sign in to comment.