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.45k stars 511 forks source link

Parameter Protocolize Audit #14585

Closed chamons closed 9 months ago

chamons commented 2 years ago

As found in https://github.com/xamarin/ios-samples/issues/446 and fixed in once place in https://github.com/xamarin/xamarin-macios/pull/14582#issuecomment-1083495997 we have a number of Protocolize attributes in arguments that may be incorrect.

We need to do an audit and review them one by one.

chamons commented 2 years ago

The number isn't that large:

 % git grep "\[Protocolize\]" | grep ',' |  grep .cs: | wc -l
      25
% git grep "\[Protocolize\]" | grep ',' |  grep .cs:  
appkit.cs:      bool SetDataProviderForTypes ([Protocolize] NSPasteboardItemDataProvider dataProvider, string [] types);
appkit.cs:      NSDraggingSession BeginDraggingSession (NSDraggingItem [] items, NSEvent evnt, [Protocolize] NSDraggingSource source);
appkit.cs:      void PresentViewController (NSViewController viewController, [Protocolize] NSViewControllerPresentationAnimator animator);
avfoundation.cs:        void SetDelegate ([Protocolize][NullAllowed] AVAssetResourceLoaderDelegate resourceLoaderDelegate, [NullAllowed] DispatchQueue delegateQueue);
avfoundation.cs:        bool IsValidForAsset ([NullAllowed] AVAsset asset, CMTimeRange timeRange, [Protocolize] [NullAllowed] AVVideoCompositionValidationHandling validationDelegate);
avfoundation.cs:        void StartRecording (NSUrl outputFileUrl, string fileType, [Protocolize] AVCaptureFileOutputRecordingDelegate recordingDelegate);
avfoundation.cs:        void StartRecordingToOutputFile (NSUrl outputFileUrl, [Protocolize] AVCaptureFileOutputRecordingDelegate recordingDelegate);
avfoundation.cs:        void SetDelegate ([NullAllowed][Protocolize] AVCaptureMetadataOutputObjectsDelegate objectsDelegate, [NullAllowed] DispatchQueue objectsCallbackQueue);
avfoundation.cs:        void SetDelegate ([Protocolize][NullAllowed] AVPlayerItemMetadataOutputPushDelegate pushDelegate, [NullAllowed] DispatchQueue delegateQueue);
avfoundation.cs:        void SetDelegate ([Protocolize] [NullAllowed] AVPlayerItemOutputPullDelegate delegateClass, [NullAllowed] DispatchQueue delegateQueue);
avfoundation.cs:        void SetDelegate ([Protocolize][NullAllowed] AVPlayerItemLegibleOutputPushDelegate delegateObject, [NullAllowed] DispatchQueue delegateQueue);
corebluetooth.cs:       NativeHandle Constructor ([NullAllowed][Protocolize] CBPeripheralManagerDelegate peripheralDelegate, [NullAllowed] DispatchQueue queue);
corebluetooth.cs:       NativeHandle Constructor ([NullAllowed][Protocolize] CBPeripheralManagerDelegate peripheralDelegate, [NullAllowed] DispatchQueue queue, [NullAllowed] NSDictionary options);
foundation.cs:      NSUrlConnection FromRequest (NSUrlRequest request, [Protocolize] NSUrlConnectionDelegate connectionDelegate);
foundation.cs:      NativeHandle Constructor (NSUrlRequest request, [Protocolize] NSUrlConnectionDelegate connectionDelegate);
foundation.cs:      NativeHandle Constructor (NSUrlRequest request, [Protocolize] NSUrlConnectionDelegate connectionDelegate, bool startImmediately);
imagekit.cs:        void RunSlideshow ([Protocolize] IKSlideshowDataSource dataSource, string slideshowMode, NSDictionary slideshowOptions);
quicklook.cs:       bool ShouldOpenUrl (QLPreviewController controller, NSUrl url, [Protocolize] QLPreviewItem item);
quicklook.cs:       CGRect FrameForPreviewItem (QLPreviewController controller, [Protocolize] QLPreviewItem item, ref UIView view);
quicklook.cs:       UIImage TransitionImageForPreviewItem (QLPreviewController controller, [Protocolize] QLPreviewItem item, CGRect contentRect);
quicklookUI.cs:     CGRect SourceFrameOnScreenForPreviewItem (QLPreviewPanel panel, [Protocolize] QLPreviewItem item);
quicklookUI.cs:     NSObject TransitionImageForPreviewItem (QLPreviewPanel panel, [Protocolize] QLPreviewItem item, CGRect contentRect);
webkit.cs:      void InitEvent (string eventType, bool canBubble, bool cancelable, DomAbstractView view, int /* int, not NSInteger */ detail, int /* int, not NSInteger */ screenX, int /* int, not NSInteger */ screenY, int /* int, not NSInteger */ clientX, int /* int, not NSInteger */ clientY, bool ctrlKey, bool altKey, bool shiftKey, bool metaKey, ushort button, [Protocolize] DomEventTarget relatedTarget);
webkit.cs:      NativeHandle Constructor (string eventType, bool canBubble, bool cancelable, DomAbstractView view, int /* int, not NSInteger */ detail, int /* int, not NSInteger */ screenX, int /* int, not NSInteger */ screenY, int /* int, not NSInteger */ clientX, int /* int, not NSInteger */ clientY, bool ctrlKey, bool altKey, bool shiftKey, bool metaKey, ushort button, [Protocolize] DomEventTarget relatedTarget);
wkwebkit.cs:        void AddScriptMessageHandler ([Protocolize] WKScriptMessageHandler scriptMessageHandler, string name);
rolfbjarne commented 2 years ago

We can probably just remove the [Protocolize] attribute, and add the I prefix to all the corresponding types to clean up our codebase.

[Protocolize] was used to build for both Classic and Unified profiles at the same time, but we're no longer building Classic, so we can update our code now.

rolfbjarne commented 1 year ago

This is an example of what we should do:

diff --git a/src/storekit.cs b/src/storekit.cs
index d84ef7fe9fb..82bb53890cd 100644
--- a/src/storekit.cs
+++ b/src/storekit.cs
@@ -241,10 +241,10 @@ namespace StoreKit {
                void FinishTransaction (SKPaymentTransaction transaction);

                [Export ("addTransactionObserver:")]
-               void AddTransactionObserver ([Protocolize]SKPaymentTransactionObserver observer);
+               void AddTransactionObserver (ISKPaymentTransactionObserver observer);

                [Export ("removeTransactionObserver:")]
-               void RemoveTransactionObserver ([Protocolize]SKPaymentTransactionObserver observer);
+               void RemoveTransactionObserver (ISKPaymentTransactionObserver observer);

                [Export ("transactions")]
                SKPaymentTransaction [] Transactions { get; }
diff --git a/src/uikit.cs b/src/uikit.cs
index db5672d1d39..1a849ede988 100644
--- a/src/uikit.cs
+++ b/src/uikit.cs
@@ -582,8 +582,7 @@ namespace UIKit {
                double UpdateInterval { get; set; }

                [Wrap ("WeakDelegate")]
-               [Protocolize]
-               UIAccelerometerDelegate Delegate { get; set; }
+               IUIAccelerometerDelegate Delegate { get; set; }

                [Export ("delegate", ArgumentSemantic.Assign)][NullAllowed]
                NSObject WeakDelegate { get; set; }

If the Protocolize attribute is only for !NET code, like this, then nothing needs to be done:

https://github.com/xamarin/xamarin-macios/blob/2d96d431459199ab1c0fc30c79bd631b2c14337f/src/appkit.cs#L2077-L2081