-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Unify roslyn analyzers on single version #78381
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
Conversation
This changes our RoslynAnalyzers layer to unify on a single version of the Microsoft.CodeAnalysis APIs
| <DefineConstants>$(DefineConstants),CODEANALYSIS_V3_7_OR_BETTER</DefineConstants> | ||
| </PropertyGroup> | ||
| <PropertyGroup Condition="'$(MicrosoftCodeAnalysisVersion)' == '' OR $([MSBuild]::VersionNotEquals('$(MicrosoftCodeAnalysisVersion)','1.2.1'))"> | ||
| <PropertyGroup Condition="'$(MicrosoftCodeAnalysisVersionForAnalyzers)' == '' OR $([MSBuild]::VersionNotEquals('$(MicrosoftCodeAnalysisVersion)','1.2.1'))"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that the min version is 4.0.1 I'm not sure we need this anymore. Could mean we could simplify a bit here.
|
|
||
| <DefineConstants Condition="'$(LEGACY_CODE_METRICS_MODE)' == 'true'">$(DefineConstants),LEGACY_CODE_METRICS_MODE</DefineConstants> | ||
|
|
||
| <MicrosoftCodeAnalysisVersionForAnalyzer>4.0.1</MicrosoftCodeAnalysisVersionForAnalyzer> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't hate if this was Analyzers instead of Analyzer, but it's basically a change everywhere so certainly not blocking on it.
|
|
||
| <!-- RS0026: Avoid public API overloads with differences in optional parameters --> | ||
| <NoWarn>$(NoWarn);RS0026</NoWarn> | ||
| <MicrosoftCodeAnalysisVersion Condition="'$(MicrosoftCodeAnalysisVersion)' == ''">$(MicrosoftCodeAnalysisVersionForCodeAnalysisAnalyzers)</MicrosoftCodeAnalysisVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is $(MicrosoftCodeAnalysisVersionForCodeAnalysisAnalyzers) defined anywhere?
| <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" VersionOverride="$(MicrosoftCodeAnalysisVersionForAnalyzer)" /> | ||
| <PackageReference Include="Microsoft.CodeAnalysis.VisualBasic" VersionOverride="$(MicrosoftCodeAnalysisVersionForAnalyzer)" /> | ||
| <PackageReference Include="Microsoft.CodeAnalysis.VisualBasic.Workspaces" VersionOverride="$(MicrosoftCodeAnalysisVersionForAnalyzer)" /> | ||
| <PackageReference Include="Microsoft.CodeAnalysis.Workspaces.Common" VersionOverride="$(MicrosoftCodeAnalysisVersionForAnalyzer)" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block should be using MicrosoftCodeAnalysisVersionForAnalyzerTests
|
|
||
| <!-- Avoid ID conflicts with the package project. --> | ||
| <PackageId>*$(MSBuildProjectFile)*</PackageId> | ||
| <MicrosoftCodeAnalysisVersion Condition="'$(MicrosoftCodeAnalysisVersion)' == ''">$(MicrosoftCodeAnalysisVersionForResxSourceGenerators)</MicrosoftCodeAnalysisVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 It would be good to lock this analyzer to an older version, because it doesn't need many APIs, doesn't look at source code, and we want it to work in a wide range of projects.
This changes our RoslynAnalyzers layer to unify on a single version of the Microsoft.CodeAnalysis APIs