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

Thousands of analyzer warnings are thrown inside Visual Studio #68764

Closed
ViktorHofer opened this issue May 2, 2022 · 23 comments · Fixed by #69162
Closed

Thousands of analyzer warnings are thrown inside Visual Studio #68764

ViktorHofer opened this issue May 2, 2022 · 23 comments · Fixed by #69162

Comments

@ViktorHofer
Copy link
Member

ViktorHofer commented May 2, 2022

Visual Studio 17.2 Preview 5
6.0.100 SDK

Severity	Code	Description	Project	File	Line	Suppression State
Warning	CS8032	An instance of analyzer Microsoft.CodeAnalysis.CSharp.UseIndexOrRangeOperator.CSharpUseIndexOperatorDiagnosticAnalyzer cannot be created from C:\Users\vihofer\.nuget\packages\microsoft.codeanalysis.csharp.codestyle\4.3.0-1.22206.2\analyzers\dotnet\cs\Microsoft.CodeAnalysis.CSharp.CodeStyle.dll: Could not load file or assembly 'Microsoft.CodeAnalysis, Version=4.3.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies. The system cannot find the file specified..	System.Text.Json (net6.0)		1	Active

Could not load file or assembly 'Microsoft.CodeAnalysis, Version=4.3.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies.

How is that assembly supposed to be referenced? Are we missing a PackageReference?

Repro: Open i.e. the System.Text.Json solution file with VS.

cc @ericstj in case you have any ideas.

@ghost
Copy link

ghost commented May 2, 2022

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details
Severity	Code	Description	Project	File	Line	Suppression State
Warning	CS8032	An instance of analyzer Microsoft.CodeAnalysis.CSharp.UseIndexOrRangeOperator.CSharpUseIndexOperatorDiagnosticAnalyzer cannot be created from C:\Users\vihofer\.nuget\packages\microsoft.codeanalysis.csharp.codestyle\4.3.0-1.22206.2\analyzers\dotnet\cs\Microsoft.CodeAnalysis.CSharp.CodeStyle.dll: Could not load file or assembly 'Microsoft.CodeAnalysis, Version=4.3.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies. The system cannot find the file specified..	System.Text.Json (net6.0)		1	Active

Repro: Open i.e. the System.Text.Json solution file with VS.

cc @ericstj in case you have any ideas.

Author: ViktorHofer
Assignees: -
Labels:

bug, area-Infrastructure

Milestone: -

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label May 2, 2022
@danmoseley
Copy link
Member

You could use procmon to see where it's looking?

@ViktorHofer
Copy link
Member Author

I believe that assembly comes with Visual Studio. The analyzers were updated with 5707668 (cc @jkoritzinsky).

Do we know what the minimum supported version of VS is when using the 4.3.0 analyzers? If it's 17.3 then we likely broke the analyzers for anyone who doesn't have access to the IntPreview builds of VS.

@ViktorHofer
Copy link
Member Author

cc @dotnet/roslyn-analysis regarding my above question.

@jmarolf
Copy link

jmarolf commented May 2, 2022

This typically means that your analyzers are referencing a newer version of the compiler than the one you are building with. Are you referencing the 4.3 compiler?

@ViktorHofer
Copy link
Member Author

ViktorHofer commented May 2, 2022

We are building with version 4.3.0-1.22215.4 of the compiler:

<MicrosoftNetCompilersToolsetVersion>4.3.0-1.22215.4</MicrosoftNetCompilersToolsetVersion>

@jmarolf
Copy link

jmarolf commented May 2, 2022

Unfortunately would need a binlog of the failing build to diagnose further.

@ViktorHofer
Copy link
Member Author

ViktorHofer commented May 2, 2022

These messages don't fail the build, they are printed onto the Error List in VS but as warnings. Just looked at all the design time binlogs and the build binlog and the warnings don't appear in them.

image

When I toggle the "Source" column, VS tells me that those are IntelliSense errors

@jmarolf
Copy link

jmarolf commented May 2, 2022

ah, and are you using a 17.3 or newer version of Visual Studio? What does help->About say?

image

@ViktorHofer
Copy link
Member Author

ViktorHofer commented May 2, 2022

No, I'm using 17.2 Preview 5. I wasn't sure if 4.3.0 analyzers require 17.3, that's what I was asking above:

Do we know what the minimum supported version of VS is when using the 4.3.0 analyzers? If it's 17.3 then we likely broke the analyzers for anyone who doesn't have access to the IntPreview builds of VS.

@jmarolf
Copy link

jmarolf commented May 2, 2022

Yep, 17.3 maps top the 4.3 version of the compiler.

@ViktorHofer
Copy link
Member Author

Thanks @jmarolf. Naively asking as I don't know anything about how roslyn analyzers are hosted inside VS but would it eventually be possible to avoid the strong dependency on a newer version of VS? We regularly update the analyzers to benefit from new features but we shouldn't move faster than what is publicly available (to not leave the community behind).

@sharwell
Copy link
Member

sharwell commented May 2, 2022

would it eventually be possible to avoid the strong dependency on a newer version of VS?

It is possible through a completely different design for the analyzers. StyleCop Analyzers employs an extensive (and complicated) lightup layer that allows it to compile against Roslyn 1.1 while lighting up to support anything newer.

During a design review a while back, the Roslyn team reviewed options and costs associated with various versioning strategies and decided to lock the analyzer build to the compiler version via the public API. This means it is possible to use an older analyzer with a newer SDK or IDE, but it is not going to be possible use a newer analyzer with an older SDK or IDE.

@ViktorHofer
Copy link
Member Author

ViktorHofer commented May 2, 2022

Thanks for the detailed response Sam. @agocke @stephentoub @danmoseley IMO we should treat analyzer updates as breaking changes. To be more precise, updating an analyzer's major or minor version will result in a dependency on a newer version of VS. In our example, our analyzers are depending on VS 17.3 which hasn't been released yet publicly which results in some analyzers being broken an tons of warnings being thrown.

@ViktorHofer ViktorHofer added this to the 7.0.0 milestone May 2, 2022
@ViktorHofer ViktorHofer removed the untriaged New issue has not been triaged by the area owner label May 2, 2022
@stephentoub
Copy link
Member

which results in some analyzers being broken

To be clear, this is for a developer working in Visual Studio on a dotnet/runtime csproj and not using a recent enough VS build, yes?

i.e. this has zero impact on command-line builds, on CI, or on anyone consuming the output of builds, correct?

@mavasani
Copy link

mavasani commented May 2, 2022

i.e. this has zero impact on command-line builds, on CI, or on anyone consuming the output of builds, correct?

Yes, correct.

@danmoseley
Copy link
Member

IMO we should treat analyzer updates as breaking changes. To be more precise, updating an analyzer's major or minor version will result in a dependency on a newer version of VS.

What does that look like -- holding off ingesting roslyn-analyzers until the monthly update day? Do all updates increment a major/minor version, if not how will we identify those?

Would this make it harder for us to test our analyzer as we're developing them?

@sharwell
Copy link
Member

sharwell commented May 2, 2022

Note that dotnet/roslyn-analyzers and dotnet/roslyn don't have exactly the same versioning for analyzers. The analyzers hard-locked to the compiler are in dotnet/roslyn (the code style analyzers with diagnostics IDExxxx). Analyzers implemented in dotnet/roslyn-analyzers have "slack" in the versioning, where we use light-up for a short period of time while things are being implemented, and then bump the referenced version of the compiler once it's readily available.

@stephentoub
Copy link
Member

updating an analyzer's major or minor version

I don't understand what this is referring to. What "analyzer" are you talking about @ViktorHofer? The roslyn-analyzer analyzers depend on an older version. To my knowledge it's the CodeAnalysis assemblies that the source generators and analyzers depend on, not the analyzers themselves, that are at issue here, e.g.

<MicrosoftCodeAnalysisVersion>4.3.0-1.22206.2</MicrosoftCodeAnalysisVersion>

not
<MicrosoftCodeAnalysisNetAnalyzersVersion>7.0.0-preview1.22229.1</MicrosoftCodeAnalysisNetAnalyzersVersion>

@sharwell
Copy link
Member

sharwell commented May 2, 2022

Super impressed that NuGet Package Explorer can show the declared dependencies of analyzer assemblies inside a package:
image

@stephentoub
Copy link
Member

And just for reference, since I mentioned roslyn-analyzers is relying on a much older version:
image

@ViktorHofer
Copy link
Member Author

@sharwell just spent some of his time explaining to me how analyzers that depend on a specific VS version can be identified.

The "Microsoft.CodeAnalysis" assembly that ships as part of VS indicates the required version of VS. In our case we use the following the analyzers that depend on the 4.3.0.0 version of Microsoft.CodeAnalysis and by that on VS 17.3:

  • MicrosoftCodeAnalysisCSharpCodeStyleVersion
  • MicrosoftCodeAnalysisCSharpWorkspacesVersion
  • MicrosoftCodeAnalysisCSharpVersion
  • MicrosoftCodeAnalysisVersion

Presumably we want to be intentional when updating these packages to not accidentally break our contributors that don't have access or don't want to use the IntPreview builds of VS.

I just today discussed this with @ericstj offline and he mentioned that we ideally would have CI protection to make sure that these kinds of errors are caught. Even though that sounds like the right long term strategy we could meanwhile establish a policy that these packages are part of the monthly roll-out as @danmoseley mentioned above. Would that make sense?

@mavasani
Copy link

mavasani commented May 3, 2022

Also want to give some more context onto why the CodeStyle analyzers (IDExxxx) move to build against a newer version of compiler and VS (basically Microsoft.CodeAnalysis version) faster than the SDK code quality analyzers (CAxxxx in MS.CA.NetAnalyzers). The CodeStyle analyzers are largely tied to recommending new styles based on new language constructs coming out of the compiler. The analyzers themselves have a strong need to be able to access the compiler APIs and object models for newer language constructs. The CAxxxx code quality analyzers are primarily recommending correct usage of .NET APIs and generic code quality improvements, and very seldomly need to move to build against the latest compiler version, so we do it at a slower cadence so it can avoid breaks like the one you are seeing here. Hope that helps!

ViktorHofer added a commit that referenced this issue May 10, 2022
Fixes #68764

The in the above issue mentioned warnings are resolved by using VS 17.3 (currently Preview1).
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 10, 2022
stephentoub pushed a commit that referenced this issue May 10, 2022
Fixes #68764

The in the above issue mentioned warnings are resolved by using VS 17.3 (currently Preview1).
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 10, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jun 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants