search

dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.02k stars 4.03k forks source link

VSIX-based analyser should be disabled when same analyser is configured as nuget, regardless of version #9177

Open blairmcg opened 8 years ago

blairmcg commented 8 years ago

At present if I have an analyser installed as a VSIX and configured into a project through nuget, then the analysis does only run once iff the versions are exactly the same (not sure which is used, however). In team scenarios configuration of analyzers and rulesets through explicit settings in projects (i.e. using the nuget option), is a far better option than installing a VSIX as it allows a consistent configuration to be shared so that all team members see the same analysis results when looking at the same code. The current VSIX mechanism does not mesh well with this, however. Ideally a VSIX-based analyser should be disabled for a project for which the same analyser is explicitly configured as a nuget import. This should happen regardless of version, i.e. even if the nuget is older, because it is of paramount importance that all team members get a consistent result. Explicit configuration should be favoured over implicit, and consistency should be favoured over being up to date, and obviously one does not want to see duplicate results from running the same analysis rules more than once.

mavasani commented 8 years ago

@heejaechang How does v2 engine deal with duplicate analyzers from VSIX and NuGet package?

mavasani commented 8 years ago

Moving over to Heejae as the fix (if not implemented already) should go in v2 engine de-dupe logic.

heejaechang commented 8 years ago

v2 de-dup logic is same as v1. vsix gets higher priority over nuget as long as analyzer reference identity is same. analyzer reference de-dup is based on analyzer reference identity they claim if 2 versions return different identity then they are 2 different analyzer references and we can't de-dup those.

http://source.roslyn.io/#Microsoft.CodeAnalysis/DiagnosticAnalyzer/AnalyzerReference.cs,d8799d59fd0989de,references

heejaechang commented 8 years ago

by the way, this is breaking change (prefering nuget package over vsix assuming analyzer reference ids are same)

heejaechang commented 8 years ago

adding @dotnet/roslyn-analysis since I am not sure what is right resolution for this.

currently, what the issue requests is not how it works. our analyzer reference id do includes assembly version so to support what is described here will require several breaking changes.

heejaechang commented 8 years ago

I think we need some kind of dedicated option panel to configure all those little things some people want to tweak. not only this but others like which analyzers runs when and etc.

heejaechang commented 8 years ago

moving it to next milestone

heejaechang commented 7 years ago

@srivatsn this need design. making milestone unknown.

jinujoseph commented 7 years ago

Design discussion here https://github.com/dotnet/roslyn/issues/18818