Tyrrrz / CliFx

Class-first framework for building command-line interfaces
MIT License
1.5k stars 61 forks source link

Assembly binding confusion results in many AD0001 analyzer warnings #127

Closed fossbrandon closed 2 years ago

fossbrandon commented 2 years ago

Version

2.2.3

Details

I upgraded from CliFx version 2.2.2 to version 2.2.3 because I wanted those fixes for IConsole.ReadKey(), however when I rebuild my solution, it results in lots of AD0001 analyzer warnings each which look like it originates from the CliFx analyzer.

Here is an example of one of the warnings:

AD0001  Analyzer 'Roslynator.CSharp.Analysis.DefaultExpressionAnalyzer' threw an exception of type
'System.InvalidCastException' with message '[A]Microsoft.CodeAnalysis.CSharp.CSharpCompilation cannot be cast to 
[B]Microsoft.CodeAnalysis.CSharp.CSharpCompilation. Type A originates from 'Microsoft.CodeAnalysis.CSharp, 
Version=4.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' in the context 'Default' at location 
'C:\Program Files\Microsoft Visual Studio\2022\Professional\MSBuild\Current\Bin\Roslyn\Microsoft.CodeAnalysis.CSharp.dll'. 
Type B originates from 'Microsoft.CodeAnalysis.CSharp, Version=4.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' 
in the context 'LoadFrom' at location 'C:\Users\branfoss\.nuget\packages\clifx\2.2.3\analyzers\dotnet\cs\Microsoft.CodeAnalysis.CSharp.dll'.'.

Each warning follows a similar format.

I've tried the following to remove the warnings with no success:

Has anyone else seen this issue? Downgrading back to version 2.2.2 removes these warnings.

Steps to reproduce

fossbrandon commented 2 years ago

I did more experimenting and I can't get these warnings to show up when doing dotnet build with the command line. This could be a Visual Studio bug then but it seems odd that I only seem to get those warnings when I update this package to the newest version. Something to look into at least.

Tyrrrz commented 2 years ago

Interesting. I've seen other warnings (also very inconsistently) but not this one. Building analyzers is unfortunately such a mess.

Are all the warnings you get the same as the first one? As in, is the error the same?

Also, do CliFx analyzers work despite the warnings?

Tyrrrz commented 2 years ago

So it looks like there are two Microsoft.CodeAnalysis.CSharp assemblies:

  1. one packaged within the CliFx nuget package
  2. default one packaged with Visual Studio

For some reason, unrelated analyzers (StyleCop, Roslynator in your case) are binding to CliFx's instance of Microsoft.CodeAnalysis.CSharp instead of the default one. My assumption is that VS loads all analyzers in the same application domain and that's why the assembly confusion happens (it's also why the order may be different, it depends on which assembler gets picked first). And then dotnet build probably loads each analyzer independently.

fossbrandon commented 2 years ago

That would make sense because every time I rebuild, it gives me a different amount of warnings. It shows anywhere between 33 warnings to 150+ warnings.

They all look like the warning in my initial post, but randomly I get a different error like the middle one in this screenshot. image

The CliFx analyzer seem to still be working: image

Tyrrrz commented 2 years ago

I wonder what's the best thing to do in this case. I'm packaging Microsoft.CodeAnalysis.CSharp to ensure that it's available in that specific version. Relying on pre-installed version is risky because it may be different depending on the version of IDE/SDK installed and potentially a million other factors.

Tyrrrz commented 2 years ago

Looks like StyleCop's approach is just to use the oldest version of the library :/

https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/9c5d064f100186515bcb45aadd629f18bc523a28/StyleCop.Analyzers/StyleCop.Analyzers/StyleCop.Analyzers.csproj#L32

fossbrandon commented 2 years ago

I don't like that haha. Roslynator uses a fairly recent version it seems. https://github.com/JosefPihrt/Roslynator/blob/master/src/Analyzers/Analyzers.csproj#:~:text=%3CPackageReference%20Include%3D%22Microsoft.CodeAnalysis.CSharp%22%20Version%3D%224.0.1%22%20/%3E

Tyrrrz commented 2 years ago

Found this on someone's blog:

image

Amazing 🙃

Tyrrrz commented 2 years ago

@fossbrandon I've pushed a pre-release version here: https://www.nuget.org/packages/CliFx/2.2.4-test

Can you test that:

  1. The warnings disappear (make sure to clean your solution just in case before)
  2. The analyzers are still working
fossbrandon commented 2 years ago

After upgrading, all the warnings are gone!

The CliFx analyzer still seems to work and I was able to get some errors from stylecop so I believe my other analyzers still work as well.

Thanks for looking into this so quick!

Tyrrrz commented 2 years ago

Great! I'll push a stable release then