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

Applying code fixes outside Visual Studio #2020

Closed
daveaglick opened this issue Apr 15, 2015 · 17 comments
Closed

Applying code fixes outside Visual Studio #2020

daveaglick opened this issue Apr 15, 2015 · 17 comments
Labels
Area-Analyzers Area-IDE Concept-API This issue involves adding, removing, clarification, or modification of an API. Discussion
Milestone

Comments

@daveaglick
Copy link
Contributor

I'm having a hard time tracking down how to do this, and I'm starting to wonder if it's even possible.

In finding all uses of CodeFixProvider, it looks isolated to the Workspaces, Features, and Diagnostics libraries. None of them appear to have an API for actually applying code fixes. In fact, in the CodeFixTestBase class I noticed that the application of a code fix appears to be performed manually by rigging up a CodeFixContext and calling the methods of CodeFixProvider directly.

If I haven't missed something, than my next question would be if there are plans to expose code fixes in the same way other functionality is exposed. For example, analyzers can be run (fairly) easily outside the confines of Visual Studio. If it's not already on the table, would this be a proposal the team would be amenable to considering (at which point I could write up a proposal)?

@Pilchie
Copy link
Member

Pilchie commented Apr 15, 2015

You haven't missed anything, and it's definitely something that we're up for changing. We'd love to have projects like Omnisharp-Roslyn be able to use our CodeFix stuff directly.

However, we probably won't have time to devote to it before VS 2015 ships. If you'd like to take a stab at it though, we'd be happy to consult!

@Pilchie Pilchie added Concept-API This issue involves adding, removing, clarification, or modification of an API. Discussion Area-IDE Area-Analyzers labels Apr 15, 2015
@daveaglick
Copy link
Contributor Author

Thanks for the quick feedback. In that case, I would like to take a look at this, though it may be a little while before I get a chance to do so in much detail. As a first step, I'll look at specifying an API so it can be discussed and iterated here before coding anything.

I have some usage examples from the CodeFixTestBase class mentioned above as well as in the internal CodeFixService class. In fact, at first glance most of the work is probably going to be exposing the workings of CodeFixService in one form or another. I notice that's in the Features library - would that also be the most appropriate place for a public code fixes API?

@nosami - any thoughts on this as far as what would work best for Omnisharp-Roslyn?

@nosami
Copy link
Contributor

nosami commented Apr 15, 2015

Right now, we are using some CodeFixProviders from NRefactory6.

The code we are using is probably pretty naive but is working fine for the small number of fixes that we have available to us.

I think we just need 1 API method :-

public async Task<IEnumerable<CodeAction>> GetFixesAsync(Document document, TextSpan range, CancellationToken cancellationToken)

@Pilchie
Copy link
Member

Pilchie commented Apr 16, 2015

Tagging some other people who might be interested: @mattwar @heejaechang @mavasani @srivatsn @shyamnamboodiripad @jmarolf

@heejaechang
Copy link
Contributor

how do you intent to use it? if you are using it with Workspace and MEF, then, anything in feature layer should/most likely just work as long as it is somehow exposed to public. only thing needed will be cleaning up APIs.

...

if you intent to use it without MEF, then we probably need to do some work to move services to be based on feature pack rather than MEF directly (even if feature pack internally could use MEF).

...

if you intent to use it even without Workspace, then.. it would be quite a bit of work and design change.

@heejaechang
Copy link
Contributor

if we want to follow existing pattern of SymbolFinder, Formatter, Renamer and etc then, we probably need to move to FeaturePack and let it accepts CodeFixProviders.

and make that CodeFixer to return Solution rather than CodeActions so that caller doesn't need to deal with CodeActionOperations.

@mattwar
Copy link
Contributor

mattwar commented Apr 16, 2015

The only reasonable consumer of this API is one that also operates like the IDE. Given a diagnostic, one or more code fixes (represented as code actions) are returned. From this, an actual user has to be presented with options to choose which action. The chosen action can then be executed and applied.

@daveaglick
Copy link
Contributor Author

@mattwar I'm not sure that "the only reasonable consumer of this API is one that also operates like the IDE" (though I agree that's certainly the most obvious). It also happens to be the use case I'm currently looking at (writing, testing, and applying code fixes in LINQPad). However, I could envision scenarios where code fixes are used as a programmatic code re-writing system. Why create a new capability for identifying target locations, specifying transforms on the syntax tree, and applying them when we already have something that does exactly that?

@mattwar
Copy link
Contributor

mattwar commented Apr 16, 2015

There can be more than one code fix provider offering fixes for a diagnostic, and code fix providers can offer more than one possible fix. Someone has to choose which fix to apply. The only information to go on is the description text, which is meant to be read by either an actual person or some futuristic AI. So, code fixes, themselves, don't work well as programmatic operations, because there is no way to programmatically distinguish between them. You could, however, also offer an API that does the same operation as the code fix.

@daveaglick
Copy link
Contributor Author

@mattwar What about the use case where we already know which CodeFixProvider we want to apply (because it was specified out-of-band in code, a config file, command-line argument, etc.)? Granted, there's still the problem of distinguishing between multiple code fixes per provider...but a default could just be to apply them all if more than one (leave it up to the developer to make a sensible choice about which providers to apply).

I guess what I'm trying to address with this is the use case where I have a code fix written by someone else (say, "make all class names uppercase") and I want to write an automated tool that applies it across the board, to specific documents, etc. I wouldn't want to rewrite that code fix using the standard syntax APIs or rip out the logic that manipulates the trees when I've already got everything right there. And in reverse, if I'm doing some general code rewriting it would be nice to write and maintain that logic in one place and use it for programmatic as well as interactive use.

That aside, there's still also the question of how to make use of code fixes even if you have an interactive envionment. For example, the Omnisharp use case or my use case where I'm trying to make use of code fixes in a totally independent "IDE" or text editor. I suppose you could probably make this work today with a lot of rigging code, but a more standard API for this would go a long way towards making it easier.


(unrelated to above, but had already started typing when your comment came in...)

Here's what I'm thinking in terms of requirements for a public API:

  • Apply a CodeAction to a Workspace
  • Apply a CodeAction to a Solution
  • Apply a CodeAction to a Document
  • Apply a CodeFixProvider (and all resulting CodeActions) to a Workspace
  • Apply a CodeFixProvider (and all resulting CodeActions) to a Solution
  • Apply a CodeFixProvider (and all resulting CodeActions) to a Document
  • Apply a CodeRefactoringProvider (and all resulting CodeActions) to a Workspace
  • Apply a CodeRefactoringProvider (and all resulting CodeActions) to a Solution
  • Apply a CodeRefactoringProvider (and all resulting CodeActions) to a Document

And some questions/comments:

  • Should there be a public API to return instances of CodeAction given CodeFixProviders and/or CodeRefactoringProviders (along with appropriate context information)?
    • This would address the Omnisharp-Roslyn use case from @nosami
  • Should there be a public API to return applicable CodeFixProviders and CodeRefactoringProviders given Workspace, Solutions, and/or Documents?
    • If so, how should candidates be "registered" (I.e., Visual Studio uses the Export...ProviderAttributes to find them)?
  • At the lowest level, all CodeActionOperations apply to Workspace. I'm unclear how a CodeAction (and thus CodeActionOperations) can be applied to Solutions and Documents.
    • In general, this is the part that I'm having the most trouble with. It appears as though there is no indication within a CodeAction of the workspace object to which it is intended to apply (though, of course, they all apply to the Workspace in the end). Is this even a meaningful distinction? Is it assumed that there will always be a Workspace available when applying CodeActions?
  • I think this could all be isolated to the Workspaces library. It contains the definitions for CodeActionOperation, CodeAction, CodeFixProvider, CodeRefactoringProvider, etc. At first glance, I don't know why we would need to bring Features into the picture.
    • The question then becomes where to put this all within Workspaces? Should it go in a helper class? Extension methods on Workspace, Solution, and/or Document? Members of (or extensions of) CodeFixProvider, CodeRefactoringProvider, and CodeAction?

@mattwar
Copy link
Contributor

