Cohost code actions by davidwengier · Pull Request #11147 · dotnet/razor

davidwengier · 2024-11-04T05:20:49Z

Fixes #10742
Needs dotnet/roslyn#75711 before it will build
Also will need to merge in main once #11141 is merged

…code actions

…tionsPart5 # Conflicts: # src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/CodeActions/CodeActionsService.cs

…ine before. I blame VS

davidwengier · 2024-11-05T05:32:42Z

Merged main and bumped to a real Roslyn build, so this is ready for review

davidwengier · 2024-11-05T06:41:05Z

@dotnet/razor-compiler Your Microsoft.AspNetCore.Razor.Language.Legacy.CSharpStatementTest.SwitchStatement_ListPattern test failed, I'm guessing due to some change in Roslyn this is bringing in inadvertently? Is this expected? Should I just regen the baselines?

jjonescz · 2024-11-05T08:19:23Z

@dotnet/razor-compiler Your Microsoft.AspNetCore.Razor.Language.Legacy.CSharpStatementTest.SwitchStatement_ListPattern test failed, I'm guessing due to some change in Roslyn this is bringing in inadvertently? Is this expected? Should I just regen the baselines?

Please try regenerating the baseline so we can see the diff, it's not very clear from the test results

…tionsPart5

davidwengier · 2024-11-05T11:13:53Z

Please try regenerating the baseline so we can see the diff, it's not very clear from the test results

Spread changed to two dots? Seems odd to me, but not necessarily anything to worry about I suppose: https://github.com/dotnet/razor/pull/11147/files#diff-77d4727ed03808a308baf4eb6f7861529d0a9d23e86b2b51dda9eb9d719b62bd

jjonescz · 2024-11-05T11:27:15Z

...anguage/test/TestFiles/ParserTests/CSharpStatementTest/SwitchStatement_ListPattern.stree.txt

                LeftBracket;[[];
-                CSharpOperator;[..];
+                Dot;[.];
+                Dot;[.];


Looks like result of dotnet/roslyn#75549.

DustinCampbell

Looks very good!

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/CodeActionRequestInfo.cs

src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/CodeActions/RoslynCodeActionHelpers.cs

src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/CodeActions/RemoteCodeActionsService.cs

DustinCampbell · 2024-11-06T00:38:15Z

src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/CodeActions/RemoteServices.cs

This file looks super cool. 😄

ehh, it's okay. I don't like that we have to just manually make sure this matches everything in the LSP server DI container.

DustinCampbell · 2024-11-06T00:39:09Z

src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/CodeActions/RoslynCodeActionHelpers.cs

+    public async Task<string?> GetFormattedNewFileContentsAsync(IProjectSnapshot projectSnapshot, Uri csharpFileUri, string newFileContent, CancellationToken cancellationToken)
+    {
+        Debug.Assert(projectSnapshot is RemoteProjectSnapshot);
+        var project = ((RemoteProjectSnapshot)projectSnapshot).Project;
+
+        var document = project.AddDocument(RazorUri.GetDocumentFilePathFromUri(csharpFileUri), newFileContent);
+
+        return await ExternalHandlers.CodeActions.GetFormattedNewFileContentAsync(document, cancellationToken).ConfigureAwait(false);
+    }


Consider not creating a new async state machine.

Suggested change

public async Task<string?> GetFormattedNewFileContentsAsync(IProjectSnapshot projectSnapshot, Uri csharpFileUri, string newFileContent, CancellationToken cancellationToken)

{

Debug.Assert(projectSnapshot is RemoteProjectSnapshot);

var project = ((RemoteProjectSnapshot)projectSnapshot).Project;

var document = project.AddDocument(RazorUri.GetDocumentFilePathFromUri(csharpFileUri), newFileContent);

return await ExternalHandlers.CodeActions.GetFormattedNewFileContentAsync(document, cancellationToken).ConfigureAwait(false);

}

public Task<string?> GetFormattedNewFileContentsAsync(IProjectSnapshot projectSnapshot, Uri csharpFileUri, string newFileContent, CancellationToken cancellationToken)

{

Debug.Assert(projectSnapshot is RemoteProjectSnapshot);

var project = ((RemoteProjectSnapshot)projectSnapshot).Project;

var document = project.AddDocument(RazorUri.GetDocumentFilePathFromUri(csharpFileUri), newFileContent);

return ExternalHandlers.CodeActions.GetFormattedNewFileContentAsync(document, cancellationToken);

}

Okay, but I need to employ a ! because the Roslyn method returns Task<string>, so its not pretty. Unless I'm forgetting some other way to avoid the nullability compiler warning?

Ahhh... I see. Yeah, that's unfortunate. However, if the Roslyn method returns a Task<string>, why does this return a Task<string?>? It doesn't look to me like this method can return null. Or, is the Roslyn method not annotated correctly? Or, can this also return a Task<string>?

It's the non-cohost impl. It makes an LSP call, so can always be null.

Had a shower thought, I can make this non-nullable by moving the bit that handles null into the non-cohost impl. Thank you for the brain spark :D

…tionsPart5

Lost hours to that bloody capital T

davidwengier · 2024-11-06T10:12:06Z

@dotnet/razor-compiler for second sign off on test baseline changes

...oft.CodeAnalysis.Razor.Workspaces/CodeActions/Razor/ExtractToCodeBehindCodeActionResolver.cs

jjonescz · 2024-11-07T11:08:58Z

@dotnet/razor-compiler for second sign off on test baseline changes

Test changes should be good to go with just one sign off.

davidwengier added 9 commits November 1, 2024 16:58

Create basic cohosting infrastrcuture

3376535

Complete cohosting code actions functionality

6f61999

Report better MEF composition errors in tests

61ca99b

Add delegated document Uri to code action data

60ed8c0

Add cohost resolve endpoint and tests

e829ffb

Fix code action translation to Roslyn types

b509f67

Move helpers to base class

a813c22

Implement additional roslyn helpers, and add tests for more involved …

8ba2bbf

…code actions

Cohost eqivalent of dotnet#11141

0a3b902

davidwengier requested a review from a team as a code owner November 4, 2024 05:20

davidwengier added 5 commits November 4, 2024 17:03

Silly mistake number 1

d7e1c01

Bump Roslyn to 4.13.0-2.24554.8

4ef1668

Merge remote-tracking branch 'upstream/main' into dev/dawengie/CodeAc…

a217d9f

…tionsPart5 # Conflicts: # src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/CodeActions/CodeActionsService.cs

Update tests. No idea why these weren't failing to compile on my mach…

9b09b25

…ine before. I blame VS

Seal endpoints (silly mistake 2?)

673f828

davidwengier requested a review from a team as a code owner November 5, 2024 05:32

davidwengier removed the request for review from a team November 5, 2024 05:32

davidwengier added 2 commits November 5, 2024 22:09

Update compiler test baselines

7338393

Merge remote-tracking branch 'upstream/main' into dev/dawengie/CodeAc…

39138eb

…tionsPart5

jjonescz approved these changes Nov 5, 2024

View reviewed changes

DustinCampbell approved these changes Nov 6, 2024

View reviewed changes

davidwengier added 5 commits November 6, 2024 14:17

PR Feedback

61b5d93

Merge remote-tracking branch 'upstream/main' into dev/dawengie/CodeAc…

04da689

…tionsPart5

FIx code actions resolve in VS.

706dc93

Lost hours to that bloody capital T

Fix registration after merge

15c0333

Fix namespace on test

6cd3c63

davidwengier mentioned this pull request Oct 17, 2024

Cohosting - Support LSP endpoints #10364

Closed

Tweak roslyn helpers and nullability

aebf195

DustinCampbell reviewed Nov 6, 2024

View reviewed changes

...oft.CodeAnalysis.Razor.Workspaces/CodeActions/Razor/ExtractToCodeBehindCodeActionResolver.cs Outdated Show resolved Hide resolved

DustinCampbell approved these changes Nov 6, 2024

View reviewed changes

Further tweak Roslyn helpers

31469c4

davidwengier merged commit 40d2a6f into dotnet:main Nov 7, 2024

davidwengier deleted the dev/dawengie/CodeActionsPart5 branch November 7, 2024 11:22

dotnet-policy-service bot added this to the Next milestone Nov 7, 2024

This was referenced Nov 9, 2024

[Automated] PRs inserted in VS build main-35508.24 #11188

Closed

[Automated] PRs inserted in VS build feature.debugger.main-35514.242 #11216

Closed

dotnet-bot mentioned this pull request Nov 19, 2024

[Automated] PRs inserted in VS build feature.debugger.shadowDebug-35518.109 #11227

Closed

jjonescz modified the milestones: Next, 17.13 P2 Nov 25, 2024

Conversation

davidwengier commented Nov 4, 2024

Uh oh!

davidwengier commented Nov 5, 2024

Uh oh!

davidwengier commented Nov 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jjonescz commented Nov 5, 2024

Uh oh!

davidwengier commented Nov 5, 2024

Uh oh!

jjonescz Nov 5, 2024

Choose a reason for hiding this comment

Uh oh!

DustinCampbell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DustinCampbell Nov 6, 2024

Choose a reason for hiding this comment

Uh oh!

davidwengier Nov 6, 2024

Choose a reason for hiding this comment

Uh oh!

DustinCampbell Nov 6, 2024

Choose a reason for hiding this comment

Uh oh!

davidwengier Nov 6, 2024

Choose a reason for hiding this comment

Uh oh!

DustinCampbell Nov 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidwengier Nov 6, 2024

Choose a reason for hiding this comment

Uh oh!

davidwengier Nov 6, 2024

Choose a reason for hiding this comment

Uh oh!

davidwengier commented Nov 6, 2024

Uh oh!

Uh oh!

jjonescz commented Nov 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

davidwengier commented Nov 5, 2024 •

edited

Loading

DustinCampbell Nov 6, 2024 •

edited

Loading