Support CSharp Light Bulbs (FQN, Add Using) #2501

TanayParikh · 2020-09-15T02:27:54Z

Summary of the changes

Support for Fully Qualified Namespace C# Light Bulb
Support for Add Using C# Light bulb
Abstraction Layer / Refactoring to create shared light bulb infrastructure between TSLanguageServer + HTMLCSharpLanguageServer.
This implementation doesn't have support for code actions which require resolving with Roslyn (ex. via textDocument/codeActionResolve). Working on an appropriate expansion for this, and will likely require changes in omnisharp-vscode.
- Tracked here: https://github.com/dotnet/aspnetcore/issues/25918
Some general perf optimizations are possible, but wanted to avoid potential pre-optimizations. Will re-visit if we see issues.
Testing: TODO assuming no issues with the design.

Fixes: https://github.com/dotnet/aspnetcore/issues/24779
Fixes: https://github.com/dotnet/aspnetcore/issues/18173
Fixes: https://github.com/dotnet/aspnetcore/issues/24778
Fixes: https://github.com/dotnet/aspnetcore/issues/25144

…/aspnetcore-tooling into taparik/csharp_lightbulbs

TanayParikh · 2020-09-15T03:17:42Z

src/Razor/src/Microsoft.AspNetCore.Razor.VSCode/src/CodeActions/CodeActionsHandler.ts

+    }
+
+    private commandAsCodeAction(command: vscode.Command): RazorCodeAction {
+        return { title: command.title } as RazorCodeAction;


This will be expanded with the code action resolve implementation.

…/aspnetcore-tooling into taparik/csharp_lightbulbs

...c/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/AddUsingsCodeActionProviderHelper.cs

TanayParikh · 2020-09-15T21:06:32Z

...azor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/CSharpVirtualDocumentPublisher.cs

-            {
-                get => true;
-                set { /* no-op */ }
-            }


Unrelated change from earlier PR feedback.

...oft.VisualStudio.LanguageServer.ContainedLanguage.Test/DefaultLSPDocumentSynchronizerTest.cs

TanayParikh · 2020-09-15T21:09:21Z

...or/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/CodeActions/CodeActionEndpointTest.cs

@@ -24,6 +24,7 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer.Test.CodeActions
 {
    public class CodeActionEndpointTest : LanguageServerTestBase
    {
+        private readonly RazorDocumentMappingService DocumentMappingService = Mock.Of<RazorDocumentMappingService>();


Tests updated to a passing state. Testing of functionality introduced with this PR is to be done.

I'd like to get this merged first to ensure the PR isn't too large. I'll subsequently implement the resolve endpoint, and do the testing for it all together because there's likely to be a good deal of overlap.

ajaybhargavb

LGTM from a quick skim. I'll look at it more in some time.

ajaybhargavb · 2020-09-15T21:11:55Z

...c/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/AddUsingsCodeActionProviderHelper.cs

+            return namespaceName;
+        }
+
+        internal static RazorCodeAction CreateAddUsingCodeAction(string fullyQualifiedName, DocumentUri uri)


Can this functionality be in a base class instead of a helper class? We generally try to avoid helper methods if possible to keep it contained to where it is intended to be used.

It's used in two separate providers RazorAddUsingsCodeActionProvider which implements RazorCodeActionProvider and CSharpAddUsingsCodeActionProvider which implements CSharpCodeActionProvider. So I don't think a base class would be possible without multiple-inheritance here.

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/CodeActionEndpoint.cs

NTaylorMullen

Hot damn this is awesome to see land! 👏

Love where you're going with it. Had some suggestions and a few questions throughout.

...c/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/AddUsingsCodeActionProviderHelper.cs

src/Razor/src/Microsoft.AspNetCore.Razor.VSCode/src/CodeActions/CodeActionsHandler.ts

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/DynamicDocumentContainer.cs

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

.../Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLanguageServerCustomMessageTarget.cs

...oft.VisualStudio.LanguageServer.ContainedLanguage.Test/DefaultLSPDocumentSynchronizerTest.cs

TanayParikh · 2020-09-16T04:27:12Z

Thanks @NTaylorMullen @ajaybhargavb. Addressed the feedback.

Noticing some issues where fully qualifying <Hello> results in <SomeComponent.Hello.Hello> despite the preview showing <SomeComponent.Hello>. Considering the preview is correct, I believe this is an external regression.

I'm also not getting any code actions with CTRL + . (shows no code actions found), but I do get them when I right click and select quick actions. I believe this is likely also an external regression.

Is anyone else seeing this on latest VS?

Edit: Seeing this in aspnetcore-tooling/master, so not blocking this PR. Created: https://github.com/dotnet/aspnetcore/issues/25964

NTaylorMullen

Poifect!! Looks good to me!

ajaybhargavb

🎉

...t.AspNetCore.Razor.LanguageServer/CodeActions/Razor/ExtractToCodeBehindCodeActionProvider.cs

TanayParikh added 25 commits August 26, 2020 10:45

Basic Refactorings

78b3f46

Merge branch 'master' into taparik/csharp_lightbulbs

a8441b0

SupportsDiagnostics PR Feedback

570f26a

Merge branch 'master' into taparik/csharp_lightbulbs

800b3f8

WIP

be245a8

WIP

b934c48

Partial fix for https://github.com/dotnet/aspnetcore/issues/25144

77cf7d1

Merge branch 'master' into taparik/csharp_lightbulbs

cb375cb

SerializableParams

b791acc

WIP

1ec2392

VSCode Integration

faeb897

Working Add Using FQN Code Actions

7b44d2a

Refactor

2f69f15

Refactor RazorCodeActions

ba66418

Fix Tests

da74129

Refactor CSharp Code Actions

e7985b0

VS WIP

7e5d1c9

Merge branch 'master' into taparik/csharp_lightbulbs

152ad61

Merge branch 'taparik/csharp_lightbulbs' of https://github.com/dotnet…

c503095

…/aspnetcore-tooling into taparik/csharp_lightbulbs

Merge branch 'master' into taparik/csharp_lightbulbs

8071a1f

VS Functionality

4869937

Refactoring Add Using Code Action

27541ef

Remove RazorCreateFile

d4cab62

getCodeActions -> provideCodeActions

49c7ed5

Remove HTMLCSharpCodeActions

4891613

TanayParikh commented Sep 15, 2020

View reviewed changes

TanayParikh added 4 commits September 15, 2020 12:56

WorkspaceEdit.Changes -> DocumentChanges

9cfcf35

Remove TSLanguageServer Impl

d706c15

Merge branch 'taparik/csharp_lightbulbs' of https://github.com/dotnet…

b00fe61

…/aspnetcore-tooling into taparik/csharp_lightbulbs

Fix build

240717e

Merge branch 'master' into taparik/csharp_lightbulbs

b523aab

TanayParikh changed the title *DO NOT REVIEW: WIP* Support CSharp Light Bulbs Support CSharp Light Bulbs (FQN, Add Using) Sep 15, 2020

TanayParikh marked this pull request as ready for review September 15, 2020 21:02

TanayParikh requested review from ajaybhargavb, NTaylorMullen and ryanbrandenburg as code owners September 15, 2020 21:02

TanayParikh commented Sep 15, 2020

View reviewed changes

ajaybhargavb reviewed Sep 15, 2020

View reviewed changes

NTaylorMullen requested changes Sep 15, 2020

View reviewed changes

PR Feedback

06c2297

TanayParikh requested review from NTaylorMullen and ajaybhargavb September 16, 2020 04:53

NTaylorMullen approved these changes Sep 16, 2020

View reviewed changes

ryanbrandenburg approved these changes Sep 16, 2020

View reviewed changes

Fix tests

38a25e7

ajaybhargavb approved these changes Sep 16, 2020

View reviewed changes

...t.AspNetCore.Razor.LanguageServer/CodeActions/Razor/ExtractToCodeBehindCodeActionProvider.cs Outdated Show resolved Hide resolved

TanayParikh and others added 3 commits September 16, 2020 11:02

Sort Using

bdc1869

Support Multiple Code Action Namespaces

0b5764c

Fix build

b2a58be

TanayParikh merged commit 66dbb85 into master Sep 16, 2020

TanayParikh deleted the taparik/csharp_lightbulbs branch September 16, 2020 20:11

This was referenced Oct 4, 2021

Ensure document version is provided in the VersionedTextDocumentIdentifier #4582

Open

Get Code Action Identifiers From Roslyn #5154

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support CSharp Light Bulbs (FQN, Add Using) #2501

Support CSharp Light Bulbs (FQN, Add Using) #2501

TanayParikh commented Sep 15, 2020 •

edited

Loading

TanayParikh Sep 15, 2020

TanayParikh Sep 15, 2020

TanayParikh Sep 15, 2020

ajaybhargavb left a comment

ajaybhargavb Sep 15, 2020

TanayParikh Sep 15, 2020

NTaylorMullen left a comment

TanayParikh commented Sep 16, 2020 •

edited

Loading

NTaylorMullen left a comment

ajaybhargavb left a comment

Support CSharp Light Bulbs (FQN, Add Using) #2501

Support CSharp Light Bulbs (FQN, Add Using) #2501

Conversation

TanayParikh commented Sep 15, 2020 • edited Loading

Summary of the changes

TanayParikh Sep 15, 2020

Choose a reason for hiding this comment

TanayParikh Sep 15, 2020

Choose a reason for hiding this comment

TanayParikh Sep 15, 2020

Choose a reason for hiding this comment

ajaybhargavb left a comment

Choose a reason for hiding this comment

ajaybhargavb Sep 15, 2020

Choose a reason for hiding this comment

TanayParikh Sep 15, 2020

Choose a reason for hiding this comment

NTaylorMullen left a comment

Choose a reason for hiding this comment

TanayParikh commented Sep 16, 2020 • edited Loading

NTaylorMullen left a comment

Choose a reason for hiding this comment

ajaybhargavb left a comment

Choose a reason for hiding this comment

TanayParikh commented Sep 15, 2020 •

edited

Loading

TanayParikh commented Sep 16, 2020 •

edited

Loading