Skip to content

Commit 98e9265

Browse files
jpobstjpobst
committed
[generator] Better support deprecated property getter/setters.
1 parent 3a9f770 commit 98e9265

File tree

3 files changed

+201
-7
lines changed

3 files changed

+201
-7
lines changed

‎tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs‎

Lines changed: 162 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,8 @@ public void ObsoletedOSPlatformAttributeSupport ()
565565
Assert.True (writer.ToString ().Contains ("[global::System.Runtime.Versioning.ObsoletedOSPlatform (\"android25.0\", @\"This is a field deprecated since 25!\")]"), writer.ToString ());
566566
Assert.True (writer.ToString ().Contains ("[global::System.Runtime.Versioning.ObsoletedOSPlatform (\"android25.0\", @\"This is a constructor deprecated since 25!\")]"), writer.ToString ());
567567
Assert.True (writer.ToString ().Contains ("[global::System.Runtime.Versioning.ObsoletedOSPlatform (\"android25.0\", @\"This is a method deprecated since 25!\")]"), writer.ToString ());
568-
Assert.True (writer.ToString ().Contains ("[global::System.Runtime.Versioning.ObsoletedOSPlatform (\"android25.0\", @\"This is a property getter deprecated since 25! This is a property setter deprecated since 25!\")]"), writer.ToString ());
568+
Assert.True (writer.ToString ().Contains ("[global::System.Runtime.Versioning.ObsoletedOSPlatform (\"android25.0\", @\"This is a property getter deprecated since 25!\")]"), writer.ToString ());
569+
Assert.True (writer.ToString ().Contains ("[global::System.Runtime.Versioning.ObsoletedOSPlatform (\"android25.0\", @\"This is a property setter deprecated since 25!\")]"), writer.ToString ());
569570
}
570571

571572
[Test]
@@ -612,7 +613,152 @@ public void ObsoletedOSPlatformAttributeUnneededSupport ()
612613
}
613614

