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

Diagnostics and Adornments, Tags and VS UI extensibility #1224

Open
suprak opened this issue Mar 12, 2015 · 14 comments
Open

Diagnostics and Adornments, Tags and VS UI extensibility #1224

suprak opened this issue Mar 12, 2015 · 14 comments
Labels
Area-Analyzers Area-IDE Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request Need Design Review The end user experience design needs to be reviewed and approved.
Milestone

Comments

@suprak
Copy link

suprak commented Mar 12, 2015

I'm developing a Diagnostics analyzer. I have been to accomplish what I want.

However I want to take it a step further and extend the look and feel using the existing VS extensibility model.

My question is how is that achieved?

How do I bridge the gap between analyzer-generated Diagnostics and say a Text Adornment service to highlight the locations that contain my Diagnostic?

I have tried several approaches, but my current hiccup is that I can't figure out a way of programatically retrieving all the analyzer-generated Diagnostics for a document. GetDiagnostics doesn't work as expected here (returning only language level compiler errors.)

Supported?

@srivatsn
Copy link
Contributor

Today unfortunately, there is no way to get the diagnostics that are computed in a file. We have talked about exposing an API so that custom presentation of diagnostics can be done in VS though. We'll use this issue to track that feature request.

@suprak
Copy link
Author

suprak commented Mar 13, 2015

@srivatsn Is there some drawback to using CompilationWithAnalyzers for what I described above?

@srivatsn
Copy link
Contributor

CompilationWithAnalyzers basically reanalyzes the entire project from scratch and wouldn't use any of the diagnostics that that IDE would have computed incrementally.It will also run the Initialize method of the analyzer again and the contract is that Initialize methods of the analyzers should only be called once per session of the host. So it's really meant for commandline scenarios where you want to run the analysis once.

@suprak
Copy link
Author

suprak commented Mar 13, 2015

Well the way I was thinking of it, is to develop my Analyzer and VS Extension.
But instead of adding the Analyzer to a project and then having the extension react to diagnostics, I would just run the analyzers using CompilationWithAnalyzers and get the diagnostics back directly.

That way I utilize the analysis component and use it in within the extension. However what's not clear to me is whether or not that's a valid use of CompilationWithAnalyzers and if there would be unexpected perf side-effects.

I guess I'll go with it for now, and see if it works for me.

@srivatsn
Copy link
Contributor

There will be a perf impact depending on the solution size. This would be similar to if the VS extension did it's own full build by invoking csc for the solution and getting the errors (if the solution was the size of roslyn.sln for example, then there will be a very visible perf impact). The VS IDE does this in a document by document case and does some caching to not run analyzers when it knows that a text change cannot affect certain analyzers in certain projects. It also does it in a cancellable way so that the next keystroke cancels previous computations which are now useless. CompilationWithAnalyzer does none of that stuff.

@suprak
Copy link
Author

suprak commented Mar 14, 2015

I hear you, but unless you've got a better path for me to follow, this is currently the only way for me to prototype what I'm envisioning. So I'll wait before I fully commit to my approach, while there is some progress made on enabling this sort of capability.

Ideally, today if I call one of the various GetDiagnostics methods, I would get back ALL diagnostics, including analyzer generated ones.

It would also be good if this kind of performance oriented insight was captured in the CompilationWithAnalyzer class documentation. I stumbled on to the class as a suggestion from somebody in the community, but obviously lack all of your context.

@srivatsn
Copy link
Contributor

Agreed. I don't have a better suggestion for you to prototype.

We originally explored having GetDiagnostics return analyzer diagnostics too but from a layering perspective, running analyzers from within the compiler created a lot of complications. CompilationWithAnalyzers has a GetAllDiagnostics method that returns everthing.

Also agree about the doc comment. I'll add some comments to that type.

@hallatore
Copy link

hallatore commented Jul 18, 2016

Any update on this @srivatsn ?
I'm trying to create an extension that show the number of stack and heap allocations in a method, And I would like to display it over the method (like codelens) and not like some error output.

@suprak
Copy link
Author

suprak commented Aug 8, 2016

I too am curious if there's any update in this area?

@srivatsn
Copy link
Contributor

srivatsn commented Aug 9, 2016

No, unfortunately we haven't worked on this feature yet. However, for the approach mentioned above of using CompilationWithAnalyzers, we've added an API to get diagnostics for a single document see. While it's still not as efficient as the IDE computing it, it's better than causing diagnostics computation for the entire compilation.

@og1sha
Copy link

og1sha commented Apr 9, 2020

I found myself in similar problem. Are there any updates for getting diagnostics in Text Adornments or similar Extension types?

@mavasani
Copy link
Contributor

Tagging @333fred

I think a good route might be to either split the IDiagnosticAnalyzerService interface into internal and public parts: https://github.com/dotnet/roslyn/blob/master/src/Features/Core/Portable/Diagnostics/IDiagnosticAnalyzerService.cs#L17 and expose the latter for external clients OR introduce a new type at Workspaces/Features layer which is equivalent to CompilationWithAnalyzers at compiler layer, but works on Document/Project/Solution. We should probably only expose pure GetDiagnostics/GetAnalysisResult APIs on this type which take in analysis scope and analysis options as inputs (keeping it as consistent as possible with the shape of public APIs on CompilationWithAnalyzers).

@333fred
Copy link
Member

333fred commented Jul 28, 2020

It feels like splitting out IDiagnosticAnalyzerService might be cleaner architectural solution. Ideally, it should imply all the state management and caching, and I'm not sure that a workspace-level version of GetDiagnostics/GetAnalysisResult would imply that level of caching. I'd like to actually take advantage of all the work the team has already done here, rather than trying to replicate it.

@mavasani mavasani added the Need Design Review The end user experience design needs to be reviewed and approved. label Apr 28, 2021
@sharwell sharwell added the Concept-API This issue involves adding, removing, clarification, or modification of an API. label May 10, 2021
@ryanbrandenburg
Copy link
Contributor

Design Meeting Notes: We'd like to do something in this area which is independent of host, but that's delayed by external factors. We'll come back to this when the prerequisites are more firmed-up.

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. Feature Request Need Design Review The end user experience design needs to be reviewed and approved.
Projects
Status: Need Proposal
Development

No branches or pull requests

9 participants