Override completion can mess up based on context

[333fred]

[warn]: OmniSharp.Roslyn.CSharp.Services.Completion.CompletionService
        Could not find the first index of the display text.
Display text: IsDefinedInSourceTree(SyntaxTree tree, TextSpan? definedWithinSpan, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)).
Completion Text: using System.Threading;

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
    internal abstract class SourceUserDefinedOperatorSymbolBase : SourceMemberMethodSymbol
    {
        internal override bool IsDefinedInSourceTree(SyntaxTree tree, TextSpan? definedWithinSpan, CancellationToken cancellationToken = default)
        {
            return base.IsDefinedInSourceTree(tree, definedWithinSpan, cancellationToken);
        }

This can happen when the DisplayText uses fully-qualified names, but the actual change adds an import, and then minimally qualifies the result/simplifies default/plays around with nullable. This then causes an exception because we assign default(ImmutableArray<LinePositionChange> to additionalTextEdits, rather than assigning null, which causes Newtonsoft.Json to barf.

[333fred]

Note there are 2 bugs here:

1. If we fail to find where to make the additional edits, we should not use a default array. https://github.com/OmniSharp/omnisharp-roslyn/blob/master/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs#L455 needs to change to return ImmutableArray<LinePositionSpanTextChange>?.
2. We need a workaround for this case of override completion where the displaytext is not in the change.

[filipw]

on that note, I think ImmutableArray should be avoided on JSON serializable contract types, we can move those to IList to be safe. I remember there were similar issues in Roslyn in the past e.g. dotnet/roslyn#21539

[333fred]

I'd prefer IReadonlyList, but yes. I really wanted to use ImmutableArray because it's both threadsafe and performant

[333fred]

What I think will work as a simple algorithm for finding the correct position in the change text for override completion is to find the last instance of the text override (including the spaces). From there, find the first instance of the display text up to but not including the first ( in that text.

[333fred]

Import completion shouldn't have this issue and there's nothing qualified in the DisplayText.