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

Extract a few more interfaces from the VS layer project system #65799

Merged

Conversation

jasonmalinowski
Copy link
Member

This introduces some more interfaces.

@jasonmalinowski jasonmalinowski self-assigned this Dec 6, 2022
@jasonmalinowski jasonmalinowski requested a review from a team as a code owner December 6, 2022 01:05
This is what we report analyzer load issues through; this probably needs
a larger refactoring because:

1. I'm not sure why we have both the base type and derived type, since
   there's only one inheriting from the base type.
2. Much of this is used for analyzer dependency conflict checking,
   which is perhaps of limited use in the first place.

namespace Microsoft.CodeAnalysis.Workspaces.ProjectSystem
{
// TODO: see if we can get rid of this interface by appropriately rewriting HostDiagnosticUpdateSource to live at the workspaces layer.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I realize I'm introducing an interface that has a TODO to get rid of it. There's a lot of other tangles for that which I don't want to fight at the moment.

/// An interface implemented by hosts to provide the host-level analyzers; for example in Visual Studio for Windows this
/// is where we'll fetch VSIX-defined analyzers.
/// </summary>
internal interface IHostDiagnosticAnalyzerProvider
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need the factory as an interface?
https://sourceroslyn.io/#Microsoft.VisualStudio.LanguageServices/Diagnostics/VisualStudioDiagnosticAnalyzerProvider.Factory.cs,19

Or what is the way we provide the implementation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be a MEF part, but just grabbed and passed in directly to the project type.

@jasonmalinowski jasonmalinowski merged commit 54a5024 into dotnet:main Dec 6, 2022
@ghost ghost added this to the Next milestone Dec 6, 2022
@Cosifne Cosifne modified the milestones: Next, 17.5 P3 Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants