Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add IsComplexTextEdit property to CompletionItem #51470

Merged
merged 6 commits into from
Mar 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/Features/Core/Portable/Completion/CommonCompletionItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ public static CompletionItem Create(
bool showsWarningIcon = false,
ImmutableDictionary<string, string> properties = null,
ImmutableArray<string> tags = default,
string inlineDescription = null)
string inlineDescription = null,
bool isComplexTextEdit = false)
{
tags = tags.NullToEmpty();

Expand Down Expand Up @@ -54,7 +55,8 @@ public static CompletionItem Create(
properties: properties,
tags: tags,
rules: rules,
inlineDescription: inlineDescription);
inlineDescription: inlineDescription,
isComplexTextEdit: isComplexTextEdit);
}

public static bool HasDescription(CompletionItem item)
Expand Down
61 changes: 54 additions & 7 deletions src/Features/Core/Portable/Completion/CompletionItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,21 @@ public sealed class CompletionItem : IComparable<CompletionItem>
/// </summary>
public CompletionItemRules Rules { get; }

/// <summary>
/// Returns true if this item's text edit requires complex resolution that
/// may impact performance. For example, an edit may be complex if it needs
/// to format or type check the resulting code, or make complex non-local
/// changes to other parts of the file.
/// Complex resolution is used so we only do the minimum amount of work
/// needed to display completion items. It is performed only for the
/// committed item just prior to commit. Thus, it is ideal for any expensive
/// completion work that does not affect the display of the item in the
/// completion list, but is necessary for committing the item.
/// An example of an item type requiring complex resolution is C#/VB
/// override completion.
/// </summary>
allisonchou marked this conversation as resolved.
Show resolved Hide resolved
public bool IsComplexTextEdit { get; }
allisonchou marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// The name of the <see cref="CompletionProvider"/> that created this
/// <see cref="CompletionItem"/>. Not available to clients. Only used by
Expand All @@ -110,7 +125,8 @@ private CompletionItem(
CompletionItemRules rules,
string displayTextPrefix,
string displayTextSuffix,
string inlineDescription)
string inlineDescription,
bool isComplexTextEdit)
{
DisplayText = displayText ?? "";
DisplayTextPrefix = displayTextPrefix ?? "";
Expand All @@ -121,6 +137,7 @@ private CompletionItem(
Properties = properties ?? ImmutableDictionary<string, string>.Empty;
Tags = tags.NullToEmpty();
Rules = rules ?? CompletionItemRules.Default;
IsComplexTextEdit = isComplexTextEdit;

if (!DisplayText.Equals(filterText, StringComparison.Ordinal))
{
Expand Down Expand Up @@ -154,6 +171,23 @@ public static CompletionItem Create(
return Create(displayText, filterText, sortText, properties, tags, rules, displayTextPrefix, displayTextSuffix, inlineDescription: null);
}

// binary back compat overload
public static CompletionItem Create(
string displayText,
string filterText,
string sortText,
ImmutableDictionary<string, string> properties,
ImmutableArray<string> tags,
CompletionItemRules rules,
string displayTextPrefix,
string displayTextSuffix,
string inlineDescription)
{
return Create(
displayText, filterText, sortText, properties, tags, rules, displayTextPrefix,
displayTextSuffix, inlineDescription, isComplexTextEdit: false);
}

public static CompletionItem Create(
string displayText,
string filterText = null,
Expand All @@ -163,7 +197,8 @@ public static CompletionItem Create(
CompletionItemRules rules = null,
string displayTextPrefix = null,
string displayTextSuffix = null,
string inlineDescription = null)
string inlineDescription = null,
bool isComplexTextEdit = false)
{
return new CompletionItem(
span: default,
Expand All @@ -175,7 +210,8 @@ public static CompletionItem Create(
rules: rules,
displayTextPrefix: displayTextPrefix,
displayTextSuffix: displayTextSuffix,
inlineDescription: inlineDescription);
inlineDescription: inlineDescription,
isComplexTextEdit: isComplexTextEdit);
}

/// <summary>
Expand Down Expand Up @@ -210,7 +246,8 @@ public static CompletionItem Create(
rules: rules,
displayTextPrefix: null,
displayTextSuffix: null,
inlineDescription: null);
inlineDescription: null,
isComplexTextEdit: false);
}

private CompletionItem With(
Expand All @@ -223,7 +260,8 @@ private CompletionItem With(
Optional<CompletionItemRules> rules = default,
Optional<string> displayTextPrefix = default,
Optional<string> displayTextSuffix = default,
Optional<string> inlineDescription = default)
Optional<string> inlineDescription = default,
Optional<bool> isComplexTextEdit = default)
{
var newSpan = span.HasValue ? span.Value : Span;
var newDisplayText = displayText.HasValue ? displayText.Value : DisplayText;
Expand All @@ -235,6 +273,7 @@ private CompletionItem With(
var newRules = rules.HasValue ? rules.Value : Rules;
var newDisplayTextPrefix = displayTextPrefix.HasValue ? displayTextPrefix.Value : DisplayTextPrefix;
var newDisplayTextSuffix = displayTextSuffix.HasValue ? displayTextSuffix.Value : DisplayTextSuffix;
var newIsComplexTextEdit = isComplexTextEdit.HasValue ? isComplexTextEdit.Value : IsComplexTextEdit;

if (newSpan == Span &&
newDisplayText == DisplayText &&
Expand All @@ -245,7 +284,8 @@ private CompletionItem With(
newRules == Rules &&
newDisplayTextPrefix == DisplayTextPrefix &&
newDisplayTextSuffix == DisplayTextSuffix &&
newInlineDescription == InlineDescription)
newInlineDescription == InlineDescription &&
newIsComplexTextEdit == IsComplexTextEdit)
{
return this;
}
Expand All @@ -260,7 +300,8 @@ private CompletionItem With(
rules: newRules,
displayTextPrefix: newDisplayTextPrefix,
displayTextSuffix: newDisplayTextSuffix,
inlineDescription: newInlineDescription)
inlineDescription: newInlineDescription,
isComplexTextEdit: newIsComplexTextEdit)
{
AutomationText = AutomationText,
ProviderName = ProviderName,
Expand Down Expand Up @@ -350,6 +391,12 @@ public CompletionItem AddTag(string tag)
public CompletionItem WithRules(CompletionItemRules rules)
=> With(rules: rules);

/// <summary>
/// Creates a copy of this <see cref="CompletionItem"/> with the <see cref="IsComplexTextEdit"/> property changed.
/// </summary>
public CompletionItem WithIsComplexTextEdit(bool isComplexTextEdit)
=> With(isComplexTextEdit: isComplexTextEdit);

private string _entireDisplayText;

int IComparable<CompletionItem>.CompareTo(CompletionItem other)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ public static CompletionItem Create(
symbols: ImmutableArray.Create(symbol),
contextPosition: descriptionPosition,
properties: props,
rules: rules);
rules: rules,
isComplexTextEdit: true);
}

public static Task<CompletionDescription> GetDescriptionAsync(CompletionItem item, Document document, CancellationToken cancellationToken)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ private static CompletionItem CreateWorker(
string filterText = null,
SupportedPlatformData supportedPlatforms = null,
ImmutableDictionary<string, string> properties = null,
ImmutableArray<string> tags = default)
ImmutableArray<string> tags = default,
bool isComplexTextEdit = false)
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
{
var props = properties ?? ImmutableDictionary<string, string>.Empty;

Expand All @@ -54,7 +55,8 @@ private static CompletionItem CreateWorker(
glyph: firstSymbol.GetGlyph(),
showsWarningIcon: supportedPlatforms != null,
properties: props,
tags: tags);
tags: tags,
isComplexTextEdit: isComplexTextEdit);

item = WithSupportedPlatforms(item, supportedPlatforms);
return symbolEncoder(symbols, item);
Expand Down Expand Up @@ -267,7 +269,8 @@ public static CompletionItem CreateWithSymbolId(
string filterText = null,
SupportedPlatformData supportedPlatforms = null,
ImmutableDictionary<string, string> properties = null,
ImmutableArray<string> tags = default)
ImmutableArray<string> tags = default,
bool isComplexTextEdit = false)
{
return CreateWithSymbolId(
displayText,
Expand All @@ -280,7 +283,8 @@ public static CompletionItem CreateWithSymbolId(
filterText,
supportedPlatforms,
properties,
tags);
tags,
isComplexTextEdit);
}

public static CompletionItem CreateWithSymbolId(
Expand All @@ -294,12 +298,13 @@ public static CompletionItem CreateWithSymbolId(
string filterText = null,
SupportedPlatformData supportedPlatforms = null,
ImmutableDictionary<string, string> properties = null,
ImmutableArray<string> tags = default)
ImmutableArray<string> tags = default,
bool isComplexTextEdit = false)
{
return CreateWorker(
displayText, displayTextSuffix, symbols, rules, contextPosition,
s_addSymbolEncoding, sortText, insertionText,
filterText, supportedPlatforms, properties, tags);
filterText, supportedPlatforms, properties, tags, isComplexTextEdit);
}

