* Stop crashing VS when logging errors.
- Ultimately this is more of a workaround for this issue: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1405849. But let me share some insight as to how we got here. When I initially ["protected VS from crashing"](95b33df1d6) due to the compiler bug I added logic that would `LogError` in the case that unexpected exceptions would occur. Now the problem with my approach is that our Language Server logger can potentially explode if the language server is in the midst of shutting down. Therefore, what would happen is the compiler would unexpectedly throw, we'd then try to log an error and if we were in a shutting down state that logging of an error would then throw again resulting in VS crashing.
- Went from logging an error to an unobserved task exception. This means things will still throw; however, they'll be translated into unobserved task exceptions.
Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1420246
* Address code review comments.
- Saw that automation was having a hard time auto-merging and noticed some stale / incorrect dependencies. Ran a few darc commands to fix things up.
- Razor no longer depends on Configuration.Json so could remove & needed to update the Extensiosn.Configuration package to have a coherent parent dependency to match.
- Marked the `RenameHandler` as `#nullable enable` and cleaned up some of the null handling in the rename flow.
- Removed an unnecessary throw if cancellation requested (the following `await` would have captured it anyhow).
- Updated the tests to reflect the new dependency on document snapshots and their virtual documents.
Part of #5017
- Marked the `OnAutoInsertHandler` as `#nullable enable`. Some of the fallout from this was that I also had to mark the HTML C# language server as nullable enable as well and react accordingly. Ultimately this made things a lot more clear on when null could be returned.
- Updated the tests to reflect the new dependency on document snapshots and their virtual documents.
Part of #5017
- When we fail to talk to a semantic endpoint we now indicate that we're out-of-sync.
- Updated the tests to reflect the new dependency on document snapshots and their virtual documents.
Part of #5017
- Updated some logging language + severities
- Removed some unnecessary null checks
- In the pull diagnostic handler removed a comment on long-term goals (we do want to talk to multiple servers)
- Since the request diagnostics on all servers API utilizes `IAsyncEnumerable` I modified the pull diagnostic signature from returning an array which is mutable to an `IReadOnlyList` so I could build a list and then return it in an immutable format.
- Updated the tests to reflect the new dependency on document snapshots and their virtual documents.
#5017
- Had to utilize our LSP document manager on the client to lookup the appropriate `TextBuffer` for translated diagnostic requests.
- Marked translation APIs as `nullable` since our new delegation APIs were. Updated callsites accordingly.
- Updated the tests to reflect the new dependency on document snapshots and their virtual documents.
Part of #5017
- One issue with migrating our pre-existing code action resolve logic to the new APIs is that we needed to know what document a code action resolve request was for. To account for this I had to update all of our resolve code action APIs to flow the corresponding Razor host document so we could look it up on the client when re-invoking. This is the bulk of this changeset.
- Updated the tests to reflect the new dependency on document snapshots and their virtual documents.
Part of #5017
- Had to update our completion resolve handler to take in our LSP document manager to ensure we can resolve corresponding text documents so on re-invocation the LSP platform knows which server to talk to.
- Updated the tests to reflect the new dependency on document snapshots and their virtual documents.
Part of #5017
- Added a `ReinvocationResponse` extension to extract results or log to make reinvocation responses easier to handle.
- Updated the tests to reflect the new dependency on document snapshots and their virtual documents.
Part of #5017
- Migrated projection and document mapping APIs to use new re-invocation APIs. Typically these two APIs will use the top-level Razor buffer as their inspection point for requesting custom messages (language query, document edit remap) so the top-level API's for the two classes didn't really have to change because we could lookup the document snapshot and grab its buffer directly.
- Update the `LSPProjectionProvider` and `LSPDocumentMappingProvider` APIs to be `nullable`
- Updated tests to reflect the new requirement of a document snapshot / `ITextBuffer` callpoint. The tests overall are in awful shape (way toooo mocky) but that's a problem for another day.
Part of #5017
- The new APIs now take `ITextBuffer`s as a route to resolve what language servers to talk to.
- New APIs are nullable decorated to ensure that callers do the right thing.
- Added new extension methods to help acquire `ITextBuffer`s for consumers. This way they can easily pass in the right buffer based on the language they're operating in.
Part of #5017
- Trimmed additional binaries from the VSIX payload that aren't required at runtime.
- Referenced the O# language server directly in the VSIX project since it's concealed at our language server layer with PrivateAssets="All"
- These packages brought in new service hub dependencies that expect us to be more explicit in our trace configuration construction.
- Updated tests to capture new API.
- In preparation for consuming latest VS language server client changes we need to decouple our Visual Studio client glue code from the OmniSharp language server. The specific reason why we need to decouple the two is that the O# bits reference `Microsoft.Bcl.AsyncInterfaces` 6.0.0.0 where as VS only has references to 5.0.0.0. Because of this any consumption of `IAsyncEnumerable` (we'll be consuming this soon) would result in a typeload exception because VS would try and load the version that O# references. We don't want to add a codebase / binding redirect for that type so our only recourse is to remove our dependency.
- Tried taking the most minimal approach to decoupling by marking the O# impl dlls as `PrivateAssets="All"` which effectively means don't let people who depend on me also depend on these dependencies. Therefore when VS.LanguageSErverClient.Razor depends on our language server to construct it no longer will transitively bring in 6.0.0.0 AsyncInterface assemblies.
- Still left the `OmniSharp.Extensions.LanguageProtocol` and top-level Razor language server assemblies intact since they expose protocol types that make it easy to communicate with the server. Eventually we may want to fully separate this dependency when we go OOP.
- Updated test / benchmark projects to now bring in protocol assemblies where needed since impl assemblies were no longer being transitively shared. Found that the AspNetCore.Testing dependency indirectly pulls in `Microsoft.Extensions.Logging` version 6.0.0.0 which put us in an interesting predicament where we needed to pin our logging dependency at test time.
Part of #5017
Microsoft.CodeAnalysis.Razor , Microsoft.AspNetCore.Testing , Microsoft.AspNetCore.Razor.Language , Microsoft.AspNetCore.Razor.Internal.Transport , Microsoft.AspNetCore.Mvc.Razor.Extensions.Version2_X , Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X , Microsoft.AspNetCore.Mvc.Razor.Extensions
From Version 6.0.0-rtm.21518.7 -> To Version 6.0.0-rtm.21518.12
Dependency coherency updates
Microsoft.Extensions.Configuration.Json,Microsoft.Extensions.Logging,System.Diagnostics.DiagnosticSource,System.Resources.Extensions,System.Text.Encodings.Web,Microsoft.Extensions.DependencyModel,Microsoft.NETCore.App.Ref,Microsoft.NETCore.BrowserDebugHost.Transport,Microsoft.NETCore.App.Runtime.win-x64,Microsoft.NETCore.Platforms
From Version 6.0.0-rtm.21517.2 -> To Version 6.0.0-rtm.21518.4 (parent: Microsoft.CodeAnalysis.Razor
[main] Update dependencies from dotnet/arcade dotnet/aspnetcore
- Coherency Updates:
- Microsoft.Extensions.Configuration.Json: from 6.0.0-rtm.21515.15 to 6.0.0-rtm.21517.2 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.Extensions.Logging: from 6.0.0-rtm.21515.15 to 6.0.0-rtm.21517.2 (parent: Microsoft.CodeAnalysis.Razor)
- System.Diagnostics.DiagnosticSource: from 6.0.0-rtm.21515.15 to 6.0.0-rtm.21517.2 (parent: Microsoft.CodeAnalysis.Razor)
- System.Resources.Extensions: from 6.0.0-rtm.21515.15 to 6.0.0-rtm.21517.2 (parent: Microsoft.CodeAnalysis.Razor)
- System.Text.Encodings.Web: from 6.0.0-rtm.21515.15 to 6.0.0-rtm.21517.2 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.Extensions.DependencyModel: from 6.0.0-rtm.21515.15 to 6.0.0-rtm.21517.2 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.App.Ref: from 6.0.0-rtm.21515.15 to 6.0.0-rtm.21517.2 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.BrowserDebugHost.Transport: from 6.0.0-rtm.21515.15 to 6.0.0-rtm.21517.2 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.App.Runtime.win-x64: from 6.0.0-rtm.21515.15 to 6.0.0-rtm.21517.2 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.Platforms: from 6.0.0-rtm.21515.15 to 6.0.0-rtm.21517.2 (parent: Microsoft.CodeAnalysis.Razor)
[main] Update dependencies from dotnet/aspnetcore
- Coherency Updates:
- Microsoft.Extensions.Configuration.Json: from 6.0.0-rtm.21515.11 to 6.0.0-rtm.21515.15 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.Extensions.Logging: from 6.0.0-rtm.21515.11 to 6.0.0-rtm.21515.15 (parent: Microsoft.CodeAnalysis.Razor)
- System.Diagnostics.DiagnosticSource: from 6.0.0-rtm.21515.11 to 6.0.0-rtm.21515.15 (parent: Microsoft.CodeAnalysis.Razor)
- System.Resources.Extensions: from 6.0.0-rtm.21515.11 to 6.0.0-rtm.21515.15 (parent: Microsoft.CodeAnalysis.Razor)
- System.Text.Encodings.Web: from 6.0.0-rtm.21515.11 to 6.0.0-rtm.21515.15 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.Extensions.DependencyModel: from 6.0.0-rtm.21515.11 to 6.0.0-rtm.21515.15 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.App.Ref: from 6.0.0-rtm.21515.11 to 6.0.0-rtm.21515.15 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.BrowserDebugHost.Transport: from 6.0.0-rtm.21515.11 to 6.0.0-rtm.21515.15 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.App.Runtime.win-x64: from 6.0.0-rtm.21515.11 to 6.0.0-rtm.21515.15 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.Platforms: from 6.0.0-rtm.21515.11 to 6.0.0-rtm.21515.15 (parent: Microsoft.CodeAnalysis.Razor)
[main] Update dependencies from dotnet/aspnetcore
- Coherency Updates:
- Microsoft.Extensions.Configuration.Json: from 6.0.0-rtm.21515.6 to 6.0.0-rtm.21515.11 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.Extensions.Logging: from 6.0.0-rtm.21515.6 to 6.0.0-rtm.21515.11 (parent: Microsoft.CodeAnalysis.Razor)
- System.Diagnostics.DiagnosticSource: from 6.0.0-rtm.21515.6 to 6.0.0-rtm.21515.11 (parent: Microsoft.CodeAnalysis.Razor)
- System.Resources.Extensions: from 6.0.0-rtm.21515.6 to 6.0.0-rtm.21515.11 (parent: Microsoft.CodeAnalysis.Razor)
- System.Text.Encodings.Web: from 6.0.0-rtm.21515.6 to 6.0.0-rtm.21515.11 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.Extensions.DependencyModel: from 6.0.0-rtm.21515.6 to 6.0.0-rtm.21515.11 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.App.Ref: from 6.0.0-rtm.21515.6 to 6.0.0-rtm.21515.11 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.BrowserDebugHost.Transport: from 6.0.0-rtm.21515.6 to 6.0.0-rtm.21515.11 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.App.Runtime.win-x64: from 6.0.0-rtm.21515.6 to 6.0.0-rtm.21515.11 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.Platforms: from 6.0.0-rtm.21515.6 to 6.0.0-rtm.21515.11 (parent: Microsoft.CodeAnalysis.Razor)
[main] Update dependencies from dotnet/aspnetcore
- Coherency Updates:
- Microsoft.Extensions.Configuration.Json: from 6.0.0-rtm.21514.7 to 6.0.0-rtm.21515.6 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.Extensions.Logging: from 6.0.0-rtm.21514.7 to 6.0.0-rtm.21515.6 (parent: Microsoft.CodeAnalysis.Razor)
- System.Diagnostics.DiagnosticSource: from 6.0.0-rtm.21514.7 to 6.0.0-rtm.21515.6 (parent: Microsoft.CodeAnalysis.Razor)
- System.Resources.Extensions: from 6.0.0-rtm.21514.7 to 6.0.0-rtm.21515.6 (parent: Microsoft.CodeAnalysis.Razor)
- System.Text.Encodings.Web: from 6.0.0-rtm.21514.7 to 6.0.0-rtm.21515.6 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.Extensions.DependencyModel: from 6.0.0-rtm.21514.7 to 6.0.0-rtm.21515.6 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.App.Ref: from 6.0.0-rtm.21514.7 to 6.0.0-rtm.21515.6 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.BrowserDebugHost.Transport: from 6.0.0-rtm.21514.7 to 6.0.0-rtm.21515.6 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.App.Runtime.win-x64: from 6.0.0-rtm.21514.7 to 6.0.0-rtm.21515.6 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.Platforms: from 6.0.0-rtm.21514.7 to 6.0.0-rtm.21515.6 (parent: Microsoft.CodeAnalysis.Razor)
- This is part 2/2 to fix `@using` statement completion in Razor files. Part 1 is over in the dotnet/roslyn repo: [PR](https://github.com/dotnet/roslyn/pull/57160)
- So the two components to this issue are as follows:
1. C# was throwing whenever they'd be in the process of returning `isIncomplete: true` completion lists and filter down to 0 items. This typically happened at the `n` of `@using`. Therefore things would explode preventing us from being able to control the completion experience
2. Found that the Razor compiler doesn't generate C# for `@using` statements. This means when we try to query what language exists at `@using|` it says "sorry nothing". My bet is that the compiler special cases C# keywords and preemptively doesn't generate C# for them. This means we bail out of our completion path entirely and return an empty completion list breaking the flow. That issue is tracked here: https://github.com/dotnet/aspnetcore/issues/37568
- Razor tooling wasn't directly responsible for the second issue; however, wes till needed to come up with a low-risk fix. The fix I landed on was to special case the bug and call it out specifically in code. The special case consists of three components:
1. Only check special cases if we failed to resolve a language. This is already a completion failure case so in the worst case we still don't provide completion
2. Detecting if we're in an `isIncomplete: true` scenario by looking at the trigger kind and seeing if it's `TriggerForIncompleteCompletions`next we look at the word span that's currently active
3. Detecting if the user is attempting to type a C# keyword (full match like `using` in `@using|`)
- Given this fix is going to go through QB for dev17 I added insane amounts of tests/checks to ensure things are of the highest quality / we don't degrade functionality.
### Before
### After
Fixes#5606
[main] Update dependencies from dotnet/aspnetcore
- Coherency Updates:
- Microsoft.Extensions.Configuration.Json: from 6.0.0-rtm.21513.26 to 6.0.0-rtm.21514.7 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.Extensions.Logging: from 6.0.0-rtm.21513.26 to 6.0.0-rtm.21514.7 (parent: Microsoft.CodeAnalysis.Razor)
- System.Diagnostics.DiagnosticSource: from 6.0.0-rtm.21513.26 to 6.0.0-rtm.21514.7 (parent: Microsoft.CodeAnalysis.Razor)
- System.Resources.Extensions: from 6.0.0-rtm.21513.26 to 6.0.0-rtm.21514.7 (parent: Microsoft.CodeAnalysis.Razor)
- System.Text.Encodings.Web: from 6.0.0-rtm.21513.26 to 6.0.0-rtm.21514.7 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.Extensions.DependencyModel: from 6.0.0-rtm.21513.26 to 6.0.0-rtm.21514.7 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.App.Ref: from 6.0.0-rtm.21513.26 to 6.0.0-rtm.21514.7 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.BrowserDebugHost.Transport: from 6.0.0-rtm.21513.26 to 6.0.0-rtm.21514.7 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.App.Runtime.win-x64: from 6.0.0-rtm.21513.26 to 6.0.0-rtm.21514.7 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.Platforms: from 6.0.0-rtm.21513.26 to 6.0.0-rtm.21514.7 (parent: Microsoft.CodeAnalysis.Razor)
[main] Update dependencies from dotnet/arcade dotnet/aspnetcore
- Coherency Updates:
- Microsoft.Extensions.Configuration.Json: from 6.0.0-rc.2.21470.23 to 6.0.0-rtm.21513.26 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.Extensions.Logging: from 6.0.0-rc.2.21470.23 to 6.0.0-rtm.21513.26 (parent: Microsoft.CodeAnalysis.Razor)
- System.Diagnostics.DiagnosticSource: from 6.0.0-rc.2.21470.23 to 6.0.0-rtm.21513.26 (parent: Microsoft.CodeAnalysis.Razor)
- System.Resources.Extensions: from 6.0.0-rc.2.21470.23 to 6.0.0-rtm.21513.26 (parent: Microsoft.CodeAnalysis.Razor)
- System.Text.Encodings.Web: from 6.0.0-rc.2.21470.23 to 6.0.0-rtm.21513.26 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.Extensions.DependencyModel: from 6.0.0-rc.2.21470.23 to 6.0.0-rtm.21513.26 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.App.Ref: from 6.0.0-rc.2.21470.23 to 6.0.0-rtm.21513.26 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.BrowserDebugHost.Transport: from 6.0.0-rc.2.21470.23 to 6.0.0-rtm.21513.26 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.App.Runtime.win-x64: from 6.0.0-rc.2.21470.23 to 6.0.0-rtm.21513.26 (parent: Microsoft.CodeAnalysis.Razor)
- Microsoft.NETCore.Platforms: from 6.0.0-rc.2.21470.23 to 6.0.0-rtm.21513.26 (parent: Microsoft.CodeAnalysis.Razor)
- Prior to this C# completion would only ever be offered at start of word. Now we offer C# completion always to work around a racey:
When a user is doing 24/7 completion (just typing letters) HTML & C# only offer 24/7 completion at the beginning of a word. Meaning, completions will be provided at `|D` but not for `|Da` which brings us to an interesting cross-roads. Razor is currently designed with two language servers:
1. HTML C#: Powers the HTML / C# experience
2. Razor: Has all of the Razor understanding / powers the generated C# & HTML for a document
Because of this split, in the middle of completion requests it's possible for additional generated content (C# or HTML) to flow into the client. When additional content flows to the client we could mean to ask for completions at `|D` but in practice it'd ask C# for `|Da` (resulting in 0 completions). Therefore, to counteract this point-in-time design flaw we translate typing completion requests to explicit in order
to ensure that we still get completion results at `|Da`.
- Added tests to validate this new behavior.
- After changing C# behavior it revealed a separate unintended bug in the Razor completion engine. Basically another inherent race in how we do didChange requests. That's being addressed in future PRs (lots coming).
Part of #5017
N. Taylor Mullen
Allison Chou
David Wengier
dotnet-maestro[bot]
dotnet-maestro[bot]
David Wengier
Jon Fortescue
