From 04dc6eab19594c8b4bff48c7912a57a8b1b20f27 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Sun, 9 Aug 2020 21:25:35 -0700 Subject: [PATCH 1/5] Start moving omnisharp to directly using Roslyn's completion service, with a separate resolve step Today, Omnisharp uses Roslyn's completion service to gather completions, but doesn't then use the other completion helpers to fill in the rest of the detail of the completion. Additionally, Omnisharp does a bunch of work when creating and populating completions, and this can cause slow and inconsistent behavior when invoking completions in the middle of a pre-typed word. Some current issues: * vscode fuzzy-matching changes based on whether you invoke completion after typing a few characters. * Completions with large numbers of elements can take a bit of time to load, as Omnisharp does filtering and documentation building for all of them. * Completion is not well setup to play nicely with LSP, which uses a much different interface. * Completion is not capable of providing unimported types, particularly because providing the edits would be extremely expensive if not done only on request. To solve these issues, this adds a new completion service that mirrors the structure of LSP completion and completion/resolve: * On request, only the completion item and initial edit are provided. * A new response service is added, which looks up documentation when a particular item is selected in the completion window. * This response service will additionally be able to provide extra edits, such as inserting the rest of an override statement or adding imports for unimported types, but these are not yet implemented. --- .../Models/v1/Completion/CompletionItem.cs | 138 ++++ .../Models/v1/Completion/CompletionRequest.cs | 42 + .../v1/Completion/CompletionResolveRequest.cs | 12 + .../Completion/CompletionResolveResponse.cs | 13 + .../v1/Completion/CompletionResponse.cs | 23 + .../OmniSharpEndpoints.cs | 3 + .../Helpers/MarkdownHelpers.cs | 201 +++++ .../Helpers/SourceTextExtensions.cs | 13 + .../Services/Completion/CompletionService.cs | 272 +++++++ .../Services/QuickInfoProvider.cs | 128 +--- .../Utilities/ImmutableArrayExtensions.cs | 50 +- .../Utilities/MarkdownHelpers.cs | 16 - .../CompletionFacts.cs | 715 ++++++++++++++++++ 13 files changed, 1486 insertions(+), 140 deletions(-) create mode 100644 src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs create mode 100644 src/OmniSharp.Abstractions/Models/v1/Completion/CompletionRequest.cs create mode 100644 src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResolveRequest.cs create mode 100644 src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResolveResponse.cs create mode 100644 src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResponse.cs create mode 100644 src/OmniSharp.Roslyn.CSharp/Helpers/MarkdownHelpers.cs create mode 100644 src/OmniSharp.Roslyn.CSharp/Helpers/SourceTextExtensions.cs create mode 100644 src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs delete mode 100644 src/OmniSharp.Shared/Utilities/MarkdownHelpers.cs create mode 100644 tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs diff --git a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs new file mode 100644 index 0000000000..33809c375a --- /dev/null +++ b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs @@ -0,0 +1,138 @@ +#nullable enable + +using System.Collections.Immutable; +using Newtonsoft.Json; + +namespace OmniSharp.Models.v1.Completion +{ + public class CompletionItem + { + /// + /// The label of this completion item. By default + /// also the text that is inserted when selecting + /// this completion. + /// + [JsonProperty("label")] + public string Label { get; set; } = null!; + + /// + /// The kind of this completion item. Based of the kind + /// an icon is chosen by the editor.The standardized set + /// of available values is defined in + /// + [JsonProperty("kind")] + public CompletionItemKind Kind { get; set; } + + /// + /// Tags for this completion item + /// + [JsonProperty("tags")] + public ImmutableArray? Tags { get; set; } + + /// + /// A human-readable string with additional information + /// about this item, like type or symbol information + /// + [JsonProperty("detail")] + public string? Detail { get; set; } + + /// + /// A human-readable string that represents a doc-comment. This is + /// formatted as markdown. + /// + [JsonProperty("documentation")] + public string? Documentation { get; set; } + + /// + /// Select this item when showing. + /// + [JsonProperty("preselect")] + public bool Preselect { get; set; } + + /// + /// A string that should be used when comparing this item + /// with other items. When null or empty the label is used. + /// + [JsonProperty("sortText")] + public string? SortText { get; set; } + + /// + /// A string that should be used when filtering a set of + /// completion items. When null or empty the label is used. + /// + [JsonProperty("filterText")] + public string? FilterText { get; set; } + + /// + /// A string that should be inserted into a document when selecting + /// this completion.When null or empty the label is used. + /// + [JsonProperty("insertText")] + public string? InsertText { get; set; } + + /// + /// The format of . + /// + [JsonProperty("insertTextFormat")] + public InsertTextFormat? InsertTextFormat { get; set; } + + /// + /// An optional set of characters that when pressed while this completion is active will accept it first and + /// then type that character. + /// + [JsonProperty("commitCharacters")] + public ImmutableArray? CommitCharacters { get; set; } + + /// + /// Index in the completions list that this completion occurred. + /// + [JsonProperty("data")] + public int Data { get; set; } + + public override string ToString() + { + return $"{{ {nameof(Label)} = {Label}, {nameof(CompletionItemKind)} = {Kind} }}"; + } + } + + public enum CompletionItemKind + { + Text = 1, + Method = 2, + Function = 3, + Constructor = 4, + Field = 5, + Variable = 6, + Class = 7, + Interface = 8, + Module = 9, + Property = 10, + Unit = 11, + Value = 12, + Enum = 13, + Keyword = 14, + Snippet = 15, + Color = 16, + File = 17, + Reference = 18, + Folder = 19, + EnumMember = 20, + Constant = 21, + Struct = 22, + Event = 23, + Operator = 24, + TypeParameter = 25, + } + + public enum CompletionItemTag + { + Deprecated = 1, + } + + public enum InsertTextFormat + { + PlainText = 1, + // TODO: Support snippets + Snippet = 2, + } +} diff --git a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionRequest.cs b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionRequest.cs new file mode 100644 index 0000000000..671cfa549c --- /dev/null +++ b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionRequest.cs @@ -0,0 +1,42 @@ +#nullable enable + +using System.ComponentModel; +using OmniSharp.Mef; + +namespace OmniSharp.Models.v1.Completion +{ + [OmniSharpEndpoint(OmniSharpEndpoints.Completion, typeof(CompletionRequest), typeof(CompletionResponse))] + public class CompletionRequest : Request + { + /// + /// How the completion was triggered + /// + public CompletionTriggerKind CompletionTrigger { get; set; } + + /// + /// The character that triggered completion if + /// is . + /// otherwise. + /// + public char? TriggerCharacter { get; set; } + } + + public enum CompletionTriggerKind + { + /// + /// Completion was triggered by typing an identifier (24x7 code + /// complete), manual invocation (e.g Ctrl+Space) or via API + /// + Invoked = 1, + /// + /// Completion was triggered by a trigger character specified by + /// the `triggerCharacters` properties of the `CompletionRegistrationOptions`. + /// + TriggerCharacter = 2, + + // We don't need to support incomplete completion lists that need to be recomputed + // later, but this is reserving the number to match LSP if we need it later. + [EditorBrowsable(EditorBrowsableState.Never)] + TriggerForIncompleteCompletions = 3 + } +} diff --git a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResolveRequest.cs b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResolveRequest.cs new file mode 100644 index 0000000000..9ebf78e411 --- /dev/null +++ b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResolveRequest.cs @@ -0,0 +1,12 @@ +#nullable enable + +using OmniSharp.Mef; + +namespace OmniSharp.Models.v1.Completion +{ + [OmniSharpEndpoint(OmniSharpEndpoints.CompletionResolve, typeof(CompletionResolveRequest), typeof(CompletionResolveResponse))] + public class CompletionResolveRequest : IRequest + { + public CompletionItem Item { get; set; } = null!; + } +} diff --git a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResolveResponse.cs b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResolveResponse.cs new file mode 100644 index 0000000000..713077fcf6 --- /dev/null +++ b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResolveResponse.cs @@ -0,0 +1,13 @@ +#nullable enable + +using Newtonsoft.Json; +using Newtonsoft.Json.Serialization; + +namespace OmniSharp.Models.v1.Completion +{ + public class CompletionResolveResponse + { + [JsonProperty("item")] + public CompletionItem? Item { get; set; } + } +} diff --git a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResponse.cs b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResponse.cs new file mode 100644 index 0000000000..5f65dfcc70 --- /dev/null +++ b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResponse.cs @@ -0,0 +1,23 @@ +#nullable enable + +using System.Collections.Generic; +using System.Collections.Immutable; +using Newtonsoft.Json; + +namespace OmniSharp.Models.v1.Completion +{ + public class CompletionResponse + { + /// + /// If true, this list is not complete. Further typing should result in recomputing the list. + /// + [JsonProperty("isIncomplete")] + public bool IsIncomplete { get; set; } + + /// + /// The completion items. + /// + [JsonProperty("items")] + public ImmutableArray Items { get; set; } + } +} diff --git a/src/OmniSharp.Abstractions/OmniSharpEndpoints.cs b/src/OmniSharp.Abstractions/OmniSharpEndpoints.cs index b20b5c3787..f8b8c98d45 100644 --- a/src/OmniSharp.Abstractions/OmniSharpEndpoints.cs +++ b/src/OmniSharp.Abstractions/OmniSharpEndpoints.cs @@ -47,6 +47,9 @@ public static class OmniSharpEndpoints public const string ReAnalyze = "/reanalyze"; public const string QuickInfo = "/quickinfo"; + public const string Completion = "/completion"; + public const string CompletionResolve = "/completion/resolve"; + public static class V2 { public const string GetCodeActions = "/v2/getcodeactions"; diff --git a/src/OmniSharp.Roslyn.CSharp/Helpers/MarkdownHelpers.cs b/src/OmniSharp.Roslyn.CSharp/Helpers/MarkdownHelpers.cs new file mode 100644 index 0000000000..555a20ef99 --- /dev/null +++ b/src/OmniSharp.Roslyn.CSharp/Helpers/MarkdownHelpers.cs @@ -0,0 +1,201 @@ +using System.Collections.Immutable; +using System.Diagnostics; +using System.Linq; +using System.Text; +using System.Text.RegularExpressions; +using Microsoft.CodeAnalysis; +using OmniSharp.Options; + +namespace OmniSharp.Roslyn.CSharp.Helpers +{ + public static class MarkdownHelpers + { + private static Regex EscapeRegex = new Regex(@"([\\`\*_\{\}\[\]\(\)#+\-\.!])", RegexOptions.Compiled); + + public static string Escape(string markdown) + { + if (markdown == null) + return null; + return EscapeRegex.Replace(markdown, @"\$1"); + } + + /// + /// Indicates the start of a text container. The elements after through (but not + /// including) the matching are rendered in a rectangular block which is positioned + /// as an inline element relative to surrounding elements. The text of the element + /// itself precedes the content of the container, and is typically a bullet or number header for an item in a + /// list. + /// + private const string ContainerStart = nameof(ContainerStart); + /// + /// Indicates the end of a text container. See . + /// + private const string ContainerEnd = nameof(ContainerEnd); + + public static void TaggedTextToMarkdown(ImmutableArray taggedParts, StringBuilder stringBuilder, FormattingOptions formattingOptions, out int lastIndex, bool untilLineBreak = false) + { + bool isInCodeBlock = false; + bool brokeLine = true; + lastIndex = 0; + for (int i = 0; i < taggedParts.Length; i++) + { + var current = taggedParts[i]; + lastIndex = i; + + if (brokeLine) + { + Debug.Assert(!isInCodeBlock); + // If we're on a new line and there are no text parts in the upcoming line, then we + // can format the whole line as C# code instead of plaintext. Otherwise, we need to + // intermix, and can only use simple ` codefences + brokeLine = false; + bool canFormatAsBlock = false; + for (int j = i; j < taggedParts.Length; j++) + { + switch (taggedParts[j].Tag) + { + case TextTags.Text: + canFormatAsBlock = false; + goto endOfLineOrTextFound; + + case ContainerStart: + case ContainerEnd: + case TextTags.LineBreak: + goto endOfLineOrTextFound; + + default: + // If the block is just newlines, then we don't want to format that as + // C# code. So, we default to false, set it to true if there's actually + // content on the line, then set to false again if Text content is + // encountered. + canFormatAsBlock = true; + continue; + } + } + + endOfLineOrTextFound: + if (canFormatAsBlock) + { + stringBuilder.Append("```csharp"); + stringBuilder.Append(formattingOptions.NewLine); + for (; i < taggedParts.Length; i++) + { + current = taggedParts[i]; + if (current.Tag == ContainerStart + || current.Tag == ContainerEnd + || current.Tag == TextTags.LineBreak) + { + // End the codeblock + stringBuilder.Append(formattingOptions.NewLine); + stringBuilder.Append("```"); + + // We know we're at a line break of some kind, but it could be + // a container start, so let the standard handling take care of it. + goto standardHandling; + } + + stringBuilder.Append(current.Text); + } + + // If we're here, that means that the last part has been reached, so just + // return. + Debug.Assert(i == taggedParts.Length); + stringBuilder.Append(formattingOptions.NewLine); + stringBuilder.Append("```"); + return; + } + } + + standardHandling: + switch (current.Tag) + { + case TextTags.Text when !isInCodeBlock: + addText(current.Text); + break; + + case TextTags.Text: + endBlock(); + addText(current.Text); + break; + + case TextTags.Space when isInCodeBlock: + if (indexIsTag(i + 1, TextTags.Text)) + { + endBlock(); + } + + addText(current.Text); + break; + + case TextTags.Space: + case TextTags.Punctuation: + addText(current.Text); + break; + + case ContainerStart: + addNewline(); + addText(current.Text); + break; + + case ContainerEnd: + addNewline(); + break; + + case TextTags.LineBreak when untilLineBreak && stringBuilder.Length != 0: + // The section will end and another newline will be appended, no need to add yet another newline. + return; + + case TextTags.LineBreak: + if (stringBuilder.Length != 0 && !indexIsTag(i + 1, ContainerStart, ContainerEnd) && i + 1 != taggedParts.Length) + { + addNewline(); + } + break; + + default: + if (!isInCodeBlock) + { + isInCodeBlock = true; + stringBuilder.Append('`'); + } + stringBuilder.Append(current.Text); + break; + } + } + + if (isInCodeBlock) + { + endBlock(); + } + + return; + + void addText(string text) + { + stringBuilder.Append(Escape(text)); + } + + void addNewline() + { + if (isInCodeBlock) + { + endBlock(); + } + + // Markdown needs 2 linebreaks to make a new paragraph + stringBuilder.Append(formattingOptions.NewLine); + stringBuilder.Append(formattingOptions.NewLine); + brokeLine = true; + } + + void endBlock() + { + stringBuilder.Append('`'); + isInCodeBlock = false; + } + + bool indexIsTag(int i, params string[] tags) + => i < taggedParts.Length && tags.Contains(taggedParts[i].Tag); + } + } +} diff --git a/src/OmniSharp.Roslyn.CSharp/Helpers/SourceTextExtensions.cs b/src/OmniSharp.Roslyn.CSharp/Helpers/SourceTextExtensions.cs new file mode 100644 index 0000000000..b7e40b2418 --- /dev/null +++ b/src/OmniSharp.Roslyn.CSharp/Helpers/SourceTextExtensions.cs @@ -0,0 +1,13 @@ +#nullable enable + +using Microsoft.CodeAnalysis.Text; +using OmniSharp.Models; + +namespace OmniSharp.Roslyn.CSharp.Helpers +{ + internal static class SourceTextExtensions + { + public static int GetTextPosition(this SourceText sourceText, Request request) + => sourceText.Lines.GetPosition(new LinePosition(request.Line, request.Column)); + } +} diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs new file mode 100644 index 0000000000..4d454999d5 --- /dev/null +++ b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs @@ -0,0 +1,272 @@ +#nullable enable + +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Composition; +using System.Diagnostics; +using System.Text; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Completion; +using Microsoft.CodeAnalysis.Tags; +using Microsoft.Extensions.Logging; +using OmniSharp.Mef; +using OmniSharp.Models.v1.Completion; +using OmniSharp.Options; +using OmniSharp.Roslyn.CSharp.Helpers; +using OmniSharp.Roslyn.CSharp.Services.Intellisense; +using OmniSharp.Utilities; +using CompletionItem = OmniSharp.Models.v1.Completion.CompletionItem; +using CompletionTriggerKind = OmniSharp.Models.v1.Completion.CompletionTriggerKind; +using CSharpCompletionItem = Microsoft.CodeAnalysis.Completion.CompletionItem; +using CSharpCompletionList = Microsoft.CodeAnalysis.Completion.CompletionList; +using CSharpCompletionService = Microsoft.CodeAnalysis.Completion.CompletionService; + +namespace OmniSharp.Roslyn.CSharp.Services.Completion +{ + [Shared] + [OmniSharpHandler(OmniSharpEndpoints.Completion, LanguageNames.CSharp)] + [OmniSharpHandler(OmniSharpEndpoints.CompletionResolve, LanguageNames.CSharp)] + public class CompletionService : + IRequestHandler, + IRequestHandler + { + private static readonly Dictionary s_roslynTagToCompletionItemKind = new Dictionary() + { + { WellKnownTags.Public, CompletionItemKind.Keyword }, + { WellKnownTags.Protected, CompletionItemKind.Keyword }, + { WellKnownTags.Private, CompletionItemKind.Keyword }, + { WellKnownTags.Internal, CompletionItemKind.Keyword }, + { WellKnownTags.File, CompletionItemKind.File }, + { WellKnownTags.Project, CompletionItemKind.File }, + { WellKnownTags.Folder, CompletionItemKind.Folder }, + { WellKnownTags.Assembly, CompletionItemKind.File }, + { WellKnownTags.Class, CompletionItemKind.Class }, + { WellKnownTags.Constant, CompletionItemKind.Constant }, + { WellKnownTags.Delegate, CompletionItemKind.Function }, + { WellKnownTags.Enum, CompletionItemKind.Enum }, + { WellKnownTags.EnumMember, CompletionItemKind.EnumMember }, + { WellKnownTags.Event, CompletionItemKind.Event }, + { WellKnownTags.ExtensionMethod, CompletionItemKind.Method }, + { WellKnownTags.Field, CompletionItemKind.Field }, + { WellKnownTags.Interface, CompletionItemKind.Interface }, + { WellKnownTags.Intrinsic, CompletionItemKind.Text }, + { WellKnownTags.Keyword, CompletionItemKind.Keyword }, + { WellKnownTags.Label, CompletionItemKind.Text }, + { WellKnownTags.Local, CompletionItemKind.Variable }, + { WellKnownTags.Namespace, CompletionItemKind.Text }, + { WellKnownTags.Method, CompletionItemKind.Method }, + { WellKnownTags.Module, CompletionItemKind.Module }, + { WellKnownTags.Operator, CompletionItemKind.Operator }, + { WellKnownTags.Parameter, CompletionItemKind.Value }, + { WellKnownTags.Property, CompletionItemKind.Property }, + { WellKnownTags.RangeVariable, CompletionItemKind.Variable }, + { WellKnownTags.Reference, CompletionItemKind.Reference }, + { WellKnownTags.Structure, CompletionItemKind.Struct }, + { WellKnownTags.TypeParameter, CompletionItemKind.TypeParameter }, + { WellKnownTags.Snippet, CompletionItemKind.Snippet }, + { WellKnownTags.Error, CompletionItemKind.Text }, + { WellKnownTags.Warning, CompletionItemKind.Text }, + }; + + private readonly OmniSharpWorkspace _workspace; + private readonly FormattingOptions _formattingOptions; + private readonly ILogger _logger; + + private (CSharpCompletionList Completions, string FileName, int Position)? _lastCompletion = null; + + [ImportingConstructor] + public CompletionService(OmniSharpWorkspace workspace, FormattingOptions formattingOptions, ILoggerFactory loggerFactory) + { + _workspace = workspace; + _formattingOptions = formattingOptions; + _logger = loggerFactory.CreateLogger(); + } + + public async Task Handle(CompletionRequest request) + { + _logger.LogTrace("Completions requested"); + _lastCompletion = null; + + var document = _workspace.GetDocument(request.FileName); + if (document is null) + { + _logger.LogInformation("Could not find document for file {0}", request.FileName); + return new CompletionResponse { Items = ImmutableArray.Empty }; + } + + var sourceText = await document.GetTextAsync(); + var position = sourceText.GetTextPosition(request); + + var completionService = CSharpCompletionService.GetService(document); + Debug.Assert(request.TriggerCharacter != null || request.CompletionTrigger != CompletionTriggerKind.TriggerCharacter); + + if (request.CompletionTrigger == CompletionTriggerKind.TriggerCharacter && + !completionService.ShouldTriggerCompletion(sourceText, position, getCompletionTrigger(includeTriggerCharacter: true))) + { + _logger.LogTrace("Should not insert completions here."); + return new CompletionResponse { Items = ImmutableArray.Empty }; + } + + var completions = await completionService.GetCompletionsAsync(document, position, getCompletionTrigger(includeTriggerCharacter: false)); + _logger.LogTrace("Found {0} completions for {1}:{2},{3}", + completions?.Items.IsDefaultOrEmpty != true ? 0 : completions.Items.Length, + request.FileName, + request.Line, + request.Column); + + if (completions is null) + { + return new CompletionResponse { Items = ImmutableArray.Empty }; + } + + var typedSpan = completionService.GetDefaultCompletionListSpan(sourceText, position); + string typedText = sourceText.GetSubText(typedSpan).ToString(); + + ImmutableArray filteredItems = typedText != string.Empty + ? completionService.FilterItems(document, completions.Items, typedText).SelectAsArray(i => i.DisplayText) + : ImmutableArray.Empty; + _logger.LogTrace("Completions filled in"); + _lastCompletion = (completions, request.FileName, position); + + var triggerCharactersBuilder = ImmutableArray.CreateBuilder(completions.Rules.DefaultCommitCharacters.Length); + return new CompletionResponse + { + IsIncomplete = false, + Items = completions.Items.SelectAsArrayWithArgumentAndIndex((completions, triggerCharactersBuilder, filteredItems), buildCompletion) + }; + + CompletionTrigger getCompletionTrigger(bool includeTriggerCharacter) + => request.CompletionTrigger switch + { + CompletionTriggerKind.Invoked => CompletionTrigger.Invoke, + // https://github.com/dotnet/roslyn/issues/42982: Passing a trigger character + // to GetCompletionsAsync causes a null ref currently. + CompletionTriggerKind.TriggerCharacter when includeTriggerCharacter => CompletionTrigger.CreateInsertionTrigger((char)request.TriggerCharacter!), + _ => CompletionTrigger.Invoke, + }; + + static CompletionItemKind getCompletionItemKind(ImmutableArray tags) + { + foreach (var tag in tags) + { + if (s_roslynTagToCompletionItemKind.TryGetValue(tag, out var itemKind)) + { + return itemKind; + } + } + + return CompletionItemKind.Text; + } + + static CompletionItem buildCompletion( + CSharpCompletionItem completion, + (CSharpCompletionList completions, ImmutableArray.Builder triigerCharactersBuilder, ImmutableArray filteredItems) completionsAndBuilder, + int index) + { + var (completions, triggerCharactersBuilder, filteredItems) = completionsAndBuilder; + triggerCharactersBuilder.AddRange(completions.Rules.DefaultCommitCharacters); + + foreach (var modifiedRule in completion.Rules.CommitCharacterRules) + { + switch (modifiedRule.Kind) + { + case CharacterSetModificationKind.Add: + triggerCharactersBuilder.AddRange(modifiedRule.Characters); + break; + + case CharacterSetModificationKind.Remove: + for (int i = 0; i < triggerCharactersBuilder.Count; i++) + { + if (modifiedRule.Characters.Contains(triggerCharactersBuilder[i])) + { + triggerCharactersBuilder.RemoveAt(i); + i--; + } + } + + break; + + case CharacterSetModificationKind.Replace: + triggerCharactersBuilder.Clear(); + triggerCharactersBuilder.AddRange(modifiedRule.Characters); + break; + } + } + + // VS has a more complex concept of a commit mode vs suggestion mode for intellisense. + // LSP doesn't have this, so mock it as best we can by removing space ` ` from the list + // of commit characters if we're in suggestion mode. + if (completions.SuggestionModeItem is object) + { + triggerCharactersBuilder.Remove(' '); + } + + ImmutableArray commitCharacters = triggerCharactersBuilder.ToImmutableAndClear(); + + return new CompletionItem + { + Label = completion.DisplayTextPrefix + completion.DisplayText + completion.DisplayTextSuffix, + InsertText = completion.TryGetInsertionText(out var insertionText) ? insertionText : completion.DisplayText, + SortText = completion.SortText, + FilterText = completion.FilterText, + Kind = getCompletionItemKind(completion.Tags), + Detail = completion.InlineDescription, + Data = index, + Preselect = completion.Rules.MatchPriority == MatchPriority.Preselect || filteredItems.Contains(completion.DisplayText), + CommitCharacters = commitCharacters + }; + } + } + + public async Task Handle(CompletionResolveRequest request) + { + if (_lastCompletion is null) + { + _logger.LogError("Cannot call completion/resolve before calling completion!"); + return new CompletionResolveResponse { Item = request.Item }; + } + + var (completions, fileName, _) = _lastCompletion.Value; + + if (request.Item is null + || request.Item.Data >= completions.Items.Length + || request.Item.Data < 0) + { + _logger.LogError("Received invalid completion resolve!"); + return new CompletionResolveResponse { Item = request.Item }; + } + + var lastCompletionItem = completions.Items[request.Item.Data]; + if (lastCompletionItem.DisplayTextPrefix + lastCompletionItem.DisplayText + lastCompletionItem.DisplayTextSuffix != request.Item.Label) + { + _logger.LogError($"Inconsistent completion data. Requested data on {request.Item.Label}, but found completion item {lastCompletionItem.DisplayText}"); + return new CompletionResolveResponse { Item = request.Item }; + } + + + var document = _workspace.GetDocument(fileName); + if (document is null) + { + _logger.LogInformation("Could not find document for file {0}", fileName); + return new CompletionResolveResponse { Item = request.Item }; + } + + var completionService = CSharpCompletionService.GetService(document); + + var description = await completionService.GetDescriptionAsync(document, lastCompletionItem); + + StringBuilder textBuilder = new StringBuilder(); + MarkdownHelpers.TaggedTextToMarkdown(description.TaggedParts, textBuilder, _formattingOptions, out _); + + request.Item.Documentation = textBuilder.ToString(); + + // TODO: Diff the document and fill in additionalTextEdits + + return new CompletionResolveResponse + { + Item = request.Item + }; + } + } +} diff --git a/src/OmniSharp.Roslyn.CSharp/Services/QuickInfoProvider.cs b/src/OmniSharp.Roslyn.CSharp/Services/QuickInfoProvider.cs index 8babf99669..bc766026fe 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/QuickInfoProvider.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/QuickInfoProvider.cs @@ -9,7 +9,7 @@ using OmniSharp.Mef; using OmniSharp.Models; using OmniSharp.Options; -using OmniSharp.Utilities; +using OmniSharp.Roslyn.CSharp.Helpers; #nullable enable @@ -24,18 +24,6 @@ public class QuickInfoProvider : IRequestHandler - /// Indicates the start of a text container. The elements after through (but not - /// including) the matching are rendered in a rectangular block which is positioned - /// as an inline element relative to surrounding elements. The text of the element - /// itself precedes the content of the container, and is typically a bullet or number header for an item in a - /// list. - /// - private const string ContainerStart = nameof(ContainerStart); - /// - /// Indicates the end of a text container. See . - /// - private const string ContainerEnd = nameof(ContainerEnd); /// /// Section kind for nullability analysis. /// @@ -92,7 +80,7 @@ public async Task Handle(QuickInfoRequest request) var summary = quickInfo.Sections.FirstOrDefault(s => s.Kind == QuickInfoSectionKinds.DocumentationComments); if (summary is object) { - buildSectionAsMarkdown(summary, sectionTextBuilder, _formattingOptions, out _); + MarkdownHelpers.TaggedTextToMarkdown(summary.TaggedParts, sectionTextBuilder, _formattingOptions, out _); appendBuiltSection(finalTextBuilder, sectionTextBuilder, _formattingOptions); } @@ -110,7 +98,7 @@ public async Task Handle(QuickInfoRequest request) case QuickInfoSectionKinds.AnonymousTypes: // The first line is "Anonymous Types:" - buildSectionAsMarkdown(section, sectionTextBuilder, _formattingOptions, out int lastIndex, untilLineBreak: true); + MarkdownHelpers.TaggedTextToMarkdown(section.TaggedParts, sectionTextBuilder, _formattingOptions, out int lastIndex, untilLineBreak: true); appendBuiltSection(finalTextBuilder, sectionTextBuilder, _formattingOptions); // Then we want all anonymous types to be C# highlighted @@ -119,12 +107,12 @@ public async Task Handle(QuickInfoRequest request) case NullabilityAnalysis: // Italicize the nullable analysis for emphasis. - buildSectionAsMarkdown(section, sectionTextBuilder, _formattingOptions, out _); + MarkdownHelpers.TaggedTextToMarkdown(section.TaggedParts, sectionTextBuilder, _formattingOptions, out _); appendBuiltSection(finalTextBuilder, sectionTextBuilder, _formattingOptions, italicize: true); break; default: - buildSectionAsMarkdown(section, sectionTextBuilder, _formattingOptions, out _); + MarkdownHelpers.TaggedTextToMarkdown(section.TaggedParts, sectionTextBuilder, _formattingOptions, out _); appendBuiltSection(finalTextBuilder, sectionTextBuilder, _formattingOptions); break; } @@ -174,112 +162,6 @@ static void appendSectionAsCsharp(QuickInfoSection section, StringBuilder builde builder.Append(formattingOptions.NewLine); builder.Append("```"); } - - static void buildSectionAsMarkdown(QuickInfoSection section, StringBuilder stringBuilder, FormattingOptions formattingOptions, out int lastIndex, bool untilLineBreak = false) - { - bool isInCodeBlock = false; - lastIndex = 0; - for (int i = 0; i < section.TaggedParts.Length; i++) - { - var current = section.TaggedParts[i]; - lastIndex = i; - - switch (current.Tag) - { - case TextTags.Text when !isInCodeBlock: - addText(current.Text); - break; - - case TextTags.Text: - endBlock(); - addText(current.Text); - break; - - case TextTags.Space when isInCodeBlock: - if (nextIsTag(i, TextTags.Text)) - { - endBlock(); - } - - stringBuilder.Append(current.Text); - break; - - case TextTags.Punctuation when isInCodeBlock && current.Text != "`": - stringBuilder.Append(current.Text); - break; - - case TextTags.Space: - case TextTags.Punctuation: - stringBuilder.Append(current.Text); - break; - - case ContainerStart: - addNewline(); - addText(current.Text); - break; - - case ContainerEnd: - addNewline(); - break; - - case TextTags.LineBreak when untilLineBreak && stringBuilder.Length != 0: - // The section will end and another newline will be appended, no need to add yet another newline. - return; - - case TextTags.LineBreak: - if (stringBuilder.Length != 0 && !nextIsTag(i, ContainerStart, ContainerEnd) && i + 1 != section.TaggedParts.Length) - { - addNewline(); - } - break; - - default: - if (!isInCodeBlock) - { - isInCodeBlock = true; - stringBuilder.Append('`'); - } - stringBuilder.Append(current.Text); - break; - } - } - - if (isInCodeBlock) - { - endBlock(); - } - - return; - - void addText(string text) - { - stringBuilder.Append(MarkdownHelpers.Escape(text)); - } - - void addNewline() - { - if (isInCodeBlock) - { - endBlock(); - } - - // Markdown needs 2 linebreaks to make a new paragraph - stringBuilder.Append(formattingOptions.NewLine); - stringBuilder.Append(formattingOptions.NewLine); - } - - void endBlock() - { - stringBuilder.Append('`'); - isInCodeBlock = false; - } - - bool nextIsTag(int i, params string[] tags) - { - int nextI = i + 1; - return nextI < section.TaggedParts.Length && tags.Contains(section.TaggedParts[nextI].Tag); - } - } } } } diff --git a/src/OmniSharp.Shared/Utilities/ImmutableArrayExtensions.cs b/src/OmniSharp.Shared/Utilities/ImmutableArrayExtensions.cs index 6b81b05dda..7ffe0c6edf 100644 --- a/src/OmniSharp.Shared/Utilities/ImmutableArrayExtensions.cs +++ b/src/OmniSharp.Shared/Utilities/ImmutableArrayExtensions.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Collections.Immutable; namespace OmniSharp.Utilities @@ -17,5 +18,52 @@ public static ImmutableArray AsImmutableOrNull(this IEnumerable items) return ImmutableArray.CreateRange(items); } + + public static ImmutableArray SelectAsArray(this ImmutableArray array, Func mapper) + { + if (array.IsDefaultOrEmpty) + { + return ImmutableArray.Empty; + } + + var builder = ImmutableArray.CreateBuilder(array.Length); + foreach (var e in array) + { + builder.Add(mapper(e)); + } + + return builder.MoveToImmutable(); + } + + public static ImmutableArray SelectAsArrayWithArgumentAndIndex(this ImmutableArray array, TArg argument, Func selector) + { + if (array.IsDefaultOrEmpty) + { + return ImmutableArray.Empty; + } + + var builder = ImmutableArray.CreateBuilder(array.Length); + + for (int i = 0; i < array.Length; i++) + { + builder.Add(selector(array[i], argument, i)); + } + + return builder.MoveToImmutable(); + } + + public static ImmutableArray ToImmutableAndClear(this ImmutableArray.Builder builder) + { + if (builder.Capacity == builder.Count) + { + return builder.MoveToImmutable(); + } + else + { + var result = builder.ToImmutable(); + builder.Clear(); + return result; + } + } } } diff --git a/src/OmniSharp.Shared/Utilities/MarkdownHelpers.cs b/src/OmniSharp.Shared/Utilities/MarkdownHelpers.cs deleted file mode 100644 index 9a2bc5a4cc..0000000000 --- a/src/OmniSharp.Shared/Utilities/MarkdownHelpers.cs +++ /dev/null @@ -1,16 +0,0 @@ -using System.Text.RegularExpressions; - -namespace OmniSharp.Utilities -{ - public static class MarkdownHelpers - { - private static Regex EscapeRegex = new Regex(@"([\\`\*_\{\}\[\]\(\)#+\-\.!])", RegexOptions.Compiled); - - public static string Escape(string markdown) - { - if (markdown == null) - return null; - return EscapeRegex.Replace(markdown, @"\$1"); - } - } -} diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs new file mode 100644 index 0000000000..a96d1f41d8 --- /dev/null +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs @@ -0,0 +1,715 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.InteropServices; +using System.Text; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using OmniSharp.Models.v1.Completion; +using OmniSharp.Roslyn.CSharp.Services.Completion; +using TestUtility; +using Xunit; +using Xunit.Abstractions; + +namespace OmniSharp.Roslyn.CSharp.Tests +{ + public class CompletionFacts : AbstractTestFixture + { + private readonly ILogger _logger; + + private string EndpointName => OmniSharpEndpoints.Completion; + + public CompletionFacts(ITestOutputHelper output, SharedOmniSharpHostFixture sharedOmniSharpHostFixture) + : base(output, sharedOmniSharpHostFixture) + { + this._logger = this.LoggerFactory.CreateLogger(); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Label_is_correct_for_property(string filename) + { + const string input = + @"public class Class1 { + public int Foo { get; set; } + public Class1() + { + Foo$$ + } + }"; + + var completions = await FindCompletionsAsync(filename, input); + Assert.Contains("Foo", completions.Items.Select(c => c.Label)); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Label_is_correct_for_variable(string filename) + { + const string input = + @"public class Class1 { + public Class1() + { + var foo = 1; + foo$$ + } + }"; + + var completions = await FindCompletionsAsync(filename, input); + Assert.Contains("foo", completions.Items.Select(c => c.Label)); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Description_has_header_and_text(string filename) + { + const string input = + @"public class Class1 { + public Class1() + { + Foo$$ + } + /// Some Text + public void Foo(int bar = 1) + { + } + }"; + + var completions = await FindCompletionsAsync(filename, input); + Assert.All(completions.Items, c => Assert.Null(c.Documentation)); + + var fooCompletion = completions.Items.Single(c => c.Label == "Foo"); + var resolvedCompletion = await ResolveCompletionAsync(fooCompletion); + Assert.Equal("```csharp\nvoid Class1.Foo([int bar = 1])\n```\n\nSome Text", resolvedCompletion.Item.Documentation); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Returns_camel_case_completions(string filename) + { + const string input = + @"public class Class1 { + public Class1() + { + System.Guid.tp$$ + } + }"; + + var completions = await FindCompletionsAsync(filename, input); + Assert.Contains("TryParse", completions.Items.Select(c => c.InsertText)); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Returns_sub_sequence_completions(string filename) + { + const string input = + @"public class Class1 { + public Class1() + { + System.Guid.ng$$ + } + }"; + + var completions = await FindCompletionsAsync(filename, input); + Assert.Contains("NewGuid", completions.Items.Select(c => c.Label)); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Returns_sub_sequence_completions_without_matching_firstletter(string filename) + { + const string input = + @"public class Class1 { + public Class1() + { + System.Guid.gu$$ + } + }"; + + var completions = await FindCompletionsAsync(filename, input); + Assert.Contains("NewGuid", completions.Items.Select(c => c.Label)); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Returns_method_header(string filename) + { + const string input = + @"public class Class1 { + public Class1() + { + System.Guid.ng$$ + } + }"; + + var completions = await FindCompletionsAsync(filename, input); + Assert.All(completions.Items, c => Assert.Null(c.Documentation)); + + var fooCompletion = completions.Items.Single(c => c.Label == "NewGuid"); + var resolvedCompletion = await ResolveCompletionAsync(fooCompletion); + Assert.Equal("```csharp\nSystem.Guid System.Guid.NewGuid()\n```", resolvedCompletion.Item.Documentation); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Returns_variable_before_class(string filename) + { + const string input = + @"public class MyClass1 { + + public MyClass1() + { + var myvar = 1; + my$$ + } + }"; + + var completions = await FindCompletionsAsync(filename, input); + Assert.Contains(completions.Items, c => c.Label == "myvar"); + Assert.Contains(completions.Items, c => c.Label == "MyClass1"); + Assert.All(completions.Items, c => + { + switch (c.Label) + { + case "myvar": + Assert.True(c.Preselect); + break; + default: + Assert.False(c.Preselect); + break; + } + }); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Returns_class_before_variable(string filename) + { + const string input = + @"public class MyClass1 { + + public MyClass1() + { + var myvar = 1; + My$$ + } + }"; + + var completions = await FindCompletionsAsync(filename, input); + Assert.Contains(completions.Items, c => c.Label == "myvar"); + Assert.Contains(completions.Items, c => c.Label == "MyClass1"); + Assert.All(completions.Items, c => + { + switch (c.Label) + { + case "MyClass1": + Assert.True(c.Preselect); + break; + default: + Assert.False(c.Preselect); + break; + } + }); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Returns_empty_sequence_in_invalid_context(string filename) + { + const string source = + @"public class MyClass1 { + + public MyClass1() + { + var x$$ + } + }"; + + var completions = await FindCompletionsAsync(filename, source); + Assert.Empty(completions.Items); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Returns_attribute_without_attribute_suffix(string filename) + { + const string source = + @"using System; + + public class BarAttribute : Attribute {} + + [B$$ + public class Foo {}"; + + var completions = await FindCompletionsAsync(filename, source); + Assert.Contains(completions.Items, c => c.Label == "Bar"); + Assert.All(completions.Items, c => + { + switch (c.Label) + { + case "Bar": + Assert.True(c.Preselect); + break; + default: + Assert.False(c.Preselect); + break; + } + }); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Returns_members_in_object_initializer_context(string filename) + { + const string source = + @"public class MyClass1 { + public string Foo {get; set;} + } + + public class MyClass2 { + + public MyClass2() + { + var c = new MyClass1 { + F$$ + } + } + "; + + var completions = await FindCompletionsAsync(filename, source); + ContainsCompletions(completions.Items.Select(c => c.InsertText), "Foo"); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Returns_parameter_name_inside_a_method(string filename) + { + const string source = + @"public class MyClass1 { + public void SayHi(string text) {} + } + + public class MyClass2 { + + public MyClass2() + { + var c = new MyClass1(); + c.SayHi(te$$ + } + } + "; + + var completions = await FindCompletionsAsync(filename, source); + Assert.Contains(completions.Items, c => c.Label == "text:"); + Assert.All(completions.Items, c => + { + switch (c.Label) + { + case "text:": + Assert.True(c.Preselect); + break; + default: + Assert.False(c.Preselect); + break; + } + }); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Returns_declaration_names(string filename) + { + const string source = + @" +public class MyClass +{ + MyClass m$$ +} + "; + + var completions = await FindCompletionsAsync(filename, source); + Assert.Equal(new[] { "myClass", "my", "@class", "MyClass", "My", "Class", "GetMyClass", "GetMy", "GetClass" }, + completions.Items.Select(c => c.Label)); + } + + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Returns_override_signatures(string filename) + { + const string source = + @"class Foo + { + public virtual void Test(string text) {} + public virtual void Test(string text, string moreText) {} + } + + class FooChild : Foo + { + override $$ + } + "; + + var completions = await FindCompletionsAsync(filename, source); + ContainsCompletions(completions.Items.Select(c => c.InsertText), "Equals(object obj)", "GetHashCode()", "Test(string text)", "Test(string text, string moreText)", "ToString()"); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Returns_cref_completion(string filename) + { + const string source = + @" /// + /// A comment. for more details + /// + public class MyClass1 { + } + "; + + var completions = await FindCompletionsAsync(filename, source); + Assert.Contains(completions.Items, c => c.Label == "MyClass1"); + Assert.All(completions.Items, c => + { + switch (c.Label) + { + case "MyClass1": + Assert.True(c.Preselect); + break; + default: + Assert.False(c.Preselect); + break; + } + }); + } + + [Fact] + public async Task Returns_host_object_members_in_csx() + { + const string source = + "Prin$$"; + + var completions = await FindCompletionsAsync("dummy.csx", source); + Assert.Contains(completions.Items, c => c.Label == "Print"); + Assert.Contains(completions.Items, c => c.Label == "PrintOptions"); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Is_suggestion_mode_true_for_lambda_expression_position1(string filename) + { + const string source = @" +using System; +class C +{ + int CallMe(int i) => 42; + + void M(Func a) { } + + void M() + { + M(c$$ + } +} +"; + + var completions = await FindCompletionsAsync(filename, source); + + Assert.True(completions.Items.All(c => c.IsSuggestionMode())); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Is_suggestion_mode_true_for_lambda_expression_position2(string filename) + { + const string source = @" +using System; +class C +{ + int CallMe(int i) => 42; + + void M() + { + Func a = c$$ + } +} +"; + + var completions = await FindCompletionsAsync(filename, source); + + Assert.True(completions.Items.All(c => c.IsSuggestionMode())); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Is_suggestion_mode_false_for_normal_position1(string filename) + { + const string source = @" +using System; +class C +{ + int CallMe(int i) => 42; + + void M(int a) { } + + void M() + { + M(c$$ + } +} +"; + + var completions = await FindCompletionsAsync(filename, source); + + Assert.True(completions.Items.All(c => !c.IsSuggestionMode())); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task Is_suggestion_mode_false_for_normal_position2(string filename) + { + const string source = @" +using System; +class C +{ + int CallMe(int i) => 42; + + void M() + { + int a = c$$ + } +} +"; + + var completions = await FindCompletionsAsync(filename, source); + + Assert.True(completions.Items.All(c => !c.IsSuggestionMode())); + } + + [Fact] + public async Task Scripting_by_default_returns_completions_for_CSharp7_1() + { + const string source = + @" + var number1 = 1; + var number2 = 2; + var tuple = (number1, number2); + tuple.n$$ + "; + + var completions = await FindCompletionsAsync("dummy.csx", source); + Assert.Contains(completions.Items, c => c.Label == "number1"); + Assert.Contains(completions.Items, c => c.Label == "number2"); + Assert.All(completions.Items, c => + { + switch (c.Label) + { + case "number1": + case "number2": + Assert.True(c.Preselect); + break; + default: + Assert.False(c.Preselect); + break; + } + }); + } + + [Fact] + public async Task Scripting_by_default_returns_completions_for_CSharp7_2() + { + const string source = + @" + public class Foo { private protected int myValue = 0; } + public class Bar : Foo + { + public Bar() + { + var x = myv$$ + } + } + "; + + var completions = await FindCompletionsAsync("dummy.csx", source); + Assert.Contains(completions.Items, c => c.Label == "myValue"); + Assert.All(completions.Items, c => + { + switch (c.Label) + { + case "myValue": + Assert.True(c.Preselect); + break; + default: + Assert.False(c.Preselect); + break; + } + }); + } + + [Fact] + public async Task Scripting_by_default_returns_completions_for_CSharp8_0() + { + const string source = + @" + class Point { + public Point(int x, int y) { + PositionX = x; + PositionY = y; + } + public int PositionX { get; } + public int PositionY { get; } + } + Point[] points = { new (1, 2), new (3, 4) }; + points[0].Po$$ + "; + + var completions = await FindCompletionsAsync("dummy.csx", source); + Assert.Contains(completions.Items, c => c.Label == "PositionX"); + Assert.Contains(completions.Items, c => c.Label == "PositionY"); + Assert.All(completions.Items, c => + { + switch (c.Label) + { + case "PositionX": + case "PositionY": + Assert.True(c.Preselect); + break; + default: + Assert.False(c.Preselect); + break; + } + }); + } + + private void ContainsCompletions(IEnumerable completions, params string[] expected) + { + if (!completions.SequenceEqual(expected)) + { + var builder = new StringBuilder(); + builder.AppendLine("Expected"); + builder.AppendLine("--------"); + + foreach (var completion in expected) + { + builder.AppendLine(completion); + } + + builder.AppendLine(); + builder.AppendLine("Found"); + builder.AppendLine("-----"); + + foreach (var completion in completions) + { + builder.AppendLine(completion); + } + + this._logger.LogError(builder.ToString()); + } + + Assert.Equal(expected, completions.ToArray()); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task TriggeredOnSpaceForObjectCreation(string filename) + { + const string input = +@"public class Class1 { + public M() + { + Class1 c = new $$ + } +}"; + + var completions = await FindCompletionsAsync(filename, input, triggerChar: ' '); + Assert.NotEmpty(completions.Items); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task ReturnsAtleastOnePreselectOnNew(string filename) + { + const string input = +@"public class Class1 { + public M() + { + Class1 c = new $$ + } +}"; + + var completions = await FindCompletionsAsync(filename, input, triggerChar: ' '); + Assert.NotEmpty(completions.Items.Where(completion => completion.Preselect == true)); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task NotTriggeredOnSpaceWithoutObjectCreation(string filename) + { + const string input = +@"public class Class1 { + public M() + { + $$ + } +}"; + + var completions = await FindCompletionsAsync(filename, input, triggerChar: ' '); + Assert.Empty(completions.Items); + } + + private CompletionService GetCompletionService(OmniSharpTestHost host) + => host.GetRequestHandler(EndpointName); + + protected async Task FindCompletionsAsync(string filename, string source, char? triggerChar = null) + { + var testFile = new TestFile(filename, source); + SharedOmniSharpTestHost.AddFilesToWorkspace(testFile); + var point = testFile.Content.GetPointFromPosition(); + + var request = new CompletionRequest + { + Line = point.Line, + Column = point.Offset, + FileName = testFile.FileName, + Buffer = testFile.Content.Code, + CompletionTrigger = triggerChar is object ? CompletionTriggerKind.TriggerCharacter : CompletionTriggerKind.Invoked, + TriggerCharacter = triggerChar + }; + + var requestHandler = GetCompletionService(SharedOmniSharpTestHost); + + return await requestHandler.Handle(request); + } + + protected async Task ResolveCompletionAsync(CompletionItem completionItem) + => await GetCompletionService(SharedOmniSharpTestHost).Handle(new CompletionResolveRequest { Item = completionItem }); + } + + internal static class CompletionResponseExtensions + { + public static bool IsSuggestionMode(this CompletionItem item) => item.CommitCharacters?.IsDefaultOrEmpty ?? false || !item.CommitCharacters.Contains(' '); + } +} From 7e17bbeca99f475894b873dffcfeb8b769882f80 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Thu, 13 Aug 2020 20:28:49 -0700 Subject: [PATCH 2/5] Implement correct override, partial, xml doc tag, and internalsvisibleto completion. LSP does not support a concept of asynchronous edits that touch the current cursor location: anything that does needs to advertise those edits up front. That makes it impossible to do override or partial completion lazily: completing the method body _requires_ touching the insertion text, as depending on editorconfig settings braces might need to be inserted at the end of the line. Further, moving the cursor requires using a snippet, which cannot be accomplished in an additionalTextEdit. So, for those 4 providers that need to compute additional work at the current cursor location, we do so. We take any work that comes before the cursor location (such as turning override into public override returntype) and turn that into an additionalTextEdit, and then snippitize the rest of the completion and use it as insertionText. --- .../Models/v1/Completion/CompletionItem.cs | 39 ++ .../Helpers/LspSnippetHelpers.cs | 19 + .../Services/Completion/CompletionService.cs | 225 ++++++++++-- .../Intellisense/CompletionItemExtensions.cs | 10 +- .../Utilities/ImmutableArrayExtensions.cs | 18 +- .../CompletionFacts.cs | 342 ++++++++++++++---- 6 files changed, 545 insertions(+), 108 deletions(-) create mode 100644 src/OmniSharp.Roslyn.CSharp/Helpers/LspSnippetHelpers.cs diff --git a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs index 33809c375a..634c5ba2a9 100644 --- a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs +++ b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs @@ -83,6 +83,18 @@ public class CompletionItem [JsonProperty("commitCharacters")] public ImmutableArray? CommitCharacters { get; set; } + /// + /// An optional array of additional text edits that are applied when + /// selecting this completion.Edits must not overlap (including the same insert position) + /// with the main edit nor with themselves. + /// + /// Additional text edits should be used to change text unrelated to the current cursor position + /// (for example adding an import statement at the top of the file if the completion item will + /// insert an unqualified type). + /// + [JsonProperty("additionalTextEdits")] + public ImmutableArray? AdditionalTextEdits { get; set; } + /// /// Index in the completions list that this completion occurred. /// @@ -95,6 +107,33 @@ public override string ToString() } } + public struct TextEdit + { + [JsonProperty("range")] + public Range Range { get; set; } + + [JsonProperty("newText")] + public string? NewText { get; set; } + } + + // These are using different versions from the normal Range/Point classes in order to apply + // json converters that match up with lsp naming conventions. + public struct Range + { + [JsonProperty("start")] + public Position Start { get; set; } + [JsonProperty("end")] + public Position End { get; set; } + } + + public struct Position + { + [JsonProperty("line")] + public int Line { get; set; } + [JsonProperty("character")] + public int Character { get; set; } + } + public enum CompletionItemKind { Text = 1, diff --git a/src/OmniSharp.Roslyn.CSharp/Helpers/LspSnippetHelpers.cs b/src/OmniSharp.Roslyn.CSharp/Helpers/LspSnippetHelpers.cs new file mode 100644 index 0000000000..29531b1dfc --- /dev/null +++ b/src/OmniSharp.Roslyn.CSharp/Helpers/LspSnippetHelpers.cs @@ -0,0 +1,19 @@ +using System.Text.RegularExpressions; + +namespace OmniSharp.Roslyn.CSharp.Helpers +{ + public static class LspSnippetHelpers + { + private static Regex EscapeRegex = new Regex(@"([\\\$}])", RegexOptions.Compiled); + + /// + /// Escape the given string for use as an LSP snippet. This escapes '\', '$', and '}'. + /// + public static string Escape(string snippet) + { + if (snippet == null) + return null; + return EscapeRegex.Replace(snippet, @"\$1"); + } + } +} diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs index 4d454999d5..5a6df6f62f 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs @@ -4,6 +4,7 @@ using System.Collections.Immutable; using System.Composition; using System.Diagnostics; +using System.Linq; using System.Text; using System.Threading.Tasks; using Microsoft.CodeAnalysis; @@ -18,7 +19,6 @@ using OmniSharp.Utilities; using CompletionItem = OmniSharp.Models.v1.Completion.CompletionItem; using CompletionTriggerKind = OmniSharp.Models.v1.Completion.CompletionTriggerKind; -using CSharpCompletionItem = Microsoft.CodeAnalysis.Completion.CompletionItem; using CSharpCompletionList = Microsoft.CodeAnalysis.Completion.CompletionList; using CSharpCompletionService = Microsoft.CodeAnalysis.Completion.CompletionService; @@ -73,7 +73,8 @@ public class CompletionService : private readonly FormattingOptions _formattingOptions; private readonly ILogger _logger; - private (CSharpCompletionList Completions, string FileName, int Position)? _lastCompletion = null; + private readonly object _lock = new object(); + private (CSharpCompletionList Completions, string FileName)? _lastCompletion = null; [ImportingConstructor] public CompletionService(OmniSharpWorkspace workspace, FormattingOptions formattingOptions, ILoggerFactory loggerFactory) @@ -86,7 +87,10 @@ public CompletionService(OmniSharpWorkspace workspace, FormattingOptions formatt public async Task Handle(CompletionRequest request) { _logger.LogTrace("Completions requested"); - _lastCompletion = null; + lock (_lock) + { + _lastCompletion = null; + } var document = _workspace.GetDocument(request.FileName); if (document is null) @@ -115,11 +119,17 @@ public async Task Handle(CompletionRequest request) request.Line, request.Column); - if (completions is null) + if (completions is null || completions.Items.Length == 0) { return new CompletionResponse { Items = ImmutableArray.Empty }; } + if (request.TriggerCharacter == ' ' && !completions.Items.Any(c => c.IsObjectCreationCompletionItem())) + { + // Only trigger on space if there is an object creation completion + return new CompletionResponse { Items = ImmutableArray.Empty }; + } + var typedSpan = completionService.GetDefaultCompletionListSpan(sourceText, position); string typedText = sourceText.GetSubText(typedSpan).ToString(); @@ -127,13 +137,154 @@ public async Task Handle(CompletionRequest request) ? completionService.FilterItems(document, completions.Items, typedText).SelectAsArray(i => i.DisplayText) : ImmutableArray.Empty; _logger.LogTrace("Completions filled in"); - _lastCompletion = (completions, request.FileName, position); + + lock (_lock) + { + _lastCompletion = (completions, request.FileName); + } var triggerCharactersBuilder = ImmutableArray.CreateBuilder(completions.Rules.DefaultCommitCharacters.Length); + var completionsBuilder = ImmutableArray.CreateBuilder(completions.Items.Length); + + for (int i = 0; i < completions.Items.Length; i++) + { + var completion = completions.Items[i]; + var commitCharacters = buildCommitCharacters(completions, completion.Rules.CommitCharacterRules, triggerCharactersBuilder); + + var insertTextFormat = InsertTextFormat.PlainText; + ImmutableArray? additionalTextEdits = null; + + if (!completion.TryGetInsertionText(out string insertText)) + { + switch (completion.GetProviderName()) + { + case CompletionItemExtensions.InternalsVisibleToCompletionProvider: + // The IVT completer doesn't add extra things before the completion + // span, only assembly keys at the end if they exist. + { + CompletionChange change = await completionService.GetChangeAsync(document, completion); + Debug.Assert(typedSpan == change.TextChange.Span); + insertText = change.TextChange.NewText!; + } + break; + + case CompletionItemExtensions.XmlDocCommentCompletionProvider: + { + // The doc comment completion might compensate for the < before + // the current word, if one exists. For these cases, if the token + // before the current location is a < and the text it's replacing starts + // with a <, erase the < from the given insertion text. + var change = await completionService.GetChangeAsync(document, completion); + + bool trimFront = change.TextChange.NewText![0] == '<' + && sourceText[change.TextChange.Span.Start] == '<'; + + Debug.Assert(!trimFront || change.TextChange.Span.Start + 1 == typedSpan.Start); + + (insertText, insertTextFormat) = getAdjustedInsertTextWithPosition(change, position, newOffset: trimFront ? 1 : 0); + } + break; + + case CompletionItemExtensions.OverrideCompletionProvider: + case CompletionItemExtensions.PartialMethodCompletionProvider: + { + // For these two, we potentially need to use additionalTextEdits. It's possible + // that override (or C# expanded partials) will cause the word or words before + // the cursor to be adjusted. For example: + // + // public class C { + // override $0 + // } + // + // Invoking completion and selecting, say Equals, wants to cause the line to be + // rewritten as this: + // + // public class C { + // public override bool Equals(object other) + // { + // return base.Equals(other);$0 + // } + // } + // + // In order to handle this, we need to chop off the section of the completion + // before the cursor and bundle that into an additionalTextEdit. Then, we adjust + // the remaining bit of the change to put the cursor in the expected spot via + // snippets. We could leave the additionalTextEdit bit for resolve, but we already + // have the data do the change and we basically have to compute the whole thing now + // anyway, so it doesn't really save us anything. + + 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) + { + (insertText, insertTextFormat) = getAdjustedInsertTextWithPosition(change, position, newOffset: 0); + break; + } + + // We know the span starts before the text we're keying off of. So, break that + // out into a separate edit. We need to cut out the space before the current word, + // 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); + var additionalRange = new Range + { + Start = new Position { Line = additionalEditStartPosition.Line, Character = additionalEditStartPosition.Character }, + End = new Position { Line = additionalEditEndPosition.Line, Character = additionalEditEndPosition.Character }, + }; + + int additionalEditEndOffset = change.TextChange.NewText!.IndexOf(completion.DisplayText); + 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}", + completion.DisplayText, change.TextChange.NewText); + (insertText, insertTextFormat) = getAdjustedInsertTextWithPosition(change, position, newOffset: 0); + break; + } + + additionalTextEdits = ImmutableArray.Create(new TextEdit + { + // Again, we cut off the space at the end of the offset + NewText = change.TextChange.NewText!.Substring(0, additionalEditEndOffset - 1), + Range = additionalRange + }); + + // Now that we have the additional edit, adjust the rest of the new text + (insertText, insertTextFormat) = getAdjustedInsertTextWithPosition(change, position, additionalEditEndOffset); + } + break; + + default: + insertText = completion.DisplayText; + break; + } + } + + completionsBuilder.Add(new CompletionItem + { + Label = completion.DisplayTextPrefix + completion.DisplayText + completion.DisplayTextSuffix, + InsertText = insertText, + InsertTextFormat = insertTextFormat, + AdditionalTextEdits = additionalTextEdits, + SortText = completion.SortText, + FilterText = completion.FilterText, + Kind = getCompletionItemKind(completion.Tags), + Detail = completion.InlineDescription, + Data = i, + Preselect = completion.Rules.MatchPriority == MatchPriority.Preselect || filteredItems.Contains(completion.DisplayText), + CommitCharacters = commitCharacters, + }); + } + return new CompletionResponse { IsIncomplete = false, - Items = completions.Items.SelectAsArrayWithArgumentAndIndex((completions, triggerCharactersBuilder, filteredItems), buildCompletion) + Items = completionsBuilder.MoveToImmutable() }; CompletionTrigger getCompletionTrigger(bool includeTriggerCharacter) @@ -159,15 +310,11 @@ static CompletionItemKind getCompletionItemKind(ImmutableArray tags) return CompletionItemKind.Text; } - static CompletionItem buildCompletion( - CSharpCompletionItem completion, - (CSharpCompletionList completions, ImmutableArray.Builder triigerCharactersBuilder, ImmutableArray filteredItems) completionsAndBuilder, - int index) + static ImmutableArray buildCommitCharacters(CSharpCompletionList completions, ImmutableArray characterRules, ImmutableArray.Builder triggerCharactersBuilder) { - var (completions, triggerCharactersBuilder, filteredItems) = completionsAndBuilder; triggerCharactersBuilder.AddRange(completions.Rules.DefaultCommitCharacters); - foreach (var modifiedRule in completion.Rules.CommitCharacterRules) + foreach (var modifiedRule in characterRules) { switch (modifiedRule.Kind) { @@ -202,20 +349,46 @@ static CompletionItem buildCompletion( triggerCharactersBuilder.Remove(' '); } - ImmutableArray commitCharacters = triggerCharactersBuilder.ToImmutableAndClear(); + return triggerCharactersBuilder.ToImmutableAndClear(); + } - return new CompletionItem + static (string, InsertTextFormat) getAdjustedInsertTextWithPosition( + CompletionChange change, + int originalPosition, + int newOffset) + { + // 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 + // the cursor location in the insertion text. We therefore need to compensate + // by cutting off the requested portion of the text, finding the adjusted + // position in the requested string, and snippetizing it. + + // NewText is annotated as nullable, but this is a misannotation that will be fixed. + string newText = change.TextChange.NewText!; + + // Easy-out, either Roslyn doesn't have an opinion on adjustment, or the adjustment is after the + // end of the new text. Just return a substring from the requested offset to the end + if (!(change.NewPosition is int newPosition) + || newPosition >= (change.TextChange.Span.Start + newText.Length)) { - Label = completion.DisplayTextPrefix + completion.DisplayText + completion.DisplayTextSuffix, - InsertText = completion.TryGetInsertionText(out var insertionText) ? insertionText : completion.DisplayText, - SortText = completion.SortText, - FilterText = completion.FilterText, - Kind = getCompletionItemKind(completion.Tags), - Detail = completion.InlineDescription, - Data = index, - Preselect = completion.Rules.MatchPriority == MatchPriority.Preselect || filteredItems.Contains(completion.DisplayText), - CommitCharacters = commitCharacters - }; + 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); + } + + // 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 afterText = LspSnippetHelpers.Escape(newText.Substring(midpoint)); + + return (beforeText + "$0" + afterText, InsertTextFormat.Snippet); } } @@ -227,7 +400,7 @@ public async Task Handle(CompletionResolveRequest req return new CompletionResolveResponse { Item = request.Item }; } - var (completions, fileName, _) = _lastCompletion.Value; + var (completions, fileName) = _lastCompletion.Value; if (request.Item is null || request.Item.Data >= completions.Items.Length @@ -261,7 +434,7 @@ public async Task Handle(CompletionResolveRequest req request.Item.Documentation = textBuilder.ToString(); - // TODO: Diff the document and fill in additionalTextEdits + // TODO: Do import completion diffing here return new CompletionResolveResponse { diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Intellisense/CompletionItemExtensions.cs b/src/OmniSharp.Roslyn.CSharp/Services/Intellisense/CompletionItemExtensions.cs index ed42561e01..f9d51e8876 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Intellisense/CompletionItemExtensions.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Intellisense/CompletionItemExtensions.cs @@ -18,8 +18,10 @@ internal static class CompletionItemExtensions private const string InsertionText = nameof(InsertionText); private const string ObjectCreationCompletionProvider = "Microsoft.CodeAnalysis.CSharp.Completion.Providers.ObjectCreationCompletionProvider"; private const string NamedParameterCompletionProvider = "Microsoft.CodeAnalysis.CSharp.Completion.Providers.NamedParameterCompletionProvider"; - private const string OverrideCompletionProvider = "Microsoft.CodeAnalysis.CSharp.Completion.Providers.OverrideCompletionProvider"; - private const string ParitalMethodCompletionProvider = "Microsoft.CodeAnalysis.CSharp.Completion.Providers.PartialMethodCompletionProvider"; + internal const string OverrideCompletionProvider = "Microsoft.CodeAnalysis.CSharp.Completion.Providers.OverrideCompletionProvider"; + internal const string PartialMethodCompletionProvider = "Microsoft.CodeAnalysis.CSharp.Completion.Providers.PartialMethodCompletionProvider"; + internal const string InternalsVisibleToCompletionProvider = "Microsoft.CodeAnalysis.CSharp.Completion.Providers.InternalsVisibleToCompletionProvider"; + internal const string XmlDocCommentCompletionProvider = "Microsoft.CodeAnalysis.CSharp.Completion.Providers.XmlDocCommentCompletionProvider"; private const string ProviderName = nameof(ProviderName); private const string SymbolCompletionItem = "Microsoft.CodeAnalysis.Completion.Providers.SymbolCompletionItem"; private const string SymbolKind = nameof(SymbolKind); @@ -37,7 +39,7 @@ static CompletionItemExtensions() _getProviderName = typeof(CompletionItem).GetProperty(ProviderName, BindingFlags.NonPublic | BindingFlags.Instance); } - private static string GetProviderName(CompletionItem item) + internal static string GetProviderName(this CompletionItem item) { return (string)_getProviderName.GetValue(item); } @@ -76,7 +78,7 @@ public static async Task> GetCompletionSymbolsAsync(this Co public static bool UseDisplayTextAsCompletionText(this CompletionItem completionItem) { var provider = GetProviderName(completionItem); - return provider == NamedParameterCompletionProvider || provider == OverrideCompletionProvider || provider == ParitalMethodCompletionProvider; + return provider == NamedParameterCompletionProvider || provider == OverrideCompletionProvider || provider == PartialMethodCompletionProvider; } public static bool TryGetInsertionText(this CompletionItem completionItem, out string insertionText) diff --git a/src/OmniSharp.Shared/Utilities/ImmutableArrayExtensions.cs b/src/OmniSharp.Shared/Utilities/ImmutableArrayExtensions.cs index 7ffe0c6edf..7481d718db 100644 --- a/src/OmniSharp.Shared/Utilities/ImmutableArrayExtensions.cs +++ b/src/OmniSharp.Shared/Utilities/ImmutableArrayExtensions.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Threading.Tasks; namespace OmniSharp.Utilities { @@ -35,23 +36,6 @@ public static ImmutableArray SelectAsArray(this ImmutableArray< return builder.MoveToImmutable(); } - public static ImmutableArray SelectAsArrayWithArgumentAndIndex(this ImmutableArray array, TArg argument, Func selector) - { - if (array.IsDefaultOrEmpty) - { - return ImmutableArray.Empty; - } - - var builder = ImmutableArray.CreateBuilder(array.Length); - - for (int i = 0; i < array.Length; i++) - { - builder.Add(selector(array[i], argument, i)); - } - - return builder.MoveToImmutable(); - } - public static ImmutableArray ToImmutableAndClear(this ImmutableArray.Builder builder) { if (builder.Capacity == builder.Count) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs index a96d1f41d8..72bf943cfa 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs @@ -1,9 +1,7 @@ using System; -using System.Collections.Generic; using System.Linq; -using System.Runtime.InteropServices; -using System.Text; using System.Threading.Tasks; +using Microsoft.CodeAnalysis; using Microsoft.Extensions.Logging; using OmniSharp.Models.v1.Completion; using OmniSharp.Roslyn.CSharp.Services.Completion; @@ -22,13 +20,13 @@ public class CompletionFacts : AbstractTestFixture public CompletionFacts(ITestOutputHelper output, SharedOmniSharpHostFixture sharedOmniSharpHostFixture) : base(output, sharedOmniSharpHostFixture) { - this._logger = this.LoggerFactory.CreateLogger(); + this._logger = this.LoggerFactory.CreateLogger(); } [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Label_is_correct_for_property(string filename) + public async Task PropertyCompletion(string filename) { const string input = @"public class Class1 { @@ -41,12 +39,13 @@ public Class1() var completions = await FindCompletionsAsync(filename, input); Assert.Contains("Foo", completions.Items.Select(c => c.Label)); + Assert.Contains("Foo", completions.Items.Select(c => c.InsertText)); } [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Label_is_correct_for_variable(string filename) + public async Task VariableCompletion(string filename) { const string input = @"public class Class1 { @@ -59,12 +58,13 @@ public Class1() var completions = await FindCompletionsAsync(filename, input); Assert.Contains("foo", completions.Items.Select(c => c.Label)); + Assert.Contains("foo", completions.Items.Select(c => c.InsertText)); } [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Description_has_header_and_text(string filename) + public async Task DocumentationIsResolved(string filename) { const string input = @"public class Class1 { @@ -89,7 +89,7 @@ public void Foo(int bar = 1) [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Returns_camel_case_completions(string filename) + public async Task ReturnsCamelCasedCompletions(string filename) { const string input = @"public class Class1 { @@ -106,7 +106,7 @@ public Class1() [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Returns_sub_sequence_completions(string filename) + public async Task ReturnsSubsequences(string filename) { const string input = @"public class Class1 { @@ -123,7 +123,7 @@ public Class1() [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Returns_sub_sequence_completions_without_matching_firstletter(string filename) + public async Task ReturnsSubsequencesWithoutFirstLetter(string filename) { const string input = @"public class Class1 { @@ -140,7 +140,7 @@ public Class1() [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Returns_method_header(string filename) + public async Task MethodHeaderDocumentation(string filename) { const string input = @"public class Class1 { @@ -161,7 +161,7 @@ public Class1() [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Returns_variable_before_class(string filename) + public async Task PreselectsCorrectCasing_Lowercase(string filename) { const string input = @"public class MyClass1 { @@ -193,7 +193,7 @@ public MyClass1() [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Returns_class_before_variable(string filename) + public async Task PreselectsCorrectCasing_Uppercase(string filename) { const string input = @"public class MyClass1 { @@ -225,7 +225,7 @@ public MyClass1() [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Returns_empty_sequence_in_invalid_context(string filename) + public async Task NoCompletionsInInvalid(string filename) { const string source = @"public class MyClass1 { @@ -243,7 +243,7 @@ var x$$ [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Returns_attribute_without_attribute_suffix(string filename) + public async Task AttributeDoesNotHaveAttributeSuffix(string filename) { const string source = @"using System; @@ -255,6 +255,7 @@ public class Foo {}"; var completions = await FindCompletionsAsync(filename, source); Assert.Contains(completions.Items, c => c.Label == "Bar"); + Assert.Contains(completions.Items, c => c.InsertText == "Bar"); Assert.All(completions.Items, c => { switch (c.Label) @@ -272,7 +273,7 @@ public class Foo {}"; [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Returns_members_in_object_initializer_context(string filename) + public async Task ReturnsObjectInitalizerMembers(string filename) { const string source = @"public class MyClass1 { @@ -290,13 +291,15 @@ public MyClass2() "; var completions = await FindCompletionsAsync(filename, source); - ContainsCompletions(completions.Items.Select(c => c.InsertText), "Foo"); + Assert.Single(completions.Items); + Assert.Equal("Foo", completions.Items[0].Label); + Assert.Equal("Foo", completions.Items[0].InsertText); } [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Returns_parameter_name_inside_a_method(string filename) + public async Task IncludesParameterNames(string filename) { const string source = @"public class MyClass1 { @@ -314,7 +317,9 @@ public MyClass2() "; var completions = await FindCompletionsAsync(filename, source); - Assert.Contains(completions.Items, c => c.Label == "text:"); + var item = completions.Items.First(c => c.Label == "text:"); + Assert.NotNull(item); + Assert.Equal("text", item.InsertText); Assert.All(completions.Items, c => { switch (c.Label) @@ -332,7 +337,7 @@ public MyClass2() [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Returns_declaration_names(string filename) + public async Task ReturnsNameSuggestions(string filename) { const string source = @" @@ -347,11 +352,10 @@ MyClass m$$ completions.Items.Select(c => c.Label)); } - [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Returns_override_signatures(string filename) + public async Task OverrideSignatures_Publics(string filename) { const string source = @"class Foo @@ -367,13 +371,205 @@ class FooChild : Foo "; var completions = await FindCompletionsAsync(filename, source); - ContainsCompletions(completions.Items.Select(c => c.InsertText), "Equals(object obj)", "GetHashCode()", "Test(string text)", "Test(string text, string moreText)", "ToString()"); + Assert.Equal(new[] { "Equals(object obj)", "GetHashCode()", "Test(string text)", "Test(string text, string moreText)", "ToString()" }, + completions.Items.Select(c => c.Label)); + Assert.Equal(new[] { "Equals(object obj)\n {\n return base.Equals(obj);$0\n \\}\n", + "GetHashCode()\n {\n return base.GetHashCode();$0\n \\}\n", + "Test(string text)\n {\n base.Test(text);$0\n \\}\n", + "Test(string text, string moreText)\n {\n base.Test(text, moreText);$0\n \\}\n", + "ToString()\n {\n return base.ToString();$0\n \\}\n" + }, + completions.Items.Select(c => c.InsertText)); + + Assert.Equal(new[] { "\n public override bool", + "\n public override int", + "\n public override void", + "\n public override void", + "\n public override string"}, + completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().NewText)); + + Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().Range), + r => Assert.Equal( + new Range { Start = new Position { Line = 7, Character = 21 }, End = new Position { Line = 8, Character = 30 } }, + r)); + + Assert.All(completions.Items, c => Assert.Equal(InsertTextFormat.Snippet, c.InsertTextFormat)); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task OverrideSignatures_UnimportedTypesFullyQualified(string filename) + { + const string source = @" +using N2; +namespace N1 +{ + public class CN1 {} +} +namespace N2 +{ + using N1; + public abstract class IN2 { protected abstract CN1 GetN1(); } +} +namespace N3 +{ + class CN3 : IN2 + { + override $$ + } +}"; + + var completions = await FindCompletionsAsync(filename, source); + Assert.Equal(new[] { "Equals(object obj)", "GetHashCode()", "GetN1()", "ToString()" }, + completions.Items.Select(c => c.Label)); + + Assert.Equal(new[] { "Equals(object obj)\n {\n return base.Equals(obj);$0\n \\}", + "GetHashCode()\n {\n return base.GetHashCode();$0\n \\}", + "GetN1()\n {\n throw new System.NotImplementedException();$0\n \\}", + "ToString()\n {\n return base.ToString();$0\n \\}" + }, + completions.Items.Select(c => c.InsertText)); + + Assert.Equal(new[] { "public override bool", + "public override int", + "protected override N1.CN1", + "public override string"}, + completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().NewText)); + + Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().Range), + r => Assert.Equal( + new Range { Start = new Position { Line = 15, Character = 8 }, End = new Position { Line = 15, Character = 16 } }, + r)); + + Assert.All(completions.Items, c => Assert.Equal(InsertTextFormat.Snippet, c.InsertTextFormat)); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task OverrideSignatures_ModifierInFront(string filename) + { + const string source = @" +class C +{ + public override $$ +}"; + + var completions = await FindCompletionsAsync(filename, source); + Assert.Equal(new[] { "Equals(object obj)", "GetHashCode()", "ToString()" }, + completions.Items.Select(c => c.Label)); + + Assert.Equal(new[] { "bool Equals(object obj)\n {\n return base.Equals(obj);$0\n \\}", + "int GetHashCode()\n {\n return base.GetHashCode();$0\n \\}", + "string ToString()\n {\n return base.ToString();$0\n \\}" + }, + completions.Items.Select(c => c.InsertText)); + + Assert.All(completions.Items.Select(c => c.AdditionalTextEdits), a => Assert.Null(a)); + Assert.All(completions.Items, c => Assert.Equal(InsertTextFormat.Snippet, c.InsertTextFormat)); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task OverrideSignatures_ModifierAndReturnTypeInFront(string filename) + { + const string source = @" +class C +{ + public override bool $$ +}"; + + var completions = await FindCompletionsAsync(filename, source); + Assert.Equal(new[] { "Equals(object obj)" }, + completions.Items.Select(c => c.Label)); + + Assert.Equal(new[] { "Equals(object obj)\n {\n return base.Equals(obj);$0\n \\}" }, + completions.Items.Select(c => c.InsertText)); + + Assert.All(completions.Items.Select(c => c.AdditionalTextEdits), a => Assert.Null(a)); + Assert.All(completions.Items, c => Assert.Equal(InsertTextFormat.Snippet, c.InsertTextFormat)); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task PartialCompletion(string filename) + { + const string source = @" +partial class C +{ + partial void M1(string param); +} +partial class C +{ + partial $$ +} +"; + + var completions = await FindCompletionsAsync(filename, source); + Assert.Equal(new[] { "M1(string param)" }, + completions.Items.Select(c => c.Label)); + + Assert.Equal(new[] { "void M1(string param)\n {\n throw new System.NotImplementedException();$0\n \\}" }, + completions.Items.Select(c => c.InsertText)); + + Assert.All(completions.Items.Select(c => c.AdditionalTextEdits), a => Assert.Null(a)); + Assert.All(completions.Items, c => Assert.Equal(InsertTextFormat.Snippet, c.InsertTextFormat)); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task OverrideSignatures_PartiallyTypedIdentifier(string filename) + { + const string source = @" +class C +{ + override Ge$$ +}"; + + var completions = await FindCompletionsAsync(filename, source); + Assert.Equal(new[] { "Equals(object obj)", "GetHashCode()", "ToString()" }, + completions.Items.Select(c => c.Label)); + + Assert.Equal(new[] { "Equals(object obj)\n {\n return base.Equals(obj);$0\n \\}", + "GetHashCode()\n {\n return base.GetHashCode();$0\n \\}", + "ToString()\n {\n return base.ToString();$0\n \\}" + }, + completions.Items.Select(c => c.InsertText)); + + Assert.Equal(new[] { "public override bool", + "public override int", + "public override string"}, + completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().NewText)); + + Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().Range), + r => Assert.Equal( + new Range { Start = new Position { Line = 3, Character = 4 }, End = new Position { Line = 3, Character = 12 } }, + r)); + + Assert.All(completions.Items, c => + { + switch (c.Label) + { + case "GetHashCode()": + Assert.True(c.Preselect); + break; + default: + Assert.False(c.Preselect); + break; + } + }); + + Assert.All(completions.Items, c => Assert.Equal(InsertTextFormat.Snippet, c.InsertTextFormat)); } [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Returns_cref_completion(string filename) + public async Task CrefCompletion(string filename) { const string source = @" /// @@ -399,8 +595,36 @@ public class MyClass1 { }); } + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task DocCommentTagCompletions(string filename) + { + const string source = + @" /// + /// A comment. <$$ + /// + public class MyClass1 { + } + "; + + var completions = await FindCompletionsAsync(filename, source); + Assert.Equal(new[] { "!--$0-->", + "![CDATA[$0]]>", + "c", + "code", + "inheritdoc$0/>", + "list type=\"$0\"", + "para", + "see cref=\"$0\"/>", + "seealso cref=\"$0\"/>" + }, + completions.Items.Select(c => c.InsertText)); + Assert.All(completions.Items, c => Assert.Equal(c.InsertText.Contains("$0"), c.InsertTextFormat == InsertTextFormat.Snippet)); + } + [Fact] - public async Task Returns_host_object_members_in_csx() + public async Task HostObjectCompletionInScripts() { const string source = "Prin$$"; @@ -413,7 +637,7 @@ public async Task Returns_host_object_members_in_csx() [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Is_suggestion_mode_true_for_lambda_expression_position1(string filename) + public async Task NoCommitOnSpaceInLambdaParameter_MethodArgument(string filename) { const string source = @" using System; @@ -422,6 +646,7 @@ class C int CallMe(int i) => 42; void M(Func a) { } + void M(string unrelated) { } void M() { @@ -438,7 +663,7 @@ void M() [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Is_suggestion_mode_true_for_lambda_expression_position2(string filename) + public async Task NoCommitOnSpaceInLambdaParameter_Initializer(string filename) { const string source = @" using System; @@ -461,7 +686,7 @@ void M() [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Is_suggestion_mode_false_for_normal_position1(string filename) + public async Task CommitOnSpaceWithoutLambda_InArgument(string filename) { const string source = @" using System; @@ -486,7 +711,7 @@ void M() [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] - public async Task Is_suggestion_mode_false_for_normal_position2(string filename) + public async Task CommitOnSpaceWithoutLambda_InInitializer(string filename) { const string source = @" using System; @@ -507,7 +732,7 @@ void M() } [Fact] - public async Task Scripting_by_default_returns_completions_for_CSharp7_1() + public async Task ScriptingIncludes7_1() { const string source = @" @@ -536,7 +761,7 @@ public async Task Scripting_by_default_returns_completions_for_CSharp7_1() } [Fact] - public async Task Scripting_by_default_returns_completions_for_CSharp7_2() + public async Task ScriptingIncludes7_2() { const string source = @" @@ -567,7 +792,7 @@ public Bar() } [Fact] - public async Task Scripting_by_default_returns_completions_for_CSharp8_0() + public async Task ScriptingIncludes8_0() { const string source = @" @@ -601,34 +826,6 @@ public Point(int x, int y) { }); } - private void ContainsCompletions(IEnumerable completions, params string[] expected) - { - if (!completions.SequenceEqual(expected)) - { - var builder = new StringBuilder(); - builder.AppendLine("Expected"); - builder.AppendLine("--------"); - - foreach (var completion in expected) - { - builder.AppendLine(completion); - } - - builder.AppendLine(); - builder.AppendLine("Found"); - builder.AppendLine("-----"); - - foreach (var completion in completions) - { - builder.AppendLine(completion); - } - - this._logger.LogError(builder.ToString()); - } - - Assert.Equal(expected, completions.ToArray()); - } - [Theory] [InlineData("dummy.cs")] [InlineData("dummy.csx")] @@ -680,6 +877,29 @@ public M() Assert.Empty(completions.Items); } + [Fact] + public async Task InternalsVisibleToCompletion() + { + var projectInfo = ProjectInfo.Create( + ProjectId.CreateNewId(), + VersionStamp.Create(), + "ProjectNameVal", + "AssemblyNameVal", + LanguageNames.CSharp, + "/path/to/project.csproj"); + + SharedOmniSharpTestHost.Workspace.AddProject(projectInfo); + + const string input = @" +[assembly: System.Runtime.CompilerServices.InternalsVisibleTo(""$$"; + + + var completions = await FindCompletionsAsync("dummy.cs", input); + Assert.Single(completions.Items); + Assert.Equal("AssemblyNameVal", completions.Items[0].Label); + Assert.Equal("AssemblyNameVal", completions.Items[0].InsertText); + } + private CompletionService GetCompletionService(OmniSharpTestHost host) => host.GetRequestHandler(EndpointName); @@ -710,6 +930,6 @@ protected async Task ResolveCompletionAsync(Completio internal static class CompletionResponseExtensions { - public static bool IsSuggestionMode(this CompletionItem item) => item.CommitCharacters?.IsDefaultOrEmpty ?? false || !item.CommitCharacters.Contains(' '); + public static bool IsSuggestionMode(this CompletionItem item) => (item.CommitCharacters?.IsDefaultOrEmpty ?? true) || !item.CommitCharacters.Contains(' '); } } From c357c62f63e74fb8c65d0525ae88be94204812e2 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Sat, 15 Aug 2020 14:59:58 -0700 Subject: [PATCH 3/5] Refactor the MarkdownHelper. This allows it to be fully shared by the QuickInfoProvider and the new CompletionService, with much less special casing and encapsulation violations. Additionally, the responses from the CompletionService now no longer specify the same lower-cased names as the the LSP completion endpoint. This brings the service into consistency with the rest of omnisharp, and it won't be very hard for the LSP provider to wrap it. --- .../Models/v1/Completion/CompletionItem.cs | 43 +----- .../Completion/CompletionResolveResponse.cs | 4 - .../v1/Completion/CompletionResponse.cs | 4 - .../Helpers/MarkdownHelpers.cs | 145 +++++++++++++----- .../Services/Completion/CompletionService.cs | 18 +-- .../Services/QuickInfoProvider.cs | 68 ++------ .../CompletionFacts.cs | 36 +++-- .../QuickInfoProviderFacts.cs | 14 +- 8 files changed, 163 insertions(+), 169 deletions(-) diff --git a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs index 634c5ba2a9..141c3a2575 100644 --- a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs +++ b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs @@ -1,7 +1,6 @@ #nullable enable using System.Collections.Immutable; -using Newtonsoft.Json; namespace OmniSharp.Models.v1.Completion { @@ -12,7 +11,6 @@ public class CompletionItem /// also the text that is inserted when selecting /// this completion. /// - [JsonProperty("label")] public string Label { get; set; } = null!; /// @@ -20,67 +18,57 @@ public class CompletionItem /// an icon is chosen by the editor.The standardized set /// of available values is defined in /// - [JsonProperty("kind")] public CompletionItemKind Kind { get; set; } /// /// Tags for this completion item /// - [JsonProperty("tags")] public ImmutableArray? Tags { get; set; } /// /// A human-readable string with additional information /// about this item, like type or symbol information /// - [JsonProperty("detail")] public string? Detail { get; set; } /// /// A human-readable string that represents a doc-comment. This is /// formatted as markdown. /// - [JsonProperty("documentation")] public string? Documentation { get; set; } /// /// Select this item when showing. /// - [JsonProperty("preselect")] public bool Preselect { get; set; } /// /// A string that should be used when comparing this item /// with other items. When null or empty the label is used. /// - [JsonProperty("sortText")] public string? SortText { get; set; } /// /// A string that should be used when filtering a set of /// completion items. When null or empty the label is used. /// - [JsonProperty("filterText")] public string? FilterText { get; set; } /// /// A string that should be inserted into a document when selecting /// this completion.When null or empty the label is used. /// - [JsonProperty("insertText")] public string? InsertText { get; set; } /// /// The format of . /// - [JsonProperty("insertTextFormat")] public InsertTextFormat? InsertTextFormat { get; set; } /// /// An optional set of characters that when pressed while this completion is active will accept it first and /// then type that character. /// - [JsonProperty("commitCharacters")] public ImmutableArray? CommitCharacters { get; set; } /// @@ -92,13 +80,11 @@ public class CompletionItem /// (for example adding an import statement at the top of the file if the completion item will /// insert an unqualified type). /// - [JsonProperty("additionalTextEdits")] - public ImmutableArray? AdditionalTextEdits { get; set; } + public ImmutableArray? AdditionalTextEdits { get; set; } /// /// Index in the completions list that this completion occurred. /// - [JsonProperty("data")] public int Data { get; set; } public override string ToString() @@ -107,33 +93,6 @@ public override string ToString() } } - public struct TextEdit - { - [JsonProperty("range")] - public Range Range { get; set; } - - [JsonProperty("newText")] - public string? NewText { get; set; } - } - - // These are using different versions from the normal Range/Point classes in order to apply - // json converters that match up with lsp naming conventions. - public struct Range - { - [JsonProperty("start")] - public Position Start { get; set; } - [JsonProperty("end")] - public Position End { get; set; } - } - - public struct Position - { - [JsonProperty("line")] - public int Line { get; set; } - [JsonProperty("character")] - public int Character { get; set; } - } - public enum CompletionItemKind { Text = 1, diff --git a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResolveResponse.cs b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResolveResponse.cs index 713077fcf6..26837f003d 100644 --- a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResolveResponse.cs +++ b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResolveResponse.cs @@ -1,13 +1,9 @@ #nullable enable -using Newtonsoft.Json; -using Newtonsoft.Json.Serialization; - namespace OmniSharp.Models.v1.Completion { public class CompletionResolveResponse { - [JsonProperty("item")] public CompletionItem? Item { get; set; } } } diff --git a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResponse.cs b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResponse.cs index 5f65dfcc70..e624f14158 100644 --- a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResponse.cs +++ b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResponse.cs @@ -1,8 +1,6 @@ #nullable enable -using System.Collections.Generic; using System.Collections.Immutable; -using Newtonsoft.Json; namespace OmniSharp.Models.v1.Completion { @@ -11,13 +9,11 @@ public class CompletionResponse /// /// If true, this list is not complete. Further typing should result in recomputing the list. /// - [JsonProperty("isIncomplete")] public bool IsIncomplete { get; set; } /// /// The completion items. /// - [JsonProperty("items")] public ImmutableArray Items { get; set; } } } diff --git a/src/OmniSharp.Roslyn.CSharp/Helpers/MarkdownHelpers.cs b/src/OmniSharp.Roslyn.CSharp/Helpers/MarkdownHelpers.cs index 555a20ef99..523929a766 100644 --- a/src/OmniSharp.Roslyn.CSharp/Helpers/MarkdownHelpers.cs +++ b/src/OmniSharp.Roslyn.CSharp/Helpers/MarkdownHelpers.cs @@ -1,6 +1,7 @@ using System.Collections.Immutable; using System.Diagnostics; using System.Linq; +using System.Runtime.InteropServices; using System.Text; using System.Text.RegularExpressions; using Microsoft.CodeAnalysis; @@ -32,50 +33,76 @@ public static string Escape(string markdown) /// private const string ContainerEnd = nameof(ContainerEnd); - public static void TaggedTextToMarkdown(ImmutableArray taggedParts, StringBuilder stringBuilder, FormattingOptions formattingOptions, out int lastIndex, bool untilLineBreak = false) + public static void TaggedTextToMarkdown( + ImmutableArray taggedParts, + StringBuilder stringBuilder, + FormattingOptions formattingOptions, + MarkdownFormat markdownFormat) { bool isInCodeBlock = false; bool brokeLine = true; - lastIndex = 0; + bool afterFirstLine = false; + + if (markdownFormat == MarkdownFormat.Italicize) + { + stringBuilder.Append("_"); + } + for (int i = 0; i < taggedParts.Length; i++) { var current = taggedParts[i]; - lastIndex = i; - if (brokeLine) + if (brokeLine && markdownFormat != MarkdownFormat.Italicize) { Debug.Assert(!isInCodeBlock); - // If we're on a new line and there are no text parts in the upcoming line, then we - // can format the whole line as C# code instead of plaintext. Otherwise, we need to - // intermix, and can only use simple ` codefences brokeLine = false; - bool canFormatAsBlock = false; - for (int j = i; j < taggedParts.Length; j++) + bool canFormatAsBlock = (afterFirstLine, markdownFormat) switch + { + (false, MarkdownFormat.FirstLineAsCSharp) => true, + (true, MarkdownFormat.FirstLineDefaultRestCSharp) => true, + (_, MarkdownFormat.AllTextAsCSharp) => true, + _ => false + }; + + if (!canFormatAsBlock) { - switch (taggedParts[j].Tag) + // If we're on a new line and there are no text parts in the upcoming line, then we + // can format the whole line as C# code instead of plaintext. Otherwise, we need to + // intermix, and can only use simple ` codefences + for (int j = i; j < taggedParts.Length; j++) { - case TextTags.Text: - canFormatAsBlock = false; - goto endOfLineOrTextFound; - - case ContainerStart: - case ContainerEnd: - case TextTags.LineBreak: - goto endOfLineOrTextFound; - - default: - // If the block is just newlines, then we don't want to format that as - // C# code. So, we default to false, set it to true if there's actually - // content on the line, then set to false again if Text content is - // encountered. - canFormatAsBlock = true; - continue; + switch (taggedParts[j].Tag) + { + case TextTags.Text: + canFormatAsBlock = false; + goto endOfLineOrTextFound; + + case ContainerStart: + case ContainerEnd: + case TextTags.LineBreak: + goto endOfLineOrTextFound; + + default: + // If the block is just newlines, then we don't want to format that as + // C# code. So, we default to false, set it to true if there's actually + // content on the line, then set to false again if Text content is + // encountered. + canFormatAsBlock = true; + continue; + } } } + else + { + // If it's just a newline, we're going to default to standard handling which will + // skip the newline. + canFormatAsBlock = !indexIsTag(i, ContainerStart, ContainerEnd, TextTags.LineBreak); + } endOfLineOrTextFound: if (canFormatAsBlock) { + afterFirstLine = true; stringBuilder.Append("```csharp"); stringBuilder.Append(formattingOptions.NewLine); for (; i < taggedParts.Length; i++) @@ -85,16 +112,23 @@ public static void TaggedTextToMarkdown(ImmutableArray taggedParts, || current.Tag == ContainerEnd || current.Tag == TextTags.LineBreak) { - // End the codeblock stringBuilder.Append(formattingOptions.NewLine); - stringBuilder.Append("```"); - // We know we're at a line break of some kind, but it could be - // a container start, so let the standard handling take care of it. - goto standardHandling; - } + if (markdownFormat != MarkdownFormat.AllTextAsCSharp + && markdownFormat != MarkdownFormat.FirstLineDefaultRestCSharp) + { + // End the codeblock + stringBuilder.Append("```"); - stringBuilder.Append(current.Text); + // We know we're at a line break of some kind, but it could be + // a container start, so let the standard handling take care of it. + goto standardHandling; + } + } + else + { + stringBuilder.Append(current.Text); + } } // If we're here, that means that the last part has been reached, so just @@ -141,10 +175,6 @@ public static void TaggedTextToMarkdown(ImmutableArray taggedParts, addNewline(); break; - case TextTags.LineBreak when untilLineBreak && stringBuilder.Length != 0: - // The section will end and another newline will be appended, no need to add yet another newline. - return; - case TextTags.LineBreak: if (stringBuilder.Length != 0 && !indexIsTag(i + 1, ContainerStart, ContainerEnd) && i + 1 != taggedParts.Length) { @@ -172,7 +202,12 @@ public static void TaggedTextToMarkdown(ImmutableArray taggedParts, void addText(string text) { - stringBuilder.Append(Escape(text)); + afterFirstLine = true; + if (!isInCodeBlock) + { + text = Escape(text); + } + stringBuilder.Append(text); } void addNewline() @@ -182,10 +217,20 @@ void addNewline() endBlock(); } + if (markdownFormat == MarkdownFormat.Italicize) + { + stringBuilder.Append("_"); + } + // Markdown needs 2 linebreaks to make a new paragraph stringBuilder.Append(formattingOptions.NewLine); stringBuilder.Append(formattingOptions.NewLine); brokeLine = true; + + if (markdownFormat == MarkdownFormat.Italicize) + { + stringBuilder.Append("_"); + } } void endBlock() @@ -198,4 +243,28 @@ bool indexIsTag(int i, params string[] tags) => i < taggedParts.Length && tags.Contains(taggedParts[i].Tag); } } + + public enum MarkdownFormat + { + /// + /// Only format entire lines as C# code if there is no standard text on the line + /// + Default, + /// + /// Italicize the section + /// + Italicize, + /// + /// Format the first line as C#, unconditionally + /// + FirstLineAsCSharp, + /// + /// Format the first line as default text, and format the rest of the lines as C#, unconditionally + /// + FirstLineDefaultRestCSharp, + /// + /// Format the entire set of text as C#, unconditionally + /// + AllTextAsCSharp + } } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs index 5a6df6f62f..e17469f1e7 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs @@ -12,6 +12,7 @@ using Microsoft.CodeAnalysis.Tags; using Microsoft.Extensions.Logging; using OmniSharp.Mef; +using OmniSharp.Models; using OmniSharp.Models.v1.Completion; using OmniSharp.Options; using OmniSharp.Roslyn.CSharp.Helpers; @@ -152,7 +153,7 @@ public async Task Handle(CompletionRequest request) var commitCharacters = buildCommitCharacters(completions, completion.Rules.CommitCharacterRules, triggerCharactersBuilder); var insertTextFormat = InsertTextFormat.PlainText; - ImmutableArray? additionalTextEdits = null; + ImmutableArray? additionalTextEdits = null; if (!completion.TryGetInsertionText(out string insertText)) { @@ -229,12 +230,6 @@ public async Task Handle(CompletionRequest request) // 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); - var additionalRange = new Range - { - Start = new Position { Line = additionalEditStartPosition.Line, Character = additionalEditStartPosition.Character }, - End = new Position { Line = additionalEditEndPosition.Line, Character = additionalEditEndPosition.Character }, - }; - int additionalEditEndOffset = change.TextChange.NewText!.IndexOf(completion.DisplayText); if (additionalEditEndOffset < 1) { @@ -247,11 +242,14 @@ public async Task Handle(CompletionRequest request) break; } - additionalTextEdits = ImmutableArray.Create(new TextEdit + additionalTextEdits = ImmutableArray.Create(new LinePositionSpanTextChange { // Again, we cut off the space at the end of the offset NewText = change.TextChange.NewText!.Substring(0, additionalEditEndOffset - 1), - Range = additionalRange + StartLine = additionalEditStartPosition.Line, + StartColumn = additionalEditStartPosition.Character, + EndLine = additionalEditEndPosition.Line, + EndColumn = additionalEditEndPosition.Character, }); // Now that we have the additional edit, adjust the rest of the new text @@ -430,7 +428,7 @@ public async Task Handle(CompletionResolveRequest req var description = await completionService.GetDescriptionAsync(document, lastCompletionItem); StringBuilder textBuilder = new StringBuilder(); - MarkdownHelpers.TaggedTextToMarkdown(description.TaggedParts, textBuilder, _formattingOptions, out _); + MarkdownHelpers.TaggedTextToMarkdown(description.TaggedParts, textBuilder, _formattingOptions, MarkdownFormat.FirstLineAsCSharp); request.Item.Documentation = textBuilder.ToString(); diff --git a/src/OmniSharp.Roslyn.CSharp/Services/QuickInfoProvider.cs b/src/OmniSharp.Roslyn.CSharp/Services/QuickInfoProvider.cs index bc766026fe..f0ae892db6 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/QuickInfoProvider.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/QuickInfoProvider.cs @@ -69,19 +69,26 @@ public async Task Handle(QuickInfoRequest request) } var finalTextBuilder = new StringBuilder(); - var sectionTextBuilder = new StringBuilder(); var description = quickInfo.Sections.FirstOrDefault(s => s.Kind == QuickInfoSectionKinds.Description); if (description is object) { - appendSectionAsCsharp(description, finalTextBuilder, _formattingOptions, includeSpaceAtStart: false); + appendSection(description, MarkdownFormat.AllTextAsCSharp); + + // The description doesn't include a set of newlines at the end, regardless + // of whether there are more sections, so if there are more sections we need + // to ensure they're separated. + if (quickInfo.Sections.Length > 1) + { + finalTextBuilder.Append(_formattingOptions.NewLine); + finalTextBuilder.Append(_formattingOptions.NewLine); + } } var summary = quickInfo.Sections.FirstOrDefault(s => s.Kind == QuickInfoSectionKinds.DocumentationComments); if (summary is object) { - MarkdownHelpers.TaggedTextToMarkdown(summary.TaggedParts, sectionTextBuilder, _formattingOptions, out _); - appendBuiltSection(finalTextBuilder, sectionTextBuilder, _formattingOptions); + appendSection(summary, MarkdownFormat.Default); } foreach (var section in quickInfo.Sections) @@ -93,27 +100,22 @@ public async Task Handle(QuickInfoRequest request) continue; case QuickInfoSectionKinds.TypeParameters: - appendSectionAsCsharp(section, finalTextBuilder, _formattingOptions); + appendSection(section, MarkdownFormat.AllTextAsCSharp); break; case QuickInfoSectionKinds.AnonymousTypes: // The first line is "Anonymous Types:" - MarkdownHelpers.TaggedTextToMarkdown(section.TaggedParts, sectionTextBuilder, _formattingOptions, out int lastIndex, untilLineBreak: true); - appendBuiltSection(finalTextBuilder, sectionTextBuilder, _formattingOptions); - // Then we want all anonymous types to be C# highlighted - appendSectionAsCsharp(section, finalTextBuilder, _formattingOptions, lastIndex + 1); + appendSection(section, MarkdownFormat.FirstLineDefaultRestCSharp); break; case NullabilityAnalysis: // Italicize the nullable analysis for emphasis. - MarkdownHelpers.TaggedTextToMarkdown(section.TaggedParts, sectionTextBuilder, _formattingOptions, out _); - appendBuiltSection(finalTextBuilder, sectionTextBuilder, _formattingOptions, italicize: true); + appendSection(section, MarkdownFormat.Italicize); break; default: - MarkdownHelpers.TaggedTextToMarkdown(section.TaggedParts, sectionTextBuilder, _formattingOptions, out _); - appendBuiltSection(finalTextBuilder, sectionTextBuilder, _formattingOptions); + appendSection(section, MarkdownFormat.Default); break; } } @@ -122,45 +124,9 @@ public async Task Handle(QuickInfoRequest request) return response; - static void appendBuiltSection(StringBuilder finalTextBuilder, StringBuilder stringBuilder, FormattingOptions formattingOptions, bool italicize = false) + void appendSection(QuickInfoSection section, MarkdownFormat format) { - // Two newlines to trigger a markdown new paragraph - finalTextBuilder.Append(formattingOptions.NewLine); - finalTextBuilder.Append(formattingOptions.NewLine); - if (italicize) - { - finalTextBuilder.Append("_"); - } - finalTextBuilder.Append(stringBuilder); - if (italicize) - { - finalTextBuilder.Append("_"); - } - stringBuilder.Clear(); - } - - static void appendSectionAsCsharp(QuickInfoSection section, StringBuilder builder, FormattingOptions formattingOptions, int startingIndex = 0, bool includeSpaceAtStart = true) - { - if (includeSpaceAtStart) - { - builder.Append(formattingOptions.NewLine); - } - builder.Append("```csharp"); - builder.Append(formattingOptions.NewLine); - for (int i = startingIndex; i < section.TaggedParts.Length; i++) - { - TaggedText part = section.TaggedParts[i]; - if (part.Tag == TextTags.LineBreak && i + 1 != section.TaggedParts.Length) - { - builder.Append(formattingOptions.NewLine); - } - else - { - builder.Append(part.Text); - } - } - builder.Append(formattingOptions.NewLine); - builder.Append("```"); + MarkdownHelpers.TaggedTextToMarkdown(section.TaggedParts, finalTextBuilder, _formattingOptions, format); } } } diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs index 72bf943cfa..8350b6e96d 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs @@ -388,10 +388,14 @@ class FooChild : Foo "\n public override string"}, completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().NewText)); - Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().Range), - r => Assert.Equal( - new Range { Start = new Position { Line = 7, Character = 21 }, End = new Position { Line = 8, Character = 30 } }, - r)); + Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Value.Single()), + r => + { + Assert.Equal(7, r.StartLine); + Assert.Equal(21, r.StartColumn); + Assert.Equal(8, r.EndLine); + Assert.Equal(30, r.EndColumn); + }); Assert.All(completions.Items, c => Assert.Equal(InsertTextFormat.Snippet, c.InsertTextFormat)); } @@ -437,10 +441,14 @@ class CN3 : IN2 "public override string"}, completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().NewText)); - Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().Range), - r => Assert.Equal( - new Range { Start = new Position { Line = 15, Character = 8 }, End = new Position { Line = 15, Character = 16 } }, - r)); + Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Value.Single()), + r => + { + Assert.Equal(15, r.StartLine); + Assert.Equal(8, r.StartColumn); + Assert.Equal(15, r.EndLine); + Assert.Equal(16, r.EndColumn); + }); Assert.All(completions.Items, c => Assert.Equal(InsertTextFormat.Snippet, c.InsertTextFormat)); } @@ -545,10 +553,14 @@ override Ge$$ "public override string"}, completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().NewText)); - Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().Range), - r => Assert.Equal( - new Range { Start = new Position { Line = 3, Character = 4 }, End = new Position { Line = 3, Character = 12 } }, - r)); + Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Value.Single()), + r => + { + Assert.Equal(3, r.StartLine); + Assert.Equal(4, r.StartColumn); + Assert.Equal(3, r.EndLine); + Assert.Equal(12, r.EndColumn); + }); Assert.All(completions.Items, c => { diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/QuickInfoProviderFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/QuickInfoProviderFacts.cs index 63d4b1b5f2..c25bbe27e3 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/QuickInfoProviderFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/QuickInfoProviderFacts.cs @@ -1,6 +1,4 @@ -using System.Collections.Generic; -using System.Collections.Immutable; -using System.IO; +using System.IO; using System.Threading.Tasks; using OmniSharp.Models; using OmniSharp.Options; @@ -232,7 +230,7 @@ public async Task DisplayFormatFor_TypeSymbol_ComplexType_DifferentNamespace() public async Task DisplayFormatFor_TypeSymbol_WithGenerics() { var response = await GetTypeLookUpResponse(line: 15, column: 36); - Assert.Equal("```csharp\ninterface System.Collections.Generic.IDictionary\n```\n```csharp\n\nTKey is string\nTValue is IEnumerable\n```", response.Markdown); + Assert.Equal("```csharp\ninterface System.Collections.Generic.IDictionary\n```\n\n\n\n```csharp\nTKey is string\nTValue is IEnumerable\n```", response.Markdown); } [Fact] @@ -340,7 +338,7 @@ class testissue } }"; var response = await GetTypeLookUpResponse(content); - Assert.Equal("```csharp\nbool testissue.Compare(int gameObject, string tagName)\n```\n\nReturns:\n\n Returns true if object is tagged with tag\\.", response.Markdown); + Assert.Equal("```csharp\nbool testissue.Compare(int gameObject, string tagName)\n```\n\n\n\nReturns:\n\n Returns true if object is tagged with tag\\.", response.Markdown); } [Fact] @@ -371,7 +369,7 @@ class testissue } }"; var response = await GetTypeLookUpResponse(content); - Assert.Equal("```csharp\nbool testissue.Compare(int gameObject, string tagName)\n```\n\nExceptions:\n\n A\n\n B", response.Markdown); + Assert.Equal("```csharp\nbool testissue.Compare(int gameObject, string tagName)\n```\n\n\n\nExceptions:\n\n A\n\n B", response.Markdown); } [Fact] @@ -602,7 +600,7 @@ class testissue }"; var response = await GetTypeLookUpResponse(content); Assert.Equal( - "```csharp\nT[] testissue.Compare(int gameObject)\n```\n\nChecks if object is tagged with the tag\\.\n\nYou may have some additional information about this class here\\.\n\nReturns:\n\n Returns an array of type `T`\\.\n\n\n\nExceptions:\n\n `System.Exception`", + "```csharp\nT[] testissue.Compare(int gameObject)\n```\n\nChecks if object is tagged with the tag\\.\n\nYou may have some additional information about this class here\\.\n\nReturns:\n\n Returns an array of type `T`\\.\n\n\n\nExceptions:\n\n```csharp\n System.Exception\n```", response.Markdown); } @@ -667,7 +665,7 @@ void M2() } }"; var response = await GetTypeLookUpResponse(content); - Assert.Equal("```csharp\nvoid C.M1<'a>('a t)\n```\n\nAnonymous Types:\n```csharp\n 'a is new { int X, int Y }\n```", response.Markdown); + Assert.Equal("```csharp\nvoid C.M1<'a>('a t)\n```\n\n\n\nAnonymous Types:\n\n```csharp\n 'a is new { int X, int Y }\n```", response.Markdown); } [Fact] From e3c7eccef1488548534f7ec31bde325a4a54d089 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Sat, 15 Aug 2020 15:52:35 -0700 Subject: [PATCH 4/5] Refactor to use new extension method. --- .../Services/Completion/CompletionService.cs | 1 + .../Services/Formatting/FormatAfterKeystrokeService.cs | 3 ++- .../Services/Formatting/FormatRangeService.cs | 3 ++- .../Services/Intellisense/IntellisenseService.cs | 2 +- .../Services/Navigation/FindImplementationsService.cs | 2 +- .../Services/Navigation/GotoDefinitionService.cs | 2 +- src/OmniSharp.Roslyn.CSharp/Services/QuickInfoProvider.cs | 3 ++- .../Services/Refactoring/GetCodeActionService.cs | 3 ++- .../Services/Refactoring/RenameService.cs | 3 ++- .../Services/Refactoring/RunCodeActionService.cs | 3 ++- .../Services/Signatures/SignatureHelpService.cs | 3 ++- .../Services/TestCommands/TestCommandService.cs | 3 ++- src/OmniSharp.Roslyn.CSharp/Services/Types/TypeLookup.cs | 3 ++- .../Extensions}/SourceTextExtensions.cs | 2 +- 14 files changed, 23 insertions(+), 13 deletions(-) rename src/{OmniSharp.Roslyn.CSharp/Helpers => OmniSharp.Roslyn/Extensions}/SourceTextExtensions.cs (88%) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs index e17469f1e7..7d89f6c69e 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs @@ -11,6 +11,7 @@ using Microsoft.CodeAnalysis.Completion; using Microsoft.CodeAnalysis.Tags; using Microsoft.Extensions.Logging; +using OmniSharp.Extensions; using OmniSharp.Mef; using OmniSharp.Models; using OmniSharp.Models.v1.Completion; diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Formatting/FormatAfterKeystrokeService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Formatting/FormatAfterKeystrokeService.cs index 11c3af9940..c62949707d 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Formatting/FormatAfterKeystrokeService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Formatting/FormatAfterKeystrokeService.cs @@ -4,6 +4,7 @@ using Microsoft.CodeAnalysis.Text; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using OmniSharp.Extensions; using OmniSharp.Mef; using OmniSharp.Models.Format; using OmniSharp.Options; @@ -35,7 +36,7 @@ public async Task Handle(FormatAfterKeystrokeRequest reques } var text = await document.GetTextAsync(); - int position = text.Lines.GetPosition(new LinePosition(request.Line, request.Column)); + int position = text.GetTextPosition(request); var changes = await FormattingWorker.GetFormattingChangesAfterKeystroke(document, position, request.Char, _omnisharpOptions, _loggerFactory); return new FormatRangeResponse() diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Formatting/FormatRangeService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Formatting/FormatRangeService.cs index aa42ae9aec..07493e24dd 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Formatting/FormatRangeService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Formatting/FormatRangeService.cs @@ -3,6 +3,7 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Text; using Microsoft.Extensions.Logging; +using OmniSharp.Extensions; using OmniSharp.Mef; using OmniSharp.Models.Format; using OmniSharp.Options; @@ -34,7 +35,7 @@ public async Task Handle(FormatRangeRequest request) } var text = await document.GetTextAsync(); - var start = text.Lines.GetPosition(new LinePosition(request.Line, request.Column)); + var start = text.GetTextPosition(request); var end = text.Lines.GetPosition(new LinePosition(request.EndLine, request.EndColumn)); var syntaxTree = await document.GetSyntaxRootAsync(); var tokenStart = syntaxTree.FindToken(start).FullSpan.Start; diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Intellisense/IntellisenseService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Intellisense/IntellisenseService.cs index b92bac7857..b042f092d8 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Intellisense/IntellisenseService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Intellisense/IntellisenseService.cs @@ -37,7 +37,7 @@ public async Task> Handle(AutoCompleteRequest foreach (var document in documents) { var sourceText = await document.GetTextAsync(); - var position = sourceText.Lines.GetPosition(new LinePosition(request.Line, request.Column)); + var position = sourceText.GetTextPosition(request); var service = CompletionService.GetService(document); var completionList = await service.GetCompletionsAsync(document, position); diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Navigation/FindImplementationsService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Navigation/FindImplementationsService.cs index dcbb374b72..2d8e198390 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Navigation/FindImplementationsService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Navigation/FindImplementationsService.cs @@ -34,7 +34,7 @@ public async Task Handle(FindImplementationsRequest request) { var semanticModel = await document.GetSemanticModelAsync(); var sourceText = await document.GetTextAsync(); - var position = sourceText.Lines.GetPosition(new LinePosition(request.Line, request.Column)); + var position = sourceText.GetTextPosition(request); var quickFixes = new List(); var symbol = await SymbolFinder.FindSymbolAtPositionAsync(semanticModel, position, _workspace); diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Navigation/GotoDefinitionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Navigation/GotoDefinitionService.cs index 06c84fdd94..18d99416dc 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Navigation/GotoDefinitionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Navigation/GotoDefinitionService.cs @@ -39,7 +39,7 @@ public async Task Handle(GotoDefinitionRequest request) { var semanticModel = await document.GetSemanticModelAsync(); var sourceText = await document.GetTextAsync(); - var position = sourceText.Lines.GetPosition(new LinePosition(request.Line, request.Column)); + var position = sourceText.GetTextPosition(request); var symbol = await SymbolFinder.FindSymbolAtPositionAsync(semanticModel, position, _workspace); // go to definition for namespaces is not supported diff --git a/src/OmniSharp.Roslyn.CSharp/Services/QuickInfoProvider.cs b/src/OmniSharp.Roslyn.CSharp/Services/QuickInfoProvider.cs index f0ae892db6..30587d1f39 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/QuickInfoProvider.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/QuickInfoProvider.cs @@ -6,6 +6,7 @@ using Microsoft.CodeAnalysis.QuickInfo; using Microsoft.CodeAnalysis.Text; using Microsoft.Extensions.Logging; +using OmniSharp.Extensions; using OmniSharp.Mef; using OmniSharp.Models; using OmniSharp.Options; @@ -59,7 +60,7 @@ public async Task Handle(QuickInfoRequest request) } var sourceText = await document.GetTextAsync(); - var position = sourceText.Lines.GetPosition(new LinePosition(request.Line, request.Column)); + var position = sourceText.GetTextPosition(request); var quickInfo = await quickInfoService.GetQuickInfoAsync(document, position); if (quickInfo is null) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/GetCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/GetCodeActionService.cs index bc84766125..c5ec6528d9 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/GetCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/GetCodeActionService.cs @@ -7,6 +7,7 @@ using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeRefactorings; using Microsoft.CodeAnalysis.Text; +using OmniSharp.Extensions; using OmniSharp.Mef; using OmniSharp.Models.CodeAction; using OmniSharp.Services; @@ -40,7 +41,7 @@ public async Task Handle(GetCodeActionRequest request) if (document != null) { var sourceText = await document.GetTextAsync(); - var position = sourceText.Lines.GetPosition(new LinePosition(request.Line, request.Column)); + var position = sourceText.GetTextPosition(request); var location = new TextSpan(position, 1); return new CodeRefactoringContext(document, location, (a) => actionsDestination.Add(a), CancellationToken.None); } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RenameService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RenameService.cs index 8b136b0761..815203cca3 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RenameService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RenameService.cs @@ -7,6 +7,7 @@ using Microsoft.CodeAnalysis.FindSymbols; using Microsoft.CodeAnalysis.Rename; using Microsoft.CodeAnalysis.Text; +using OmniSharp.Extensions; using OmniSharp.Mef; using OmniSharp.Models; using OmniSharp.Models.Rename; @@ -33,7 +34,7 @@ public async Task Handle(RenameRequest request) if (document != null) { var sourceText = await document.GetTextAsync(); - var position = sourceText.Lines.GetPosition(new LinePosition(request.Line, request.Column)); + var position = sourceText.GetTextPosition(request); var symbol = await SymbolFinder.FindSymbolAtPositionAsync(document, position); Solution solution = _workspace.CurrentSolution; diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RunCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RunCodeActionService.cs index 357fd2a95e..bc6fe92012 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RunCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RunCodeActionService.cs @@ -7,6 +7,7 @@ using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeRefactorings; using Microsoft.CodeAnalysis.Text; +using OmniSharp.Extensions; using OmniSharp.Mef; using OmniSharp.Models.CodeAction; using OmniSharp.Roslyn.Utilities; @@ -73,7 +74,7 @@ public async Task Handle(RunCodeActionRequest request) if (document != null) { var sourceText = await document.GetTextAsync(); - var position = sourceText.Lines.GetPosition(new LinePosition(request.Line, request.Column)); + var position = sourceText.GetTextPosition(request); var location = new TextSpan(position, 1); return new CodeRefactoringContext(document, location, (a) => actionsDestination.Add(a), CancellationToken.None); } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Signatures/SignatureHelpService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Signatures/SignatureHelpService.cs index 3c5a3a7f7d..ec85681855 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Signatures/SignatureHelpService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Signatures/SignatureHelpService.cs @@ -6,6 +6,7 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Text; +using OmniSharp.Extensions; using OmniSharp.Mef; using OmniSharp.Models; using OmniSharp.Models.SignatureHelp; @@ -104,7 +105,7 @@ throughExpression is LiteralExpressionSyntax || private async Task GetInvocation(Document document, Request request) { var sourceText = await document.GetTextAsync(); - var position = sourceText.Lines.GetPosition(new LinePosition(request.Line, request.Column)); + var position = sourceText.GetTextPosition(request); var tree = await document.GetSyntaxTreeAsync(); var root = await tree.GetRootAsync(); var node = root.FindToken(position).Parent; diff --git a/src/OmniSharp.Roslyn.CSharp/Services/TestCommands/TestCommandService.cs b/src/OmniSharp.Roslyn.CSharp/Services/TestCommands/TestCommandService.cs index 1fc153cc02..63f62386b5 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/TestCommands/TestCommandService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/TestCommands/TestCommandService.cs @@ -7,6 +7,7 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Text; +using OmniSharp.Extensions; using OmniSharp.Mef; using OmniSharp.Models; using OmniSharp.Models.TestCommand; @@ -38,7 +39,7 @@ public async Task Handle(TestCommandRequest request) var semanticModel = await document.GetSemanticModelAsync(); var syntaxTree = semanticModel.SyntaxTree; var sourceText = await document.GetTextAsync(); - var position = sourceText.Lines.GetPosition(new LinePosition(request.Line, request.Column)); + var position = sourceText.GetTextPosition(request); var node = syntaxTree.GetRoot().FindToken(position).Parent; SyntaxNode method = node.FirstAncestorOrSelf(); diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Types/TypeLookup.cs b/src/OmniSharp.Roslyn.CSharp/Services/Types/TypeLookup.cs index 89fc1a4bd5..a721a7a52b 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Types/TypeLookup.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Types/TypeLookup.cs @@ -3,6 +3,7 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.FindSymbols; using Microsoft.CodeAnalysis.Text; +using OmniSharp.Extensions; using OmniSharp.Mef; using OmniSharp.Models.TypeLookup; using OmniSharp.Options; @@ -44,7 +45,7 @@ public async Task Handle(TypeLookupRequest request) { var semanticModel = await document.GetSemanticModelAsync(); var sourceText = await document.GetTextAsync(); - var position = sourceText.Lines.GetPosition(new LinePosition(request.Line, request.Column)); + var position = sourceText.GetTextPosition(request); var symbol = await SymbolFinder.FindSymbolAtPositionAsync(semanticModel, position, _workspace); if (symbol != null) { diff --git a/src/OmniSharp.Roslyn.CSharp/Helpers/SourceTextExtensions.cs b/src/OmniSharp.Roslyn/Extensions/SourceTextExtensions.cs similarity index 88% rename from src/OmniSharp.Roslyn.CSharp/Helpers/SourceTextExtensions.cs rename to src/OmniSharp.Roslyn/Extensions/SourceTextExtensions.cs index b7e40b2418..efefc56f99 100644 --- a/src/OmniSharp.Roslyn.CSharp/Helpers/SourceTextExtensions.cs +++ b/src/OmniSharp.Roslyn/Extensions/SourceTextExtensions.cs @@ -3,7 +3,7 @@ using Microsoft.CodeAnalysis.Text; using OmniSharp.Models; -namespace OmniSharp.Roslyn.CSharp.Helpers +namespace OmniSharp.Extensions { internal static class SourceTextExtensions { From a22d6cb26e7793f480c309ab354732d3b2f4edba Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Sat, 15 Aug 2020 16:36:00 -0700 Subject: [PATCH 5/5] Standardize formatting so Linux and Mac tests work as well. --- .../CompletionFacts.cs | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs index 8350b6e96d..fc60da18ec 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs @@ -357,44 +357,44 @@ MyClass m$$ [InlineData("dummy.csx")] public async Task OverrideSignatures_Publics(string filename) { - const string source = - @"class Foo - { - public virtual void Test(string text) {} - public virtual void Test(string text, string moreText) {} - } + const string source = @" +class Foo +{ + public virtual void Test(string text) {} + public virtual void Test(string text, string moreText) {} +} - class FooChild : Foo - { - override $$ - } - "; +class FooChild : Foo +{ + override $$ +} +"; var completions = await FindCompletionsAsync(filename, source); Assert.Equal(new[] { "Equals(object obj)", "GetHashCode()", "Test(string text)", "Test(string text, string moreText)", "ToString()" }, completions.Items.Select(c => c.Label)); - Assert.Equal(new[] { "Equals(object obj)\n {\n return base.Equals(obj);$0\n \\}\n", - "GetHashCode()\n {\n return base.GetHashCode();$0\n \\}\n", - "Test(string text)\n {\n base.Test(text);$0\n \\}\n", - "Test(string text, string moreText)\n {\n base.Test(text, moreText);$0\n \\}\n", - "ToString()\n {\n return base.ToString();$0\n \\}\n" + Assert.Equal(new[] { "Equals(object obj)\n {\n return base.Equals(obj);$0\n \\}", + "GetHashCode()\n {\n return base.GetHashCode();$0\n \\}", + "Test(string text)\n {\n base.Test(text);$0\n \\}", + "Test(string text, string moreText)\n {\n base.Test(text, moreText);$0\n \\}", + "ToString()\n {\n return base.ToString();$0\n \\}" }, - completions.Items.Select(c => c.InsertText)); + completions.Items.Select(c => c.InsertText)); - Assert.Equal(new[] { "\n public override bool", - "\n public override int", - "\n public override void", - "\n public override void", - "\n public override string"}, + Assert.Equal(new[] { "public override bool", + "public override int", + "public override void", + "public override void", + "public override string"}, completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().NewText)); Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Value.Single()), r => { - Assert.Equal(7, r.StartLine); - Assert.Equal(21, r.StartColumn); - Assert.Equal(8, r.EndLine); - Assert.Equal(30, r.EndColumn); + Assert.Equal(9, r.StartLine); + Assert.Equal(4, r.StartColumn); + Assert.Equal(9, r.EndLine); + Assert.Equal(12, r.EndColumn); }); Assert.All(completions.Items, c => Assert.Equal(InsertTextFormat.Snippet, c.InsertTextFormat));