Remove LocateOwner from HoverInfoService #9112
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
davidwengier
approved these changes
Aug 9, 2023
ghost
added this to the 
ghost
pushed a commit
that referenced
this pull request
Aug 28, 2023
* Make Checksum a record and use full 32-byte SHA-256 hash * Add #if..#endif around a few using directives to make CI happy * Declare correct parameter type in JsonDataWriter * Revert "Improve Perf for Semantic Tokens (#8968)" (#9059) This reverts commit 837dc71. * Convert HashData to readonly record struct * PR Feedback * Rework tag helper serialization to avoid using builders * Deserialize DocumentSnapshotHandle properties in a particular order * Deserialize ProjectWorkspaceState properties in a particular order * Deserialize RazorConfiguration properties in a particular order * Deserialize RazorDiagnostic properties in a particular order * Remove JsonDataReader APIs that are no longer used * Increment ProjectRazorJson version number * Add comment to clarify downstream impact of updating ProjectRazorJson version Co-authored-by: David Wengier <[email protected]> * Update dependencies from https://github.com/dotnet/arcade build 20230801.3 (#9074) Microsoft.DotNet.Arcade.Sdk From Version 8.0.0-beta.23381.1 -> To Version 8.0.0-beta.23401.3 Dependency coherency updates Microsoft.DotNet.XliffTasks From Version 1.0.0-beta.23374.1 -> To Version 1.0.0-beta.23381.1 (parent: Microsoft.DotNet.Arcade.Sdk Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Refactor and clean up tag helper resolvers This change refactors the various tag helper resolvers to prefer composition over inheritance and use an `ITagHelperResolver` as the base type. * Remove unused types and mark public types as internal in MS.VS.Editor.Razor * Mark public types as internal in MS.ANC.Razor.ProjectEngineHost * Mark public types as internal in MS.CA.Razor.Workspaces * Mark public types as internal in MS.ANC.Razor.LS.* * Mark public types as internal in MS.CA.Remote.Razor * Mark public types as internal in MS.VS.LanguageServerClient.Razor * Mark public types as internal in MS.VS.LanguageServices.Razor * Fix up a few tests to account for internal types * Add public API files for MS.VS.LiveShare.Razor * Mark public types as internal in MS.VS.RazorExtension * Update a few missed types in MS.ANC.Razor.ProjectEngineHost * Rework MS.VS.LiveShare.Razor types to only expose necessary types * Fix race where we're modifying a TextDocumentIdentifier before we use it * Make class partial * Move UpdateCSharpBuffer * Move UpdateHtmlBuffer * Move html formatting * Move code actions * Move semantic tokens * Move document color * fixup: code actions * Move workspace configuration * Move wrap with tag * Move folding range * Move uri and text presentation * Move text document position related endpoints * Move rename * Move completion * Move diagnostics * Move on auto insert * Move validate breakpoint * Move spell check * Move project contexts * Move document symbol * Rename type and remove base class * Clean up type name in various other places * Rename RazorLanguageServerCustomMessageTargets while we're here * Update dependencies from https://github.com/dotnet/arcade build 20230802.2 Microsoft.DotNet.Arcade.Sdk From Version 8.0.0-beta.23401.3 -> To Version 8.0.0-beta.23402.2 * Remove CancellationToken parameter default value * Remove some unnecessary argument-null checks * Remove redundant RazorServices class * Don't allow a null factory type name to be passed into RemoteTagHelperResolver * Stop calling obsolete API in RemoteTagHelperProviderService (fixes #6316) * Clean up RemoteTagHelperResolver and cache IProjectEngineFactories * Rename class based on PR feedback * Clean up tag helper deltas and avoid O(n^2) loops * Don't send multiple buffer updates for the same C# generated document * Make the hack hackier for our hacky tests * Update dependencies from https://github.com/dotnet/arcade build 20230803.7 Microsoft.DotNet.Arcade.Sdk From Version 8.0.0-beta.23402.2 -> To Version 8.0.0-beta.23403.7 * Localized file check-in by OneLocBuild Task: Build definition ID 262: Build ID 2236407 * Remove sourcelink dependency * Remove TagHelperResolutionResult and friends `TagHelperResolutionResult` purpose was to be returned from `IRemoteTagHelperProviderService.GetTagHelpersAsync(...)`. However, that API is no longer called in Razor because it has been supplanted by `GetTagHelpersDeltaAsync(...)`. This change removes that type and its serialization machinery, simplifying code. * Remove MS.ANC.Razor.LanguageServer.Common.Test and relocate tests * Remove MS.ANC.Razor.LanguageServer.Common and relocate types to MS.ANC.Razor.LanguageServer * Remove last vestiges of MS.ANC.Razor.LanguageServer.Common * Fix OSSkipConditionFactAttribute to work correctly on .NET Framework The way our OSSkipConditionFactAttribute was implemented does work for "Windows" when running on .NET Framework. This is due to the way that `RuntimeInformation.IsOSPlatform(...)` is written for .NET Framework. Instead of comparing strings, it performs a reference comparison for the give `OSPlatform` to `OSPlatform.Windows`. Since the code creates new `OSPlatform` with the string "Windows" instead of using `OSPlatform.Windows`, it fails. * Update src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultGeneratedDocumentPublisher.cs Co-authored-by: Dustin Campbell <[email protected]> * Update src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultGeneratedDocumentPublisher.cs * Update dependencies from https://github.com/dotnet/arcade build 20230804.2 Microsoft.DotNet.Arcade.Sdk From Version 8.0.0-beta.23403.7 -> To Version 8.0.0-beta.23404.2 * Keep source info from intermediate nodes for `@bind:set` attributes * Update test baselines * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20230731.3 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 8.0.0-alpha.1.23371.1 -> To Version 8.0.0-alpha.1.23381.3 * Fix up after merge * Support opening multiple virtual documents, and simplify a bunch of stuff * Update dependencies from https://github.com/dotnet/arcade build 20230807.1 Microsoft.DotNet.Arcade.Sdk From Version 8.0.0-beta.23404.2 -> To Version 8.0.0-beta.23407.1 * Generate EventHandler CodeAction: Simplify Type Names (#9070) * Hook up call to new SimplifyTypeNamesHandler * rework * update comments * feedback * react to experimental server capability type change * rebase oopsies * Update dependencies from https://github.com/dotnet/arcade build 20230808.6 Microsoft.DotNet.Arcade.Sdk From Version 8.0.0-beta.23407.1 -> To Version 8.0.0-beta.23408.6 Dependency coherency updates Microsoft.DotNet.XliffTasks From Version 1.0.0-beta.23381.1 -> To Version 1.0.0-beta.23407.1 (parent: Microsoft.DotNet.Arcade.Sdk * Remove LocateOwner from HoverInfoService (#9112) * Make sure we still process changes even if a document is open. * Update dependencies from https://github.com/dotnet/arcade build 20230809.5 Microsoft.DotNet.Arcade.Sdk From Version 8.0.0-beta.23407.1 -> To Version 8.0.0-beta.23409.5 Dependency coherency updates Microsoft.DotNet.XliffTasks From Version 1.0.0-beta.23381.1 -> To Version 1.0.0-beta.23408.1 (parent: Microsoft.DotNet.Arcade.Sdk * Localized file check-in by OneLocBuild Task: Build definition ID 262: Build ID 2241017 * Update dependencies from https://github.com/dotnet/arcade build 20230811.1 Microsoft.DotNet.Arcade.Sdk From Version 8.0.0-beta.23407.1 -> To Version 8.0.0-beta.23411.1 Dependency coherency updates Microsoft.DotNet.XliffTasks From Version 1.0.0-beta.23381.1 -> To Version 1.0.0-beta.23408.1 (parent: Microsoft.DotNet.Arcade.Sdk * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20230808.2 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 8.0.0-alpha.1.23381.3 -> To Version 8.0.0-alpha.1.23408.2 * Log output window to test results for integration tests This makes them viewable in Test Explorer for local runs * Add timestamps to output window log messages, and log more when running local builds * Add a couple of useful log messages, and fix one bad one The Range used to log as "{Range}", which didn't help much. * Update dependencies from https://github.com/dotnet/arcade build 20230814.5 Microsoft.DotNet.Arcade.Sdk From Version 8.0.0-beta.23407.1 -> To Version 8.0.0-beta.23414.5 Dependency coherency updates Microsoft.DotNet.XliffTasks From Version 1.0.0-beta.23381.1 -> To Version 1.0.0-beta.23412.1 (parent: Microsoft.DotNet.Arcade.Sdk * Remove a couple duplicated GetContent calls (#9123) ComponentFileKindVisitor.VisitMarkupAttributeBlock's call to GetContent is duplicated and appears to be about 0.8% of allocations in the customer profile I'm looking at. I looked at the IL in a release version of the assembly and the compiler at least doesn't remove the call (unclear whether the jitter would be able to). Either way, it's clearer and potentially faster to remove the duplicated calls. Didn't look into whether any of the remaining 1.2% of allocations due to calling GetContent could be optimized. * Fix logic error causing allocations in tag helper comparers * Update dependencies from https://github.com/dotnet/arcade build 20230815.4 Microsoft.DotNet.Arcade.Sdk From Version 8.0.0-beta.23407.1 -> To Version 8.0.0-beta.23415.4 Dependency coherency updates Microsoft.DotNet.XliffTasks From Version 1.0.0-beta.23381.1 -> To Version 1.0.0-beta.23415.1 (parent: Microsoft.DotNet.Arcade.Sdk * Update Roslyn to 4.8.0-1.23415.13 Needed to fix errors CS9057: The analyzer assembly '/__w/1/s/.dotnet/sdk/8.0.100-preview.7.23376.3/Sdks/Microsoft.NET.Sdk/ codestyle/cs/Microsoft.CodeAnalysis.CodeStyle.dll' references version '4.8.0.0' of the compiler, which is newer than the currently running version '4.7.0.0'. * Use StringComparer.Ordinal to avoid current culture string comparisons Array.BinarySearch uses IComparable<T> if an IComparer<T> isn't provided. For string, that's StringComparer.CurrentCulture which we definitely don't want for MetadataCollection keys. * Don't unnecessarily sort or do dictionary lookup in MetadataCollection Both Equals and ComputeHashCode are implemented for FourOrMoreItems are implemented in a way that assumes that keys are unsorted. However, the constructor ensures that they're sorted, so this extra effort is unnecessary and those method can be implemented more efficiently. * Use Assumed.Unreachable() instead of throwing * Don't foreach over IReadOnlyList<T> to avoid boxing struct-based enumerators * Add MetadataCollection.Create(...) overloads for concrete collections to avoid struct-enumerator boxing * Reimplement FourOrMoreItems constructor to avoid array copying * Avoid excess field lookups * Add doc comments and seal classes * Optimize MetadataCollection.Create(...) that takes a Dictionary * Remove a boxed enumerator allocated during TryMapToHostDocumentPosition (#9130) * Add generic Create and CreateOrEmpty methods to remove duplication * Revert back to a string key in the document version cache In hindsight this change was just causing more pain than it was worth. The document version cache maintains a list of document snapshots, and those snapshots are inherently tied to a project. Tieing the key to a project as well just made more work for more scenarios, eg when a document moves from the miscellaneous project to a real one, we wouldn't get a "document open" for it, but we would need to check if we were tracking the document in the misc project, and if so start tracking it in the real project etc. Doing this on just the file path makes that problem go away, and since a single Razor document across all projects has the same content, it shouldn't cause any issues. * Don't defer calculation of folding ranges, or error if they go wrong The HandleCoreAsync method is called in a loop, and retries things, but also uses IEnumerables which meant that the actual calculation was deferred until the result was sent across the wire. I thought maybe this was causing issues with things operating on newer versions of a Razor document, with delegated responses coming from an older version. It also just didn't make sense with the retry logic. The other change here is to stop a notification appearing in VS Code, that the user can't do anything about anyway, and switch to a log message. * When adding a document, check if it's open and act accordingly This matches what happens in OpenDocument, just doing it at Add time if its a document that is already open. The worry is that if we open a document in Misc Project, when we move it to a real project, we won't have "primed the pump" like we did when it was first opened. * Apply the same fix to UpdateHtml as we did to UpdateCSharp One potential cause of incorrect folding ranges is if the Html content is "doubled" in the generated document. This is exactly the behaviour we saw with C# documents, I just completely missed applying it to Html. As with C# documents, the "hack" part of this will be addressed in a future PR I'm preparing. * PR feedback * Fix Uri issues with language server tests * Don't use the suffix properties if we can avoid it This is just cleaning up a couple of uses of the two VirtualDocumentSuffix properties that weren't really needed. There are two uses remaining, one will definitely be changing, and I think the other could too, which would allow the properties to be make non-public or similar, to avoid future tempatation. * Create DocumentFilePathProvider The LanguageServerFeatureOptions class was getting a bit silly, and doing duty as a general purpose settings holder, and file path creator. With file path creation getting more important, I figured it would be good to get this out of the way early. * Switch to the DocumentFilePathProvider to generate file paths There aren't any real functional changes here, just getting all of the plumbing out of the way. Feel free to skip this one :) * Add option to LanguageServerFeatureOptions to control generated file path behaviour The C# extension in VS Code also generates file paths for generated files, and unless the logic in both sides match, nothing will work. Unfortunately we require more changes to give the C# extension the info it needs before it can change behaviour. This also gives us an escape hatch to essentially turn off multi-target support if we need to, or a user needs to. * Include a suffix from the project as part of the generated document Also fixed GetRazorFilePath so its better. * Move ProjectSnapshotManagerAccessor down, and provide MEF exports in the VS layer(s) I originally thought I'd use IProjectSnapshotChangeTrigger to get project info, but it turns out there are races in needing that to be initialized before other things (Roslyn, VS) call into things. * Pass ProjectKey when creating generated file path * Actually generate a project token in the generated file path * Don't assume that a document being not found is always a problem * Provide project contexts and wire everything up with them We're moving away from Roslyn project contexts to our own, which means we can now click all of the Lego pieces together so everything works (in theory!) * Gracefully handle a lack of project context As a point in time, the nav bar in VS isn't working and we're not getting ProjectContext on a lot of requests, so these asserts are firing way more than you would expect. Opting them out might mask a bug or two, but at least makes mostly things usable. * Formalize the decision to send multiple buffer updates * Update contained language to support dynamic virtual documents This probably should have gone into the other PR, or a separate one, but I wasn't smart enough to realise I would need it. It's all entirely new methods though, so seems safe enough. * Implement dynamically creating virtual documents for C# files This mainly is for the case where a Razor document is re-opened on project open, and VS creates buffers before we know about any projects. It also covers new files being added that are immediately opened, before we see them in the project system, and (in theory) covers the case of adding a new target framework to the list that a project supports. * Wait for a virtual document to be added, if we get a request for one before we know about it being in a project Also moved from extension method to private method, to make consumption easier and more obvious, and use the feature flag in a few choice places so we can avoid the waiting altogether if we need to opt people out. * Various test and other cleanup Additionally, tweak the timeout on the sync test as it failed once in CI. Must have been a slow/busy test machine. * Provide a nicer project contexts response * Add some log messages. Over the cause of development I added about a thousand of these, but figured these were the ones that might be useful in future. * Update dependencies from https://github.com/dotnet/arcade build 20230817.3 Microsoft.DotNet.Arcade.Sdk From Version 8.0.0-beta.23407.1 -> To Version 8.0.0-beta.23417.3 Dependency coherency updates Microsoft.DotNet.XliffTasks From Version 1.0.0-beta.23381.1 -> To Version 1.0.0-beta.23416.1 (parent: Microsoft.DotNet.Arcade.Sdk * Update dependencies from https://github.com/dotnet/arcade build 20230819.1 Microsoft.DotNet.Arcade.Sdk From Version 8.0.0-beta.23407.1 -> To Version 8.0.0-beta.23419.1 Dependency coherency updates Microsoft.DotNet.XliffTasks From Version 1.0.0-beta.23381.1 -> To Version 1.0.0-beta.23418.1 (parent: Microsoft.DotNet.Arcade.Sdk * Ensure we don't send duplicate changes when unique filenames aren't used * Honour log level passed from VS Code * Log when the server starts so it's not empty by default * Make sure we don't introduce double content in generated C# files * Add some tests * Add regular test which will actually fail * Fix log message * Fix * Fix a stupid mistake * Update src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/LspLogger.cs * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20230814.1 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 8.0.0-alpha.1.23408.2 -> To Version 8.0.0-alpha.1.23414.1 * Removes the TelemetryComplexProperty wrapper to allow numeric type (#9137) * Fix log message * Fail tests when a Debug.Assert or Debug.Fail is encountered * Update after merge * Better ProjectKey comparisons * Encapsulate timeout in one spot * Comments etc. * Rename DocumentFilePathProvider to FilePathService * Remove development code * Fix stupid mistake I made, forgetting one of the post-merge changes, that fortunately lots of integration tests found * Minor logging and code cleanup, that helped me track down the issue * Fix a silly mistake I made applying PR feedback * Allow range to extend past the end of the last line, per LSP spec * Remap and translate diagnostics on the server * Rename endpoint to separate it from the VS edition * Rename feature flag to better indicate what it does * Fix tests * Random integration test cleanup * Fix angle bracket encoding in Razor components (#9121) * Add tests * Fix angle bracket encoding in Razor components * Add more tests * Update dependencies from https://github.com/dotnet/arcade build 20230822.1 Microsoft.DotNet.Arcade.Sdk From Version 8.0.0-beta.23407.1 -> To Version 8.0.0-beta.23422.1 Dependency coherency updates Microsoft.DotNet.XliffTasks From Version 1.0.0-beta.23381.1 -> To Version 1.0.0-beta.23418.1 (parent: Microsoft.DotNet.Arcade.Sdk * Attempt to make output logger not block * Use UI thread when needed * Tweak the queue and threading * Possible paranoia * Cleanup * Fix build post-merge * Fix merge from main * Don't rely on templates for integration tests * Skip flaky tests * Revert ThrowingTraceListener change * Fail test if a Debug.Fail call happens * Stop hard coding Roslyn token modifiers, and add test * Use TryEnqueue instead * put in an assert * Wait for Razor project system before doing things * Remove unused using * Skip flaky tests * Format attributes that start with a transition correctly As usual, the Html formatter is doing the right thing here, and our C# adjustments then break things. Essentially it sees `@bind` as a Razor directive, so wants it at column 0. This change simply makes the `@` on an attribute pretend to be Html, so the C# formatter leaves it alone. * Fix workitem attributes * Rework BoundAttributeDescriptionInfo to avoid failures due to unexpected inputs * Update dependencies from https://github.com/dotnet/arcade build 20230825.2 Microsoft.DotNet.Arcade.Sdk From Version 8.0.0-beta.23422.1 -> To Version 8.0.0-beta.23425.2 Dependency coherency updates Microsoft.DotNet.XliffTasks From Version 1.0.0-beta.23418.1 -> To Version 1.0.0-beta.23423.1 (parent: Microsoft.DotNet.Arcade.Sdk * Don't return a range from Roslyn and hope its correct, because it won't be * Add test * Fix completion in self closing tags inside C# * Defend against bad input in the Inferred mapping * Rename tests to match method under test --------- Co-authored-by: Dustin Campbell <[email protected]> Co-authored-by: David Wengier <[email protected]> Co-authored-by: Phil Allen <[email protected]> Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: dotnet bot <[email protected]> Co-authored-by: Nikola Milosavljevic <[email protected]> Co-authored-by: Loni Tra <[email protected]> Co-authored-by: Andrew Hall <[email protected]> Co-authored-by: David Wengier <[email protected]> Co-authored-by: Todd Grunke <[email protected]> Co-authored-by: Jan Jones <[email protected]> Co-authored-by: Maryam Ariyan <[email protected]> Co-authored-by: Fred Silberberg <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.