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

Expose workspace level feature APIs as libraries #19908

Closed
masaeedu opened this issue May 31, 2017 · 11 comments
Closed

Expose workspace level feature APIs as libraries #19908

masaeedu opened this issue May 31, 2017 · 11 comments
Labels
Area-IDE Feature Request help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Milestone

Comments

@masaeedu
Copy link

I'm writing a Roslyn code generator and found that I needed the ability to generate a fake implementation of every member in an interface in an arbitrary class/struct. Rather than implement this from scratch, I wanted to reuse the logic already present in the "implement missing interface members" code fix.

Unfortunately, this is not exposed as a public API. At a glance it seems to have a sufficiently internals-agnostic API that it could just be wrapped up into a library and published, so it'd be great if this (and other code-refactorings + analyzers) could be published on NuGet.

@CyrusNajmabadi
Copy link
Member

This API is heavily designed around the use cases of our two 'implement interface' features. i.e. the one that gets invoked by VB if you hit return after an implements-clause, and the light-bulb we show in VB/C#.

It is not a generalized API to say "here is a type, give me that type back with the appropriate interface implemented."

We'd need to do a fair amount of work to support that, and once we exposed it we would not be able to change this. So we're not likely to get around to this for a long time FYI.

@masaeedu
Copy link
Author

@CyrusNajmabadi The primary interface into this code seems to be:

Task<Document> ImplementInterfaceAsync(Document document, SyntaxNode node, CancellationToken cancellationToken);
IEnumerable<CodeAction> GetCodeActions(Document document, SemanticModel model, SyntaxNode node, CancellationToken cancellationToken);

I'm not aware of whether the CodeAction being returned is actionable on an arbitrary documents, so maybe the second one isn't so useful to publish.

The first however takes an arbitrary Document, a SyntaxNode, a CancellationToken, and returns an updated Document, which is fairly general purpose. We can haggle over whether it should take SyntaxNode + Compilations or Documents, but even as-is, if I want to run it for an arbitrary syntax node I'll just make a workspace and stick whatever I want to transform into a new document.

I've skimmed the implementation, it seems to extract the relevant Compilation from the Document, then drill straight down to the symbol of the class and the interfaces it is trying to implement. If desired this can be indirected into an API that takes a Compilation and a SyntaxNode and returns a SyntaxNode with the members implemented, just as you say. You can publish that and your existing Document based API can use it.

@CyrusNajmabadi
Copy link
Member

You can publish that and your existing Document based API can use it.

But if we then ever want to change that, it would be a breaking change. We have to support these APIs for 10+ years. That takes planning and care to make sure we do it properly.

@masaeedu
Copy link
Author

masaeedu commented May 31, 2017

Oh wow. Maybe something less official than the Roslyn package itself then? CodeFixes.ImplementInterface, CodeFixes.Foo, etc? I'd imagine people fiddling with this stuff would be fine with you major rev-ing as fast as you need, since this isn't the actual compiler API.

I'm just looking for a way to exploit the work that's gone into all the existing Roslyn analyzers and code-fixes. At a high level, an analyzer is just a function that takes a workspace and produces a list of problems, and a code-fix is a function that takes a workspace and produces a different workspace. It would be great if these could be published and composed at a granular level, similarly to how Babel works.

I hear you on the planning and care front, just bringing up an idea.

@masaeedu
Copy link
Author

@CyrusNajmabadi Also see: dotnet/vscode-csharp#43. If the feature level API is not exposed in some way, all of the analyzers+code-fixes+refactorings you have here will have to be reimplemented in OmniSharp.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented May 31, 2017

Omnisharp can use our public DiagnosticAnalyzer+CodeActions API for this. There is no need to get at internal implementation details.

@masaeedu
Copy link
Author

masaeedu commented Jun 1, 2017

@CyrusNajmabadi Could you point me to the API you're referring to? If it's this, do you mean they should derive from that to implement analyzers?

@Pilchie
Copy link
Member

Pilchie commented Jun 1, 2017

Tagging @DustinCampbell on this one - there is a plan to support all the Roslyn code actions in OmniSharp.

As for the feature specific request, I could imagine making something public if there was a specific proposal for a public API that we could support as @CyrusNajmabadi mentioned above.

@Pilchie Pilchie added this to the Unknown milestone Jun 1, 2017
@Pilchie Pilchie added the help wanted The issue is "up for grabs" - add a comment if you are interested in working on it label Jun 1, 2017
@DustinCampbell
Copy link
Member

We're all working together here. 😄

OmniSharp provides Code Fixes and Refactorings today by calling into them directly. Unfortunately, OmniSharp does not have a diagnostic analysis engine, which is something we might expose from Roslyn in the future to allow OmniSharp and other editors to take advantage of. That said, the current experience works well enough for now in VS Code:

image

@masaeedu
Copy link
Author

masaeedu commented Jun 1, 2017

@DustinCampbell Right, so at the code level, how are you calling into Roslyn to populate that dropdown? Additionally, what API are you using to actually apply the fix? Skimming the source, I can see you're doing some kind of reflection magic here: https://github.com/OmniSharp/omnisharp-roslyn/blob/dev/src/OmniSharp.Roslyn.CSharp/CodeActions/CodeActionHelper.cs, but it's hard for me to understand what's going on; the commit messages suggest you're interacting with Roslyn via MEF somehow.

@DustinCampbell
Copy link
Member

We're calling directly into the fixes and refactorings to see if they're available. Additionally, we have some reflection code to specifically drill into nested code actions, since this isn't exposed today. This works well for most built-in code actions, but there is no diagnostic analyzer engine for OmniSharp today. So, code fixes that are driven off of custom diagnostics will not work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Feature Request help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Projects
None yet
Development

No branches or pull requests

4 participants