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

Incorrect SupportedOSPlatform attributes on various properties #19209

Closed
tipa opened this issue Oct 15, 2023 · 8 comments · Fixed by #19211
Closed

Incorrect SupportedOSPlatform attributes on various properties #19209

tipa opened this issue Oct 15, 2023 · 8 comments · Fixed by #19211
Labels
enhancement The issue or pull request is an enhancement
Milestone

Comments

@tipa
Copy link

tipa commented Oct 15, 2023

Similar bug report as here: #19066

Steps to Reproduce

Use NSMutableParagraphStyle.TextLists in a project with MinimumOSVersion < 16.0

Expected Behavior

No warning

Actual Behavior

/Users/tipa/GitHub/diarium/Diarium/Classes/Helper_macios.cs(99,25): warning CA1416: This call site is reachable on: 'iOS' 15.0 and later, 'maccatalyst' 15.0 and later. 'NSParagraphStyle.TextLists' is only supported on: 'ios' 16.0 and later, 'maccatalyst' 16.0 and later, 'tvos' 16.0 and later. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1416)

Comments

Documented to be available since iOS 7:
https://developer.apple.com/documentation/uikit/nsparagraphstyle/1534193-textlists?language=objc

xamarin-macios/src/xkit.cs

Lines 1797 to 1799 in 3436992

[iOS (16, 0), TV (16, 0), NoWatch, MacCatalyst (16, 0)]
[Export ("textLists")]
NSTextList [] TextLists { get; [NotImplemented] set; }

I haven't tested functionality yet, but at least setting & getting the property does not crash on iOS 15:

var xx = new NSMutableParagraphStyle();
xx.TextLists = new[] { new NSTextList(NSTextListMarkerFormats.Circle) };
var yy = xx.TextLists[0];

Documented to be available since iOS 9:
https://developer.apple.com/documentation/uikit/nsparagraphstyle/3667463-linebreakstrategy?language=objc

xamarin-macios/src/xkit.cs

Lines 1811 to 1814 in 3436992

[Mac (11, 0), Watch (7, 0), TV (14, 0), iOS (14, 0)]
[MacCatalyst (14, 0)]
[Export ("lineBreakStrategy")]
NSLineBreakStrategy LineBreakStrategy { get; [NotImplemented] set; }

There's probably even more properties for which Apple changed their min API level with the last SDK...

Environment

.NET 8 RC2

@rolfbjarne
Copy link
Member

Apple's attributes are interesting to say the least...

The NSLineBreakStrategy enum is available in iOS 14+: https://developer.apple.com/documentation/uikit/nslinebreakstrategy?language=objc

But the NSLineBreakStrategy.PushOut enum value is available in iOS 9+: https://developer.apple.com/documentation/uikit/nslinebreakstrategy/nslinebreakstrategypushout?language=objc

rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Oct 16, 2023
…agraphStyle. Fixes xamarin#19209.

Both headers and documentation agree that:

* The TextLists and LineBreakStrategy properties are available in all OS
  versions we support, so adjust the availability attributes accordingly.

* While the NSLineBreakStrategy enum is supposedly not available in all OS
  versions we support, the NSLineBreakStrategy.PushOut enum value is. This
  doesn't make much sense, so I've made our enum available in all OS versions
  + the PushOut enum value as well. All the other enum values are only
  available in iOS 14+ (according to docs + headers).

Fixes xamarin#19209.
@rolfbjarne rolfbjarne added the enhancement The issue or pull request is an enhancement label Oct 16, 2023
@rolfbjarne rolfbjarne added this to the Future milestone Oct 16, 2023
rolfbjarne added a commit that referenced this issue Oct 19, 2023
…agraphStyle. Fixes #19209. (#19211)

Both headers and documentation agree that:

* The TextLists and LineBreakStrategy properties are available in all OS
  versions we support, so adjust the availability attributes accordingly.

* While the NSLineBreakStrategy enum is supposedly not available in all OS
  versions we support, the NSLineBreakStrategy.PushOut enum value is. This
  doesn't make much sense, so I've made our enum available in all OS versions
  + the PushOut enum value as well. All the other enum values are only
  available in iOS 14+ (according to docs + headers).

Fixes #19209.
@tipa
Copy link
Author

tipa commented Nov 10, 2023

Not sure if this is related or fixed with the changes you made, but I still get warnings when accessing the property even when guarded by OperatingSystem.IsIOSVersionAtLeast

var p = new NSMutableParagraphStyle();
if (OperatingSystem.IsIOSVersionAtLeast(17))
{
    p.TextLists = new[] { new NSTextList("{decimal}.") };
}

However no warnings when using NSParagraphStyle

var p = new NSParagraphStyle();
if (OperatingSystem.IsIOSVersionAtLeast(17))
{
    p.TextLists = new[] { new NSTextList("{decimal}.") };
}

And also this works

var p = new NSMutableParagraphStyle();
if (OperatingSystem.IsIOSVersionAtLeast(17))
{
  ((NSParagraphStyle)p).TextLists = new[] { new NSTextList("{decimal}.") };
}

Maybe also worth noting that textLists of NSParagraphStyle doesn't has a setter, but the .NET bindings do

@rolfbjarne
Copy link
Member

That's because the fix didn't get in in time for the initial .NET 8 release. We'll probably ship it when we ship Xcode 15.1 support.

@tipa
Copy link
Author

tipa commented Feb 10, 2024

@rolfbjarne I am using Xcode 15.2 and the latest .NET versions, but the warning is still there. Has this change been shipped yet?

@rolfbjarne
Copy link
Member

@tipa no, I just checked, this fix hasn't been released yet. Our release supporting Xcode 15.1 turned out to be a minimal service release, with very few changes. Xcode 15.2 didn't really change anything for us (just added the visionOS SDK, which we don't support), so that didn't necessitate any release for us. At this point we might release this fix when we add support for Xcode 15.3.

@rolfbjarne

This comment was marked as outdated.

@rolfbjarne rolfbjarne added the need-info Waiting for more information before the bug can be investigated label Feb 12, 2024

This comment was marked as outdated.

@rolfbjarne rolfbjarne removed the need-info Waiting for more information before the bug can be investigated label Feb 12, 2024
@rolfbjarne
Copy link
Member

NVM my last comment, that was for a different issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue or pull request is an enhancement
Projects
None yet
2 participants