-
Notifications
You must be signed in to change notification settings - Fork 31
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
Decide on how to support different versions of Roslyn within a single VS extension #40
Comments
@cezarypiatek Did you ever face a similar problem? Any better ideas? Before I do anything stupid I would appreciate if you could take a look :-) |
@ironcev, Yes, I had the same problem when I wanted to add support for nullable references to my extension while referencing the old roslyn version (VS2017). I ended up with a simpler version of your solution https://github.com/cezarypiatek/MappingGenerator/blob/master/MappingGenerator/MappingGenerator/MappingGenerator/Mappings/SourceFinders/NullableExtensions.cs I needed only nullable-references related features so I created an abstraction over that and provided two implementations: one for the new versions of roslyn where desired methods are called by reflection and the fallback one for old roslyn versions. Everything is in the single project, but when you need to have access to broader ranger of new roslyn feature then this split into the separated projects seems to be a more maintainable solution. There's also an option to split your analyzers into a separated projects that reference different version of roslyn and put them in a single VSIX package. VS should load only those that have roslyn version supported by a given VS instance, but there's still on open issue related to that dotnet/roslyn#42538 |
Thanks a lot for your detailed and valuable comment @cezarypiatek ! What you are presenting is the fifth possible solution I didn't think of before :-) The big difference and advantage of your shim-based solution is that you do not need to load any assemblies at runtime. You rely on what is already loaded by VS and just put the right shim in place if the support for a particular method is there. Being someone who has seen thousands of issues coming from loading assembly dependencies in complex real-life constelations I am always for avoiding dynamic loading of assemblies if possible. From that perspective, I like your simplified approach very much. Weighing between those two options - shims without risks of potential problems at assembly loading or direct API access with assembly loading - at this moment, I also see the second one as better fitting. Still, it is good to know if I ever run into any issues with assemblies, I can always switch to the "Option 5", shims. dotnet/roslyn#42538 is definitely the right way to go for the regular analyzers, and I am looking forward to seeing that feature implemented. It will, unfortunately, be of no direct use to Sharpen, because Sharpen does not use regular Analyzers infrastructure. VS does not recognize Sharpen as a Roslyn analyzer. Thanks a lot once again! I'll then go with the multiple projects and DLLs loaded at runtime approach. |
The issue is explained in detail in: https://github.com/sharpenrocks/Sharpen/tree/master/lab/RoslynDependenciesAtBuildAndRuntime
Here is a short summary:
This way we do not have access to the new Roslyn API like e.g.
IOperation
but we assume that most of the analyzers can be comfortably written using this old Roslyn version.E.g. ,
IsNullableContextEnabled(...)
method can simply returnfalse
in VS2017 but must do some actual work in VS2019. This is the reason for the issue False positives in c# 8 nullable reference suggestions #38.So, we want to achieve the following:
IsNullableContextEnabled(...)
method has different implementations for Roslyn version older than v3.0.0.0 (C# 8.0) and newer or equal then v3.0.0.0.Proposed solution and its alterantives are described in detail here: https://github.com/sharpenrocks/Sharpen/tree/master/lab/RoslynDependenciesAtBuildAndRuntime
Here is the summary of the proposed solution:
The proof-of-concept of the approach can be seen here: https://github.com/sharpenrocks/Sharpen/tree/master/lab/RoslynDependenciesAtBuildAndRuntime/solution-04
Questions:
The text was updated successfully, but these errors were encountered: