Support UIDevice.CheckSystemVersion from Microsoft.iOS/Catalyst workloads or provide SupportedOSPlatformGuard/UnsupportedOSPlatformGuard that supports dynamic versions

[chamons]

Analyzer

Diagnostic ID: CA1416

Analyzer source

NuGet Package: Microsoft.CodeAnalysis.NetAnalyzers

Version: 7.0.0-preview1.22403.2

Describe the bug

CA1416 is incorrectly determining call site reachability as it does not understand that UIDevice.CheckSystemVersion is excluding them.

The supported extension mechanism SupportedOSPlatformGuard/UnsupportedOSPlatformGuard does not allow dynamic versions, so we can not fix this on our end.

As an example:

UIDevice.CurrentDevice.CheckSystemVersion(13, 0) would imply [SupportedOSPlatformGuard("ios13.0")] but

UIDevice.CurrentDevice.CheckSystemVersion(14, 0) would imply [SupportedOSPlatformGuard("ios14.0")] instead.

The Windows API IsWindowsVersionAtLeast does not use attributes at all but I'm told is hard coded in the analyzer directly.

Please either hard code UIDevice.CheckSystemVersion or extend SupportedOSPlatformGuard/UnsupportedOSPlatformGuard to support arguments.

Steps To Reproduce

An example of this behavior can be seen in #6189

Additional context

Originally #6189 and then dotnet/macios#16250

Discussed with @buyaa-n offline prior to filing.

[buyaa-n]

Again, this is not a bug, this scenario is not covered in the design, but we will consider extend SupportedOSPlatformGuard/UnsupportedOSPlatformGuard to support custom versions

CC @jeffhandley @terrajobst

[terrajobst]

Considering that we can easily add or remove code from the analyzer but we can't easily add or remove features from APIs I'm inclined to start with hardcoding the iOS API in the analyzer.

We can do the same for all platform-provided version checking APIs, I don't think there are gonna be that many to begin with. Once we have a list it will become more obvious whether we can (should) extend the guarding attributes to support arguments or whether the semantics are too specialized and the number of cases low enough that hardcoding them in the analyzer makes more sense.

[buyaa-n]

For me extending the guard attributes to support arguments is better as it can be used for any other similar APIs without needing to add them into hardcoded APIs

Anyway, question for @chamons:

1. Is UIDevice.CurrentDevice.CheckSystemVersion(int major, int minor) is only API needs to be supported?
2. Versions will be loaded from the parameters, but what OS platforms that API should guard? It seems the API relate to several platforms, we need to hard code them too if we decide to go with hardcoding

[chamons]

1. From what I can today, yes. I'll get back to you if we find another, but that is the only API I can find right now that would need such support.
2. The runtime check is against whatever platform you are running against, but it is on a UIKit type, so it could be iOS, Catalyst or TvOS. Since they generally share the same version numbers, I think it would be safe to imply them all.

@rolfbjarne / @dalexsoto / @mandel-macaque - ^ Check me (and also be in this thread for me after my last day).

[buyaa-n]

so it could be iOS, Catalyst or TvOS

OK, then the method UIDevice.CurrentDevice.CheckSystemVersion(13, 0) will be parsed to a guard similar to this
OperatingSystem. IsIOSVersionAtLeast(13, 0) || OperatingSystem. IsTvOSVersionAtLeast(13, 0) and can be used for guarding APIs supported on ios/maccatalys 13+ AND TvOS 13+. but not individual support. Hope this is matching with the expected result, example:

        [SupportedOSPlatform("ios13.0")]
        [SupportedOSPlatform("tvos13.0")]
        void ApiSupportedOnIosAndTvOs13() { }

        [SupportedOSPlatform("ios13.0")]
        void ApiSupportedOnIos13() { }

        [SupportedOSPlatform("tvos13.0")]
        void ApiSupportedOnTvOs13() { }

        void Test()
        {
           // equivalent of `if (OperatingSystem.IsIOSVersionAtLeast(13, 0) || OperatingSystem.IsTvOSVersionAtLeast(13, 0))`
            if (UIDevice.CurrentDevice.CheckSystemVersion(13, 0)) 
            {
                ApiSupportedOnIos13(); // warns because call site also reachable on TvOS
                ApiSupportedOnTvOs13(); // warns because call site reachable on iOS
                ApiSupportedOnIosAndTvOs13(); // not warn, API supported on both 
            }

           // individual supports need to be checked individually
            if (OperatingSystem.IsIOSVersionAtLeast(13, 0, 2))
            {
                ApiSupportedOnIos13(); // not warn
                ApiSupportedOnTvOs13(); // warns as only reachable on ios 13+
                ApiSupportedOnIosAndTvOs13(); // not warn
            }
        }

[rolfbjarne]

@buyaa-n that makes sense as long as the project is building for both iOS and tvOS (TargetFrameworks=net6.0-ios;net6.0-tvos). If the project is building for only iOS, then it doesn't make much sense to warn about API not being available on tvOS - is that how it would work?

[buyaa-n]

I think in that case we better use guard attributes for OS platform names, then can apply attributes with if-def

[terrajobst]

@rolfbjarne is there a single version of the file that has the API surface for iOS, tvOS, macOS, and Mac Catalyst or do you guys have dedicated files for each OS?

[rolfbjarne]

@terrajobst we just have a single file (https://github.com/xamarin/xamarin-macios/blob/95429d3aa2560321451b5d323494a543aedb3019/src/UIKit/UIDevice.cs?rgh-link-date=2022-10-13T17%3A54%3A34Z#L15-L24) for iOS, tvOS and Mac Catalyst. This particular API does not exist on macOS (nor do we have any equivalent API).

FWIW the reason we don't have this API on macOS, is that until recently we supported macOS 10.9, and Apple didn't provide a supported API to check the OS version on macOS 10.9 (that we could find). We could implement something now using [NSProcessInfo operationSystemVersion], but nobody's asked for it (yet). Also, if we were to implement something for macOS, it would have to be in a different namespace (because the UIKit namespace doesn't exist on macOS).