public static CompletionItem CreateWithNameAndKind(
Expand All @@ -313,12 +318,13 @@ public static CompletionItem CreateWithNameAndKind(
string filterText = null,
SupportedPlatformData supportedPlatforms = null,
ImmutableDictionary<string, string> properties = null,
ImmutableArray<string> tags = default)
ImmutableArray<string> tags = default,
bool isComplexTextEdit = false)
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
{
return CreateWorker(
displayText, displayTextSuffix, symbols, rules, contextPosition,
s_addSymbolInfo, sortText, insertionText,
filterText, supportedPlatforms, properties, tags);
filterText, supportedPlatforms, properties, tags, isComplexTextEdit);
}

internal static string GetSymbolName(CompletionItem item)
Expand Down
1 change: 0 additions & 1 deletion src/Features/Core/Portable/PublicAPI.Shipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,6 @@ static Microsoft.CodeAnalysis.Completion.CompletionChange.Create(Microsoft.CodeA
static Microsoft.CodeAnalysis.Completion.CompletionChange.Create(System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Text.TextChange> textChanges, int? newPosition = null, bool includesCommitCharacter = false) -> Microsoft.CodeAnalysis.Completion.CompletionChange
static Microsoft.CodeAnalysis.Completion.CompletionDescription.Create(System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.TaggedText> taggedParts) -> Microsoft.CodeAnalysis.Completion.CompletionDescription
static Microsoft.CodeAnalysis.Completion.CompletionDescription.FromText(string text) -> Microsoft.CodeAnalysis.Completion.CompletionDescription
static Microsoft.CodeAnalysis.Completion.CompletionItem.Create(string displayText, string filterText = null, string sortText = null, System.Collections.Immutable.ImmutableDictionary<string, string> properties = null, System.Collections.Immutable.ImmutableArray<string> tags = default(System.Collections.Immutable.ImmutableArray<string>), Microsoft.CodeAnalysis.Completion.CompletionItemRules rules = null, string displayTextPrefix = null, string displayTextSuffix = null, string inlineDescription = null) -> Microsoft.CodeAnalysis.Completion.CompletionItem
static Microsoft.CodeAnalysis.Completion.CompletionItem.Create(string displayText, string filterText, string sortText, Microsoft.CodeAnalysis.Text.TextSpan span, System.Collections.Immutable.ImmutableDictionary<string, string> properties, System.Collections.Immutable.ImmutableArray<string> tags, Microsoft.CodeAnalysis.Completion.CompletionItemRules rules) -> Microsoft.CodeAnalysis.Completion.CompletionItem
static Microsoft.CodeAnalysis.Completion.CompletionItem.Create(string displayText, string filterText, string sortText, System.Collections.Immutable.ImmutableDictionary<string, string> properties, System.Collections.Immutable.ImmutableArray<string> tags, Microsoft.CodeAnalysis.Completion.CompletionItemRules rules) -> Microsoft.CodeAnalysis.Completion.CompletionItem
static Microsoft.CodeAnalysis.Completion.CompletionItem.Create(string displayText, string filterText, string sortText, System.Collections.Immutable.ImmutableDictionary<string, string> properties, System.Collections.Immutable.ImmutableArray<string> tags, Microsoft.CodeAnalysis.Completion.CompletionItemRules rules, string displayTextPrefix, string displayTextSuffix) -> Microsoft.CodeAnalysis.Completion.CompletionItem
Expand Down
4 changes: 4 additions & 0 deletions src/Features/Core/Portable/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
const Microsoft.CodeAnalysis.TextTags.Record = "Record" -> string
Microsoft.CodeAnalysis.Completion.CompletionItem.IsComplexTextEdit.get -> bool
Microsoft.CodeAnalysis.Completion.CompletionItem.WithIsComplexTextEdit(bool isComplexTextEdit) -> Microsoft.CodeAnalysis.Completion.CompletionItem
static Microsoft.CodeAnalysis.Completion.CompletionChange.Create(Microsoft.CodeAnalysis.Text.TextChange textChange, System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Text.TextChange> textChanges, int? newPosition = null, bool includesCommitCharacter = false) -> Microsoft.CodeAnalysis.Completion.CompletionChange
static Microsoft.CodeAnalysis.Completion.CompletionItem.Create(string displayText, string filterText = null, string sortText = null, System.Collections.Immutable.ImmutableDictionary<string, string> properties = null, System.Collections.Immutable.ImmutableArray<string> tags = default(System.Collections.Immutable.ImmutableArray<string>), Microsoft.CodeAnalysis.Completion.CompletionItemRules rules = null, string displayTextPrefix = null, string displayTextSuffix = null, string inlineDescription = null, bool isComplexTextEdit = false) -> Microsoft.CodeAnalysis.Completion.CompletionItem
static Microsoft.CodeAnalysis.Completion.CompletionItem.Create(string displayText, string filterText, string sortText, System.Collections.Immutable.ImmutableDictionary<string, string> properties, System.Collections.Immutable.ImmutableArray<string> tags, Microsoft.CodeAnalysis.Completion.CompletionItemRules rules, string displayTextPrefix, string displayTextSuffix, string inlineDescription) -> Microsoft.CodeAnalysis.Completion.CompletionItem
Original file line number Diff line number Diff line change
Expand Up @@ -215,13 +215,6 @@ static async Task<TCompletionItem> CreateCompletionItemAsync<TCompletionItem>(
var completeDisplayText = stringBuilder.ToString();
stringBuilder.Clear();

// The TextEdits for override and partial method completions are provided in the resolve handler.
// We do not provide them in this handler.
// HACK: We should not be accessing the completion item's properties directly.
// See https://github.com/dotnet/roslyn/issues/51396.
item.Properties.TryGetValue("Modifiers", out var modifier);
var isOverrideOrPartialMethodCompletion = modifier != null && (modifier.Contains("Override") || modifier.Contains("Partial"));
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved

var completionItem = new TCompletionItem
{
Label = completeDisplayText,
Expand All @@ -239,13 +232,13 @@ static async Task<TCompletionItem> CreateCompletionItemAsync<TCompletionItem>(
Preselect = item.Rules.SelectionBehavior == CompletionItemSelectionBehavior.HardSelection,
};

// Override and partial method completions are always populated in the resolve handler, so we
// leave both TextEdit and InsertText unpopulated in these cases.
if (isOverrideOrPartialMethodCompletion)
// Complex text edits (e.g. override and partial method completions) are always populated in the
// resolve handler, so we leave both TextEdit and InsertText unpopulated in these cases.
if (item.IsComplexTextEdit)
{
// Razor C# is currently the only language client that supports LSP.InsertTextFormat.Snippet.
// We can enable it for regular C# once LSP is used for local completion.
if (snippetsSupported && clientName == ProtocolConstants.RazorCSharp)
if (snippetsSupported)
{
completionItem.InsertTextFormat = LSP.InsertTextFormat.Snippet;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,16 @@ private static CompletionResolveData GetCompletionResolveData(LSP.CompletionItem
.Select(tp => new ClassifiedTextRun(tp.Tag.ToClassificationTypeName(), tp.Text)));
}

// We compute the TextEdit resolves for override and partial method completions here.
// Lazily resolving TextEdits is technically a violation of the LSP spec, but is
// currently supported by the VS client anyway. Once the VS client adheres to the spec,
// this logic will need to change and VS will need to provide official support for
// TextEdit resolution in some form.
if (completionItem.InsertText == null && completionItem.TextEdit == null)
// We compute the TextEdit resolves for complex text edits (e.g. override and partial
// method completions) here. Lazily resolving TextEdits is technically a violation of
// the LSP spec, but is currently supported by the VS client anyway. Once the VS client
// adheres to the spec, this logic will need to change and VS will need to provide
// official support for TextEdit resolution in some form.
if (selectedItem.IsComplexTextEdit)
{
Contract.ThrowIfTrue(completionItem.InsertText != null);
Contract.ThrowIfTrue(completionItem.TextEdit != null);

var snippetsSupported = context.ClientCapabilities.TextDocument?.Completion?.CompletionItem?.SnippetSupport ?? false;

completionItem.TextEdit = await GenerateTextEditAsync(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ void M()
}

[Fact]
[WorkItem(51125, "https://github.com/dotnet/roslyn/issues/51125")]
public async Task TestResolveOverridesCompletionItemAsync()
{
var markup =
Expand Down Expand Up @@ -88,6 +89,7 @@ class B : A
}

[Fact]
[WorkItem(51125, "https://github.com/dotnet/roslyn/issues/51125")]
public async Task TestResolveOverridesCompletionItem_SnippetsEnabledAsync()
{
var markup =
Expand Down Expand Up @@ -138,6 +140,7 @@ class B : A
}

[Fact]
[WorkItem(51125, "https://github.com/dotnet/roslyn/issues/51125")]
public async Task TestResolveOverridesCompletionItem_SnippetsEnabled_CaretOutOfSnippetScopeAsync()
{
var markup =
Expand Down