[cecil-tests] Add two more tests. (#21625)

Add two more Cecil tests: * Verify that we don't call the NSObject.Handle setter, we're supposed to call NSObject.InitializeHandle instead (which throws an exception if the handle is null), * Verify that we never call the base class' default constructor (very easy mistake to make) in manually bound constructors - we're supposed to call the overload that takes an `NSObjectFlag` parameter instead.
xamarin · Nov 18, 2024 · 98bee4d · 98bee4d
1 parent 1a7def0
commit 98bee4d
Show file tree

Hide file tree

Showing 5 changed files with 332 additions and 1 deletion.
diff --git a/tests/cecil-tests/ConstructorTest.KnownFailures.cs b/tests/cecil-tests/ConstructorTest.KnownFailures.cs
@@ -0,0 +1,84 @@
+using System.Collections.Generic;
+
+#nullable enable
+
+namespace Cecil.Tests {
+	public partial class ConstructorTest {
+		static HashSet<string> knownFailuresNonDefaultCtorDoesNotCallBaseDefaultCtor = new HashSet<string> {
+			"AppKit.ActionDispatcher::.ctor(System.EventHandler)",
+			"AppKit.NSAlertDidEndDispatcher::.ctor(System.Action`1<System.IntPtr>)",
+			"AppKit.NSGradient::.ctor(AppKit.NSColor[],System.Single[],AppKit.NSColorSpace)",
+			"AppKit.NSImage::.ctor(Foundation.NSData,System.Boolean)",
+			"AppKit.NSImage::.ctor(System.String,System.Boolean)",
+			"AppKit.NSTextContainer::.ctor(CoreGraphics.CGSize,System.Boolean)",
+			"AVFoundation.AVAudioRecorder::.ctor(Foundation.NSUrl,AVFoundation.AudioSettings,Foundation.NSError&)",
+			"AVFoundation.AVAudioRecorder::.ctor(Foundation.NSUrl,AVFoundation.AVAudioFormat,Foundation.NSError&)",
+			"AVFoundation.InternalAVAudioSessionDelegate::.ctor(AVFoundation.AVAudioSession)",
+			"CarPlay.CPListSection::.ctor(CarPlay.CPListItem[],System.String,System.String)",
+			"CarPlay.CPListSection::.ctor(CarPlay.CPListItem[])",
+			"CloudKit.CKSyncEngineFetchChangesScope::.ctor(Foundation.NSSet`1<CloudKit.CKRecordZoneID>,System.Boolean)",
+			"CloudKit.CKSyncEngineSendChangesScope::.ctor(Foundation.NSSet`1<CloudKit.CKRecordZoneID>,System.Boolean)",
+			"CloudKit.CKUserIdentityLookupInfo::.ctor(System.String,System.Int32)",
+			"CoreAnimation.CALayer::.ctor(CoreAnimation.CALayer)",
+			"Foundation.InternalNSNotificationHandler::.ctor(Foundation.NSNotificationCenter,System.Action`1<Foundation.NSNotification>)",
+			"Foundation.NSActionDispatcher::.ctor(System.Action)",
+			"Foundation.NSAppleEventDescriptor::.ctor(Foundation.NSAppleEventDescriptorType)",
+			"Foundation.NSAsyncActionDispatcher::.ctor(System.Action)",
+			"Foundation.NSAsyncSynchronizationContextDispatcher::.ctor(System.Threading.SendOrPostCallback,System.Object)",
+			"Foundation.NSAttributedString::.ctor(Foundation.NSData,Foundation.NSAttributedStringDataType,Foundation.NSDictionary&)",
+			"Foundation.NSHttpCookie::.ctor(System.Net.Cookie)",
+			"Foundation.NSHttpCookie::.ctor(System.String,System.String,System.String,System.String)",
+			"Foundation.NSMutableArray`1::.ctor(TValue[])",
+			"Foundation.NSObject::.ctor(Foundation.NSObjectFlag)",
+			"Foundation.NSObject::.ctor(ObjCRuntime.NativeHandle,System.Boolean)",
+			"Foundation.NSString::.ctor(System.String,System.Int32,System.Int32)",
+			"Foundation.NSString::.ctor(System.String)",
+			"Foundation.NSSynchronizationContextDispatcher::.ctor(System.Threading.SendOrPostCallback,System.Object)",
+			"Foundation.NSThread::.ctor(Foundation.NSObject,ObjCRuntime.Selector,Foundation.NSObject)",
+			"Foundation.NSTimerActionDispatcher::.ctor(System.Action`1<Foundation.NSTimer>)",
+			"Foundation.NSUserDefaults::.ctor(System.String,Foundation.NSUserDefaultsType)",
+			"GameKit.GKScore::.ctor(System.String)",
+			"GameplayKit.GKPath::.ctor(System.Numerics.Vector2[],System.Single,System.Boolean)",
+			"GameplayKit.GKPath::.ctor(System.Numerics.Vector3[],System.Single,System.Boolean)",
+			"HomeKit.HMMatterHome::.ctor(Foundation.NSCoder)",
+			"HomeKit.HMMatterHome::.ctor(Foundation.NSObjectFlag)",
+			"HomeKit.HMMatterHome::.ctor(Foundation.NSUuid,System.String)",
+			"HomeKit.HMMatterHome::.ctor(ObjCRuntime.NativeHandle)",
+			"HomeKit.HMMatterRequestHandler::.ctor(Foundation.NSObjectFlag)",
+			"HomeKit.HMMatterRequestHandler::.ctor(ObjCRuntime.NativeHandle)",
+			"HomeKit.HMMatterRoom::.ctor(Foundation.NSCoder)",
+			"HomeKit.HMMatterRoom::.ctor(Foundation.NSObjectFlag)",
+			"HomeKit.HMMatterRoom::.ctor(Foundation.NSUuid,System.String)",
+			"HomeKit.HMMatterRoom::.ctor(ObjCRuntime.NativeHandle)",
+			"HomeKit.HMMatterTopology::.ctor(Foundation.NSCoder)",
+			"HomeKit.HMMatterTopology::.ctor(Foundation.NSObjectFlag)",
+			"HomeKit.HMMatterTopology::.ctor(HomeKit.HMMatterHome[])",
+			"HomeKit.HMMatterTopology::.ctor(ObjCRuntime.NativeHandle)",
+			"Intents.INPriceRange::.ctor(Intents.INPriceRangeOption,Foundation.NSDecimalNumber,System.String)",
+			"Intents.INSaveProfileInCarIntent::.ctor(Foundation.NSNumber,System.String)",
+			"Intents.INSetProfileInCarIntent::.ctor(Foundation.NSNumber,System.String,Foundation.NSNumber)",
+			"MapKit.MKMapCameraZoomRange::.ctor(System.Double,MapKit.MKMapCameraZoomRangeType)",
+			"MapKit.MKPointOfInterestFilter::.ctor(MapKit.MKPointOfInterestCategory[],MapKit.MKPointOfInterestFilterType)",
+			"ModelIO.MDLMesh::.ctor(ModelIO.MDLMesh,System.Int32,System.UInt32,ModelIO.IMDLMeshBufferAllocator)",
+			"ModelIO.MDLMesh::.ctor(System.Numerics.Vector3,CoreGraphics.NVector2i,ModelIO.MDLGeometryType,ModelIO.IMDLMeshBufferAllocator)",
+			"ModelIO.MDLMesh::.ctor(System.Numerics.Vector3,CoreGraphics.NVector2i,System.Boolean,ModelIO.MDLGeometryType,ModelIO.IMDLMeshBufferAllocator,System.Nullable`1<System.Int32>,System.Nullable`1<System.Boolean>,System.Nullable`1<System.Boolean>)",
+			"ModelIO.MDLMesh::.ctor(System.Numerics.Vector3,CoreGraphics.NVector2i,System.Boolean,System.Boolean,System.Boolean,ModelIO.MDLGeometryType,ModelIO.IMDLMeshBufferAllocator)",
+			"ModelIO.MDLMesh::.ctor(System.Numerics.Vector3,CoreGraphics.NVector3i,System.Boolean,ModelIO.MDLGeometryType,ModelIO.IMDLMeshBufferAllocator)",
+			"ModelIO.MDLMesh::.ctor(System.Numerics.Vector3,System.Boolean,ModelIO.MDLGeometryType,ModelIO.IMDLMeshBufferAllocator)",
+			"ModelIO.MDLNoiseTexture::.ctor(System.Single,System.String,CoreGraphics.NVector2i,ModelIO.MDLTextureChannelEncoding,ModelIO.MDLNoiseTextureType)",
+			"NetworkExtension.NEHotspotConfiguration::.ctor(System.String,System.Boolean)",
+			"NetworkExtension.NEHotspotConfiguration::.ctor(System.String,System.String,System.Boolean,System.Boolean)",
+			"NetworkExtension.NEHotspotConfiguration::.ctor(System.String,System.String,System.Boolean)",
+			"NetworkExtension.NEHotspotConfiguration::.ctor(System.String)",
+			"SpriteKit.SKUniform::.ctor(System.String,System.Numerics.Vector2)",
+			"SpriteKit.SKUniform::.ctor(System.String,System.Numerics.Vector3)",
+			"SpriteKit.SKUniform::.ctor(System.String,System.Numerics.Vector4)",
+			"SpriteKit.SKVideoNode::.ctor(Foundation.NSUrl)",
+			"SpriteKit.SKVideoNode::.ctor(System.String)",
+			"SpriteKit.SKWarpGeometryGrid::.ctor(System.IntPtr,System.IntPtr,System.Numerics.Vector2[],System.Numerics.Vector2[])",
+			"UIKit.UIControlEventProxy::.ctor(UIKit.UIControl,System.EventHandler)",
+			"UIKit.UIImageStatusDispatcher::.ctor(UIKit.UIImage/SaveStatus)",
+			"UIKit.UIVideoStatusDispatcher::.ctor(UIKit.UIVideo/SaveStatus)",
+		};
+	}
+}
diff --git a/tests/cecil-tests/ConstructorTest.cs b/tests/cecil-tests/ConstructorTest.cs
@@ -1,6 +1,7 @@
 using System;
 using System.Linq;
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 
 using Mono.Cecil;
 using Mono.Cecil.Cil;
@@ -10,7 +11,57 @@
 using Xamarin.Utils;
 
 namespace Cecil.Tests {
-	public class ConstructorTest {
+	public partial class ConstructorTest {
+		static bool TryFindBaseConstructorCalled (MethodDefinition ctor, [NotNullWhen (true)] out MethodReference? ctorCalled, [NotNullWhen (false)] out string? failureReason)
+		{
+			failureReason = null;
+			ctorCalled = null;
+
+			var instructions = ctor.Body.Instructions;
+			foreach (var instr in instructions) {
+				if (instr.OpCode != OpCodes.Call)
+					continue;
+
+				var target = ((MethodReference) instr.Operand).Resolve ();
+				if (target is null) {
+					failureReason = $"Unable to resolve base method call: {instr}";
+					return false;
+				}
+
+				if (!target.IsConstructor)
+					continue;
+
+				if (target.IsStatic)
+					continue;
+
+				if (target.DeclaringType == ctor.DeclaringType) {
+					ctorCalled = target;
+					return true; // forwarding to another ctor in the same class.
+				}
+
+				if (target.DeclaringType != ctor.DeclaringType.BaseType.Resolve ()) {
+					failureReason = $"Not immediate base type? calling {target.DeclaringType} from {ctor.DeclaringType} whose base type is {ctor.DeclaringType.BaseType}";
+					return false;
+				}
+
+				ctorCalled = target;
+				return true;
+			}
+
+			// Ctor just throws - this is still a failure.
+			if (instructions.Count == 4 &&
+				instructions [0].OpCode == OpCodes.Ldarg_0 &&
+				instructions [1].OpCode == OpCodes.Ldstr &&
+				instructions [2].OpCode == OpCodes.Newobj &&
+				instructions [3].OpCode == OpCodes.Throw) {
+				failureReason = $"A ctor must call the NSObjectFlag base ctor even if it only throws an exception.";
+				return false;
+			}
+
+			failureReason = $"No base ctor call found?\n\t{string.Join ("\n\t", ctor.Body.Instructions.Select (v => v.ToString ()))}";
+			return false;
+		}
+
 		static bool IsMatch (MethodDefinition ctor, params (string Namespace, string Name) [] parameterTypes)
 		{
 			if (!ctor.IsConstructor)
@@ -428,5 +479,44 @@ public void NativeObjectIntPtrBoolConstructorIsPreserved (ApplePlatform platform
 			}
 			Assert.That (failures, Is.Empty, "No failures");
 		}
+
+		[Test]
+		public void NonDefaultCtorDoesNotCallBaseDefaultCtor ()
+		{
+			Configuration.IgnoreIfAnyIgnoredPlatforms ();
+
+			var failures = new Dictionary<string, FailureWithMessageAndLocation> ();
+			foreach (var info in Helper.NetPlatformImplementationAssemblyDefinitions) {
+				foreach (var type in info.Assembly.MainModule.Types) {
+					if (!type.IsClass || !type.HasMethods)
+						continue;
+
+					if (!SubclassesNSObject (type))
+						continue;
+
+					foreach (var ctor in type.Methods.Where (v => v.HasBody && !v.IsStatic && v.IsConstructor && v.HasParameters && v.Parameters.Count > 0)) {
+						if (!TryFindBaseConstructorCalled (ctor, out var ctorCalled, out var failureMessage)) {
+							var msg = $"{ctor.RenderMethod ()}: {failureMessage}";
+							// Uncomment this to make finding and fixing known failures easier
+							// Console.WriteLine ($"{GetLocation (ctor)}{msg}");
+							failures [ctor.RenderMethod ()] = new FailureWithMessageAndLocation (msg, GetLocation (ctor));
+							continue;
+						}
+
+						if (!ctorCalled.HasParameters || ctorCalled.Parameters.Count == 0) {
+							var msg = $"{ctor.RenderMethod ()}: Calls the base class' default constructor";
+							// Uncomment this to make finding and fixing known failures easier
+							// Console.WriteLine ($"{GetLocation (ctor)}{msg}");
+							failures [ctor.RenderMethod ()] = new FailureWithMessageAndLocation (msg, GetLocation (ctor));
+							continue;
+						}
+					}
+				}
+			}
+			Helper.AssertFailures (failures,
+				knownFailuresNonDefaultCtorDoesNotCallBaseDefaultCtor,
+				nameof (knownFailuresNonDefaultCtorDoesNotCallBaseDefaultCtor),
+				"Non-default ctors that call the base class' default ctor.", (v) => $"{v.Location}: {v.Message}");
+		}
 	}
 }
diff --git a/tests/cecil-tests/Helper.cs b/tests/cecil-tests/Helper.cs
@@ -693,4 +693,30 @@ public OSPlatformAttributes (ICustomAttributeProvider api, ApplePlatform platfor
 			Platform = platform;
 		}
 	}
+
+	public record FailureWithMessageAndLocation : IComparable {
+		public string Message { get; }
+		public string Location { get; }
+
+		public FailureWithMessageAndLocation (string message, string location)
+		{
+			Message = message;
+			Location = location;
+		}
+
+		public override string ToString ()
+		{
+			if (string.IsNullOrEmpty (Location))
+				return Message;
+			return $"{Message} at {Location}";
+		}
+
+		public int CompareTo (object? obj)
+		{
+			if (obj is FailureWithMessageAndLocation other)
+				return ToString ().CompareTo (other.ToString ());
+			return -1;
+		}
+	}
+
 }
diff --git a/tests/cecil-tests/SetHandleTest.KnownFailures.cs b/tests/cecil-tests/SetHandleTest.KnownFailures.cs
@@ -0,0 +1,49 @@
+using System.Collections.Generic;
+
+#nullable enable
+
+namespace Cecil.Tests {
+	public partial class SetHandleTest {
+		static HashSet<string> knownFailuresNobodyCallsHandleSetter = new HashSet<string> {
+			"AddressBook.ABGroup::.ctor(AddressBook.ABRecord)",
+			"AppKit.NSBitmapImageRep::.ctor(Foundation.NSObjectFlag,Foundation.NSObjectFlag)",
+			"AppKit.NSGradient::Initialize(AppKit.NSColor[],System.Void*,AppKit.NSColorSpace)",
+			"AppKit.NSImage::.ctor(Foundation.NSData,System.Boolean)",
+			"AppKit.NSImage::.ctor(System.String,System.Boolean)",
+			"AppKit.NSOpenGLPixelFormat::.ctor(AppKit.NSOpenGLPixelFormatAttribute[])",
+			"AppKit.NSOpenGLPixelFormat::.ctor(System.Object[])",
+			"AppKit.NSTextContainer::.ctor(CoreGraphics.CGSize,System.Boolean)",
+			"AVFoundation.AVAudioRecorder::.ctor(Foundation.NSUrl,AVFoundation.AudioSettings,Foundation.NSError&)",
+			"AVFoundation.AVAudioRecorder::.ctor(Foundation.NSUrl,AVFoundation.AVAudioFormat,Foundation.NSError&)",
+			"CoreFoundation.CFMutableString::.ctor(CoreFoundation.CFString,System.IntPtr)",
+			"CoreFoundation.CFMutableString::.ctor(System.String,System.IntPtr)",
+			"CoreFoundation.CFSocket::Initialize(CoreFoundation.CFRunLoop,CoreFoundation.CFSocket/CreateSocket)",
+			"CoreFoundation.CFString::.ctor(System.String)",
+			"CoreFoundation.DispatchBlock::Retain()",
+			"CoreFoundation.OSLog::.ctor(System.String,System.String)",
+			"CoreGraphics.CGPattern::.ctor(CoreGraphics.CGRect,CoreGraphics.CGAffineTransform,System.Runtime.InteropServices.NFloat,System.Runtime.InteropServices.NFloat,CoreGraphics.CGPatternTiling,System.Boolean,CoreGraphics.CGPattern/DrawPattern)",
+			"Foundation.NSAttributedString::.ctor(Foundation.NSData,Foundation.NSAttributedStringDataType,Foundation.NSDictionary&)",
+			"Foundation.NSHttpCookie::CreateCookie(System.String,System.String,System.String,System.String,System.String,System.String,System.Nullable`1<System.Boolean>,System.Nullable`1<System.DateTime>,System.Nullable`1<System.Int32>,System.String,System.Nullable`1<System.Boolean>,System.Nullable`1<System.Int32>)",
+			"Foundation.NSKeyedUnarchiver::.ctor(Foundation.NSData)",
+			"Foundation.NSString::.ctor(System.String,System.Int32,System.Int32)",
+			"Foundation.NSString::.ctor(System.String)",
+			"Foundation.NSThread::.ctor(Foundation.NSObject,ObjCRuntime.Selector,Foundation.NSObject)",
+			"Foundation.NSUrlProtectionSpace::.ctor(System.String,System.Int32,System.String,System.String,System.String,System.Boolean)",
+			"Foundation.NSUrlProtectionSpace::.ctor(System.String,System.Int32,System.String,System.String,System.String)",
+			"Foundation.NSUserDefaults::.ctor(System.String,Foundation.NSUserDefaultsType)",
+			"Foundation.NSUuid::.ctor(System.Byte[])",
+			"GameKit.GKScore::.ctor(System.String)",
+			"GameplayKit.GKPath::.ctor(System.Numerics.Vector2[],System.Single,System.Boolean)",
+			"GameplayKit.GKPath::.ctor(System.Numerics.Vector3[],System.Single,System.Boolean)",
+			"Intents.INPriceRange::.ctor(Intents.INPriceRangeOption,Foundation.NSDecimalNumber,System.String)",
+			"ModelIO.MDLNoiseTexture::.ctor(System.Single,System.String,CoreGraphics.NVector2i,ModelIO.MDLTextureChannelEncoding,ModelIO.MDLNoiseTextureType)",
+			"MultipeerConnectivity.MCSession::.ctor(MultipeerConnectivity.MCPeerID,Security.SecIdentity,MultipeerConnectivity.MCEncryptionPreference)",
+			"MultipeerConnectivity.MCSession::.ctor(MultipeerConnectivity.MCPeerID,Security.SecIdentity,Security.SecCertificate[],MultipeerConnectivity.MCEncryptionPreference)",
+			"ScreenCaptureKit.SCContentFilter::.ctor(ScreenCaptureKit.SCDisplay,ScreenCaptureKit.SCRunningApplication[],ScreenCaptureKit.SCWindow[],ScreenCaptureKit.SCContentFilterOption)",
+			"ScreenCaptureKit.SCContentFilter::.ctor(ScreenCaptureKit.SCDisplay,ScreenCaptureKit.SCWindow[],ScreenCaptureKit.SCContentFilterOption)",
+			"Security.SecTrust2::.ctor(Security.SecTrust)",
+			"SpriteKit.SKVideoNode::.ctor(Foundation.NSUrl)",
+			"SpriteKit.SKVideoNode::.ctor(System.String)",
+		};
+	}
+}
diff --git a/tests/cecil-tests/SetHandleTest.cs b/tests/cecil-tests/SetHandleTest.cs
@@ -0,0 +1,82 @@
+using System;
+using System.Linq;
+using System.Collections.Generic;
+
+using Mono.Cecil;
+using Mono.Cecil.Cil;
+using NUnit.Framework;
+
+using Xamarin.Tests;
+using Xamarin.Utils;
+
+namespace Cecil.Tests {
+	public partial class SetHandleTest {
+		static string GetLocation (MethodDefinition? method)
+		{
+			if (method?.DebugInformation?.HasSequencePoints == true) {
+				var seq = method.DebugInformation.SequencePoints [0];
+				return seq.Document.Url + ":" + seq.StartLine + ": ";
+			}
+			return string.Empty;
+		}
+
+		static bool VerifyMethod (MethodDefinition method, IList<Instruction> instructions, out string? reason)
+		{
+			reason = null;
+
+			foreach (var instr in instructions) {
+				if (instr.OpCode != OpCodes.Call)
+					continue;
+
+				var target = instr.Operand as MethodReference;
+				if (target is null) {
+					reason = $"Call instructions calling null?";
+					return false;
+				}
+				if (target.Name != "set_Handle")
+					continue;
+
+				// A class can call its own Handle property
+				if (target.DeclaringType == method.DeclaringType)
+					continue;
+
+				if (target.DeclaringType.Is ("Foundation", "NSObject")) {
+					reason = $"Call to NSObject.Handle setter.";
+					return false;
+				}
+
+				if (target.DeclaringType.Is ("ObjCRuntime", "DisposableObject")) {
+					reason = $"Call to DisposableObject.Handle setter.";
+					return false;
+				}
+
+				Console.WriteLine ($"Another Handle property? {target.DeclaringType} called in {method.RenderMethod ()}");
+			}
+
+			return true;
+		}
+
+		[Test]
+		public void NobodyCallsHandleSetter ()
+		{
+			Configuration.IgnoreIfAnyIgnoredPlatforms ();
+
+			var failures = new Dictionary<string, FailureWithMessageAndLocation> ();
+			foreach (var info in Helper.NetPlatformImplementationAssemblyDefinitions) {
+				foreach (var method in info.Assembly.EnumerateMethods (v => v.HasBody)) {
+					if (!VerifyMethod (method, method.Body.Instructions, out var failureReason)) {
+						var msg = $"{method.RenderMethod ()} Failed NSObject.Handle setter verification: {failureReason}";
+						// Uncomment this to make finding and fixing known failures easier
+						// Console.WriteLine ($"{GetLocation (method)}{msg}");
+						failures [method.RenderMethod ()] = new FailureWithMessageAndLocation (msg, GetLocation (method));
+					}
+				}
+			}
+
+			Helper.AssertFailures (failures,
+				knownFailuresNobodyCallsHandleSetter,
+				nameof (knownFailuresNobodyCallsHandleSetter),
+				"Don't call NSObject.Handle's setter directly, use InitializeHandle instead.", (v) => $"{v.Location}: {v.Message}");
+		}
+	}
+}