mattwar commented Apr 16, 2015

@daveaglick I'm providing some information hopefully answering some of your questions from the second half.

A CodeAction is basically a delegate with a description. The delegate does some action (on inputs that it has already captured) and yields CodeActionOperation's. These operations represent the output of the action, with all the changes already made. You get them by calling CodeAction.GetOperationsAsync(), which invokes action and returns the operations produced. However, none of these changes have affected the workspace yet. They've all been done to a isolated copy of the solution (the captured input). You push these changes back into the workspace by calling the CodeActionOperation.Apply method on each operation.

The primary reason CodeActionOperations exist in the API and not just implied by the code action, is so the Apply methods can get called on the UI thread (a constraint imposed by VS), and the code action author is free to operate in blissful ignorance.

There is already a way to get code fixes that apply to all occurrences in a solution. It's called a FixAll provider and can be accessed from a normal provider. It doesn't have scopes like Document and Project, but possibly could in the future. The reason this is a separate provider that produces aggregate code actions and not some aggregate operation over multiple code actions is that often fixes need to have special case handling in order to avoid conflicting with one another. There is a default FixAll provider that can be used by an author if they know their fixes can be applied correctly in sequence.

Other than that there is no way to apply a code fix to other scopes, because the scope of the fix has already been captured within each CodeAction instance.

@mavasani
Copy link
Contributor

@mattwar
"It's called a FixAll provider and can be accessed from a normal provider. It doesn't have scopes like Document and Project, but possibly could in the future."

FixAllProvider does have the concept of FixAllScope for which the aggregate fix is desired: http://source.roslyn.io/Microsoft.CodeAnalysis.Workspaces/R/b9bff38165d5d1e8.html
The scope is currently chosen by explicit user action of selecting "Fix All in Document" OR "Fix All in Project" OR "Fix All in Solution" on light bulb in IDE.

However, constructor for FixAllContext is internal and only instantiated by CodeFixService in the Features layer. It is based upon the DiagnosticService in that layer to compute all instances of a given diagnostic ID in scope of interest (document/project/solution).
We may want to consider investing some effort in exposing a public API at Workspaces/Features layer to compute and apply FixAll fixes based on a given diagnostic instance, i.e. say fix all unnecessary usings in the scope of interest. The API consumer can then decide if they want to do this iteratively and apply fix all for different diagnostic IDs in their scope.

@heejaechang
Copy link
Contributor

I think CodeActionOperation except ApplyChangeOpeartion is a bit host specific such as opening a file in host, providing its own preview and etc. with ProjectCodeFixProvider (not MEF based CodeFixProvider) we might be able to get around it but still CodeFixProvider work on top of Document where Host might have its own IWorkspaceService specific to the Host, so I think it would be safer not to expose CodeActionOperation and make CodeFixer work like others where it is solely a solution to a solution transformation API.

@Pilchie Pilchie added this to the Unknown milestone Apr 20, 2015
@aelij
Copy link
Contributor

aelij commented Nov 26, 2016

Slightly related to this issue - I've been using the CodeFixService class in RoslynPad and it works great (thanks to IVT ;). Lately I've decided to remove the EditorFeatures assembly to make the code more platform-agnostic, so I had to copy the entire class.

Can I submit a PR to move it to the Features assembly? It's one of the only types in EditorFeatures that's not in the Microsoft.CodeAnalysis.Editor namespace, and it has no other dependencies.

@CyrusNajmabadi
Copy link
Member

Sure!

@IanKemp
Copy link
Contributor

IanKemp commented Jun 1, 2018

Since #2709 is now closed, shouldn't this issue be too?

@CyrusNajmabadi
Copy link
Member

This would need to go through an API proposal. in general, the reason we don't just make things public is because it increases maintenance costs and locks down our ability to change things in teh future.

@CyrusNajmabadi CyrusNajmabadi closed this as not planned Won't fix, can't repro, duplicate, stale Oct 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Analyzers Area-IDE Concept-API This issue involves adding, removing, clarification, or modification of an API. Discussion
Projects
None yet
Development

No branches or pull requests

9 participants