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

Compiler needs to declaratively fail when loading newer analyzers #61252

Closed
jaredpar opened this issue May 11, 2022 · 3 comments · Fixed by #61261
Closed

Compiler needs to declaratively fail when loading newer analyzers #61252

jaredpar opened this issue May 11, 2022 · 3 comments · Fixed by #61261
Assignees
Milestone

Comments

@jaredpar
Copy link
Member

The .NET SDK supports installation into older versions of Visual Studio. For example 17.0 installs the 6.0.100 SDK by default but loading 6.0.200, 6.0.300 is supported. While this is supported it means that there is a bit of a mismatch in which compiler is used:

  • When building from Visual Studio or msbuild the C# compiler installed with Visual Studio is used.
  • When building from the CLI via dotnet build the compiler installed with the SDK is used.

This means that analyzers and generators can experience two different compiler versions based on how the build is invoked. An analyzer / generator that depends on a new version of the Roslyn APIs can get into a situation where dotnet build succeeds but msbuild fails because the compiler with Visual Studio is too old. That ends up resulting in errors like the following:

`##[warning]CSC(0,0): Warning CS8032: An instance of analyzer Microsoft.CodeAnalysis.MakeFieldReadonly.MakeFieldReadonlyDiagnosticAnalyzer cannot be created from C:\hostedtoolcache\windows\dotnet\sdk\6.0.300\Sdks\Microsoft.NET.Sdk\codestyle\cs\Microsoft.CodeAnalysis.CodeStyle.dll : Could not load file or assembly 'Microsoft.CodeAnalysis, Version=4.2.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies. The system cannot find the file specified..
CSC : warning CS8032: An instance of analyzer Microsoft.CodeAnalysis.MakeFieldReadonly.MakeFieldReadonlyDiagnosticAnalyzer cannot be created from C:\hostedtoolcache\windows\dotnet\sdk\6.0.300\Sdks\Microsoft.NET.Sdk\codestyle\cs\Microsoft.CodeAnalysis.CodeStyle.dll : Could not load file or assembly 'Microsoft.CodeAnalysis, Version=4.2.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies. The system cannot find the file specified.. [f:\a\1\s\src\XX\XX.csproj]

This situation has existed for a while but this error is becoming more common for a variety of reasons:

  1. Source generator APIs are new and hence depend on much newer versions of the compiler
  2. Code style analyzers are effectively shipping in lock step with the language now and that requires them to use latest compiler APIs.
  3. There are more scenarios where customers are taking advantage of mismatched VS and .NET SDK versions

Given that we should address this scenario and I believe we should do so in two ways:

  1. The compiler needs a declarative diagnostic here. I believe our analyzer loader should just scan the reference table of any analyzer we load and if it depends on a newer version of the compiler then we issue a proactive diagonstic and do not load the analyzer.
  2. The diagnostic should be a warning not an error. The reason is that when a customer gets into this situation there is often nothing they can do. Using an error means the build is fundamentally blocked until they can find the MSBuild logic to remove an analyzer, which then degrades the dotnet build scenario. Using a warning means the customer is alerted to the problem and the build can continue.

Examples of customers hitting this:

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels May 11, 2022
@jaredpar jaredpar added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels May 11, 2022
@jaredpar jaredpar added this to the 17.3 milestone May 11, 2022
@RikkiGibson
Copy link
Contributor

warning: Analyzer {CLASS_NAME} from {ASSEMBLY_PATH} cannot be used because the analyzer targets compiler version {TARGET_VERSION}, which is newer than the currently running version {CURRENT_VERSION}.

@jaredpar
Copy link
Member Author

jaredpar commented May 11, 2022

:shipit:

@edwiles
Copy link

edwiles commented May 23, 2022

Thanks for this. I see that my comment is quoted above. This I believe is a different issue, because the errors occur outside of Visual Studio i.e. when using "dotnet build". For example, we've needed to downgrade our Azure DevOps builds to .NET SDK 6.0.203 to avoid the issue.

An example that I believe does describe this issue is here.

@arunchndr arunchndr modified the milestones: 17.3, 17.4 Oct 3, 2022
@jaredpar jaredpar closed this as completed Oct 7, 2022
jaredpar added a commit to jaredpar/roslyn that referenced this issue Jan 3, 2023
This is the Roslyn side of the fix for [msbuild 7832](dotnet/msbuild#7832)
It provides a NuPkg that contains _only_ the .NET Framework version of
the Roslyn compilers. This matches, almost identically, what is inserted
into the MSBuild install of Visual Studio.

The .NET SDK can now use this package to use a .NET Framework toolset
when building C# and VB projects. This will resolve the analyzer
inconsistency issues that come up when mixing .NET SDK and MSBuild
Framework toolsets at different versions in CI.

Related
- dotnet#61252
- dotnet/msbuild#7832

The package verification
jaredpar added a commit to jaredpar/roslyn that referenced this issue Mar 14, 2023
This is the Roslyn side of the fix for [msbuild 7832](dotnet/msbuild#7832)
It provides a NuPkg that contains _only_ the .NET Framework version of
the Roslyn compilers. This matches, almost identically, what is inserted
into the MSBuild install of Visual Studio.

The .NET SDK can now use this package to use a .NET Framework toolset
when building C# and VB projects. This will resolve the analyzer
inconsistency issues that come up when mixing .NET SDK and MSBuild
Framework toolsets at different versions in CI.

Related
- dotnet#61252
- dotnet/msbuild#7832

The package verification
jaredpar added a commit to jaredpar/roslyn that referenced this issue Mar 14, 2023
This is the Roslyn side of the fix for [msbuild 7832](dotnet/msbuild#7832)
It provides a NuPkg that contains _only_ the .NET Framework version of
the Roslyn compilers. This matches, almost identically, what is inserted
into the MSBuild install of Visual Studio.

The .NET SDK can now use this package to use a .NET Framework toolset
when building C# and VB projects. This will resolve the analyzer
inconsistency issues that come up when mixing .NET SDK and MSBuild
Framework toolsets at different versions in CI.

Related
- dotnet#61252
- dotnet/msbuild#7832

The package verification
jaredpar added a commit to jaredpar/roslyn that referenced this issue Mar 15, 2023
This is the Roslyn side of the fix for [msbuild 7832](dotnet/msbuild#7832)
It provides a NuPkg that contains _only_ the .NET Framework version of
the Roslyn compilers. This matches, almost identically, what is inserted
into the MSBuild install of Visual Studio.

The .NET SDK can now use this package to use a .NET Framework toolset
when building C# and VB projects. This will resolve the analyzer
inconsistency issues that come up when mixing .NET SDK and MSBuild
Framework toolsets at different versions in CI.

Related
- dotnet#61252
- dotnet/msbuild#7832

The package verification
jaredpar pushed a commit that referenced this issue Mar 16, 2023
* Framework compilers package for SDK consumption

This is the Roslyn side of the fix for [msbuild 7832](dotnet/msbuild#7832)
It provides a NuPkg that contains _only_ the .NET Framework version of
the Roslyn compilers. This matches, almost identically, what is inserted
into the MSBuild install of Visual Studio.

The .NET SDK can now use this package to use a .NET Framework toolset
when building C# and VB projects. This will resolve the analyzer
inconsistency issues that come up when mixing .NET SDK and MSBuild
Framework toolsets at different versions in CI.

Related
- #61252
- dotnet/msbuild#7832

The package verification

* PR feedback

* Full run only

* YAML we meet again

* round 2 YAML

* make the pain stop

* yaml variable expansion part 2

* yaml hates typos

* wohoo a code bug!

* it's always yaml

* Apply suggestions from code review

Co-authored-by: Jan Jones <[email protected]>

* readme

* can't type xml

* Exclude from source build

---------

Co-authored-by: Jan Jones <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants