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

Add support for category-specific AnalysisLevel properties #4556

Merged
merged 2 commits into from
Dec 13, 2020

Conversation

mavasani
Copy link
Contributor

@mavasani mavasani commented Dec 11, 2020

Implements category specific AnalysisLevel properties as per dotnet/roslyn#49250 (comment).

Users would now be able to specify the core AnalysisLevel and override category specific AnalysisLevel for each category.

<AnalysisLevel>5-minimum</AnalysisLevel>
<AnalysisLevelSecurity>latest-all</AnalysisLevelSecurity>
<AnalysisLevelPerformance>preview-recommended</AnalysisLevelPerformance>
<AnalysisLevelGlobalization>5-none</AnalysisLevelGlobalization>
  1. The post-build tool now generates category-specific global configs for each supported analysis mode
  2. The tool also generates category-specific AddGlobalAnalyzerConfigForPackage targets so that if user overrides a category-specific AnalysisLevel property, we pass in both the configs
  3. Category-specific config files have a higher precedence compared to category-agnostic config file. This is done via setting the global_level property as per the feature Global analyzer config precedence roslyn#48634, which is currently in code review: Support global analyzer config levels: roslyn#49834

Testing

Verified that setting the above matrix of category-agnostic and category-specific AnalysisLevel property leads to correct global config files being passed to the compiler. Currently, we get warnings about conflicting entries for same diagnostic ID in different config files, but that would be resolved with dotnet/roslyn#49834

Implements category specific AnalysisLevel properties as per dotnet/roslyn#49250 (comment).

Users would now be able to specify the core AnalysisLevel and override category specific AnalysisLevel for each category.

```xml
<AnalysisLevel>5-minimum</AnalysisLevel>
<AnalysisLevelSecurity>latest-all</ AnalysisLevelSecurity >
<AnalysisLevelPerformance>preview-recommended</AnalysisLevelPerformance>
<AnalysisLevelGlobalization>5-none</AnalysisLevelGlobalization>
```

1. The post-build tool now generates category-specific global configs for each supported analysis mode
2. The tool also generates category-specific `AddGlobalAnalyzerConfigForPackage` targets so that if user overrides a category-specific AnalysisLevel property

NOTE: This PR depends on feature dotnet/roslyn#48634, which is currently in code review: dotnet/roslyn#49834
@mavasani mavasani requested a review from a team as a code owner December 11, 2020 01:36
@mavasani
Copy link
Contributor Author

mavasani commented Dec 11, 2020

Also tagging @chsienki as an FYI. This analyzer configuration feature for .NET 6 relies on Chris' PR dotnet/roslyn#49834 to support precedence levels for global configs.

@codecov
Copy link

codecov bot commented Dec 11, 2020

Codecov Report

Merging #4556 (ee5f74e) into master (9084731) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4556      +/-   ##
==========================================
- Coverage   95.84%   95.84%   -0.01%     
==========================================
  Files        1177     1177              
  Lines      268158   268257      +99     
  Branches    16192    16202      +10     
==========================================
+ Hits       257009   257100      +91     
- Misses       9070     9090      +20     
+ Partials     2079     2067      -12     

@mavasani
Copy link
Contributor Author

Thanks @jmarolf!

@mavasani mavasani merged commit 990a966 into dotnet:master Dec 13, 2020
@mavasani mavasani deleted the PerCategoryAnalysisLevel branch December 13, 2020 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants