search

xamarin / xamarin-macios

.NET for iOS, Mac Catalyst, macOS, and tvOS provide open-source bindings of the Apple SDKs for use with .NET managed languages such as C#
Other
2.49k stars 515 forks source link

SecKeychain wrapper methods not marked as obsolete/deprecated #21185

Open tipa opened 2 months ago

tipa commented 2 months ago

Framework version

net8.0-ios

Description

The SecKeychain class has what I believe are convenience methods like AddGenericPassword: https://github.com/xamarin/xamarin-macios/blob/a6b5d22d7420c0fb655026968832fe348dc72c77/src/Security/Items.cs#L511-L532 They call other methods under the hood, like SecKeychainAddGenericPassword. As those methods are deprecated, the "wrapper" method should perhaps also be marked with the obsolete attribute.

rolfbjarne commented 2 months ago

I can confirm this, our code is missing availability attributes in quite a few places.

The reason we don't get the usual warnings in our build is because we have a rather old build system in place, and we don't run the corresponding analyzers.

So the fix here is to:

  1. Run the analyzers during our build.
  2. Fix the resulting build warnings.
rolfbjarne commented 2 months ago

Here's code to build with the analyzers: https://github.com/rolfbjarne/xamarin-macios/commit/cec7a10a0709427d1c2e37798f02dffaa986f03e

That ends up reporting ~1200 warnings across all platforms (many duplicates though).

tipa commented 1 month ago

@rolfbjarne I have found another case where a method is not correctly annotated. AvAudioSession.RequestRecordPermission seems to be deprecated since iOS 17.

Will this deprecation be detected with the changes you made to the analyzer? Let me know if I should open a new issue.

rolfbjarne commented 1 month ago

@rolfbjarne I have found another case where a method is not correctly annotated. AvAudioSession.RequestRecordPermission seems to be deprecated since iOS 17.

Will this deprecation be detected with the changes you made to the analyzer? Let me know if I should open a new issue.

Our tests already detects that one, so no need to file an issue: https://github.com/xamarin/xamarin-macios/blob/c8300a15021bad46e0a48fd6fa9d169cb2c6dc31/tests/xtro-sharpie/api-annotations-dotnet/iOS-AVFoundation.todo#L294