Skip to content

Commit

Permalink
[generator] Fix an issue where we'd not copy attributes from inlined …
Browse files Browse the repository at this point in the history 
…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 xamarin#14802.
  • Loading branch information
@rolfbjarne
rolfbjarne committed Jan 26, 2023
1 parent 63abb2d commit 5eec307
Show file tree
Hide file tree
Showing 4 changed files with 358 additions and 28 deletions.
7 changes: 7 additions & 0 deletions src/btouch.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
47 changes: 40 additions & 7 deletions src/generator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3634,6 +3634,36 @@ static void CopyValidAttributes (List<AvailabilityBaseAttribute> 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<AvailabilityBaseAttribute> dest, IEnumerable<AvailabilityBaseAttribute> 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)
{
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -5306,6 +5337,7 @@ void GenerateProperty (Type type, PropertyInfo pi, List<string> 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);

Expand Down Expand Up @@ -5494,7 +5526,7 @@ void GenerateProperty (Type type, PropertyInfo pi, List<string> 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
Expand Down Expand Up @@ -5566,7 +5598,7 @@ void GenerateProperty (Type type, PropertyInfo pi, List<string> 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
Expand Down Expand Up @@ -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 ";
Expand All @@ -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<NotImplementedAttribute> (setMethod))
PrintExport (minfo, GetSetterExportAttribute (pi));
print ("set;");
Expand Down Expand Up @@ -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)
Expand Down
231 changes: 210 additions & 21 deletions tests/generator/BGenTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> ();
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]
Expand Down Expand Up @@ -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<string> ();

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);
Expand Down
Loading

0 comments on commit 5eec307

Please sign in to comment.