614615
[Test]
615-
[NonParallelizable] // We are setting a static property on Report
616+
public void ObsoleteGetterOnlyProperty ()
617+
{
618+
var xml = @"<api>
619+
<package name='java.lang' jni-name='java/lang'>
620+
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
621+
</package>
622+
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
623+
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='MyClass' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyClass;'>
624+
<method abstract='false' deprecated='deprecated' final='false' name='getCount' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public'></method>
625+
</class>
626+
</package>
627+
</api>";
628+
629+
var gens = ParseApiDefinition (xml);
630+
var iface = gens.Single (g => g.Name == "MyClass");
631+
632+
generator.Context.ContextTypes.Push (iface);
633+
generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
634+
generator.Context.ContextTypes.Pop ();
635+
636+
// This should use [Obsolete] on the entire property because the getter is obsolete and there is no setter
637+
Assert.True (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete (@\"deprecated\")]public virtual unsafe int Count".NormalizeLineEndings ()), writer.ToString ());
638+
639+
// Ensure we don't write getter attribute
640+
Assert.False (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete(@\"deprecated\")]get"), writer.ToString ());
641+
}
642+
643+
[Test]
644+
public void ObsoletePropertyGetter ()
645+
{
646+
var xml = @"<api>
647+
<package name='java.lang' jni-name='java/lang'>
648+
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
649+
</package>
650+
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
651+
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='MyClass' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyClass;'>
652+
<method abstract='false' deprecated='deprecated' final='false' name='getCount' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public'></method>
653+
<method abstract='false' deprecated='not deprecated' final='false' name='setCount' jni-signature='(I)V' bridge='false' native='false' return='void' jni-return='V' static='false' synchronized='false' synthetic='false' visibility='public'>
654+
<parameter name='count' type='int' jni-type='I'></parameter>
655+
</method>
656+
</class>
657+
</package>
658+
</api>";
659+
660+
var gens = ParseApiDefinition (xml);
661+
var iface = gens.Single (g => g.Name == "MyClass");
662+
663+
generator.Context.ContextTypes.Push (iface);
664+
generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
665+
generator.Context.ContextTypes.Pop ();
666+
667+
// This should use [Obsolete] on just the property getter since the setter is not obsolete
668+
Assert.True (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete(@\"deprecated\")]get"), writer.ToString ());
669+
}
670+
671+
[Test]
672+
public void ObsoletePropertySetter ()
673+
{
674+
var xml = @"<api>
675+
<package name='java.lang' jni-name='java/lang'>
676+
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
677+
</package>
678+
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
679+
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='MyClass' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyClass;'>
680+
<method abstract='false' deprecated='not deprecated' final='false' name='getCount' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public'></method>
681+
<method abstract='false' deprecated='deprecated' final='false' name='setCount' jni-signature='(I)V' bridge='false' native='false' return='void' jni-return='V' static='false' synchronized='false' synthetic='false' visibility='public'>
682+
<parameter name='count' type='int' jni-type='I'></parameter>
683+
</method>
684+
</class>
685+
</package>
686+
</api>";
687+
688+
var gens = ParseApiDefinition (xml);
689+
var iface = gens.Single (g => g.Name == "MyClass");
690+
691+
generator.Context.ContextTypes.Push (iface);
692+
generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
693+
generator.Context.ContextTypes.Pop ();
694+
695+
// This should use [Obsolete] on just the property setter since the getter is not obsolete
696+
Assert.True (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete(@\"deprecated\")]set"), writer.ToString ());
697+
}
698+
699+
[Test]
700+
public void ObsoleteBothPropertyMethods ()
701+
{
702+
var xml = @"<api>
703+
<package name='java.lang' jni-name='java/lang'>
704+
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
705+
</package>
706+
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
707+
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='MyClass' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyClass;'>
708+
<method abstract='false' deprecated='getter_message' final='false' name='getCount' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public'></method>
709+
<method abstract='false' deprecated='setter_message' final='false' name='setCount' jni-signature='(I)V' bridge='false' native='false' return='void' jni-return='V' static='false' synchronized='false' synthetic='false' visibility='public'>
710+
<parameter name='count' type='int' jni-type='I'></parameter>
711+
</method>
712+
</class>
713+
</package>
714+
</api>";
715+
716+
var gens = ParseApiDefinition (xml);
717+
var iface = gens.Single (g => g.Name == "MyClass");
718+
719+
generator.Context.ContextTypes.Push (iface);
720+
generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
721+
generator.Context.ContextTypes.Pop ();
722+
723+
// This should use [Obsolete] on both property methods because the deprecation messages are different
724+
Assert.True (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete(@\"getter_message\")]get"), writer.ToString ());
725+
Assert.True (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete(@\"setter_message\")]set"), writer.ToString ());
726+
}
727+
728+
[Test]
729+
public void ObsoleteEntireProperty ()
730+
{
731+
var xml = @"<api>
732+
<package name='java.lang' jni-name='java/lang'>
733+
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
734+
</package>
735+
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
736+
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='MyClass' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyClass;'>
737+
<method abstract='false' deprecated='deprecated' final='false' name='getCount' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public'></method>
738+
<method abstract='false' deprecated='deprecated' final='false' name='setCount' jni-signature='(I)V' bridge='false' native='false' return='void' jni-return='V' static='false' synchronized='false' synthetic='false' visibility='public'>
739+
<parameter name='count' type='int' jni-type='I'></parameter>
740+
</method>
741+
</class>
742+
</package>
743+
</api>";
744+
745+
var gens = ParseApiDefinition (xml);
746+
var iface = gens.Single (g => g.Name == "MyClass");
747+
748+
generator.Context.ContextTypes.Push (iface);
749+
generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
750+
generator.Context.ContextTypes.Pop ();
751+
752+
// This should use [Obsolete] on the entire property because the getter and setter are both obsoleted with the same message
753+
Assert.True (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete (@\"deprecated\")]public virtual unsafe int Count".NormalizeLineEndings ()), writer.ToString ());
754+
755+
// Ensure we don't write getter/setter attributes
756+
Assert.False (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete(@\"deprecated\")]get"), writer.ToString ());
757+
Assert.False (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete(@\"deprecated\")]set"), writer.ToString ());
758+
}
759+
760+
[Test]
761+
[NonParallelizable] // We are setting a static property on Report
616762
public void WarnIfTypeNameMatchesNamespace ()
617763
{
618764
var @class = new TestClass ("Object", "java.myclass.MyClass");
@@ -651,6 +797,20 @@ public void DontWarnIfNestedTypeNameMatchesNamespace ()
651797
// The warning should not be raised if the nested type matches enclosing namespace
652798
Assert.False (sb.ToString ().Contains ("warning BG8403"));
653799
}
800+
801+
static string StripRegisterAttributes (string str)
802+
{
803+
// It is hard to test if the [Obsolete] is on the setter/etc due to the [Register], so remove all [Register]s
804+
// [global::System.Obsolete (@"setter_message")]
805+
// [Register ("setCount", "(I)V", "GetSetCount_IHandler")]
806+
// set {
807+
int index;
808+
809+
while ((index = str.IndexOf ("[Register", StringComparison.Ordinal)) > -1)
810+
str = str.Substring (0, index) + str.Substring (str.IndexOf (']', index) + 1);
811+
812+
return str;
813+
}
654814
}
655815

656816
[TestFixture]

‎tools/generator/Java.Interop.Tools.Generator.ObjectModel/Property.cs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,5 +60,32 @@ public void AutoDetectEnumifiedOverrideProperties (AncestorDescendantCache cache
6060
}
6161

6262
public string ExplicitInterface => Getter?.ExplicitInterface ?? Setter?.ExplicitInterface;
63+
64+
public bool IsWholePropertyDeprecated {
65+
get {
66+
// If the getter isn't deprecated then the property isn't
67+
if (Getter?.Deprecated is null)
68+
return false;
69+
70+
// If the getter is deprecated and there is no setter then the property is deprecated
71+
if (Setter is null)
72+
return true;
73+
74+
// If the setter isn't deprecated then the property isn't
75+
if (Setter.Deprecated is null)
76+
return false;
77+
78+
// If the getter/setter deprecation messages differ, don't use whole property deprecation
79+
if (Getter.Deprecated != Setter.Deprecated)
80+
return false;
81+
82+
// If the getter/setter deprecation versions differ, don't use whole property deprecation
83+
if (Getter.DeprecatedSince != Setter.DeprecatedSince)
84+
return false;
85+
86+
// Getter/Setter deprecation is the same, use whole property deprecation
87+
return true;
88+
}
89+
}
6390
}
6491
}

‎tools/generator/SourceWriters/BoundProperty.cs‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,18 @@ public BoundProperty (GenBase gen, Property property, CodeGenerationOptions opt,
7272
IsOverride = false;
7373
}
7474

75-
// Unlike [Register], [Obsolete] cannot be put on property accessors, so we can apply them only under limited condition...
76-
if (property.Getter.Deprecated != null && (property.Setter == null || property.Setter.Deprecated != null)) {
77-
var message = property.Getter.Deprecated.Trim () + (property.Setter != null && property.Setter.Deprecated != property.Getter.Deprecated ? " " + property.Setter.Deprecated.Trim () : null);
78-
var since = property.Getter?.DeprecatedSince ?? property.Setter?.DeprecatedSince;
79-
SourceWriterExtensions.AddObsolete (Attributes, message, opt, deprecatedSince: since);
75+
// Add [Obsolete] or [ObsoletedOSPlatform]
76+
if (property.IsWholePropertyDeprecated) {
77+
// This case applies [Obsolete] to the entire property
78+
SourceWriterExtensions.AddObsolete (Attributes, property.Getter.Deprecated.Trim (), opt, deprecatedSince: property.Getter.DeprecatedSince);
79+
} else {
80+
// This case applies [Obsolete] to just the getter
81+
if (property.Getter?.Deprecated != null)
82+
SourceWriterExtensions.AddObsolete (GetterAttributes, property.Getter.Deprecated.Trim (), opt, deprecatedSince: property.Getter?.DeprecatedSince);
83+
84+
// This case applies [Obsolete] to just the setter
85+
if (property.Setter?.Deprecated != null)
86+
SourceWriterExtensions.AddObsolete (SetterAttributes, property.Setter.Deprecated.Trim (), opt, deprecatedSince: property.Setter?.DeprecatedSince);
8087
}
8188

8289
SourceWriterExtensions.AddSupportedOSPlatform (Attributes, property.Getter, opt);

0 commit comments

Comments
 (0)