-
Notifications
You must be signed in to change notification settings - Fork 472
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add ObsoletedInOSPlatform attribute support in Platform Compat Analyzer #6082
Conversation
Codecov Report
@@ Coverage Diff @@
## main #6082 +/- ##
========================================
Coverage 96.04% 96.05%
========================================
Files 1338 1339 +1
Lines 308692 309416 +724
Branches 9824 9855 +31
========================================
+ Hits 296497 297198 +701
- Misses 9817 9833 +16
- Partials 2378 2385 +7 |
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 left a few fit-and-finish and test recommendations, but the implementation looks good.
...tAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs
Outdated
Show resolved
Hide resolved
...etCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.ObsoletedInOSPlatformTests.cs
Outdated
Show resolved
Hide resolved
...etCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.ObsoletedInOSPlatformTests.cs
Show resolved
Hide resolved
...etCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.ObsoletedInOSPlatformTests.cs
Show resolved
Hide resolved
46aa74c
to
45fe345
Compare
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.
Looks good. I only saw a few test code comments that are out of date with the message format.
...etCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.ObsoletedInOSPlatformTests.cs
Outdated
Show resolved
Hide resolved
...etCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.ObsoletedInOSPlatformTests.cs
Outdated
Show resolved
Hide resolved
...etCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.ObsoletedInOSPlatformTests.cs
Outdated
Show resolved
Hide resolved
...etCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.ObsoletedInOSPlatformTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Jeff Handley <jeffhandley@users.noreply.github.com>
We have added ObsoletedInOSPlatform Attribute in 7.0 recently, now need to add the analyzer support for this in PlatformCompatibilityAnalyzer. By the approved API:
ObsoletedInOSPlatform
orUnsupportedOSPlatform
attributes with greater than or equal version could suppress obsoleted warningsOperatingSystem
APIs (for example:if (!OperatingSystem.IsLinux()
)UnsupportedOSPlatformGuard
attribute having matching platform/version, same for negatedSupportedOSPlatformGuard
UnsupportedOSPlatform
no warning for cross platform build in case the platform is not included in MSBuild SupportedPlatforms listFixes #6081