CA1416 MacCatalyst is a superset of iOS

[buyaa-n]

Related to dotnet/runtime#53084

MacCatalyst is a superset of iOS

Acceptance Criteria

To consider the proposal of using SupportedOSPlatformGuard attributes on the OperatingSystem methods to achieve a relationship of MacCatalyst as a superset of IOS, we will utilize the following acceptance criteria.

1. APIs annotated as supported on IOS are available on MacCatalyst without warning
   • This allows existing APIs annotated as supported on IOS to be automatically supported on MacCatalyst
2. Guarding those calls only requires checking a single platform
   • OperatingSystem.IsIOS() matches both IOS and MacCatalyst
3. APIs can still be annotated as being supported on one but not both IOS and MacCatalyst
   • OperatingSystem.IsIOS() && !OperatingSystem.IsMacCatalyst() matches only IOS but not MacCatalyst
   • OperatingSystem.IsMacCatalyst() matches only MacCatalyst but not IOS
4. New usage of [SupportedOSPlatformGuard] attributes is consistent with existing behavior
5. [UnsupportedOSPlatform] and [UnsupportedOSPlatformGuard] attributes must continue to work as expected
   • [UnsupportedOSPlatform] attributes must respect the platform relationship defined
   • This allows existing APIs annotated as unsupported on IOS to be automatically unsupported on MacCatalyst
6. The performance impact of the change must be negligible

Proposal Summary

1. Update OperatingSystem.IsIOS to return true when the platform is "ios" OR "maccatalyst"
2. Update the Platform Compatibility Analyzer to respect [SupportedOSPlatformGuard] attributes on System.OperatingSystem
   • Load these into a map during analyzer initialization
   • Use fallback logic of inferring the method's platform guard from the method name if no attributes exist
3. Annotate OperatingSystem.IsIOS with [SupportedOSPlatformGuard("maccatalyst")]
4. Update the Platform Compatibility Analyzer such that when identifying an API's supported/unsupported platforms, the platform set is expanded to include all platforms indicated through [SupportedOSPlatformGuard] attributes on that platform's Is{Platform} method

Illustration of Acceptance Criteria

public class OperatingSystem
{
    // Guard methods that simulate the OperatingSystem members
    // [SupportedOSPlatformGuard("ios")] this annotation is not needed as the method name express that it is iOS guard 
    [SupportedOSPlatformGuard("maccatalyst")]
    public static bool IsIOS() => IsIOS() || IsMacCatalyst();

    // [SupportedOSPlatformGuard("maccatalyst")] for same reason annotation doesn't need
    public static bool IsMacCatalyst() => IsMacCatalyst();
    ...
}

public class AnnotationScenarios    // APIs for the scenarios
{
    // The [SupportedOSPlatform("maccatalyst")] attribute would not exist in the code
    // but it would be inferred by the analyzer.
    [SupportedOSPlatform("ios")]
    void SupportedOnIOSAndMacCatalyst() { }

    // Having the [SupportedOSPlatform("maccatalyst")] explicitly
    // has no effect, it is same as above annotation
    [SupportedOSPlatform("maccatalyst")]
    [SupportedOSPlatform("ios")]
    void SupportedOnIOSAndMacCatalyst() { }

    [SupportedOSPlatform("ios")]
    // The following attribute could be used
    // to cancel the inferred "maccatalyst" support
    [UnsupportedOSPlatform("maccatalyst")]
    void SupportedOnIOS_ButNotMacCatalyst() { }

    // Unlike the [SupportedOSPlatform("ios")] 
    // this will not infer iOS support, only support  maccatalyst
    [SupportedOSPlatform("maccatalyst")]
    void SupportedOnMacCatalyst() { }

    [UnsupportedOSPlatform("ios")]
    // The following attribute would not exist in the code
    // but it would be inferred by the analyzer.
    // that also [UnsupportedOSPlatform("maccatalyst")]
    void UnsupportedOnIOSAndMacCatalyst() { }

    // only unsupported on maccatalyst
    [UnsupportedOSPlatform("maccatalyst")]
    void UnsupportedOnMacCatalyst() { }

    void Main()
    {
        // This block is reachable on both IOS and MacCatalyst
        if (OperatingSystem.IsIOS())
        {
            SupportedOnIOSAndMacCatalyst();      // Scenario 1a - No warning
            SupportedOnMacCatalyst();           // Scenario 1b - Warning
            SupportedOnIOS();                   // Scenario 1c - No warning
            SupportedOnIOS_ButNotMacCatalyst(); // Scenario 1d - Warning
            UnsupportedOnIOS();                 // Scenario 1e - Warning
            UnsupportedOnMacCatalyst();         // Scenario 1f - Warning
        }

        // This block is reachable on MacCatalyst but not IOS
        if (OperatingSystem.IsMacCatalyst())
        {
            SupportedOnIOSAndMacCatalyst();      // Scenario 2a - No warning
            SupportedOnMacCatalyst();           // Scenario 2b - No warning
            SupportedOnIOS();                   // Scenario 2c - No warning
            SupportedOnIOS_ButNotMacCatalyst(); // Scenario 2d - Warning
            UnsupportedOnIOS();                 // Scenario 2e - Warning
            UnsupportedOnMacCatalyst();         // Scenario 2f - Warning
        }

        // This code block is reachable on IOS but not MacCatalyst
        if (OperatingSystem.IsIOS() && !OperatingSystem.IsMacCatalyst())
        {
            SupportedOnIOSAndMacCatalyst();      // Scenario 3a - No warning
            SupportedOnMacCatalyst();           // Scenario 3b - Warning
            SupportedOnIOS();                   // Scenario 3c - No warning
            SupportedOnIOS_ButNotMacCatalyst(); // Scenario 3d - UPDATE: NO WARNING
            UnsupportedOnIOS();                 // Scenario 3e - Warning
            UnsupportedOnMacCatalyst();         // Scenario 3f - No warning
        }
    }
}

[buyaa-n]

This is for merging OperatingSystem.IsIOS() && !OperatingSystem.IsMacCatalyst() conditional which flow analysis value is (OperatingSystem.IsIOS() || OperatingSystem.IsMacCatalyst()) && !OperatingSystem.IsMacCatalyst(). Ideally it should be handled/merged by the flow analisys into just OperatingSystem.IsIOS() CC @mavasani

[codecov]

Codecov Report

Merging #5266 (7824cf6) into release/6.0.1xx (19af447) will increase coverage by 0.02%.
The diff coverage is 98.10%.

@@                 Coverage Diff                 @@
##           release/6.0.1xx    #5266      +/-   ##
===================================================
+ Coverage            95.59%   95.62%   +0.02%     
===================================================
  Files                 1233     1236       +3     
  Lines               283394   284484    +1090     
  Branches             16971    17068      +97     
===================================================
+ Hits                270922   272025    +1103     
+ Misses               10162    10155       -7     
+ Partials              2310     2304       -6     

[jeffhandley]

I see a lot of test cases that have comments indicating the message states the code is "reachable on all platforms" but the code is within some sort of a guard. I wasn't sure if those were accurate messages or copy/paste errors in the comments.

[buyaa-n]

I see a lot of test cases that have comments indicating the message states the code is "reachable on all platforms" but the code is within some sort of a guard. I wasn't sure if those were accurate messages or copy/paste errors in the comments.

That is accurate, if the diagnostic is found inside a platform check method and the issue is caused by platform check then the message shows This call site is reachable/unreachable on: 'Platform'. But If the platform check is not causing the problem, for example, if the condition is !OperatingSystem.IsWindows() and if the calling method is Linux only API then the message This call site is reachable on all platforms. 'LinuxOnlyAPI()' is only supported on : 'Linux' was more meaningful instead of This call site is unreachable on: 'Windows'. The 'LinuxOnlyAPI()' is only supported on : 'Linux'

[buyaa-n]

Seems @mavasani is out, if he gives a feedback later i will apply them with different PR, gonna merge