Skip to content

Commit

Permalink
Addresses the concern raised in dotnet#52991 (comment)
Browse files Browse the repository at this point in the history
The core idea is to define and thread in a new numeric `AnalysisLevel` value specific to IDE Code style, that gets passed to the analyzers to conditionally enable any new breaking change features, such as the one proposed in dotnet#52991.

Currently, we support `AnalysisLevel` and per-category `AnalysisLevel{Category}` properties to determine the set of first party CA and IDE rules to enable by default: https://learn.microsoft.com/dotnet/core/project-sdk/msbuild-props#analysislevelcategory. This also includes `AnalysisLevelStyle` property for IDE CodeStyle analyzers, which already defaults to `AnalysisLevel`. However, this property can hold non-numeric well-known values such as `none`, `preview`, `latest` or compound values in the form `<version>-<mode>` (documented [here](https://learn.microsoft.com/dotnet/core/project-sdk/msbuild-props#analysislevel). This PR adds the below additional logic:

1. Add a new MSBuild 'EffectiveAnalysisLevelStyle' property, that contains a numeric value derived from `AnalysisLevelStyle` property, similar to the way we have `EffectiveAnalysisLevel` derived from `AnalysisLevel` property (see https://github.com/dotnet/sdk/blob/fcc367b2164e4b41b8fb6622110aeb9f3247dd92/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.Analyzers.targets#L35-L56). Note that the category specific logic added here (for 'Style' category) is identical to the existing logic in Microsoft.CodeAnalysis.NetAnalyzers.targets file for different CA analyzer categories that is generated by tooling and inserted into the .NET SDK: https://github.com/dotnet/roslyn-analyzers/blob/b924542a1b526322929725a1aaa9586c21b1b231/src/Tools/GenerateDocumentationAndConfigFiles/Program.cs#L1528C62-L1553.

2. Mark this new 'EffectiveAnalysisLevelStyle' property as a `CompilerVisibileProperty` to pass it to the analyzers via analyzer config options.

Note that this Target gets added to C# and VB CodeStyle targets file that is imported here: https://github.com/dotnet/sdk/blob/f52240f11ad291e6ee3cff86e83c0f7a21b60370/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.Analyzers.targets#L117-L120. I verified locally that `<%ProjectName%>.GeneratedMSBuildEditorConfig.editorconfig` generated by our tooling contains an entry with key `build_property.EffectiveAnalysisLevelStyle`.
  • Loading branch information
mavasani committed Nov 14, 2023
1 parent 591d47e commit 08453ff
Showing 1 changed file with 24 additions and 3 deletions.
27 changes: 24 additions & 3 deletions src/CodeStyle/Tools/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,19 @@ static string GetTargetContents(string language)
{
return $"""
<Target Name="AddGlobalAnalyzerConfigForPackage_MicrosoftCodeAnalysis{language}CodeStyle" BeforeTargets="CoreCompile" Condition="'$(SkipGlobalAnalyzerConfigForPackage)' != 'true'">
<Target Name="AddGlobalAnalyzerConfigForPackage_MicrosoftCodeAnalysis{language}CodeStyle" BeforeTargets="GenerateMSBuildEditorConfigFileCore;CoreCompile" Condition="'$(SkipGlobalAnalyzerConfigForPackage)' != 'true'">
<!-- PropertyGroup to compute global analyzer config file to be used -->
<PropertyGroup>
<!-- Default 'AnalysisLevelStyle' to the core 'AnalysisLevel' -->
<AnalysisLevelStyle Condition="'$(AnalysisLevelStyle)' == ''">$(AnalysisLevel)</AnalysisLevelStyle>
<!-- AnalysisLevelStyle can also contain compound values with a prefix and suffix separated by a '-' character.
The prefix indicates the core AnalysisLevel for CA analyzers, which we ignore for IDE style analyzers.
The suffix indicates the bucket of rules to enable for 'Style' rules by default. It is used to map to the correct global config.
The prefix indicates the core AnalysisLevel for 'Style' rules and the suffix indicates the bucket of
rules to enable for 'Style' rules by default. For example, some valid compound values for AnalysisLevelStyle are:
1. '5-all' - Indicates core AnalysisLevelStyle = '5' with 'all' the 'Style' rules enabled by default.
2. 'latest-none' - Indicates core AnalysisLevelStyle = 'latest' with 'none' of the 'Style' rules enabled by default.
AnalysisLevelPrefixStyle is used to set the EffectiveAnalysisLevelStyle below.
AnalysisLevelSuffixStyle is used to map to the correct global config.
-->
<AnalysisLevelPrefixStyle Condition="$(AnalysisLevelStyle.Contains('-'))">$([System.Text.RegularExpressions.Regex]::Replace($(AnalysisLevelStyle), '-(.)*', ''))</AnalysisLevelPrefixStyle>
<AnalysisLevelSuffixStyle Condition="'$(AnalysisLevelPrefixStyle)' != ''">$([System.Text.RegularExpressions.Regex]::Replace($(AnalysisLevelStyle), '$(AnalysisLevelPrefixStyle)-', ''))</AnalysisLevelSuffixStyle>
Expand All @@ -228,6 +232,18 @@ static string GetTargetContents(string language)
<!-- Default 'AnalysisModeStyle' to the core 'AnalysisMode' -->
<AnalysisModeStyle Condition="'$(AnalysisModeStyle)' == ''">$(AnalysisMode)</AnalysisModeStyle>
<!-- EffectiveAnalysisLevelStyle is used to differentiate from user specified strings (such as 'none')
and an implied numerical option (such as '4') -->
<EffectiveAnalysisLevelStyle Condition="'$(AnalysisLevelStyle)' == 'none' or '$(AnalysisLevelPrefixStyle)' == 'none'">$(_NoneAnalysisLevel)</EffectiveAnalysisLevelStyle>
<EffectiveAnalysisLevelStyle Condition="'$(AnalysisLevelStyle)' == 'latest' or '$(AnalysisLevelPrefixStyle)' == 'latest'">$(_LatestAnalysisLevel)</EffectiveAnalysisLevelStyle>
<EffectiveAnalysisLevelStyle Condition="'$(AnalysisLevelStyle)' == 'preview' or '$(AnalysisLevelPrefixStyle)' == 'preview'">$(_PreviewAnalysisLevel)</EffectiveAnalysisLevelStyle>
<!-- Set EffectiveAnalysisLevelStyle to the value of AnalysisLevelStyle if it is a version number -->
<EffectiveAnalysisLevelStyle Condition="'$(EffectiveAnalysisLevelStyle)' == '' And
'$(AnalysisLevelPrefixStyle)' != ''">$(AnalysisLevelPrefixStyle)</EffectiveAnalysisLevelStyle>
<EffectiveAnalysisLevelStyle Condition="'$(EffectiveAnalysisLevelStyle)' == '' And
'$(AnalysisLevelStyle)' != ''">$(AnalysisLevelStyle)</EffectiveAnalysisLevelStyle>
<!-- Set the default analysis mode, if not set by the user -->
<_GlobalAnalyzerConfigAnalysisMode_MicrosoftCodeAnalysis{language}CodeStyle>$(AnalysisLevelSuffixStyle)</_GlobalAnalyzerConfigAnalysisMode_MicrosoftCodeAnalysis{language}CodeStyle>
<_GlobalAnalyzerConfigAnalysisMode_MicrosoftCodeAnalysis{language}CodeStyle Condition="'$(_GlobalAnalyzerConfigAnalysisMode_MicrosoftCodeAnalysis{language}CodeStyle)' == ''">$(AnalysisModeStyle)</_GlobalAnalyzerConfigAnalysisMode_MicrosoftCodeAnalysis{language}CodeStyle>
Expand All @@ -247,6 +263,11 @@ static string GetTargetContents(string language)
('$(AnalysisLevelStyle)' != '$(AnalysisLevel)' or '$(AnalysisModeStyle)' != '$(AnalysisMode)')">
<EditorConfigFiles Include="$(_GlobalAnalyzerConfigFile_MicrosoftCodeAnalysis{language}CodeStyle)" />
</ItemGroup>
<!-- Pass the MSBuild property value for 'EffectiveAnalysisLevelStyle' to the analyzers via analyzer config options. -->
<ItemGroup>
<CompilerVisibleProperty Include="EffectiveAnalysisLevelStyle" />
</ItemGroup>
</Target>
""";
Expand Down

0 comments on commit 08453ff

Please sign in to comment.