CA1416 about API not being available on Mac Catalyst when it is

[rolfbjarne]

Analyzer

Diagnostic ID: CA1416: This call site is reachable on: 'iOS' 13.0 and later, 'maccatalyst' 15.0 and later. 'AppDelegate.DoSomething()' is only supported on: 'ios' 14.0 and later.

Describe the bug

An API decorated with [SupportedOSPlatform("maccatalyst")] (no version) doesn't seem to be detected as supported on Mac Catalyst.

Steps To Reproduce

1. Create a new iOS project (dotnet new ios).
2. Set SupportedOSPlatformVersion=13.0 in the csproj.
3. Add the following code to it (for instance in the AppDelegate class / in AppDelegate.cs):

private void Tapped()
{
	if (OperatingSystem.IsIOSVersionAtLeast(15,0))
		DoSomething ();
}

[SupportedOSPlatform ("ios14.0")]
[SupportedOSPlatform ("maccatalyst")]
public void DoSomething () {}

Complete test project (+ binlog):
ios-plain-52d66f2.zip

Expected behavior

No warning.

Actual behavior

warning CA1416: This call site is reachable on: 'iOS' 13.0 and later, 'maccatalyst' 15.0 and later. 'AppDelegate.DoSomething()' is only supported on: 'ios' 14.0 and later.

Note that the warning message is wrong on more than one account:

1. The call site is reachable on iOS 15.0 (not 13.0).
2. The called API is (correctly) supported on iOS 14+, but also on all versions of Mac Catalyst.

Additional context

Adding a version to the maccatalyst attribute text fixes the warning.

So there's no warning with this:

private void Tapped()
{
	if (OperatingSystem.IsIOSVersionAtLeast(15,0))
		DoSomething ();
}

[SupportedOSPlatform ("ios14.0")]
[SupportedOSPlatform ("maccatalyst14.0")]
public void DoSomething () {}

[rolfbjarne]

CC @buyaa-n

[riccardominato]

Same issue here. If you're not using MacCatalyst, adding this before the namespace makes the warning disappear.

[assembly:System.Runtime.Versioning.UnsupportedOSPlatform("maccatalyst")]

Thank you @rolfbjarne.

[MartyIX]

I tried to debug this issue a bit and I think I figured out how to write a failing test in this repo: https://github.com/dotnet/roslyn-analyzers/pull/7526/files#diff-fe676a982c8118266a3aa3b031ca04dc69395a41f78ac1cd98a397d7e475c668R4141 (based on @rolfbjarne's test)

I think that the reason why the analyzer misbehaves is:

1. roslyn-analyzers/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs

   Line 1517 in 5ed3367

   mandatorySupportFound = true;

   here we figure out that maccatalyst attribute can be suppressed. Something like that these two guys can be safely disregarded:

   using System;
   using System.Runtime.Versioning;
   
   class TestType
   {
       [SupportedOSPlatform(""ios13.0"")]
       [SupportedOSPlatform(""maccatalyst13.0"")]   // THIS GUY 
       private void Tapped()
       {
   	    if (OperatingSystem.IsIOSVersionAtLeast(15,0))
   		    DoSomething();
       }
   
       [SupportedOSPlatform(""ios14.0"")]
       [SupportedOSPlatform(""maccatalyst"")]   // ... AND THIS GUY
       public void DoSomething() {}
   }

2. However, later an analyzer called PlatformCompatibilityAnalyzer (?) is started and it figures out that OperatingSystem.IsIOSVersionAtLeast below

   using System;
   using System.Runtime.Versioning;
   
   class TestType
   {
       [SupportedOSPlatform(""ios13.0"")]
       [SupportedOSPlatform(""maccatalyst13.0"")]
       private void Tapped()
       {
   	    if (OperatingSystem.IsIOSVersionAtLeast(15,0)) // THIS GUY
   		    DoSomething();
       }
   
       [SupportedOSPlatform(""ios14.0"")]
       [SupportedOSPlatform(""maccatalyst"")]
       public void DoSomething() {}
   }

   is implemented with the SupportedOSPlatformGuard attribute

    /// <summary>
    /// Check for the iOS/MacCatalyst version (returned by 'libobjc.get_operatingSystemVersion') with a >= version comparison. Used to guard APIs that were added in the given iOS release.
    /// </summary>
    [SupportedOSPlatformGuard("maccatalyst")]
    [NonVersionable]
    public static bool IsIOSVersionAtLeast(int major, int minor = 0, int build = 0)
        => IsIOS() && IsOSVersionAtLeast(major, minor, build, 0);

3. And parents of DoSomething() call in Tapped method has then 2 parents (ios and maccatalyst) instead of just one (given that maccatalyst was suppressed) and consequetly in this block:

   roslyn-analyzers/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs

   Lines 610 to 639 in 5ed3367

   	foreach (var parent in value.Parents)
   	{
   	// NOTE: IsKnownValueGuarded mutates the input values, so we pass in cloned values
   	// to ensure that evaluation of each part of || is independent of evaluation of other parts.
   	var parentAttributes = CopyAttributes(attributes);
   	var parentCsAttributes = csAttributes == null ? null : CopyAttributes(csAttributes);
   	using var parentCapturedVersions = PooledDictionary<string, Version>.GetInstance(capturedVersions);
   	if (parent.AnalysisValues.Count > 0)
   	{
   	if (parentCsAttributes != null && parentCsAttributes.Any() &&
   	IsNegationOfCallsiteAttributes(parentCsAttributes, parent.AnalysisValues))
   	{
   	continue;
   	}
   	if (value.AnalysisValues.Count == parent.AnalysisValues.Count &&
   	IsNegationOfParentValues(value, parent.AnalysisValues.GetEnumerator()))
   	{
   	continue;
   	}
   	}
   	if (!IsKnownValueGuarded(parentAttributes, ref parentCsAttributes, parent, parentCapturedVersions, originalCsAttributes))
   	{
   	csAttributes = parentCsAttributes;
   	return false;
   	}
   	}
   	}

   we fail on

   roslyn-analyzers/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs

   Line 636 in 5ed3367

   return false;

   for maccatalyst.

I'm not 100 per cent sure this analysis is correct but it appears more or less to make sense so I tend to think it's more or less what happens.

@buyaa-n Not sure if you are the right person to ping. If you are, does it make sense to you please?

PS: I opened this #7526 PR in case somebody is able to fix the issue based on my previous text. I'm out of assigned time for this issue, so I cannot unfortunately finish it.

[buyaa-n]

I'm not 100 per cent sure this analysis is correct but it appears more or less to make sense so I tend to think it's more or less what happens.

Thank you @MartyIX your analysis looks correct. I have put up a PR that puts the suppressed maccatalyst attribute back in case ios was not suppressed. As I cannot contribute to your PR created separate PR. You can copy the changes into your PR and push the fix too. Hopefully the team will review any of the PRs and merge one soon

[MartyIX]

Not seeking credits that hard. I'm very happy that my work was useful though. :) It's great to see this being fixed.