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

[xtro] Report missing-protocol-conformance when protocols are defined… #3187

Merged
merged 18 commits into from
Jan 18, 2018
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
d9c3c68
[xtro] Report missing-protocol-conformance when protocols are defined…
VincentDondain Jan 8, 2018
d9ce8eb
Fix indentation
VincentDondain Jan 10, 2018
0f00cb1
Fix encodeWithCoder not found
VincentDondain Jan 10, 2018
b641107
Remove Internal check from VisitObjCCategoryDecl and VisitObjCInterfa…
VincentDondain Jan 10, 2018
75e4646
Ignore NSSecureCoding missing-protocol-conformance (not available on …
VincentDondain Jan 10, 2018
a391212
Fix GKSavedGameListener
VincentDondain Jan 10, 2018
9b80dc8
Ignore previewItemTitle failure (normal since it's optional)
VincentDondain Jan 10, 2018
90d1d8e
Only skip UIStateRestoring for subclasses of UIViewController
VincentDondain Jan 10, 2018
6996ac0
Ignore UIStateRestoring test on watchOS (UIViewController not available)
VincentDondain Jan 11, 2018
6268870
Properly implement NSSecureCoding for MKMapItem (ignore encodeWithCod…
VincentDondain Jan 11, 2018
ac6346e
Cleanup MapKit.ignore files
VincentDondain Jan 11, 2018
cd007c3
Fix newline noise
VincentDondain Jan 11, 2018
57ef7c4
Remove protocol conformances that generated wrong availability attrib…
VincentDondain Jan 12, 2018
fc3aee5
Avoid new virtual or virtual when adding protocol conformance
VincentDondain Jan 12, 2018
e942535
Removed objectRestorationClass and restorationParent (optional)
VincentDondain Jan 12, 2018
4fe8324
Fix indentation in webkit
VincentDondain Jan 18, 2018
94bb764
Add XAMCORE_4_0 to previewItemTitle case
VincentDondain Jan 18, 2018
0f89482
Revert "Fix GKSavedGameListener"
VincentDondain Jan 18, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions src/avfoundation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4205,11 +4205,7 @@ interface AVAssetCache
[BaseType (typeof (AVAsset), Name="AVURLAsset")]
// 'init' returns NIL
[DisableDefaultCtor]
interface AVUrlAsset {

[TV (10, 2), Mac (10, 12, 4), iOS (10, 3)]
[Export ("mayRequireContentKeysForMediaDataProcessing")]
bool MayRequireContentKeysForMediaDataProcessing { get; }
interface AVUrlAsset : AVContentKeyRecipient {

[Export ("URL", ArgumentSemantic.Copy)]
NSUrl Url { get; }
Expand Down Expand Up @@ -11517,7 +11513,13 @@ interface AVSampleBufferDisplayLayer {
bool ReadyForMoreMediaData { [Bind ("isReadyForMoreMediaData")] get; }

[Export ("enqueueSampleBuffer:")]
void Enqueue (CMSampleBuffer sampleBuffer);

#if !XAMCORE_4_0
[Wrap ("Enqueue (sampleBuffer)", IsVirtual = true)]
[Obsolete ("Use the 'Enqueue' method instead.")]
void EnqueueSampleBuffer (CMSampleBuffer sampleBuffer);
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume it's a conflict with the protocol name ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conflict it is (:


[Export ("flush")]
void Flush ();
Expand All @@ -11526,11 +11528,22 @@ interface AVSampleBufferDisplayLayer {
void FlushAndRemoveImage ();

[Export ("requestMediaDataWhenReadyOnQueue:usingBlock:")]
void RequestMediaData (DispatchQueue queue, Action handler);

#if !XAMCORE_4_0
[Wrap ("RequestMediaData (queue, enqueuer)", IsVirtual = true)]
[Obsolete ("Use the 'RequestMediaData' method instead.")]
void RequestMediaDataWhenReadyOnQueue (DispatchQueue queue, Action enqueuer);
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same ;)


[Export ("stopRequestingMediaData")]
void StopRequestingMediaData ();


// TODO: Remove (alongside others) when https://github.com/xamarin/xamarin-macios/issues/3213 is fixed and conformance to 'AVQueuedSampleBufferRendering' is restored.
[TV (11,0), Mac (10,13), iOS (11,0)]
[Export ("timebase", ArgumentSemantic.Retain)]
CMTimebase Timebase { get; }

[iOS (8, 0), Mac (10,10)]
[Field ("AVSampleBufferDisplayLayerFailedToDecodeNotification")]
[Notification]
Expand Down
4 changes: 2 additions & 2 deletions src/cloudkit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace XamCore.CloudKit {
[iOS (8,0), Mac (10,10, onlyOn64 : true)]
[DisableDefaultCtor] // NSInvalidArgumentException Reason: You must call -[CKAsset initWithFileURL:] or -[CKAsset initWithData:]
[BaseType (typeof (NSObject))]
interface CKAsset : NSCoding, NSSecureCoding {
interface CKAsset : NSCoding, NSSecureCoding, CKRecordValue {

[Export ("initWithFileURL:")]
IntPtr Constructor (NSUrl fileUrl);
Expand Down Expand Up @@ -1511,7 +1511,7 @@ interface CKRecordZoneID : NSSecureCoding, NSCopying {
[iOS (8,0), Mac (10,10, onlyOn64 : true)]
[DisableDefaultCtor] // NSInvalidArgumentException Reason: You must call -[CKReference initWithRecordID:] or -[CKReference initWithRecord:] or -[CKReference initWithAsset:]
[BaseType (typeof (NSObject))]
interface CKReference : NSSecureCoding, NSCopying {
interface CKReference : NSSecureCoding, NSCopying, CKRecordValue {

[DesignatedInitializer]
[Export ("initWithRecordID:action:")]
Expand Down
2 changes: 1 addition & 1 deletion src/coreaudiokit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace XamCore.CoreAudioKit {
#if XAMCORE_2_0 || !MONOMAC
[iOS (9,0)][Mac (10,11, onlyOn64 : true)]
[BaseType (typeof(AUViewControllerBase))]
interface AUViewController : NSExtensionRequestHandling {
interface AUViewController {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VincentDondain: This is a breaking change:

screenshot 2018-01-29 10 39 14

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rolfbjarne It's UIViewController that implements NSExtensionRequestHandling now (as part of this PR).

Then based on https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/breaking-change-rules.md#types:

Allowed

Removing an interface implementation from a type when the interface is already implemented lower in the hierarchy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I asked about this exact case and you answered me :P

https://xamarinhq.slack.com/archives/C03CCJNR7/p1515786537000437

Vincent:

I’m getting a breaking change from API diff but I don’t understand how come it’s a breaking change
I move a protocol from a subclass to its parent. So the generated method from the protocol is removed from the subclass (that’s what’s reported as a breaking change) but that method is now on the base class…
so even if a subclass of a subclass was overriding that virtual method, it should still be fine
https://gist.github.com/VincentDondain/dbd1efe6976e748a2d72df10775a4524#file-new-diff-L189-L213 and https://gist.github.com/VincentDondain/dbd1efe6976e748a2d72df10775a4524#file-new-diff-L599-L623

Rolf:

You’re assuming apidiff is infallible 🙂
Moving members to a base class is one case where it will say it’s a breaking change when it isn’t

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VincentDondain heh, ok, hopefully I'll remember the next time I run into this 😄 👍

[Export ("initWithNibName:bundle:")]
[PostGet ("NibBundle")]
IntPtr Constructor ([NullAllowed] string nibName, [NullAllowed] NSBundle bundle);
Expand Down
4 changes: 2 additions & 2 deletions src/coredata.cs
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,7 @@ interface NSIncrementalStoreNode {
// 'init' issues a warning: CoreData: error: Failed to call designated initializer on NSManagedObject class 'NSManagedObject'
// then crash while disposing the instance
[DisableDefaultCtor]
interface NSManagedObject {
interface NSManagedObject : NSFetchRequestResult {
[DesignatedInitializer]
[Export ("initWithEntity:insertIntoManagedObjectContext:")]
IntPtr Constructor (NSEntityDescription entity, [NullAllowed] NSManagedObjectContext context);
Expand Down Expand Up @@ -1145,7 +1145,7 @@ interface NSManagedObjectChangeEventArgs {
[BaseType (typeof (NSObject))]
// Objective-C exception thrown. Name: NSInvalidArgumentException Reason: *** -URIRepresentation cannot be sent to an abstract object of class NSManagedObjectID: Create a concrete instance!
[DisableDefaultCtor]
interface NSManagedObjectID : NSCopying {
interface NSManagedObjectID : NSCopying, NSFetchRequestResult {

[Export ("entity", ArgumentSemantic.Strong)]
NSEntityDescription Entity { get; }
Expand Down
3 changes: 2 additions & 1 deletion src/corelocation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
//
using XamCore.ObjCRuntime;
using XamCore.Foundation;
using XamCore.CloudKit;
using XamCore.CoreGraphics;
using XamCore.CoreLocation;
#if !MONOMAC
Expand Down Expand Up @@ -70,7 +71,7 @@ partial interface CLHeading : NSSecureCoding, NSCopying {
}

[BaseType (typeof (NSObject))]
partial interface CLLocation : NSSecureCoding, NSCopying {
partial interface CLLocation : NSSecureCoding, NSCopying, CKRecordValue {
[Export ("coordinate")]
CLLocationCoordinate2D Coordinate { get; }

Expand Down
32 changes: 24 additions & 8 deletions src/foundation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,21 @@
//
#define DOUBLE_BLOCKS
using XamCore.ObjCRuntime;
using XamCore.CloudKit;
using XamCore.CoreData;
using XamCore.CoreFoundation;
using XamCore.Foundation;
using XamCore.CoreGraphics;
#if IOS
using XamCore.QuickLook;
#endif
#if !TVOS
using XamCore.Contacts;
#endif
#if !WATCH
using XamCore.CoreAnimation;
using XamCore.CoreMedia;
using XamCore.CoreSpotlight;
using XamCore.CloudKit;
#endif
using XamCore.SceneKit;
using XamCore.Security;
Expand All @@ -48,6 +56,7 @@

#if MONOMAC
using XamCore.AppKit;
using XamCore.QuickLookUI;
#else
using XamCore.CoreLocation;
using XamCore.UIKit;
Expand Down Expand Up @@ -102,7 +111,7 @@ namespace XamCore.Foundation
interface NSArray<TValue> : NSArray {}

[BaseType (typeof (NSObject))]
interface NSArray : NSSecureCoding, NSMutableCopying, INSFastEnumeration {
interface NSArray : NSSecureCoding, NSMutableCopying, INSFastEnumeration, CKRecordValue {
[Export ("count")]
nuint Count { get; }

Expand Down Expand Up @@ -1372,7 +1381,7 @@ interface NSCompoundPredicate : NSCoding {
delegate void NSDataByteRangeEnumerator (IntPtr bytes, NSRange range, ref bool stop);

[BaseType (typeof (NSObject))]
interface NSData : NSSecureCoding, NSMutableCopying {
interface NSData : NSSecureCoding, NSMutableCopying, CKRecordValue {
[Export ("dataWithContentsOfURL:")]
[Static]
NSData FromUrl (NSUrl url);
Expand Down Expand Up @@ -3495,7 +3504,7 @@ interface NSMutableData {
}

[BaseType (typeof (NSObject))]
interface NSDate : NSSecureCoding, NSCopying {
interface NSDate : NSSecureCoding, NSCopying, CKRecordValue {
[Export ("timeIntervalSinceReferenceDate")]
double SecondsSinceReferenceDate { get; }

Expand Down Expand Up @@ -3542,7 +3551,7 @@ interface NSDate : NSSecureCoding, NSCopying {
}

[BaseType (typeof (NSObject))]
interface NSDictionary : NSSecureCoding, NSMutableCopying {
interface NSDictionary : NSSecureCoding, NSMutableCopying, NSFetchRequestResult, INSFastEnumeration {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we do not have correct support for NSFastEnumeration
it better remains in the .ignore files (like others) until we do (or we'll miss this case when we add support)

Copy link
Contributor Author

@VincentDondain VincentDondain Jan 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup I saw that: https://github.com/xamarin/xamarin-macios/blob/master/src/foundation.cs#L13034-L13043

But INSFastEnumeration has a message saying: "Placeholer, just so we can start flagging things" and NSArray is already using INSFastEnumeration so I figured it was valid to add it to NSDictionary.

I already added it to the .ignore file: https://github.com/xamarin/xamarin-macios/pull/3187/files#diff-e909b6d483a4c2c9cbff0a2076e1406aR889

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure your first link is related - bad copy/paste ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, updated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please add INSFastEnumeration when applicable, that way we'll automatically get it when it's implemented.

[Export ("dictionaryWithContentsOfFile:")]
[Static]
NSDictionary FromFile (string path);
Expand Down Expand Up @@ -4069,7 +4078,11 @@ partial interface NSExtensionItem : NSCopying, NSSecureCoding {
}

[BaseType (typeof (NSObject))]
interface NSNull : NSSecureCoding, NSCopying {
interface NSNull : NSSecureCoding, NSCopying
#if !WATCH
, CAAction
#endif
{
[Export ("null"), Static]
NSNull Null { get; }
}
Expand Down Expand Up @@ -5264,6 +5277,9 @@ partial interface NSUrl : NSSecureCoding, NSCopying
#endif
#if !(MONOMAC && !XAMCORE_2_0) // exclude Classic/XM
, NSItemProviderWriting, NSItemProviderReading
#endif
#if IOS || MONOMAC
, QLPreviewItem
#endif
{
[Export ("initWithScheme:host:path:")]
Expand Down Expand Up @@ -7726,7 +7742,7 @@ interface NSStreamDelegate {
}

[BaseType (typeof (NSObject)), Bind ("NSString")]
interface NSString2 : NSSecureCoding, NSMutableCopying
interface NSString2 : NSSecureCoding, NSMutableCopying, CKRecordValue
#if MONOMAC
, NSPasteboardReading, NSPasteboardWriting // Documented that it implements NSPasteboard protocols even if header doesn't show it
#endif
Expand Down Expand Up @@ -10522,7 +10538,7 @@ interface NSValueTransformer {
[BaseType (typeof (NSValue))]
// init returns NIL
[DisableDefaultCtor]
interface NSNumber {
interface NSNumber : CKRecordValue, NSFetchRequestResult {
[Export ("charValue")]
sbyte SByteValue { get; }

Expand Down
14 changes: 8 additions & 6 deletions src/gamekit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,11 @@ interface GKLeaderboardViewController : UIAppearance
[Watch (3,0)]
[Mac (10, 8)]
[BaseType (typeof (GKPlayer))]
interface GKLocalPlayer {
interface GKLocalPlayer
#if !WATCH
: GKSavedGameListener
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's something that should be fixed

{
[Export ("authenticated")]
bool Authenticated { [Bind ("isAuthenticated")] get; }

Expand Down Expand Up @@ -855,13 +859,14 @@ interface GKSavedGame : NSCopying {
}

[NoWatch]
[NoTV]
[Protocol, Model]
[BaseType (typeof (NSObject))]
interface GKSavedGameListener {
[NoTV]
[Export ("player:didModifySavedGame:")]
void DidModifySavedGame (GKPlayer player, GKSavedGame savedGame);

[NoTV]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me go back one step (I might have induced you into an error earlier)

This looks like the protocol does not really exists in tvOS and that Apple just annotated the members and not the type itself. In doubt let's open a radar and add a comment/link into the xtro data

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea this is confusing.

Also one comment in xtro said:

## all members are not available so the protocol is empty
## however this is confusing because some protocols have no members (so it can't just be ignored)

I was thinking they added the conformance (even if the protocol is empty on tvOS) for later.

But yea I can also revert that, open a radar and annotate xtro.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's tricky since exposing the protocol (if not supported) could be considered a private API (when apps are analyzed) on tvOS. Since I don't see any benefit from an empty protocol then let's play it safe :)

[Export ("player:hasConflictingSavedGames:")]
void HasConflictingSavedGames (GKPlayer player, GKSavedGame [] savedGames);
}
Expand Down Expand Up @@ -2256,11 +2261,8 @@ interface GKTurnBasedExchangeReply
[Watch (3,0)]
[Model, Protocol, BaseType (typeof (NSObject))]
interface GKLocalPlayerListener : GKTurnBasedEventListener
#if !TVOS && !WATCH
, GKSavedGameListener
#endif
#if !WATCH
, GKChallengeListener, GKInviteEventListener
, GKChallengeListener, GKInviteEventListener, GKSavedGameListener
#endif
{
}
Expand Down
4 changes: 2 additions & 2 deletions src/mapkit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,9 @@ interface MKDirectionsRequest {
[iOS (6,0)]
[TV (9,2)]
[Mac (10,9, onlyOn64 : true)]
interface MKMapItem
interface MKMapItem : NSSecureCoding
#if IOS // #if TARGET_OS_IOS
: NSItemProviderReading, NSItemProviderWriting
, NSItemProviderReading, NSItemProviderWriting
#endif
{
[Export ("placemark", ArgumentSemantic.Retain)]
Expand Down
2 changes: 1 addition & 1 deletion src/uikit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13596,7 +13596,7 @@ interface UILayoutGuide_UIConstraintBasedLayoutDebugging {
interface IUIContentContainer {}

[BaseType (typeof (UIResponder))]
interface UIViewController : NSCoding, UIAppearanceContainer, UIContentContainer, UITraitEnvironment, UIFocusEnvironment {
interface UIViewController : NSCoding, UIAppearanceContainer, UIContentContainer, UITraitEnvironment, UIFocusEnvironment, NSExtensionRequestHandling {
[DesignatedInitializer]
[Export ("initWithNibName:bundle:")]
[PostGet ("NibBundle")]
Expand Down
6 changes: 5 additions & 1 deletion src/webkit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2426,7 +2426,11 @@ partial interface WebScriptObject {
"WeakUIDelegate",
"WeakPolicyDelegate" }
)]
partial interface WebView {
partial interface WebView
#if MONOMAC
: NSUserInterfaceValidations
#endif
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: indent

[Static]
[Export ("canShowMIMEType:")]
bool CanShowMimeType (string MimeType);
Expand Down
6 changes: 5 additions & 1 deletion src/wkwebkit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,11 @@ interface WKUserScript : NSCopying {
#endif
)]
[DisableDefaultCtor ()] // Crashes during deallocation in Xcode 6 beta 2. radar 17377712.
interface WKWebView {
interface WKWebView
#if MONOMAC
: NSUserInterfaceValidations
#endif
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: indent

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: indent


[DesignatedInitializer]
[Export ("initWithFrame:configuration:")]
Expand Down
13 changes: 13 additions & 0 deletions tests/introspection/ApiSelectorTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,19 @@ protected virtual bool Skip (Type type, string selectorName)
return !TestRuntime.CheckXcodeVersion (9, 0);
}
break;
case "NSUrl":
switch (selectorName) {
case "previewItemTitle":
// 'previewItemTitle' should be optional (fixed in XAMCORE_4_0)
return true;
}
break;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: that block could have been inside a #if !XAMCORE_4_0

case "MKMapItem": // Selector not available on iOS 32-bit
switch (selectorName) {
case "encodeWithCoder:":
return !TestRuntime.CheckXcodeVersion (9, 0);
}
break;
#if !MONOMAC
case "MTLCaptureManager":
case "NEHotspotEapSettings": // Wireless Accessory Configuration is not supported in the simulator.
Expand Down
6 changes: 6 additions & 0 deletions tests/introspection/iOS/iOSApiProtocolTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,12 @@ protected override bool Skip (Type type, string protocolName)

case "UIPasteConfigurationSupporting": // types do not conform to protocol but protocol methods work on those types (base type tests in monotouch-test)
return true; // Skip everything because 'UIResponder' implements 'UIPasteConfigurationSupporting' and that's 130+ types

#if !__WATCHOS__
// Undocumented conformance (members were inlinded in 'UIViewController' before so all subtypes should conform)
case "UIStateRestoring":
return type.Name == "UIViewController" || type.IsSubclassOf (typeof (UIViewController));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

#endif
}
return base.Skip (type, protocolName);
}
Expand Down
Loading