Generate EventHandler CodeAction: Simplify Type Names #9070

lonitra · 2023-08-01T22:53:47Z

Summary of the changes

Related: Generate method lightbulb for Blazor event callbacks. #4381
Roslyn PR with new LSP Handler: Add LSP Simplify Method for Razor roslyn#69323
Adds the necessary logic to call to the new LSP method roslyn/simplifyMethod that will exist in Roslyn once above PR is merged
Update to generate the event handler with the simplified type names that we get from the new LSP call.

VS:

VSCode:

davidwengier · 2023-08-02T00:46:18Z

...rosoft.AspNetCore.Razor.LanguageServer/CodeActions/Razor/GenerateMethodCodeActionResolver.cs

+        var templateWithMethodSignature = await PopulateMethodSignatureAsync(
+            documentContext,
+            actionParams,
+            csharpSource.ToString(),


Can we avoid doing this? Isn't there logic already that works out where to put the generated method, that would already have a position? Or does that come later? Might be worth refactoring.

davidwengier · 2023-08-02T00:48:50Z

...rosoft.AspNetCore.Razor.LanguageServer/CodeActions/Razor/GenerateMethodCodeActionResolver.cs

-        return templateWithMethodSignature.Replace(s_eventArgs, eventArgsType);
+            : eventTagHelper.GetEventArgsType();
+
+        var fullyQualifiedTypeNames = new[] { "System.NotImplementedException", returnType, eventArgsType };


This shape of this API feels like an odd fit to me. I think it would make more sense for us to create the code for a method with fully qualified types, and send it to Roslyn saying "if I were to insert this code at position X in document Y, what should we actually insert?". That would be more future proof, and cover more bases than just type name qualifications. It could also mean we can do fancy things like generating with shortened type names, and inserting necessary using statements, or other simplifications that Roslyn might be able to do if we ever need to generate code that does something other than just throw.

davidwengier · 2023-08-02T00:50:20Z

...oft.VisualStudio.LanguageServerClient.Razor/DefaultRazorLanguageServerCustomMessageTarget.cs

+        var textBuffer = virtualDocument.Snapshot.TextBuffer;
+        var response = await _requestInvoker.ReinvokeRequestOnServerAsync<SimplifyTypeNamesParams, string[]?>(
+            textBuffer,
+            "textDocument/simplifyTypeNames",


Nit: should put this in a constants file so it can be reused in tests etc.

davidwengier · 2023-08-02T00:59:08Z

Draft until dotnet/roslyn#69323 is merged as Roslyn version needs to be bumped

Roslyn doesn't need to be bumped, it just needs to be inserted in VS/VS Code. Though having said that, would be good to add a capability check for this so it doesn't matter if Razor goes in first.

davidwengier · 2023-08-02T22:07:23Z

...oft.VisualStudio.LanguageServerClient.Razor/DefaultRazorLanguageServerCustomMessageTarget.cs

+        var (synchronized, virtualDocument) = await _documentSynchronizer.TrySynchronizeVirtualDocumentAsync<CSharpVirtualDocumentSnapshot>(
+            request.Identifier.Version,
+            request.Identifier.TextDocumentIdentifier.Uri,
+            cancellationToken).ConfigureAwait(false);


A note for the future, but the pattern for calling TrySynchronize in this file has changed a little, if you merge in latest main. Of course, if you merge in latest main you'll break your local roslyn/razor dev experience, so I'm mainly leaving this note here for later :)

lonitra · 2023-08-03T00:19:13Z

Roslyn doesn't need to be bumped, it just needs to be inserted in VS/VS Code. Though having said that, would be good to add a capability check for this so it doesn't matter if Razor goes in first.

By capability check do you mean adding a LanguageServerOption and checking that? Similar to #8622?

Also, if it just needs to be inserted in VS/VS Code, would I update the tests by basing what is expected off of capability check results? Originally, I was imagining that I would need to go through the tests and update the expected generated event handler to have simplified type names, but if Roslyn hasn't been updated, the tests it wouldn't know how to handle roslyn/simplifyMethod and fail.

davidwengier · 2023-08-03T00:43:59Z

capability check

No, I mean a true LSP capability check. LSP servers and clients communicate what features they support when they connect, so that either side knows whether to bother trying to send a message. eg, when we delegate to Roslyn for spell checking, we check if they have the spell checking capability before sending the request. The Roslyn side advertises that they have the capability. In this case it would be an "expermental" capability, which essentially is just any random string, and it would mean that Razor simply wouldn't call the LSP method if the server doesn't support it.

Originally, I was imagining that I would need to go through the tests and update the expected generated event handler to have simplified type names, but if Roslyn hasn't been updated, the tests it wouldn't know how to handle roslyn/simplifyMethod and fail.

This is a great point, and yes without updating Roslyn the tests won't know how to simplify things. The call from the code action to razor/simplify would come in here though, and we can just hardcode a response for now. In theory we could also update the version of Roslyn that is referenced by tests, to be newer than the version referenced by product code, without affecting integration tests, but that's probably not worth it.

I would imagine you'd have tests in Roslyn that test the simplify handler itself, so on our side we can just rely on knowing that the message is sent, and that can be enough. In fact, might be better too, as the test would not be fragile based on changes in Roslyn's simplifier.

lonitra · 2023-08-04T21:16:33Z

.../Microsoft.AspNetCore.Razor.LanguageServer.Common/RazorLanguageServerCustomMessageTargets.cs

+
+    public const string RazorMethodEndpointName = "razor/simplifyMethod";
+
+    public const string RoslynSimplifyMethodEndpointName = "roslyn/simplifyMethod";


Let me know if I should put this somewhere else (:

RazorMethodEndpointName is correctly located here, but RoslynSimplifyMethodEndpointName should probably go in Microsoft.VisualStudio.LanguageServerClient.Razor.RazorLSPConstants.
Thats where the RazorCSharpLanguageServerName comes from, which is also a string value that is shared with Roslyn.

lonitra · 2023-08-04T21:21:05Z

...rosoft.AspNetCore.Razor.LanguageServer/CodeActions/Razor/GenerateMethodCodeActionResolver.cs

+                delegatedParams,
+                cancellationToken).ConfigureAwait(false);
+
+            if (result is not null)


Unfortunately because of the possibility of result = null, I did not remove a lot of the formatting code in CodeBlockService. If result = null, we should still produce a nicely formatted event handler, but we cannot use the RazorFormattingService at that point since we do not have C# edits, so we still do manual formatting work.

lonitra · 2023-08-04T21:27:12Z

...est/Microsoft.AspNetCore.Razor.LanguageServer.Test/SingleServerDelegatingEndpointTestBase.cs

+        private static TextEdit[] HandleSimplifyMethod<TParams>(TParams @params)
+        {
+            Assert.IsType<DelegatedSimplifyMethodParams>(@params);
+            return null;


I tried returning the text edit that was passed in as a hardcoded response, but got grief with the razor formatter and have been unable to debug tests for some reason :/... Right now the tests do not take simplification into account at all, but there is a test that checks the simplification works as expected in the roslyn repo.

davidwengier · 2023-08-07T05:34:32Z

.../Microsoft.AspNetCore.Razor.LanguageServer.Common/RazorLanguageServerCustomMessageTargets.cs

+
+    public const string RazorMethodEndpointName = "razor/simplifyMethod";
+
+    public const string RoslynSimplifyMethodEndpointName = "roslyn/simplifyMethod";


RazorMethodEndpointName is correctly located here, but RoslynSimplifyMethodEndpointName should probably go in Microsoft.VisualStudio.LanguageServerClient.Razor.RazorLSPConstants.
Thats where the RazorCSharpLanguageServerName comes from, which is also a string value that is shared with Roslyn.

davidwengier · 2023-08-07T05:44:23Z

...rosoft.AspNetCore.Razor.LanguageServer/CodeActions/Razor/GenerateMethodCodeActionResolver.cs

+            {
+                // Roslyn will have removed the beginning formatting, put it back.
+                var formatting = editToSendToRoslyn.NewText[0..editToSendToRoslyn.NewText.IndexOf("private")];
+                editToSendToRoslyn.NewText = $"{formatting}{result.First().NewText.TrimEnd()}";


The First() here feels a little fragile. Can there be some validation that the Roslyn side doesn't change (eg, do we only expect 1 edit?), and if it does, just act as thought it returned null (and Debug.Assert to we find out about it)

davidwengier · 2023-08-07T05:45:29Z

...rosoft.AspNetCore.Razor.LanguageServer/CodeActions/Razor/GenerateMethodCodeActionResolver.cs

+        var edits = CodeBlockService.CreateFormattedTextEdit(code, templateWithMethodSignature, _razorLSPOptionsMonitor.CurrentValue);
+        var editToSendToRoslyn = edits.Length == 1 ? edits[0] : edits[1];
+
+        if (edits.Length == 3


Would be good to include a comment explaining this. eg "if there are 3 edits then it means .... otherwise ..."

* 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]>

This was referenced Aug 1, 2023

Route razor/simplifyMethod to Roslyn dotnet/vscode-csharp#5982

Merged

Add LSP Simplify Method for Razor dotnet/roslyn#69323

Merged

davidwengier reviewed Aug 2, 2023

View reviewed changes

lonitra commented Aug 4, 2023

View reviewed changes

lonitra marked this pull request as ready for review August 4, 2023 22:13

lonitra requested a review from a team as a code owner August 4, 2023 22:13

davidwengier approved these changes Aug 7, 2023

View reviewed changes

lonitra force-pushed the simplifytypenames branch from a8202a3 to c3abcda Compare August 7, 2023 17:15

lonitra added 6 commits August 7, 2023 16:50

Hook up call to new SimplifyTypeNamesHandler

b2720ad

rework

44c4fa5

update comments

6f44a35

feedback

216d863

react to experimental server capability type change

f79de7e

rebase oopsies

7f38b1e

lonitra force-pushed the simplifytypenames branch from a37c94d to 7f38b1e Compare August 8, 2023 00:14

lonitra merged commit 02a2571 into dotnet:main Aug 8, 2023
12 checks passed

lonitra deleted the simplifytypenames branch August 8, 2023 21:18

ghost added this to the Next milestone Aug 8, 2023

allisonchou mentioned this pull request Aug 9, 2023

[Automated] PRs inserted in VS build main-34009.248 #9111

Closed

Cosifne modified the milestones: Next, 17.8 P3 Sep 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate EventHandler CodeAction: Simplify Type Names #9070

Generate EventHandler CodeAction: Simplify Type Names #9070

lonitra commented Aug 1, 2023 •

edited

Loading

davidwengier Aug 2, 2023

davidwengier Aug 2, 2023

davidwengier Aug 2, 2023

davidwengier commented Aug 2, 2023

davidwengier Aug 2, 2023

lonitra commented Aug 3, 2023

davidwengier commented Aug 3, 2023

lonitra Aug 4, 2023

davidwengier Aug 7, 2023

lonitra Aug 4, 2023

lonitra Aug 4, 2023

davidwengier Aug 7, 2023

davidwengier Aug 7, 2023

davidwengier Aug 7, 2023


		public const string RazorMethodEndpointName = "razor/simplifyMethod";

		public const string RoslynSimplifyMethodEndpointName = "roslyn/simplifyMethod";

Generate EventHandler CodeAction: Simplify Type Names #9070

Generate EventHandler CodeAction: Simplify Type Names #9070

Conversation

lonitra commented Aug 1, 2023 • edited Loading

Summary of the changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidwengier commented Aug 2, 2023

Choose a reason for hiding this comment

lonitra commented Aug 3, 2023

davidwengier commented Aug 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lonitra commented Aug 1, 2023 •

edited

Loading