Runtime: Enable the Platform Compatibility Analyzer and address warnings it introduces

[jeffhandley]

We intend to merge the Platform Compatibility Analyzer into the .NET 5.0 RC1 SDK. When the updated version of the NetAnalyzers package is taken into the dotnet/runtime repo that contains this analyzer, it will introduce build warnings. Those build warnings surface because:

• The Runtime repo applies [SupportedOSPlatform] and [UnsupportedOSPlatform] attributes to many APIs now
• We began applying those attributes before having the analyzer merged in because we wanted to have those attributes in place for the RC1 release
• There are cases, specifically around our multi-targeting, where we will need to either apply further annotations or utilize guard methods around calls to the platform-specific APIs

Because these warnings will occur, we are going to disable the analyzer in the dotnet/runtime repo before merging it into the SDK. Before we ship .NET 5.0 RC2 though, we want to re-enable the analyzer and address all of the warnings introduced.

See #41209 for a draft PR where we updated the infrastructure to allow MSBuild properties for applying assembly-wide attributes. We should include that infrastructure work as part of this issue, including the detail of producing a build error if an assembly attempts to define _both_ Supported and Unsupported platforms at the assembly-level.

[marek-safar]

It'd be fine to enable the analyzer for master branch only and we'd backport fixes if there would be any.

[buyaa-n]

Overall 41 warnings found with local build, all looks valid warnings and should be suppressed if window targets are appropriately produced the SupportedOSPlatform attribute local warnings.txt

Overall 48 warnings found with -allconfigurations build 22 of them repeated, so its 26 build warnings for real. Somehow warnings in Microsoft.Extensions.Logging.EventLog.csproj and System.IO.Ports.csproj are missing from -allconfigurations build. Results attached warnings with allconfiguration.txt

[jeffhandley]

@buyaa-n can you post instructions on how to recreate these lists so that if someone else picks this issue up, they'll be able to reproduce the list and test their work?

[buyaa-n]

Sure:

1. build roslyn-analyzer repo (release/dotnet5-rc1 branch) in release mode and copy produced dlls into the corresponding nuget package consumed by runtime repo instructions here (or just ping me for the roslyn-analyzers dlls)
2. command used for producing the errors with local configuration: build -subset libs -runtimeConfiguration release /p:TreatWarningsAsErrors=false , command used for producing errors in allconfig: build -subset libs -allconfiguration /p:TreatWarningsAsErrors=false

[jeffhandley]

To resolve this, we should:

1. Use a local build of the analyzer and locally enable it to resolve all of the issues in runtime, and submit a PR for those fixes into master, while leaving the analyzer disabled
2. Port the PR from step 1 into the release/5.0-rc2 branch (with an Ask Mode approval request)
3. Take in an official build of the NetAnalyzers package and enable the analyzer in master; this will not be ported into release/5.0

[ericstj]

@jeffhandley is this work still planned for the 5.0 codebase? Should it be marked blocking?

[jeffhandley]

Update to the plan above based on the results of #41760: /cc @ericstj

• We will not port any of the changes into 5.0
  
  
  
  • None of the changes affected any API surface area
  
  
  • Therefore there's no reason to port the changes into 5.0
  
  
  • That means we don't need to chunk the work up into multiple PRs
  
  
• We will then use a single PR (#41760) to complete the work in 6.0
  
  
  
  1. Enable the analyzer
  
  
  2. Update the analyzer build
  
  
  3. Resolve all of the issues in runtime
  
  

[jeffhandley]

This was resolved for 6.0.0 (master) with #41760. We will not port any of this into the 5.0 branches since no public API surface area was affected.