Add TextEdits support to InlayHints

[JoeRobich]

No description provided.

[333fred]

LGTM, just a couple minor comments.

[JoeRobich]

@bjorkstromm Seeing an odd Cake completion failure in the Windows net6.0 tests. The override completion is returning AdditionalTextEdits which appear to be formatting the DocComments from DotNetCoreAliases.cs. I saw the CompletionResolve handler was nulling out the AdditionalTextEdits property and was unsure if the CompletionRequest handler should be doing something similar. Surprised we only see this on Windows.

[xUnit.net 00:00:57.29]     OmniSharp.Cake.Tests.CompletionFacts.ShouldGetAdditionalTextEditsFromOverrideCompletion [FAIL][xUnit.net 00:00:57.29]       Assert.All() Failure: 5 out of 5 items in the collection did not pass.
[xUnit.net 00:00:57.29]       [4]: Item: { Label = ToString(), CompletionItemKind = Method }
[xUnit.net 00:00:57.29]            Xunit.Sdk.NullException: Assert.Null() Failure
[xUnit.net 00:00:57.29]            Expected: (null)
[xUnit.net 00:00:57.29]            Actual:   List<LinePositionSpanTextChange> [LinePositionSpanTextChange { StartLine=4155, StartColumn=7, EndLine=4203, EndColumn=7, NewText= }, LinePositionSpanTextChange { StartLine=4203, StartColumn=30, EndLine=4203, EndColumn=101, NewText= }, LinePositionSpanTextChange { StartLine=4207, StartColumn=76, EndLine=4207, EndColumn=86, NewText= }, LinePositionSpanTextChange { StartLine=4212, StartColumn=15, EndLine=4232, EndColumn=4, NewText=[deprecated] DotNetCoreRestore is obsolete and will be removed in a future release. Use <see cref="M:Cake.Common.Tools.DotNet.DotNetAliases.DotNetRestore(Cake.Core.ICakeContext,System.String)" /> instead.\r\n///            Restore all NuGet Packages in the specified path.\r\n///            </summary>\r\n/// <param name="context">The context.</param>\r\n/// <param name="root">Path to the project file to restore.</param>\r\n/// <example>\r\n///            <code>\r\n///            DotNetCoreRestore("./src/MyProject/MyProject.csproj");\r\n///            </code>\r\n/// }, LinePositionSpanTextChange { StartLine=4233, StartColumn=48, EndLine=4233, EndColumn=121, NewText= }, ...]
[xUnit.net 00:00:57.29]               at Xunit.Assert.Null(Object object) in C:\Dev\xunit\xunit\src\xunit.assert\Asserts\NullAsserts.cs:line 31
[xUnit.net 00:00:57.29]               at OmniSharp.Cake.Tests.CompletionFacts.<>c.<ShouldGetAdditionalTextEditsFromOverrideCompletion>b__9_3(CompletionItem c) in D:\a\omnisharp-roslyn\omnisharp-roslyn\tests\OmniSharp.Cake.Tests\CompletionFacts.cs:line 168
[xUnit.net 00:00:57.29]               at Xunit.Assert.All[T](IEnumerable`1 collection, Action`1 action) in C:\Dev\xunit\xunit\src\xunit.assert\Asserts\CollectionAsserts.cs:line 36
... Other completion items carry the same set of AdditionalTextEdits ...

[bjorkstromm]

@bjorkstromm Seeing an odd Cake completion failure in the Windows net6.0 tests. The override completion is returning AdditionalTextEdits which appear to be formatting the DocComments from DotNetCoreAliases.cs. I saw the CompletionResolve handler was nulling out the AdditionalTextEdits property and was unsure if the CompletionRequest handler should be doing something similar. Surprised we only see this on Windows.

[xUnit.net 00:00:57.29]     OmniSharp.Cake.Tests.CompletionFacts.ShouldGetAdditionalTextEditsFromOverrideCompletion [FAIL][xUnit.net 00:00:57.29]       Assert.All() Failure: 5 out of 5 items in the collection did not pass.
[xUnit.net 00:00:57.29]       [4]: Item: { Label = ToString(), CompletionItemKind = Method }
[xUnit.net 00:00:57.29]            Xunit.Sdk.NullException: Assert.Null() Failure
[xUnit.net 00:00:57.29]            Expected: (null)
[xUnit.net 00:00:57.29]            Actual:   List<LinePositionSpanTextChange> [LinePositionSpanTextChange { StartLine=4155, StartColumn=7, EndLine=4203, EndColumn=7, NewText= }, LinePositionSpanTextChange { StartLine=4203, StartColumn=30, EndLine=4203, EndColumn=101, NewText= }, LinePositionSpanTextChange { StartLine=4207, StartColumn=76, EndLine=4207, EndColumn=86, NewText= }, LinePositionSpanTextChange { StartLine=4212, StartColumn=15, EndLine=4232, EndColumn=4, NewText=[deprecated] DotNetCoreRestore is obsolete and will be removed in a future release. Use <see cref="M:Cake.Common.Tools.DotNet.DotNetAliases.DotNetRestore(Cake.Core.ICakeContext,System.String)" /> instead.\r\n///            Restore all NuGet Packages in the specified path.\r\n///            </summary>\r\n/// <param name="context">The context.</param>\r\n/// <param name="root">Path to the project file to restore.</param>\r\n/// <example>\r\n///            <code>\r\n///            DotNetCoreRestore("./src/MyProject/MyProject.csproj");\r\n///            </code>\r\n/// }, LinePositionSpanTextChange { StartLine=4233, StartColumn=48, EndLine=4233, EndColumn=121, NewText= }, ...]
[xUnit.net 00:00:57.29]               at Xunit.Assert.Null(Object object) in C:\Dev\xunit\xunit\src\xunit.assert\Asserts\NullAsserts.cs:line 31
[xUnit.net 00:00:57.29]               at OmniSharp.Cake.Tests.CompletionFacts.<>c.<ShouldGetAdditionalTextEditsFromOverrideCompletion>b__9_3(CompletionItem c) in D:\a\omnisharp-roslyn\omnisharp-roslyn\tests\OmniSharp.Cake.Tests\CompletionFacts.cs:line 168
[xUnit.net 00:00:57.29]               at Xunit.Assert.All[T](IEnumerable`1 collection, Action`1 action) in C:\Dev\xunit\xunit\src\xunit.assert\Asserts\CollectionAsserts.cs:line 36
... Other completion items carry the same set of AdditionalTextEdits ...

@JoeRobich, hmm unsure what the correct approach is. I'll have a look soon. But I think nulling might be the correct approach. Ok if I push to your branch if I find a solution?

[bjorkstromm]

@JoeRobich, I checked the code.

So, previously (if my old code comment was correct) AdditionalTextEdits didn't include incremental changes, but instead the whole text document, which made it problematic for Cake (since DSL is part of the document). The correct fix would be to translate lines in the AdditionalTextEdits so that generated DSL is not part of the response. I can do this later. Quick fix is to just make it null like in the resolve handler. I can update your branch soon.

Did this PR change behavior so that AdditionalTextEdits now is returned in the completion request, and not in the completion resolve request? If not, it's weird that we haven't seen this before.

[JoeRobich]

Quick fix is to just make it null like in the resolve handler. I can update your branch soon.

I'll give that a shot.

Did this PR change behavior so that AdditionalTextEdits now is returned in the completion request, and not in the completion resolve request? If not, it's weird that we haven't seen this before.

No intentional behavior changes for completion, so it is odd indeed.

[bjorkstromm]

A closer look at the code, and I see that we currently are translating line numbers for the AdditionalTextEdits, thus edits for DSL should be removed.

omnisharp-roslyn/src/OmniSharp.Cake/Extensions/ResponseExtensions.cs

Line 214 in 04a561c

foreach (var additionalTextEdit in item.AdditionalTextEdits ?? Enumerable.Empty<LinePositionSpanTextChange>())

I was however unable to reproduce locally on Windows. I'd say go ahead and make them null still. It "shouldn't" break anything.