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

[release/7.0] Fix bugs found in MAUI repo #6405

Merged
merged 3 commits into from
Jan 11, 2023

Conversation

buyaa-n
Copy link
Contributor

@buyaa-n buyaa-n commented Jan 5, 2023

Backport #6361 into .NET 7 release branch.
Issue: .NET MAUI seeing lots of new CA1416 warnings

Description

With dotnet/runtime#68557 we added ObsoletedOSPlatformAttribute for annotating APIs that is Obsoleted but still works with a custom message for suggested alternative, then added support for reporting new diagnostic with custom message in PCA which introduced one of the 2 bugs that fixed. The other bug was not known edge case scenario happening when a complex guard is used for calling an API that has annotations in the parent and the child API.

Customer Impact

The CA1416 Platform compatibility analyzer is included in SDK and warns by default.

The ObsoletedOSPlatform attribute support in Platform Compatibility Analyzer introduced the regression in .Net 7 which was not caught until xamarin team started using the ObsoletedOSPlatformAttribute in their APIs. When those bits flowed through and consumed by MAUI repo it introduces new warnings in their repo. As it was regression introduced in .NET 7 the fix better to be ported into . NET 7. The MAUI team wants the fix ported to .NET 7.0

The other bug causes false positive warning when the API call was properly guarded which could happen for any customer that has a code section that reproes it.

Regression?

One of the 2 bugs that fixed is regression from .NET 6.0.

Testing

Added unit tests that reproduces the false positive warnings and manually tested the fix with MAUI repo build.

Risk

Very low. This fix will not introduce additional warning, instead it fixes/ suppresses false positive warnings

@buyaa-n buyaa-n requested a review from a team as a code owner January 5, 2023 23:31
@buyaa-n buyaa-n added False_Positive A diagnostic is reported for non-problematic case Area-Microsoft.CodeAnalysis.NetAnalyzers labels Jan 10, 2023
@buyaa-n buyaa-n added this to the .NET 7.x milestone Jan 10, 2023
@codecov
Copy link

codecov bot commented Jan 10, 2023

Codecov Report

Merging #6405 (5a6e351) into release/7.0.1xx (66aefac) will increase coverage by 0.51%.
The diff coverage is 90.16%.

Additional details and impacted files
@@                 Coverage Diff                 @@
##           release/7.0.1xx    #6405      +/-   ##
===================================================
+ Coverage            95.53%   96.05%   +0.51%     
===================================================
  Files                 1273     1360      +87     
  Lines               290075   312286   +22211     
  Branches             17478    10049    -7429     
===================================================
+ Hits                277131   299961   +22830     
+ Misses               10575     9917     -658     
- Partials              2369     2408      +39     

Copy link
Member

@jeffhandley jeffhandley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. We will await Tactics approval on the backport.

@buyaa-n buyaa-n merged commit 897aa84 into dotnet:release/7.0.1xx Jan 11, 2023
@buyaa-n buyaa-n deleted the fix_bug branch January 11, 2023 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Microsoft.CodeAnalysis.NetAnalyzers False_Positive A diagnostic is reported for non-problematic case
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants