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

OperatingSystem.IsIOS API is problematic #53084

Closed
marek-safar opened this issue May 21, 2021 · 36 comments
Closed

OperatingSystem.IsIOS API is problematic #53084

marek-safar opened this issue May 21, 2021 · 36 comments
Assignees
Labels
area-System.Runtime design-discussion Ongoing discussion about design without consensus
Milestone

Comments

@marek-safar
Copy link
Contributor

Background and Motivation

.NET 6 heavily relies on the platform compatibility analyzer, linker and operating system detection on the mobile platforms. Currently there's a parity between the values returned by OperatingSystem.IsXXX() APIs, the UnsupportedOSPlatform("xxx") and attributes. The target framework moniker (TFM) also uses the same XXX syntax for platform suffix. All the OperatingSystem.IsXXX() APIs are mutually exclusive and at most one of them returns true on a given platform.

Unlike most TFMs the Mac Catalyst has an implicit relationship with the iOS TFM. Application targeting net6.0-maccatalyst may consume library assets that were built with net6.0-ios TFMs. This creates a disparity where this relationship is not captured by the OperatingSystem.IsIOS/IsMacCatalyst APIs and the unavailable Mac Catalyst APIs have to include explicit UnsupportedOSPlatform("maccatalyst") annotations even though they don't target net6.0-maccatalyst directly. Failure to do so would currently be silently ignored and a transitive library consumption will not produce Platform Compatibility Analyzer warnings.

Additionally, libraries targeting net6.0 and including iOS specific logic can easily fall into a trap of guarding the code with OperatingSystem.IsIOS() when the correct condition is OperatingSystem.IsIOS() || OperatingSystem.IsMacCatalyst().

Similarly, in native C / Objective-C / Swift code the platform availability guards implicitly imply the Mac Catalyst as a variant of iOS.

Platform guard example in C

Consider the following C code:

#include <stdio.h>

int main()
{
#if __is_target_os(ios)
    printf("__is_target_os(ios): true\n");
#endif
    if (__builtin_available(iOS 16, *)) {
        printf("__builtin_available iOS 16\n");
    }
    if (__builtin_available(iOS 10, *)) {
        printf("__builtin_available iOS 10\n");
    }
    if (__builtin_available(iOS 16, macCatalyst 11, *)) {
        printf("__builtin_available macCatalyst\n");
    }
}

It can be compiled for Mac Catalyst by running clang -target x86_64-apple-ios13.0-macabi avail.c -o avail and it produces the following output:

__is_target_os(ios): true
__builtin_available iOS 10
__builtin_available macCatalyst

The interpretation is that __is_target_os(...) treats Mac Catalyst as iOS variant. __builtin_available uses the iOS <version> value on Mac Catalyst unless an explict check is specified.

Proposed solutions

Proposal A

  • OperatingSystem.IsIOS() would return true on both Mac Catalyst and iOS. In majority of the cases that is what the developer wants to check since Mac Catalyst is supposed to be a superset of iOS. Current runtime checks that do OperatingSystem.IsIOS() || OperatingSystem.IsTvOS() || OperatingSystem.IsMacCatalyst() would be shortened to OperatingSystem.IsIOS() || OperatingSystem.IsTvOS().
  • Keep UnsupportedOSPlatform("XXX") consistent with OperatingSystem.IsXXX, both in the Platform Compatibility Analyzer and in linker. Thus specifying UnsupportedOSPlatform("ios") would imply that an API is also unsupported on Mac Catalyst. Duplicate UnsupportedOSPlatform("ios") and UnsupportedOSPlatform("maccatalyst") attributes would coalesce into one.
  • For the rare case where you actually want to behave differently on iOS and MacCatalyst you would use a combination of the checks / attributes. An iOS-only API would be decorated with [UnsupportedOSPlatform("maccatalyst")] and runtime check would be !OperatingSystem.IsMacCatalyst(). A MacCatalyst-only API would be decorated with [UnsupportedOSPlatform("ios")] and [SupportedOSPlatform("maccatalyst")] (or similar). Code block guarding specifically for iOS and not Mac Catalyst would use OperatingSystem.IsIOS() && !OperatingSystem.IsMacCatalyst().

Proposal B

  • Add OperatingSystem.IsIOSOrMacCatalyst() API with appropriate UnsupportedOSPlatformGuard attributes. This would simplify the checks in code while keeping the OperatingSystem.IsXXX APIs more consistent. There's a potential error for the caller to keep using IsIOS where IsIOSOrMacCatalyst should have been used. Casual observation suggests that most of the IsIOS() API usages in .NET runtime itself would be replaceable with this alternate API since they do IsIOS() || IsMacCatalyst() check anyway.

  • Teach the Platform Compatibility analyzer about the additional TFM relationship and enforce additional rules when targeting net6.0-ios and not targeting net6.0-maccatalyst in a library code (ie. adding explicit supported/unsupported MacCatalyst annotations where iOS annotations are present; additional checks for use of the IsIOS() API). [TODO]

Additional design considerations

  • Should OperatingSystem.IsXXX map the TFM fallbacks in general?
  • Should there be a relation to how RIDs are structured too?
  • Should IsLinux() return true on Android?
    Likely not; the API surface is significantly different, there's prior art with Flutter:

    This value is false if the operating system is a specialized version of Linux that identifies itself by a different name, for example Android (see isAndroid).

/cc @terrajobst @jeffhandley for design decisions

Kudos to @filipnavara for the write-up.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label May 21, 2021
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented May 21, 2021

Tagging subscribers to this area: @tannergooding
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and Motivation

.NET 6 heavily relies on the platform compatibility analyzer, linker and operating system detection on the mobile platforms. Currently there's a parity between the values returned by OperatingSystem.IsXXX() APIs, the UnsupportedOSPlatform("xxx") and attributes. The target framework moniker (TFM) also uses the same XXX syntax for platform suffix. All the OperatingSystem.IsXXX() APIs are mutually exclusive and at most one of them returns true on a given platform.

Unlike most TFMs the Mac Catalyst has an implicit relationship with the iOS TFM. Application targeting net6.0-maccatalyst may consume library assets that were built with net6.0-ios TFMs. This creates a disparity where this relationship is not captured by the OperatingSystem.IsIOS/IsMacCatalyst APIs and the unavailable Mac Catalyst APIs have to include explicit UnsupportedOSPlatform("maccatalyst") annotations even though they don't target net6.0-maccatalyst directly. Failure to do so would currently be silently ignored and a transitive library consumption will not produce Platform Compatibility Analyzer warnings.

Additionally, libraries targeting net6.0 and including iOS specific logic can easily fall into a trap of guarding the code with OperatingSystem.IsIOS() when the correct condition is OperatingSystem.IsIOS() || OperatingSystem.IsMacCatalyst().

Similarly, in native C / Objective-C / Swift code the platform availability guards implicitly imply the Mac Catalyst as a variant of iOS.

Platform guard example in C

Consider the following C code:

#include <stdio.h>

int main()
{
#if __is_target_os(ios)
    printf("__is_target_os(ios): true\n");
#endif
    if (__builtin_available(iOS 16, *)) {
        printf("__builtin_available iOS 16\n");
    }
    if (__builtin_available(iOS 10, *)) {
        printf("__builtin_available iOS 10\n");
    }
    if (__builtin_available(iOS 16, macCatalyst 11, *)) {
        printf("__builtin_available macCatalyst\n");
    }
}

It can be compiled for Mac Catalyst by running clang -target x86_64-apple-ios13.0-macabi avail.c -o avail and it produces the following output:

__is_target_os(ios): true
__builtin_available iOS 10
__builtin_available macCatalyst

The interpretation is that __is_target_os(...) treats Mac Catalyst as iOS variant. __builtin_available uses the iOS <version> value on Mac Catalyst unless an explict check is specified.

Proposed solutions

Proposal A

  • OperatingSystem.IsIOS() would return true on both Mac Catalyst and iOS. In majority of the cases that is what the developer wants to check since Mac Catalyst is supposed to be a superset of iOS. Current runtime checks that do OperatingSystem.IsIOS() || OperatingSystem.IsTvOS() || OperatingSystem.IsMacCatalyst() would be shortened to OperatingSystem.IsIOS() || OperatingSystem.IsTvOS().
  • Keep UnsupportedOSPlatform("XXX") consistent with OperatingSystem.IsXXX, both in the Platform Compatibility Analyzer and in linker. Thus specifying UnsupportedOSPlatform("ios") would imply that an API is also unsupported on Mac Catalyst. Duplicate UnsupportedOSPlatform("ios") and UnsupportedOSPlatform("maccatalyst") attributes would coalesce into one.
  • For the rare case where you actually want to behave differently on iOS and MacCatalyst you would use a combination of the checks / attributes. An iOS-only API would be decorated with [UnsupportedOSPlatform("maccatalyst")] and runtime check would be !OperatingSystem.IsMacCatalyst(). A MacCatalyst-only API would be decorated with [UnsupportedOSPlatform("ios")] and [SupportedOSPlatform("maccatalyst")] (or similar). Code block guarding specifically for iOS and not Mac Catalyst would use OperatingSystem.IsIOS() && !OperatingSystem.IsMacCatalyst().

Proposal B

  • Add OperatingSystem.IsIOSOrMacCatalyst() API with appropriate UnsupportedOSPlatformGuard attributes. This would simplify the checks in code while keeping the OperatingSystem.IsXXX APIs more consistent. There's a potential error for the caller to keep using IsIOS where IsIOSOrMacCatalyst should have been used. Casual observation suggests that most of the IsIOS() API usages in .NET runtime itself would be replaceable with this alternate API since they do IsIOS() || IsMacCatalyst() check anyway.

  • Teach the Platform Compatibility analyzer about the additional TFM relationship and enforce additional rules when targeting net6.0-ios and not targeting net6.0-maccatalyst in a library code (ie. adding explicit supported/unsupported MacCatalyst annotations where iOS annotations are present; additional checks for use of the IsIOS() API). [TODO]

Additional design considerations

  • Should OperatingSystem.IsXXX map the TFM fallbacks in general?
  • Should there be a relation to how RIDs are structured too?
  • Should IsLinux() return true on Android?
    Likely not; the API surface is significantly different, there's prior art with Flutter:

    This value is false if the operating system is a specialized version of Linux that identifies itself by a different name, for example Android (see isAndroid).

/cc @terrajobst @jeffhandley for design decisions

Kudos to @filipnavara for the write-up.

Author: marek-safar
Assignees: -
Labels:

area-System.Runtime, untriaged

Milestone: -

@marek-safar marek-safar added design-discussion Ongoing discussion about design without consensus and removed untriaged New issue has not been triaged by the area owner labels May 21, 2021
@marek-safar marek-safar added this to the 6.0.0 milestone May 21, 2021
@jkotas
Copy link
Member

jkotas commented May 21, 2021

My vote would be for Proposal A since it matches the underlying platform model.

@jeffhandley
Copy link
Member

/cc @buyaa-n @adamsitnik

@filipnavara
Copy link
Member

@terrajobst Was there any traction on this?

@terrajobst
Copy link
Member

terrajobst commented Jun 16, 2021

All the OperatingSystem.IsXXX() APIs are mutually exclusive and at most one of them returns true on a given platform.

While I think that's true today this wasn't an explicit design goal. In fact, we said before that returning true for multiple operating systems should be allowed because it allows us to add "virtual" operating systems like browser where we want to be able to add more specific checks later, such as IsBrowserChromium().

I think modelling MacCatalyst as a variant of iOS makes sense because that's how Apple positioned it. So my preference would be proposal A but instead of teaching the platform compat analzer about this relationship I propose we explicitly declare the model using attributes:

namespace System
{
    public partial class OperatingSystem
    {
        [SupportedOSPlatformGuard("ios")]
        public static bool IsMacCatalyst();
    }
}

This would mean that code that checks for IsMacCatalyst() never has to check for iOS -- this check is now always implied. I'm not sure how this plays for version numbers. If iOS is supposed to map to MacCatalyst versions 1:1 we can add a property to SupportedOSPlatformGuard to indicate that:

namespace System
{
    public partial class OperatingSystem
    {
        [SupportedOSPlatformGuard("ios", MapVersions = true)]
        public static bool IsMacCatalystVersionAtLeast(int major, int minor = 0, int build = 0);
    }
}

Please note that SupportedOSPlatformGuard is a feature we added for .NET 6. It was meant to only decorate user-provided IsXxxx methods. We'd extend the semantics when being applied to the native OperatingSystem.IsXxx APIs to model an actual compat relationsip between platforms such that [UnsupportedOSPlatform("ios")] also implies that it's unsupported on MacCatalyst.

@jeffhandley @buyaa-n, any thoughts?

@filipnavara
Copy link
Member

If iOS is supposed to map to MacCatalyst versions 1:1

It is. There are multiple versions available on MacCatalyst (Darwin, macOS, iOS) but this API and Evironment.OSVersion operates on the iOS one (consistent with Apple and documentation).

@jeffhandley
Copy link
Member

This is still on my radar to try to field for Preview 7. I expect we'll be able to look into it middle of next week.

@buyaa-n
Copy link
Contributor

buyaa-n commented Jul 8, 2021

Proposal A

I do not completely understand their relation, is iOS is the parent of MacCatalys? iOS supports MacCatalyst but MacCatalyst not support iOS?

[SupportedOSPlatformGuard("ios")]
public static bool IsMacCatalyst();

Interesting solution, makes sense for above relation, but it is not directly comply with Proposal A expectation:
OperatingSystem.IsIOS() || OperatingSystem.IsTvOS() || OperatingSystem.IsMacCatalyst() would be shortened to
OperatingSystem.IsTvOS() || OperatingSystem.IsMacCatalyst() not OperatingSystem.IsIOS() || OperatingSystem.IsTvOS().

@filipnavara
Copy link
Member

I do not completely understand their relation, is iOS is the parent of MacCatalyst? iOS supports MacCatalyst but MacCatalyst not support iOS?

MacCatalyst works kinda like a superset of iOS API (with very rare exception where some APIs are missing). There's TFM fallback that allows managed iOS code to be consumed by application targeting MacCatalyst. Effectively you can think of MacCatalyst as child of iOS even if it doesn't quite reflect the real heritage.

Interesting solution, makes sense for above relation, but...

Yeah, we want to check for OperatingSystem.IsIOS() in a library that targets net6.0-ios whether it is compiled in the end into net6.0-maccatalyst or net6.0-ios application. The guard seems to do the reverse.

@jeffhandley
Copy link
Member

jeffhandley commented Jul 8, 2021

Here's my understanding of the desired experience:

Environment Guard Result Change
iOS OperatingSystem.IsIOS() true no
iOS OperatingSystem.IsTvOS() false no
iOS OperatingSystem.IsWatchOS() false no
iOS OperatingSystem.IsMacCatalyst() false no
MacCatalyst OperatingSystem.IsIOS() true YES
MacCatalyst OperatingSystem.IsTvOS() false no
MacCatalyst OperatingSystem.IsWatchOS() false no
MacCatalyst OperatingSystem.IsMacCatalyst() true no
tvOS OperatingSystem.IsIOS() false no
tvOS OperatingSystem.IsTvOS() true no
tvOS OperatingSystem.IsWatchOS() false no
tvOS OperatingSystem.IsMacCatalyst() false no
watchOS OperatingSystem.IsIOS() false no
watchOS OperatingSystem.IsTvOS() false no
watchOS OperatingSystem.IsWatchOS() true no
watchOS OperatingSystem.IsMacCatalyst() false no
Linux OperatingSystem.IsLinux() true no
Linux OperatingSystem.IsAndroid() false no
Android OperatingSystem.IsLinux() false no
Android OperatingSystem.IsAndroid() true no

@filipnavara does that match your expectations?

Note that the Android/Linux behavior has already been changed in #53034.

@filipnavara
Copy link
Member

@filipnavara does that match your expectations?

Yes

@buyaa-n
Copy link
Contributor

buyaa-n commented Jul 8, 2021

I guess the same apply to the attributes:
[SupportedOSPlatform("iOS")] cover iOS and MacCatalyst
[SupportedOSPlatform("maccatalyst")] only cover MacCatalyst
[UnsupportedOSPlatform("iOS")] cover iOS and MacCatalyst
[UnsupportedOSPlatform("maccatalyst")] only cover MacCatalyst

@filipnavara
Copy link
Member

filipnavara commented Jul 8, 2021

@buyaa-n Correct.

Note that there could be some APIs (none in dotnet/runtime AFAIK; few in Xamarin bindings) which exist on iOS and don't exist in Mac Catalyst. In that case I would expect an annotation like this to work:

[SupportedOSPlatform("ios")]
[UnsupportedOSPlatform("maccatalyst")]

This is mostly covering frameworks that don't make sense outside of the phone environment (CoreTelephony, CoreNFC, ARKit, VisionKit, HealthKit, HealthKitUI, AddressBookUI, CarPlay, ...). It's feasible that some of them may appear at later time if Apple decides to do so.

@jeffhandley
Copy link
Member

[SupportedOSPlatform("iOS")] cover iOS and MacCatalyst
[UnsupportedOSPlatform("iOS")] cover iOS and MacCatalyst

@buyaa-n Would this have to happen inside the analyzer itself, or can you think of a way for this to work such that the analyzer doesn't need special handling of these platforms?

@buyaa-n
Copy link
Contributor

buyaa-n commented Jul 8, 2021

@buyaa-n Would this have to happen inside the analyzer itself,

Yes its need analyzer update, same for removing coverage by:

[SupportedOSPlatform("ios")]
[UnsupportedOSPlatform("maccatalyst")]

or can you think of a way for this to work such that the analyzer doesn't need special handling of these platforms?

No, nothing

@terrajobst
Copy link
Member

@buyaa-n

Have you looked at my comment? Ideally we'd make this data driven and don't hard code compat rules.

@jeffhandley
Copy link
Member

I agree on making it data-driven so that we don't have to hard-code the compat rules into the analyzer. But the guard attributes as illustrated wouldn't cover the cases of:

[SupportedOSPlatform("ios")]
[UnsupportedOSPlatform("maccatalyst")]

or

[SupportedOSPlatform("maccatalyst")]
[UnsupportedOSPlatform("ios")]

We could potentially have the SDK automatically apply [assembly:SupportedOSPlatform("maccatalyst")] when targeting net6.0-ios, but that still wouldn't help in the [UnsupportedOSPlatform("ios")] case. I can't think of anything that would avoid coding that into the analyzer unless we create new OperatingSystem API for getting which platforms are inferred to be supported/unsupported by other platforms, and the analyzer was calling into that API.

@buyaa-n
Copy link
Contributor

buyaa-n commented Jul 8, 2021

Have you looked at my comment? Ideally we'd make this data driven and don't hard code compat rules.

Yes, that makes sense, but that doesn't look like could cover all scenarios, especially for handling the relation between called API attributes and the call-site attributes

@terrajobst
Copy link
Member

terrajobst commented Jul 8, 2021

@jeffhandley @buyaa-n

I'm not entirely following. What I'm proposing is that we use the guard attributes on the OperatingSystem methods in order to communicate the compat relationship to the analyzer. We still need to add the generic support to the analyzer to understand the idea of "if I'm running on this OS I can also run on this other OS". Otherwise everyone who has to apply these attributes would need special case both as well, which isn't desirable.

Does this make sense?

@jeffhandley
Copy link
Member

  1. That we use the guard attributes on the OperatingSystem methods -- ✔️
  2. In order to communicate the compat relationship to the analyzer -- _for the OperatingSystem methods, yes -- ✔️
  3. We still need to add the generic support to the analyzer to understand the idea of "if I'm running on this OS I can also run on this other OS" -- that's where our communication break down is
  4. Otherwise everyone who has to apply these attributes would need special case both as well, which isn't desirable ✔️

So the trick is how to add the support to the analyzer to understand the platform relationships. I'd prefer for that to be data-driven and not have those specific relationships coded into the analyzer. Are you proposing the analyzer would read the guard attributes in the OperatingSystem class to glean the relationships?

@terrajobst
Copy link
Member

So the trick is how to add the support to the analyzer to understand the platform relationships.

Agreed, that's inescapable IMHO.

I'd prefer for that to be data-driven and not have those specific relationships coded into the analyzer.

Agreed

Are you proposing the analyzer would read the guard attributes in the OperatingSystem class to glean the relationships?

@jeffhandley
Copy link
Member

Gotcha; thanks, @terrajobst. OK, I'm doing a proof of concept of the guard attributes being applied to the methods to make sure the analyzer will respect those as implemented now. I'll follow up once I have those findings and we can explore what it would look like for the analyzer to consume that data (during initialization) to understand platform aliases/relationships.

One question though...

Should these relationships be defined within the runtime, or within the tooling? In other words, if the analyzer gets the data from the OperatingSystem methods, then the relationship wouldn't be understood when targeting net5.0.

@terrajobst
Copy link
Member

Should these relationships be defined within the runtime, or within the tooling? In other words, if the analyzer gets the data from the OperatingSystem methods, then the relationship wouldn't be understood when targeting net5.0.

That seems fine because if you target .NET 5 you can't see OperatingSystem.IsMacCatalyst() anyway.

@jeffhandley
Copy link
Member

jeffhandley commented Jul 11, 2021

I've been exploring options for this and I have an updated proposal. I found that the Platform Compatibility Analyzer does not currently respect SupportedOSPlatformGuard attributes on the System.OperatingSystem methods; it identifies this type specifically and infers the method's platform guard. I also found flaws in the proposal of adding [SupportedOSPlatformGuard("ios")] to IsMacCatalyst, but the updated proposal is similar and addresses the flaws.

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 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

There are a few other design considerations as well, although they are not required goals.

  1. OperatingSystem methods should be treated like any other method with [SupportedOSPlatformGuard] attributes
  2. Customers should be able to define their own platforms and relationships through extensibility
  3. The existing "macos" / "osx" alias relationship should become modeled through attributes instead of hard-coded

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("ios")] and [SupportedOSPlatformGuard("maccatalyst")]
    • Optionally, annotate all OperatingSystem.Is{Platform} methods with [SupportedOSPlatformGuard("{Platform}")] attributes
    • These attribute values could become the source of truth for casing of the platform names
  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
  5. Optionally, refactor the macOS/OSX alias logic to use this same mapping logic
    • This would require a new IsOsx member on OperatingSystem (either private or [Obsolete])

Illustration of Acceptance Criteria

The following code simulates this behavior by using custom guard properties with [SupportedOSPlatformGuard] attributes and showing where [SupportedOSPlatform] and [UnsupportedOSPlatform] attributes would be inferred by the updated logic.

    // Guard methods that simulate the OperatingSystem members
    [SupportedOSPlatformGuard("ios")]
    [SupportedOSPlatformGuard("maccatalyst")]
    bool IsIOS => OperatingSystem.IsIOS() || OperatingSystem.IsMacCatalyst();

    [SupportedOSPlatformGuard("maccatalyst")]
    bool IsMacCatalyst => OperatingSystem.IsMacCatalyst();

    // This would be a new member added to OperatingSystem, but it could be
    // private or Obsolete as an error with EditorBrowsable: Never
    [SupportedOSPlatformGuard("macos")]
    [SupportedOSPlatformGuard("osx")]
    private bool IsOsx() => OperatingSystem.IsMacOS();

    [SupportedOSPlatformGuard("macos")]
    [SupportedOSPlatformGuard("osx")]
    bool IsMacOs => OperatingSystem.IsMacOS();


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

    [SupportedOSPlatform("ios")]
    // The following attribute would exist in code, but
    // would be canceled out by the analyzer because
    // "maccatalyst" would be inferred as supported,
    // and then the unsupported attribute cancels it out.
    // [UnsupportedOSPlatform("maccatalyst")]
    void SupportedOnIOS_ButNotMacCatalyst() { }

    [SupportedOSPlatform("maccatalyst")]
    void SupportedOnMacCatalyst() { }

    [SupportedOSPlatform("ios")]
    [SupportedOSPlatform("maccatalyst")]
    void SupportedOnIOSOrMacCatalyst() { }

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

    [UnsupportedOSPlatform("maccatalyst")]
    void UnsupportedOnMacCatalyst() { }

    [SupportedOSPlatform("macos")]
    // The following attribute would not exist in the code
    // but it would be inferred by the analyzer.
    [SupportedOSPlatform("osx")]
    void SupportedOnMacOS() { }

    [SupportedOSPlatform("osx")]
    // The following attribute would not exist in the code
    // but it would be inferred by the analyzer.
    [SupportedOSPlatform("macos")]
    void SupportedOnOsx() { }


    // Test cases
    void Main()
    {
        // This block is reachable on both IOS and MacCatalyst but not macOS/OSX
        if (IsIOS)
        {
            SupportedOnIOSOrMacCatalyst();      // 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
            SupportedOnMacOS();                 // Scenario 1g - Warning
            SupportedOnOsx();                   // Scenario 1h - Warning
        }

        // This block is reachable on MacCatalyst but not IOS or macOS/OSX
        if (IsMacCatalyst)
        {
            SupportedOnIOSOrMacCatalyst();      // 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
            SupportedOnMacOS();                 // Scenario 2g - Warning
            SupportedOnOsx();                   // Scenario 2h - Warning
        }

        // This code block is reachable on IOS but not MacCatalyst or macOS/OSX
        if (IsIOS && !IsMacCatalyst)
        {
            SupportedOnIOSOrMacCatalyst();      // Scenario 3a - No warning
            SupportedOnMacCatalyst();           // Scenario 3b - Warning
            SupportedOnIOS();                   // Scenario 3c - No warning
            SupportedOnIOS_ButNotMacCatalyst(); // Scenario 3d - No warning
            UnsupportedOnIOS();                 // Scenario 3e - Warning
            UnsupportedOnMacCatalyst();         // Scenario 3f - No warning
            SupportedOnMacOS();                 // Scenario 3g - Warning
            SupportedOnOsx();                   // Scenario 3h - Warning
        }

        // This code block is reachable on macOS/OSX but not IOS or MacCatalyst
        if (IsMacOs)
        {
            SupportedOnIOSOrMacCatalyst();      // Scenario 4a - Warning
            SupportedOnMacCatalyst();           // Scenario 4b - Warning
            SupportedOnIOS();                   // Scenario 4c - Warning
            SupportedOnIOS_ButNotMacCatalyst(); // Scenario 4d - Warning
            UnsupportedOnIOS();                 // Scenario 4e - No warning
            UnsupportedOnMacCatalyst();         // Scenario 4f - No warning
            SupportedOnMacOS();                 // Scenario 4g - No warning
            SupportedOnOsx();                   // Scenario 4h - No warning
        }

        // This code block is reachable on IOS, MacCatalyst, and macOS/OSX
        if (IsMacOs || IsIOS)
        {
            SupportedOnIOSOrMacCatalyst();      // Scenario 5a - Warning
            SupportedOnMacCatalyst();           // Scenario 5b - Warning
            SupportedOnIOS();                   // Scenario 5c - Warning
            SupportedOnIOS_ButNotMacCatalyst(); // Scenario 5d - Warning
            UnsupportedOnIOS();                 // Scenario 5e - Warning
            UnsupportedOnMacCatalyst();         // Scenario 5f - Warning
            SupportedOnMacOS();                 // Scenario 5g - Warning
            SupportedOnOsx();                   // Scenario 5h - Warning
        }
    }

This approach and design meets all of the acceptance criteria defined above.

  1. APIs annotated as supported on IOS are available on MacCatalyst without warning
  2. Guarding those calls only requires checking a single platform (IsIOS())
  3. APIs can still be annotated as being being supported on one but not both IOS and MacCatalyst
  4. New usage of [SupportedOSPlatformGuard] attributes is consistent with existing behavior
  5. [UnsupportedOSPlatform] and [UnsupportedOSPlatformGuard] attributes must continue to work as expected
  6. The performance impact of the change must be negligible
    • This will be achieved by loading the map of SupportedOSPlatformGuard annotations during initialization
    • The map would be utilized during identification of supported/unsupported platforms on APIs
    • If the map is empty, then the current behavior of only identifying the specified platform would persist

The other design considerations can all be achieved as well.

  1. OperatingSystem methods should be treated like any other method with SupportedOSPlatformGuard attributes
    • This is true
  2. Customers should be able to define their own platforms and relationships through extensibility
    • Customers could define internal OperatingSystem implementations to augment/override built-in behavior
  3. The existing "macos" / "osx" alias relationship should become modeled through attributes instead of hard-coded
    • This can be true if an IsOSX() method is added to OperatingSystem (private or [Obsolete])
The lengthy discovery process that led to this proposal

SupportedOSPlatformGuard Attribute Behavior

I needed a refresher for how the guard attributes are applied. Let's start a basic single-platform scenario where the guard attribute is useful.

Single Platform without the Guard Attribute

    [SupportedOSPlatform("android")]
    void DoWork() { }

    void Main()
    {
        if (OperatingSystem.IsAndroid())
            DoWork();
    }

Abstracting away the OperatingSystem Check

    [SupportedOSPlatformGuard("android")]
    bool IsSupported => OperatingSystem.IsAndroid();

    void Main()
    {
        if (IsSupported)
            DoWork();
    }

Adding a Second Platform

In this case, DoWork is supported on both "android" and "ios" and IsSupported asserts that the current platform is either of those through an OR condition.

    [SupportedOSPlatform("android")]
    [SupportedOSPlatform("ios")]
    void DoWork() { }

    [SupportedOSPlatformGuard("android")]
    [SupportedOSPlatformGuard("ios")]
    bool IsSupported => OperatingSystem.IsAndroid() || OperatingSystem.IsIOS();

    void Main()
    {
        if (IsSupported)
            DoWork();
    }

Differentiating Between Platforms

Making the scenario more complex, we can expand this out such that there's one implementation for the Apple platforms and a different implementation for Android.

To accomplish this, we will keep IsSupported as the higher-level check, but use additional guard methods to identify the Apple platforms.

    [SupportedOSPlatform("android")]
    void SupportedOnAndroid() { }

    [SupportedOSPlatform("ios")]
    [SupportedOSPlatform("maccatalyst")]
    void SupportedOnApplePlatforms() { }

    [SupportedOSPlatformGuard("ios")]
    [SupportedOSPlatformGuard("maccatalyst")]
    bool IsApplePlatform => OperatingSystem.IsIOS() ||
                            OperatingSystem.IsMacCatalyst();

    [SupportedOSPlatformGuard("android")]
    [SupportedOSPlatformGuard("ios")]
    [SupportedOSPlatformGuard("maccatalyst")]
    bool IsSupported => IsApplePlatform ||
                        OperatingSystem.IsAndroid();

    void Main()
    {
        if (IsSupported)
        {
            if (IsApplePlatform)
                SupportedOnApplePlatforms();
            else if (OperatingSystem.IsAndroid())
                SupportedOnAndroid();
        }
    }

In this scenario:

  • IsApplePlatform is true for "ios" OR "maccatalyst"
  • IsSupported is true if the current platform is "android" OR "ios" OR "maccatalyst"

This scenario illustrates why the guards must be treated with OR logic.

MacCatalyst or IOS

Here is the behavior we desire to allow a single check for either MacCatalyst or IOS:

Environment Guard Result
iOS OperatingSystem.IsIOS() true
iOS OperatingSystem.IsTvOS() false
iOS OperatingSystem.IsWatchOS() false
iOS OperatingSystem.IsMacCatalyst() false
MacCatalyst OperatingSystem.IsIOS() true
MacCatalyst OperatingSystem.IsTvOS() false
MacCatalyst OperatingSystem.IsWatchOS() false
MacCatalyst OperatingSystem.IsMacCatalyst() true

In this table, IsIOS() should return true for either "ios" or "maccatalyst", but IsMacCatalyst() should only return true on "maccatalyst". In earlier comments on this issue, we were proposing the following:

    [SupportedOSPlatformGuard("ios")]
    public static bool IsIOS();

    [SupportedOSPlatformGuard("maccatalyst")]
    [SupportedOSPlatformGuard("ios")]
    public static bool IsMacCatalyst();

However, this is the inverse of what was actually intended. If we want IsIOS() to return true for either "ios" or "maccatalyst", then our annotations should instead be:

    [SupportedOSPlatformGuard("ios")]
    [SupportedOSPlatformGuard("maccatalyst")]
    public static bool IsIOS(); // returns true on either ios or maccatalyst

    [SupportedOSPlatformGuard("maccatalyst")]
    public static bool IsMacCatalyst(); // returns true only on maccatalyst

With those annotations, IsIOS() would match the behavior of the multi-platform guard methods illustrated above, returning true for either of the platforms. The IsMacCatalyst() method is annotated to only return true if the platform is "maccatalyst" (but not "ios").

Current Guard Behavior

The current guard behavior for this configuration can be observed by using custom guard methods with the illustrated annotations.

    [SupportedOSPlatform("ios")]
    [SupportedOSPlatform("maccatalyst")]
    void SupportedOnIOSOrMacCatalyst() { }

    [SupportedOSPlatform("ios")]
    void SupportedOnIOS() { }

    [SupportedOSPlatform("maccatalyst")]
    void SupportedOnMacCatalyst() { }

    [SupportedOSPlatformGuard("ios")]
    [SupportedOSPlatformGuard("maccatalyst")]
    bool IsIOS => OperatingSystem.IsIOS() ||
                  OperatingSystem.IsMacCatalyst();

    [SupportedOSPlatformGuard("maccatalyst")]
    bool IsMacCatalyst => OperatingSystem.IsMacCatalyst();

    void Main()
    {
        if (IsIOS)
        {
            // Correct: No warning
            SupportedOnIOSOrMacCatalyst();

            // Correct: Reachable on 'ios', only supported on 'maccatalyst'
            SupportedOnMacCatalyst();

            // INCORRECT: Reachable on 'maccatalyst', only supported on 'ios'
            SupportedOnIOS();
        }

        if (IsMacCatalyst)
        {
            // Correct: No warning
            SupportedOnIOSOrMacCatalyst();

            // Correct: No warning
            SupportedOnMacCatalyst();

            // INCORRECT: Reachable on 'maccatalyst', only supported on 'ios'
            SupportedOnIOS();
        }
    }
  1. Within the if (IsIOS) guard, the code is understood to be reachable on either "ios" OR "maccatalyst". That implies that all code within the block needs to be supported on both of those platforms, and that's not the case.

  2. Within the if (IsMacCatalyst) guard, the code is understood to be reachable only on "maccatalyst" (and not traditional "ios")--which is the accurate interpretation of asserting that we are on the superset platform. As is, that implies code within the block must be supported on "maccatalyst", but SupportedOnIOS is not.

The proposal above included changing the behavior of the second point such that when IsMacCatalyst() returns true, references to APIs supported only on "ios" should succeed, with the analyzer recognizing a relationship between these platforms.

Recognizing the Platform Relationship

We asserted above that we could glean the relationship between "maccatalyst" and "ios" by finding an annotation of [SupportedOSPlatformGuard("ios")] on IsMacCatalyst(). As is illustrated here though, that annotation would be backwards.

Two alternatives to illustrate based on the proposal are:

  1. Invert the SupportedOSPlatformGuard annotations
  2. Glean the relationship from the annotations on IsIOS() instead

Inverting the Annotations

If we invert the annotations in the sample, we would achieve the correct results within the if (IOS) block, but we still don't achieve the correct results within the if (IsMacCatalyst) block.

    [SupportedOSPlatformGuard("ios")]
    bool IsIOS => OperatingSystem.IsIOS() ||
                  OperatingSystem.IsMacCatalyst();

    [SupportedOSPlatformGuard("maccatalyst")]
    [SupportedOSPlatformGuard("ios")]
    bool IsMacCatalyst => OperatingSystem.IsMacCatalyst();

    void Main()
    {
        if (IsIOS)
        {
            // Correct: No warning
            SupportedOnIOSOrMacCatalyst();

            // Correct: Reachable on 'ios', only supported on 'maccatalyst'
            SupportedOnMacCatalyst();

            // Correct: No warning
            SupportedOnIOS();
        }

        if (IsMacCatalyst)
        {
            // Correct: No warning
            SupportedOnIOSOrMacCatalyst();

            // INCORRECT: Reachable on 'ios', only supported on 'maccatalyst'
            SupportedOnMacCatalyst();

            // INCORRECT: Reachable on 'maccatalyst', only supported on 'ios'
            SupportedOnIOS();
        }
    }

On the surface, this result seems close enough to the desired effect that the analyzer could be updated to understand the relationship between "maccatalyst" and "ios" and suppress the two incorrect warnings. The logic would be:

  1. When a call is guarded by an OperatingSystem method, check that method for SupportedOSPlatformGuard annotations
  2. When there are extra platforms indicated by the annotations, assert that calls to APIs only supported on those platforms are also supported

This would have the effect of applying AND logic between the platforms, which would be equivalent to changing the code above to:

    [SupportedOSPlatformGuard("ios")]
    bool IsIOS => OperatingSystem.IsIOS() ||
                  OperatingSystem.IsMacCatalyst();

    [SupportedOSPlatformGuard("maccatalyst")]
    // [SupportedOSPlatformGuard("ios")] this would exist
    bool IsMacCatalyst => OperatingSystem.IsMacCatalyst();

    void Main()
    {
        if (IsIOS)
        {
            // Correct: No warning
            SupportedOnIOSOrMacCatalyst();

            // Correct: Reachable on 'ios', only supported on 'maccatalyst'
            SupportedOnMacCatalyst();

            // Correct: No warning
            SupportedOnIOS();
        }

        // Simulating the _AND_ logic that would be inferred from the
        // extra "ios" guard annotation on IsMacCatalyst
        if (IsMacCatalyst && IsIOS)
        {
            // Correct: No warnings
            SupportedOnIOSOrMacCatalyst();

            // Correct: No warnings
            SupportedOnMacCatalyst();

            // Correct: No warnings
            SupportedOnIOS();
        }
    }

Everything in this example is correct, but there is a flaw in this approach when we look back at the early examples of how guards are interpreted with OR logic: this behavior would have to be special-cased to apply only for methods on the OperatingSystem class and behave differently elsewhere. This violates the acceptance criteria of the new SupportedOSPlatformGuard attributes being handled consistently with existing behavior.

Glean the relationship from the annotations on IsIOS() instead

Let's look back at the previous example to see how we could check the IsIOS() method for annotations about additional platforms.

    [SupportedOSPlatformGuard("ios")]
    [SupportedOSPlatformGuard("maccatalyst")]
    bool IsIOS => OperatingSystem.IsIOS() ||
                  OperatingSystem.IsMacCatalyst();

    [SupportedOSPlatformGuard("maccatalyst")]
    bool IsMacCatalyst => OperatingSystem.IsMacCatalyst();

    void Main()
    {
        if (IsIOS)
        {
            SupportedOnIOSOrMacCatalyst(); // Scenario 1 - Should not warn
            SupportedOnMacCatalyst();      // Scenario 2 - Should warn
            SupportedOnIOS();              // Scenario 3 - Should not warn
        }

        if (IsMacCatalyst)
        {
            SupportedOnIOSOrMacCatalyst(); // Scenario 4 - Should not warn
            SupportedOnMacCatalyst();      // Scenario 5 - Should not warn
            SupportedOnIOS();              // Scenario 6 - Should not warn
        }
    }

Given the numbered scenarios, the behavior would need to be:

Scenario Behavior
1 1. Guarded by a property with multiple platforms indicated.
2. Reachable on ios and maccatalyst, supported on ios and maccatalyst.
3. No warning is produced. This is the expected result.
2 1. Guarded by a property with multiple platforms indicated.
2. Reachable on ios and maccatalyst, supported on maccatalyst.
3. Iterate over the supported platforms (in this case, maccatalyst).
4. Look for a corresponding OperatingSystem.IsMacCatalyst() method.
5. Collect the SupportedOSPlatformGuard attributes on the method.
6. Look for all reachable platforms in the list (in this case, ios).
7. No match is found.
8. A warning is produced. This is the expected result.
3 1. Guarded by a property with multiple platforms indicated.
2. Reachable on ios and maccatalyst, supported on ios.
3. Iterate over the supported platforms (in this case, ios).
4. Look for a corresponding OperatingSystem.IsIOS() method.
5. Collect the SupportedOSPlatformGuard attributes on the method.
6. Look for all reachable platforms in the list (in this case, maccatalyst).
7. A match is found for maccatalyst, so the warning is eliminated.
8. No warning is produced. This is the expected result.
4 1. Reachable on maccatalyst, supported on maccatalyst and ios.
2. All reachable platforms are supported.
3. No warning is produced. This is the expected result.
5 1. Reachable on maccatalyst, supported on maccatalyst.
2. All reachable platforms are supported.
3. No warning is produced. This is the expected result.
6 1. Reachable on maccatalyst, supported on ios.
2. Iterate over the supported platforms (in this case, ios)
3. Look for a corresponding OperatingSystem.IsIOS() method.
4. Collect the SupportedOSPlatformGuard attributes on the method.
5. Look for all reachable platforms in the list (in this case, maccatalyst).
6. A match is found for maccatalyst, so the warning is eliminated.
7. No warning is produced. This is the expected result.

Generalizing the Behavior

In order for this behavior to work, we would introduce behavior into the analyzer such that:

  1. Any time an API call is reachable on an unsupported platform
  2. Iterate over all platforms identified as reachable but not supported
  3. Check for a corresponding OperatingSystem.Is{Platform}() method
  4. Inspect the method's SupportedOSPlatformGuard attributes for other platforms
  5. Eliminate warnings for platforms included in the guard attributes

An alternate approach to the implementation could be done at the time of identifying an API's supported/unsupported platforms.

  1. For each platform marked as supported/unsupported
  2. Check for a corresponding OperatingSystem.Is{Platform}() method
  3. Inspect the method's SupportedOSPlatformGuard attributes
  4. Set the API's supported platforms to include all of the platforms indicated by the attributes

We can simulate this approach by explicitly applying the attributes that would be inferred. This example adds scenarios for the other acceptance criteria as well, including the scenario of an API being marked as supported on IOS but not MacCatalyst.

@jeffhandley
Copy link
Member

jeffhandley commented Jul 13, 2021

Here's the plan to get this implemented:

  1. Update the runtime to have OperatingSystem.IsIOS return true when on MacCatalyst, and apply the minimal guard attributes (.NET 6, Preview 7)
  2. Update the analyzer per the proposal (.NET 6, RC1)
  3. Update the runtime code where iOS/MacCatalyst logic is currently used to benefit from the updated analyzer behavior (.NET 6, RC1)
    4. [ ] Optionally make another change to the runtime to model the macOS/OSX alias through the guard attributes (this would likely be during .NET 7) An issue filed for 7.0

@buyaa-n
Copy link
Contributor

buyaa-n commented Jul 20, 2021

Modified Illustration of Acceptance Criteria

I have modified the proposal with exact use case scenarios with have agreed to add within 6.0 (like excluding macOS/OSX scenario we moved to 7.0).

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
Copy link
Contributor

buyaa-n commented Jul 27, 2021

Here's the plan to get this implemented:

  1. Update the runtime to have OperatingSystem.IsIOS return true when on MacCatalyst, and apply the minimal guard attributes (.NET 6, Preview 7)
  2. Update the analyzer per the proposal (.NET 6, RC1)
  3. Update the runtime code where iOS/MacCatalyst logic is currently used to benefit from the updated analyzer behavior (.NET 6, RC1)
  4. Optionally make another change to the runtime to model the macOS/OSX alias through the guard attributes (this would likely be during .NET 7)

@jeffhandley as part of testing step 3 found that we left out the guard attributes scenario, most likely the when API is [UnsupportedOSPlatformGuard("ios")] it should also imply [UnsupportedOSPlatformGuard("macCatalyst")], but just want to make sure (same for [SupportedOSPlatformGuard("ios")] and [SupportedOSPlatformGuard("macCatalyst")]):

    [UnsupportedOSPlatformGuard("ios")] // is this should imply [UnsupportedOSPlatformGuard("macCatalyst")] too?
    // [UnsupportedOSPlatformGuard("macCatalyst")]  or do we want to require the attribute explicitly 
    [UnsupportedOSPlatformGuard("tvos")]
    public static bool IsDSASupported => !OperatingSystem.IsIOS() && !OperatingSystem.IsTvOS();

@jeffhandley
Copy link
Member

@jeffhandley as part of testing step 3 found that we left out the guard attributes scenario, most likely the when API is [UnsupportedOSPlatformGuard("ios")] it should also imply [UnsupportedOSPlatformGuard("macCatalyst")], but just want to make sure (same for [SupportedOSPlatformGuard("ios")] and [SupportedOSPlatformGuard("macCatalyst")]):

That is correct. However, you should be able to override that behavior if needed.

[UnsupportedOSPlatform("ios")]
[SupportedOSPlatform("maccatalyst")]
public void WorksOnMacCatalyst_ButNotStandardIOS() { }

[SupportedOSPlatform("ios")]
[UnsupportedOSPlatform("maccatalyst")]
public void WorksOnStandardIOS_ButNotMacCatalyst() {}

@jeffhandley
Copy link
Member

Sorry -- I misread -- you were asking about the guard attributes... Re-reviewing...

@terrajobst
Copy link
Member

terrajobst commented Jul 27, 2021

Do guard methods provide the same stacking behavior as the supported attributes do? If so, then @jeffhandley's logic should apply to guard methods too.

In fact, they probably should be mirrors because the guard methods are supposed to be used before calling the annotated APIs, so I'd say it would be odd if guard attributes can't be stacked like this.

@jeffhandley
Copy link
Member

Here's a representation of what I think can be inferred for the guard attributes:

[SupportedOSPlatformGuard("ios")] // specified
// [SupportedOSPlatformGuard("maccatalyst")] cannot be inferred, because
// it's possible for IsIOS() to be true while IsMacCatalyst() is false.
public bool WorksOnIOS => OperatingSystem.IsIOS();


[SupportedOSPlatformGuard("maccatalyst")] // specified
[SupportedOSPlatformGuard("ios")] // can be inferred, because
// if IsMacCatalyst() returns true, then IsIOS() will also return true.
public bool WorksOnMacCatalyst => OperatingSystem.IsMacCatalyst();


[UnsupportedOSPlatformGuard("ios")] // specified
[UnsupportedOSPlatformGuard("maccatalyst")] // can be inferred, because
// if IsIOS() returns false, then IsMacCatalyst() will also return false.
public bool DoesNotWorkOnIOS => !OperatingSystem.IsIOS();

[UnsupportedOSPlatformGuard("maccatalyst")] // specified
// [UnsupportedOSPlatformGuard("ios")] cannot be inferred, because
// it's possible for IsMacCatalyst() to be false while IsIOS() is true.
public bool DoesNotWorkOnMacCatalyst => !OperatingSystem.IsMacCatalyst();

@buyaa-n
Copy link
Contributor

buyaa-n commented Jul 29, 2021

Here's a representation of what I think can be inferred for the guard attributes:

Thanks, @jeffhandley, all looked right initially, but now I think the section covering the SupportedOSPlatformGuard part needs update because multiple SupportedOSPlatformGuard attributes produce OR logic

[SupportedOSPlatformGuard("ios")] // specified
[SupportedOSPlatformGuard("maccatalyst")] // both specified
// The guard attributes would produce OperatingSystem.IsIOS() || OperatingSystem.IsMacCatalyst() logic
public bool WorksOnIOSMacCatalyst => true;

// And because the guard method OperatingSystem.IsIOS() now has [SupportedOSPlatformGuard("maccatalyst")] 
// OperatingSystem.IsIOS()  is now infer OperatingSystem.IsIOS() || OperatingSystem.IsMacCatalyst();
// therefore [SupportedOSPlatformGuard("ios")] snould inferl [SupportedOSPlatformGuard("maccatalyst")]  too

// Further as OperatingSystem.IsMacCatalyst() has no attribute and it only infer  OperatingSystem.IsMacCatalyst()
[SupportedOSPlatformGuard("maccatalyst")] // specified
// [SupportedOSPlatformGuard("ios")] // can not be inferred, therefore to guard ios user need to add the attribute explicitly
public bool WorksOnMacCatalyst => OperatingSystem.IsMacCatalyst();

@jeffhandley
Copy link
Member

Makes sense, @buyaa-n; thanks. I forgot that the guards produce OR logic.

@buyaa-n
Copy link
Contributor

buyaa-n commented Aug 11, 2021

For step 4 filed an issue, this one can be closed now

@buyaa-n buyaa-n closed this as completed Aug 11, 2021
@filipnavara
Copy link
Member

Thanks everyone!

@ghost ghost locked as resolved and limited conversation to collaborators Sep 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime design-discussion Ongoing discussion about design without consensus
Projects
None yet
Development

No branches or pull requests

6 participants