Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add LSP light bulb support for usings in Razor #5526

Closed
ryanbrandenburg opened this issue Jan 7, 2020 · 5 comments · Fixed by #2501
Closed

Add LSP light bulb support for usings in Razor #5526

ryanbrandenburg opened this issue Jan 7, 2020 · 5 comments · Fixed by #2501
Assignees
Milestone

Comments

@ryanbrandenburg
Copy link
Contributor

This isn't totally straightforward because the C# using won't be offered today because of #line hidden directives. We can work around this by making design-time generated C# files have a #line default around the using directives. This looks something like:

#pragma checksum "C:\Users\nimullen\source\repos\31Preview1_RazorPages\31Preview1_RazorPages\Pages\Index.cshtml" "{ff1816ec-aa5e-4d10-87f7-6f4963833460}" "9fc7ba627f8b2ee157f4d4d6eda1a40a92856ec8"
// <auto-generated/>
#line default
namespace ConsoleApp5
{
#line default
    using System.Collections.Generic;
    using System.Linq;
    using System.Threading.Tasks;
    using Microsoft.AspNetCore.Mvc;
    using Microsoft.AspNetCore.Mvc.Rendering;
    using Microsoft.AspNetCore.Mvc.ViewFeatures;
#nullable restore
#line 1 "C:\Users\nimullen\source\repos\31Preview1_RazorPages\31Preview1_RazorPages\Pages\_ViewImports.cshtml"
    using _31Preview1_RazorPages;
#line default
#line hidden
#nullable disable
    class Program
    {
        static void Main(string[] args)
        {
#nullable restore
#line 12 "C:\Users\nimullen\source\repos\31Preview1_RazorPages\31Preview1_RazorPages\Pages\Index.cshtml"
            Console.WriteLine("Hello World!");
#line hidden
        }
    }
}

Once we've done that we'll be able to create a Razor CodeAction which can translate the c# codeaction.

@ryanbrandenburg
Copy link
Contributor Author

More generally here we've got two options for CodeActions edits where we need context (IE, create local function vs create class function):

  1. Allow optional metadata to be passed through _languageMiddlewareFeature.remap to remapWorkspaceEdit. By passing in the CodeAction as this metadata we no longer have to "guess" about which one we're operating on.
  2. Add a new function remapCodeActionEdit(codeAction: vscode.CodeAction, workspaceEdit: vscode.WorkspaceEdit, token: vscode.CancellationToken) to https://github.com/OmniSharp/omnisharp-vscode/blob/master/src/omnisharp/LanguageMiddlewareFeature.ts.

Both of these options provide the same utility (giving us context about what an edit relates to), but have different levels of specificity. The choice between the too likely ends up lying with the Omnisharp folks.

@ryanbrandenburg
Copy link
Contributor Author

While we're at it we should re-examine our format a bit. Currently we aren't really using LSP, so we'll have to rewrite things a bunch of times. Because we need to go out to OmniSharp some of that is unavoidable (and we probably can't use the base codeactions endpoint from the spec ☹️) but we can definitely reduce the amount of rewriting we'll need to do with some custom LSP endpoints.

@ryanbrandenburg
Copy link
Contributor Author

The current thinking is to take the SyntaxTreeComparison structure from Roslyn EditAndContinue and apply it to this problem. We would take the EditScript produced by that and inspect it to see if the node modifications on it are handle-able by us (IE: "Create a using node with this parent node" would be add using, "Move function from parent class to parent statement" would be make function local). This means we'll have to create the syntax-tree that the edits would result in on the fly.

Making those APIs public is non-trivial though, so they want us to try that out first using local copies of some of the relevant classes (like this and this) to make sure that the whole approach will work.

@TanayParikh
Copy link
Contributor

Note, per discussion, the uri provided into the current tryApplyCodeActions (584aafd#diff-6c27f33b4510169ba1e1fc7bbc6c70f9R73) is not the correct URI for these purposes. We'll need to overhaul how we handle code actions / formatting to facilitate situations like these.

@TanayParikh TanayParikh self-assigned this Aug 20, 2020
@TanayParikh
Copy link
Contributor

Blocked on: https://github.com/dotnet/aspnetcore/issues/25250

Prioritizing that and other tasks, pushing this to preview 4.

TanayParikh referenced this issue Sep 16, 2020
### Summary of the changes

- Support for Fully Qualified Namespace C# Light Bulb
![FullyQualifiedCodeAction](https://user-images.githubusercontent.com/14852843/93262976-f1da7f80-f759-11ea-9877-05e9df2ed88b.gif)

- Support for Add Using C# Light bulb
![AddUsingCodeAction](https://user-images.githubusercontent.com/14852843/93263130-2d754980-f75a-11ea-890d-bfb07516a354.gif)

- 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
@ghost ghost locked as resolved and limited conversation to collaborators Oct 16, 2020
@dotnet dotnet unlocked this conversation Oct 9, 2021
@allisonchou allisonchou transferred this issue from dotnet/aspnetcore Oct 9, 2021
@allisonchou allisonchou added this to the 16.8-Preview4 milestone Oct 9, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Nov 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants