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

Require analyzers to target netstandard1.x or netstandard2.0 #39988

Open
tmat opened this issue Nov 23, 2019 · 6 comments
Open

Require analyzers to target netstandard1.x or netstandard2.0 #39988

tmat opened this issue Nov 23, 2019 · 6 comments
Labels
Area-Analyzers Concept-OOP Related to out-of-proc
Milestone

Comments

@tmat
Copy link
Member

tmat commented Nov 23, 2019

This requirement is needed for moving analysis to OOP.

Report a load error when an analyzer does not target netstandard2.0. We could start by reporting a warning and flip it to error later.

We currently support analyzers to be loaded from a VSIX package. This means the analyzer assembly can actually call VS APIs. For example, analyzer authors might have options UI for their analyzer. We need a migration path for these kind of analyzers. E.g. for the options UI, the recommendation would be to persist the options in .editorconfig file that the analyzer then reads.

We need to document this requirement and recommendations how to migrate existing analyzers that do not target netstandard2.0.

@tmat tmat added the Concept-OOP Related to out-of-proc label Nov 23, 2019
@tmat
Copy link
Member Author

tmat commented Nov 23, 2019

@jmarolf
Copy link
Contributor

jmarolf commented Nov 23, 2019

@tmat can you file an issue on the roslyn-sdk so I can remember to update all the templates to do this.

@sharwell
Copy link
Member

sharwell commented Nov 23, 2019

@tmat netstandard1.x will also need to be supported

This requirement is needed for moving analysis to OOP.

Eventually it would be a requirement, but today our OOP runs with .NET Framework right? After the transition, netcoreappx.y (for some x.y) would be supported in addition to netstandard2.0.

This means the analyzer assembly can actually call VS APIs.

This was never officially supported, and even today would break for both FSA and Fix All scenarios. I don't believe a migration path is necessary.

@tmat
Copy link
Member Author

tmat commented Nov 23, 2019

netstandard1.x will also need to be supported

How can netstandard1.x be supported when the analyzer assembly needs to reference
Microsoft.CodeAnalysis.dll, which targets netstandard2.0?

Eventually it would be a requirement, but today our OOP runs with .NET Framework right? After the transition, netcoreappx.y (for some x.y) would be supported in addition to netstandard2.0.

Correct. Once we're in OOP and on Core we can also support netcoreappx.y.

This was never officially supported, and even today would break for both FSA and Fix All scenarios. I don't believe a migration path is necessary.

@mavasani Is telling me he saw analyzers doing this. So maybe they are broken already.

@sharwell
Copy link
Member

sharwell commented Nov 23, 2019

How can netstandard1.x be supported when the analyzer assembly needs to reference Microsoft.CodeAnalysis.dll, which targets netstandard2.0?

The analyzers can compile against the public API of an older build of Microsoft.CodeAnalysis. Even the latest 2.9.x releases of dotnet/roslyn-analyzers do this.

Is telling me he saw analyzers doing this. So maybe they are broken already.

Some analyzers do, and they are broken in some scenarios already. The supported replacement is using the light bulb extensibility APIs directly.

@tmat
Copy link
Member Author

tmat commented Nov 23, 2019

The analyzers can compile against the public API of an older build of Microsoft.CodeAnalysis. Even the latest 2.9.x releases of dotnet/roslyn-analyzers do this.

I see. Makes sense.

@tmat tmat changed the title Require analyzers to target netstandard2.0 Require analyzers to target netstandard1.x or netstandard2.0 Nov 23, 2019
@jinujoseph jinujoseph added this to the Backlog milestone Dec 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Analyzers Concept-OOP Related to out-of-proc
Projects
None yet
Development

No branches or pull requests

5 participants