From 5eec3077325f857de21d54f9dbc702b8ac818654 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Thu, 26 Jan 2023 11:59:50 +0100 Subject: [PATCH] [generator] Fix an issue where we'd not copy attributes from inlined protocols. Protocols with one set of introduced attributes ([TV (12, 0)]) inlined in types that were introduced in a different version ([TV (10, 0)]) would always use the attributes from the type. This is wrong if the protocol was introduced after the type, in which case we should instead use the introduced attributes from the protocol. Fix this by choosing the latest introduced attribute when we have multiple to choose from. This required passing a bit more information around so that we always know if a member is being inlined in another type. This PR will also print availability attributes on the protocol members themselves: [Protocol] interface IProtocol { [TV (12, 0)] // this was not printed before [Export ("someProperty")] string SomeProperty { get; set; } } Also add and improve some tests. Contributes towards https://github.com/xamarin/xamarin-macios/issues/14802. --- src/btouch.cs | 7 + src/generator.cs | 47 +++- tests/generator/BGenTests.cs | 231 ++++++++++++++++-- .../newer-availability-in-inlined-protocol.cs | 101 ++++++++ 4 files changed, 358 insertions(+), 28 deletions(-) create mode 100644 tests/generator/tests/newer-availability-in-inlined-protocol.cs diff --git a/src/btouch.cs b/src/btouch.cs index 3dc765ee9707..e0ed8650ccf4 100644 --- a/src/btouch.cs +++ b/src/btouch.cs @@ -44,6 +44,13 @@ public class BindingTouch : IDisposable { TargetFramework? target_framework; +#if NET + public static ApplePlatform [] AllPlatforms = new ApplePlatform [] { ApplePlatform.iOS, ApplePlatform.MacOSX, ApplePlatform.TVOS, ApplePlatform.MacCatalyst }; + public static PlatformName [] AllPlatformNames = new PlatformName [] { PlatformName.iOS, PlatformName.MacOSX, PlatformName.TvOS, PlatformName.MacCatalyst }; +#else + public static ApplePlatform [] AllPlatforms = new ApplePlatform [] { ApplePlatform.iOS, ApplePlatform.MacOSX, ApplePlatform.TVOS, ApplePlatform.WatchOS }; + public static PlatformName [] AllPlatformNames = new PlatformName [] { PlatformName.iOS, PlatformName.MacOSX, PlatformName.TvOS, PlatformName.WatchOS }; +#endif public PlatformName CurrentPlatform; public bool BindThirdPartyLibrary = true; public bool skipSystemDrawing; diff --git a/src/generator.cs b/src/generator.cs index 88df56d43fad..6a6bb16210db 100644 --- a/src/generator.cs +++ b/src/generator.cs @@ -3634,6 +3634,36 @@ static void CopyValidAttributes (List dest, IEnumerab } } + // Find the introduced attribute with the highest version between the target list and the additions. + // If the destination list has an introduced attribute, replace it if it's not the one with the highest version + // If the destination list does not have an introduced attribute, then add one if there's one in the additions and there's not already an unavailable attribute. + static void FindHighestIntroducedAttributes (List dest, IEnumerable additions) + { + if (!additions.Any ()) + return; + + foreach (var platform in BindingTouch.AllPlatformNames) { + // find the availability attribute with the highest version we're trying to add + var latestAddition = additions + .Where (v => v.AvailabilityKind == AvailabilityKind.Introduced && v.Platform == platform) + .OrderBy (v => v.Version) + .LastOrDefault (); + if (latestAddition is null) + continue; + + var added = CloneFromOtherPlatform (latestAddition, latestAddition.Platform); + var idx = dest.FindIndex (v => v.Platform == platform && v.AvailabilityKind == AvailabilityKind.Introduced); + if (idx == -1) { + // no existing introduced attribute: add it unless there's already an unavailable attribute + if (!dest.Any (v => v.Platform == platform && v.AvailabilityKind == AvailabilityKind.Unavailable)) + dest.Add (added); + } else if (added.Version > dest [idx].Version) { + // replace any existing introduced attribute if the existing version is lower than the added one + dest [idx] = added; + } + } + } + // This assumes the compiler implements property methods as get_ or set_ prefixes static PropertyInfo GetProperyFromGetSetMethod (MethodInfo method) { @@ -3705,6 +3735,7 @@ AvailabilityBaseAttribute [] GetPlatformAttributesToPrint (MemberInfo mi, Member // Don't copy parent attributes if the conflict with the type we're inlining members into // Example: don't copy Introduced on top of Unavailable. CopyValidAttributes (availabilityToConsider, parentContextAvailability); + FindHighestIntroducedAttributes (availabilityToConsider, parentContextAvailability); } else { availabilityToConsider.AddRange (parentContextAvailability); } @@ -3741,7 +3772,7 @@ AvailabilityBaseAttribute [] GetPlatformAttributesToPrint (MemberInfo mi, Member AddImpliedPlatforms (memberAvailability); // Now copy it down introduced from the parent - CopyValidAttributes (memberAvailability, availabilityToConsider.Where (attr => attr.AvailabilityKind == AvailabilityKind.Introduced)); + FindHighestIntroducedAttributes (memberAvailability, availabilityToConsider.Where (attr => attr.AvailabilityKind == AvailabilityKind.Introduced)); // Now expand the implied catalyst\TVOS from [iOS] a second time // This is needed in some cases where the only iOS information is in the @@ -5306,6 +5337,7 @@ void GenerateProperty (Type type, PropertyInfo pi, List instance_fields_ var minfo = new MemberInformation (this, this, pi, type, is_interface_impl); bool use_underscore = minfo.is_unified_internal; var mod = minfo.GetVisibility (); + Type inlinedType = pi.DeclaringType == type ? null : type; minfo.protocolize = Protocolize (pi); GetAccessorInfo (pi, out var getter, out var setter, out var generate_getter, out var generate_setter); @@ -5494,7 +5526,7 @@ void GenerateProperty (Type type, PropertyInfo pi, List instance_fields_ PrintExport (minfo, sel, export.ArgumentSemantic); } - PrintAttributes (pi.GetGetMethod (), platform: true, preserve: true, advice: true, notImplemented: true); + PrintAttributes (pi.GetGetMethod (), platform: true, preserve: true, advice: true, notImplemented: true, inlinedType: inlinedType); #if NET if (false) { #else @@ -5566,7 +5598,7 @@ void GenerateProperty (Type type, PropertyInfo pi, List instance_fields_ if (not_implemented_attr == null && (!minfo.is_sealed || !minfo.is_wrapper)) PrintExport (minfo, sel, export.ArgumentSemantic); - PrintAttributes (pi.GetSetMethod (), platform: true, preserve: true, advice: true, notImplemented: true); + PrintAttributes (pi.GetSetMethod (), platform: true, preserve: true, advice: true, notImplemented: true, inlinedType: inlinedType); #if NET if (false) { #else @@ -6410,6 +6442,7 @@ void GenerateProtocolTypes (Type type, string class_visibility, string TypeName, minfo.is_export = true; print ("[Preserve (Conditional = true)]"); + PrintAttributes (pi, platform: true); if (minfo.is_unsafe) mod = "unsafe "; @@ -6430,12 +6463,12 @@ void GenerateProtocolTypes (Type type, string class_visibility, string TypeName, else PrintExport (minfo, ea); } - PrintAttributes (getMethod, notImplemented: true); + PrintAttributes (getMethod, notImplemented: true, platform: true); print ("get;"); } if (generate_setter) { PrintBlockProxy (pi.PropertyType); - PrintAttributes (setMethod, notImplemented: true); + PrintAttributes (setMethod, notImplemented: true, platform: true); if (!AttributeManager.HasAttribute (setMethod)) PrintExport (minfo, GetSetterExportAttribute (pi)); print ("set;"); @@ -6767,10 +6800,10 @@ public void PrintBindAsAttribute (ICustomAttributeProvider mi, StringBuilder sb print (attribstr); } - public void PrintAttributes (ICustomAttributeProvider mi, bool platform = false, bool preserve = false, bool advice = false, bool notImplemented = false, bool bindAs = false, bool requiresSuper = false) + public void PrintAttributes (ICustomAttributeProvider mi, bool platform = false, bool preserve = false, bool advice = false, bool notImplemented = false, bool bindAs = false, bool requiresSuper = false, Type inlinedType = null) { if (platform) - PrintPlatformAttributes (mi as MemberInfo); + PrintPlatformAttributes (mi as MemberInfo, inlinedType); if (preserve) PrintPreserveAttribute (mi); if (advice) diff --git a/tests/generator/BGenTests.cs b/tests/generator/BGenTests.cs index 012bffccec0f..737a05a5cb49 100644 --- a/tests/generator/BGenTests.cs +++ b/tests/generator/BGenTests.cs @@ -1047,45 +1047,55 @@ public void AttributesFromInlinedProtocols (Profile profile) bgen.AssertExecute ("build"); var type = bgen.ApiAssembly.MainModule.GetType ("NS", "TypeA"); - var someMethod1 = type.Methods.Single (v => v.Name == "SomeMethod1"); - var someMethod2 = type.Methods.Single (v => v.Name == "SomeMethod2"); - var someMethod3 = type.Methods.Single (v => v.Name == "SomeMethod3"); - var someMethod4 = type.Methods.Single (v => v.Name == "SomeMethod4"); - var renderedSomeMethod1 = string.Join ("\n", someMethod1.CustomAttributes.Select (ca => RenderSupportedOSPlatformAttribute (ca)).OrderBy (v => v)); - var renderedSomeMethod2 = string.Join ("\n", someMethod2.CustomAttributes.Select (ca => RenderSupportedOSPlatformAttribute (ca)).OrderBy (v => v)); - var renderedSomeMethod3 = string.Join ("\n", someMethod3.CustomAttributes.Select (ca => RenderSupportedOSPlatformAttribute (ca)).OrderBy (v => v)); - var renderedSomeMethod4 = string.Join ("\n", someMethod4.CustomAttributes.Select (ca => RenderSupportedOSPlatformAttribute (ca)).OrderBy (v => v)); - - const string expectedAttributes1 = + var expectedAttributes = new string [] { @"[BindingImpl(3)] [Export(""someMethod1:"")] [SupportedOSPlatform(""ios12.0"")] [SupportedOSPlatform(""maccatalyst"")] -[UnsupportedOSPlatform(""tvos"")]"; - const string expectedAttributes2 = +[UnsupportedOSPlatform(""tvos"")]", + @"[BindingImpl(3)] [Export(""someMethod2:"")] [SupportedOSPlatform(""ios12.0"")] [SupportedOSPlatform(""maccatalyst"")] -[UnsupportedOSPlatform(""tvos"")]"; - const string expectedAttributes3 = +[UnsupportedOSPlatform(""tvos"")]", + @"[BindingImpl(3)] [Export(""someMethod3:"")] [SupportedOSPlatform(""ios11.0"")] [SupportedOSPlatform(""maccatalyst"")] -[UnsupportedOSPlatform(""tvos"")]"; - const string expectedAttributes4 = +[UnsupportedOSPlatform(""tvos"")]", + @"[BindingImpl(3)] [Export(""someMethod4:"")] [SupportedOSPlatform(""ios11.0"")] [SupportedOSPlatform(""maccatalyst"")] -[UnsupportedOSPlatform(""tvos"")]"; +[UnsupportedOSPlatform(""tvos"")]", + }; - Assert.AreEqual (expectedAttributes1, renderedSomeMethod1, "SomeMethod1"); - Assert.AreEqual (expectedAttributes2, renderedSomeMethod2, "SomeMethod2"); - Assert.AreEqual (expectedAttributes3, renderedSomeMethod3, "SomeMethod3"); - Assert.AreEqual (expectedAttributes4, renderedSomeMethod4, "SomeMethod4"); + int someMethodCount = expectedAttributes.Length; + var someMethod = new MethodDefinition [someMethodCount]; + var renderedSomeMethod = new string [someMethodCount]; + var failures = new List (); + for (var i = 0; i < someMethodCount; i++) { + someMethod [i] = type.Methods.Single (v => v.Name == "SomeMethod" + (i + 1).ToString ()); + renderedSomeMethod [i] = string.Join ("\n", someMethod [i].CustomAttributes.Select (ca => RenderSupportedOSPlatformAttribute (ca)).OrderBy (v => v)); + + if (expectedAttributes [i] == renderedSomeMethod [i]) + continue; + + var msg = + $"{someMethod [i].Name} has different attributes.\n" + + $"Expected attributes:\n" + + expectedAttributes [i] + "\n" + + "Actual attributes:\n" + + renderedSomeMethod [i]; + Console.WriteLine ($"❌ {msg}\n"); + failures.Add (msg); + } + + Assert.That (failures, Is.Empty, "Failures"); } [Test] @@ -1184,6 +1194,185 @@ public void GeneratedAttributeOnPropertyAccessors2 () Assert.AreEqual (expectedSetterAttributes, RenderSupportedOSPlatformAttributes (setter), "Setter Attributes"); } +#if !NET + [Ignore ("This only applies to .NET")] +#endif + [Test] + [TestCase (Profile.iOS)] + public void NewerAvailabilityInInlinedProtocol (Profile profile) + { + var bgen = BuildFile (profile, "tests/newer-availability-in-inlined-protocol.cs"); + + var expectedMethods = new [] { + new { + Type = "Whatever", + MethodCount = 21, + Methods = new [] { + new { Method = "get_IPropA", Attributes = "[SupportedOSPlatform(\"tvos140.0\")]" }, + new { Method = "get_IPropAOpt", Attributes = "[SupportedOSPlatform(\"tvos140.0\")]" }, + new { Method = "set_IPropB", Attributes = "[SupportedOSPlatform(\"tvos150.0\")]" }, + new { Method = "set_IPropBOpt", Attributes = "[SupportedOSPlatform(\"tvos150.0\")]" }, + new { Method = "get_IPropC", Attributes = "[SupportedOSPlatform(\"tvos130.0\")]" }, + new { Method = "set_IPropC", Attributes = "[SupportedOSPlatform(\"tvos130.0\")]" }, + new { Method = "get_IPropCOpt", Attributes = "[SupportedOSPlatform(\"tvos130.0\")]" }, + new { Method = "set_IPropCOpt", Attributes = "[SupportedOSPlatform(\"tvos130.0\")]" }, + + new { Method = "get_IPropD", Attributes = "[SupportedOSPlatform(\"tvos120.0\")]" }, + new { Method = "get_IPropDOpt", Attributes = "[SupportedOSPlatform(\"tvos120.0\")]" }, + new { Method = "set_IPropE", Attributes = "[SupportedOSPlatform(\"tvos120.0\")]" }, + new { Method = "set_IPropEOpt", Attributes = "[SupportedOSPlatform(\"tvos120.0\")]" }, + new { Method = "get_IPropF", Attributes = "[SupportedOSPlatform(\"tvos120.0\")]" }, + new { Method = "set_IPropF", Attributes = "[SupportedOSPlatform(\"tvos120.0\")]" }, + new { Method = "get_IPropFOpt", Attributes = "[SupportedOSPlatform(\"tvos120.0\")]" }, + new { Method = "set_IPropFOpt", Attributes = "[SupportedOSPlatform(\"tvos120.0\")]" }, + }, + }, + new { + Type = "IIProtocol", + MethodCount = 4, + Methods = new [] { + new { Method = "get_IPropA", Attributes = "" }, + new { Method = "set_IPropB", Attributes = "[SupportedOSPlatform(\"tvos150.0\")]" }, + new { Method = "get_IPropC", Attributes = "" }, + new { Method = "set_IPropC", Attributes = "" }, + }, + }, + new { + Type = "IProtocol_Extensions", + MethodCount = 4, + Methods = new [] { + new { Method = "GetIPropAOpt", Attributes = "" }, + new { Method = "SetIPropBOpt", Attributes = "[SupportedOSPlatform(\"tvos150.0\")]" }, + new { Method = "GetIPropCOpt", Attributes = "" }, + new { Method = "SetIPropCOpt", Attributes = "" }, + }, + }, + new { + Type = "IIProtocolLower", + MethodCount = 4, + Methods = new [] { + new { Method = "get_IPropD", Attributes = "" }, + new { Method = "set_IPropE", Attributes = "[SupportedOSPlatform(\"tvos110.0\")]" }, + new { Method = "get_IPropF", Attributes = "" }, + new { Method = "set_IPropF", Attributes = "" }, + }, + }, + new { + Type = "IProtocolLower_Extensions", + MethodCount = 4, + Methods = new [] { + new { Method = "GetIPropDOpt", Attributes = "" }, + new { Method = "SetIPropEOpt", Attributes = "[SupportedOSPlatform(\"tvos110.0\")]" }, + new { Method = "GetIPropFOpt", Attributes = "" }, + new { Method = "SetIPropFOpt", Attributes = "" }, + }, + }, + }; + + var expectedProperties = new [] { + new { + Type = "Whatever", + PropertyCount = 13, + Properties = new [] { + new { Property = "IPropA", Attributes = "[SupportedOSPlatform(\"tvos140.0\")]" }, + new { Property = "IPropB", Attributes = "[SupportedOSPlatform(\"tvos130.0\")]" }, + new { Property = "IPropAOpt", Attributes = "[SupportedOSPlatform(\"tvos140.0\")]" }, + new { Property = "IPropBOpt", Attributes = "[SupportedOSPlatform(\"tvos130.0\")]" }, + new { Property = "IPropC", Attributes = "[SupportedOSPlatform(\"tvos130.0\")]" }, + new { Property = "IPropCOpt", Attributes = "[SupportedOSPlatform(\"tvos130.0\")]" }, + new { Property = "IPropD", Attributes = "[SupportedOSPlatform(\"tvos120.0\")]" }, + new { Property = "IPropE", Attributes = "[SupportedOSPlatform(\"tvos120.0\")]" }, + new { Property = "IPropDOpt", Attributes = "[SupportedOSPlatform(\"tvos120.0\")]" }, + new { Property = "IPropEOpt", Attributes = "[SupportedOSPlatform(\"tvos120.0\")]" }, + new { Property = "IPropF", Attributes = "[SupportedOSPlatform(\"tvos120.0\")]" }, + new { Property = "IPropFOpt", Attributes = "[SupportedOSPlatform(\"tvos120.0\")]" }, + }, + }, + new { + Type = "IIProtocol", + PropertyCount = 3, + Properties = new [] { + new { Property = "IPropA", Attributes = "[SupportedOSPlatform(\"tvos140.0\")]" }, + new { Property = "IPropB", Attributes = "" }, + new { Property = "IPropC", Attributes = "" }, + }, + }, + new { + Type = "IProtocol_Extensions", + PropertyCount = 0, + Properties = new [] { + new { Property = "fake property for c# anonymous type compilation", Attributes = "..." }, + }, + }, + new { + Type = "IIProtocolLower", + PropertyCount = 3, + Properties = new [] { + new { Property = "IPropD", Attributes = "[SupportedOSPlatform(\"tvos100.0\")]" }, + new { Property = "IPropE", Attributes = "" }, + new { Property = "IPropF", Attributes = "" }, + }, + }, + new { + Type = "IProtocolLower_Extensions", + PropertyCount = 0, + Properties = new [] { + new { Property = "fake property for c# anonymous type compilation", Attributes = "..." }, + }, + }, + }; + + var failures = new List (); + + foreach (var expected in expectedMethods) { + var type = bgen.ApiAssembly.MainModule.Types.FirstOrDefault (v => v.Name == expected.Type); + Assert.IsNotNull (type, $"Type not found: {expected.Type}"); + Assert.AreEqual (expected.MethodCount, type.Methods.Count, $"Unexpected method count.\n\tActual methods:\n\t\t{string.Join ("\n\t\t", type.Methods.Select (v => v.FullName))}"); + if (expected.MethodCount == 0) + continue; + foreach (var expectedMember in expected.Methods) { + var member = type.Methods.SingleOrDefault (v => v.Name == expectedMember.Method); + Assert.IsNotNull (member, $"Method not found: {expectedMember.Method} in {type.FullName}"); + var renderedAttributes = RenderSupportedOSPlatformAttributes (member); + if (renderedAttributes != expectedMember.Attributes) { + var msg = + $"Property: {type.FullName}::{member.Name}\n" + + $"\tExpected attributes:\n" + + $"\t\t{string.Join ("\n\t\t", expectedMember.Attributes.Split ('\n'))}\n" + + $"\tActual attributes:\n" + + $"\t\t{string.Join ("\n\t\t", renderedAttributes.Split ('\n'))}"; + failures.Add (msg); + Console.WriteLine ($"❌ {msg}"); + } + } + } + + foreach (var expected in expectedProperties) { + var type = bgen.ApiAssembly.MainModule.Types.FirstOrDefault (v => v.Name == expected.Type); + Assert.IsNotNull (type, $"Type not found: {expected.Type}"); + Assert.AreEqual (expected.PropertyCount, type.Properties.Count, $"Unexpected property count.\n\tActual properties:\n\t\t{string.Join ("\n\t\t", type.Properties.Select (v => v.Name))}"); + if (expected.PropertyCount == 0) + continue; + foreach (var expectedMember in expected.Properties) { + var member = type.Properties.SingleOrDefault (v => v.Name == expectedMember.Property); + Assert.IsNotNull (member, $"Property not found: {expectedMember.Property} in {type.FullName}"); + var renderedAttributes = RenderSupportedOSPlatformAttributes (member); + if (renderedAttributes != expectedMember.Attributes) { + var msg = + $"Property: {type.FullName}::{member.Name}\n" + + $"\tExpected attributes:\n" + + $"\t\t{string.Join ("\n\t\t", expectedMember.Attributes.Split ('\n'))}\n" + + $"\tActual attributes:\n" + + $"\t\t{string.Join ("\n\t\t", renderedAttributes.Split ('\n'))}"; + failures.Add (msg); + Console.WriteLine ($"❌ {msg}"); + } + } + } + + Assert.That (failures, Is.Empty, "Failures"); + } + BGenTool BuildFile (Profile profile, params string [] filenames) { return BuildFile (profile, true, false, filenames); diff --git a/tests/generator/tests/newer-availability-in-inlined-protocol.cs b/tests/generator/tests/newer-availability-in-inlined-protocol.cs new file mode 100644 index 000000000000..4ced9ee6ecac --- /dev/null +++ b/tests/generator/tests/newer-availability-in-inlined-protocol.cs @@ -0,0 +1,101 @@ +using System; + +using Foundation; +using ObjCRuntime; + +namespace NS { + [Introduced (PlatformName.TvOS, 120, 0)] + [BaseType (typeof (NSObject))] + interface Whatever : IProtocol, IProtocolLower { + } + + [Introduced (PlatformName.TvOS, 130, 0)] + [Protocol] + interface IProtocol { + [Introduced (PlatformName.TvOS, 140, 0)] + [Abstract] + [Export ("iPropA")] + NSObject IPropA { + get; + [NoiOS] + set; + } + + [Abstract] + [Export ("iPropB")] + NSObject IPropB { + [NoiOS] + get; + [Introduced (PlatformName.TvOS, 150, 0)] + set; + } + + [Abstract] + [Export ("iPropC")] + NSObject IPropC { get; set; } + + [Introduced (PlatformName.TvOS, 140, 0)] + [Export ("iPropAOpt")] + NSObject IPropAOpt { + get; + [NoiOS] + set; + } + + [Export ("iPropBOpt")] + NSObject IPropBOpt { + [NoiOS] + get; + [Introduced (PlatformName.TvOS, 150, 0)] + set; + } + + [Export ("iPropCOpt")] + NSObject IPropCOpt { get; set; } + } + + [Introduced (PlatformName.TvOS, 90, 0)] + [Protocol] + interface IProtocolLower { + [Introduced (PlatformName.TvOS, 100, 0)] + [Abstract] + [Export ("iPropD")] + NSObject IPropD { + get; + [NoiOS] + set; + } + + [Abstract] + [Export ("iPropE")] + NSObject IPropE { + [NoiOS] + get; + [Introduced (PlatformName.TvOS, 110, 0)] + set; + } + + [Abstract] + [Export ("iPropF")] + NSObject IPropF { get; set; } + + [Introduced (PlatformName.TvOS, 100, 0)] + [Export ("iPropDOpt")] + NSObject IPropDOpt { + get; + [NoiOS] + set; + } + + [Export ("iPropEOpt")] + NSObject IPropEOpt { + [NoiOS] + get; + [Introduced (PlatformName.TvOS, 110, 0)] + set; + } + + [Export ("iPropFOpt")] + NSObject IPropFOpt { get; set; } + } +}