Skip to content

[generator] Better support deprecated property getter/setters. #1062

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 14, 2022
Merged

[generator] Better support deprecated property getter/setters. #1062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
164 changes: 162 additions & 2 deletions tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,8 @@ public void ObsoletedOSPlatformAttributeSupport ()
Assert.True (writer.ToString ().Contains ("[global::System.Runtime.Versioning.ObsoletedOSPlatform (\"android25.0\", @\"This is a field deprecated since 25!\")]"), writer.ToString ());
Assert.True (writer.ToString ().Contains ("[global::System.Runtime.Versioning.ObsoletedOSPlatform (\"android25.0\", @\"This is a constructor deprecated since 25!\")]"), writer.ToString ());
Assert.True (writer.ToString ().Contains ("[global::System.Runtime.Versioning.ObsoletedOSPlatform (\"android25.0\", @\"This is a method deprecated since 25!\")]"), writer.ToString ());
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 ());
Assert.True (writer.ToString ().Contains ("[global::System.Runtime.Versioning.ObsoletedOSPlatform (\"android25.0\", @\"This is a property getter deprecated since 25!\")]"), writer.ToString ());
Assert.True (writer.ToString ().Contains ("[global::System.Runtime.Versioning.ObsoletedOSPlatform (\"android25.0\", @\"This is a property setter deprecated since 25!\")]"), writer.ToString ());
}

[Test]
Expand Down Expand Up @@ -612,7 +613,152 @@ public void ObsoletedOSPlatformAttributeUnneededSupport ()
}

[Test]
[NonParallelizable] // We are setting a static property on Report
public void ObsoleteGetterOnlyProperty ()
{
var xml = @"<api>
<package name='java.lang' jni-name='java/lang'>
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
</package>
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
<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;'>
<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>
</class>
</package>
</api>";

var gens = ParseApiDefinition (xml);
var iface = gens.Single (g => g.Name == "MyClass");

generator.Context.ContextTypes.Push (iface);
generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
generator.Context.ContextTypes.Pop ();

// This should use [Obsolete] on the entire property because the getter is obsolete and there is no setter
Assert.True (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete (@\"deprecated\")]public virtual unsafe int Count".NormalizeLineEndings ()), writer.ToString ());

// Ensure we don't write getter attribute
Assert.False (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete(@\"deprecated\")]get"), writer.ToString ());
}

[Test]
public void ObsoletePropertyGetter ()
{
var xml = @"<api>
<package name='java.lang' jni-name='java/lang'>
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
</package>
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
<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;'>
<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>
<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'>
<parameter name='count' type='int' jni-type='I'></parameter>
</method>
</class>
</package>
</api>";

var gens = ParseApiDefinition (xml);
var iface = gens.Single (g => g.Name == "MyClass");

generator.Context.ContextTypes.Push (iface);
generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
generator.Context.ContextTypes.Pop ();

// This should use [Obsolete] on just the property getter since the setter is not obsolete
Assert.True (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete(@\"deprecated\")]get"), writer.ToString ());
}

[Test]
public void ObsoletePropertySetter ()
{
var xml = @"<api>
<package name='java.lang' jni-name='java/lang'>
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
</package>
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
<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;'>
<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>
<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'>
<parameter name='count' type='int' jni-type='I'></parameter>
</method>
</class>
</package>
</api>";

var gens = ParseApiDefinition (xml);
var iface = gens.Single (g => g.Name == "MyClass");

generator.Context.ContextTypes.Push (iface);
generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
generator.Context.ContextTypes.Pop ();

// This should use [Obsolete] on just the property setter since the getter is not obsolete
Assert.True (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete(@\"deprecated\")]set"), writer.ToString ());
}

[Test]
public void ObsoleteBothPropertyMethods ()
{
var xml = @"<api>
<package name='java.lang' jni-name='java/lang'>
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
</package>
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
<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;'>
<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>
<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'>
<parameter name='count' type='int' jni-type='I'></parameter>
</method>
</class>
</package>
</api>";

var gens = ParseApiDefinition (xml);
var iface = gens.Single (g => g.Name == "MyClass");

generator.Context.ContextTypes.Push (iface);
generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
generator.Context.ContextTypes.Pop ();

// This should use [Obsolete] on both property methods because the deprecation messages are different
Assert.True (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete(@\"getter_message\")]get"), writer.ToString ());
Assert.True (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete(@\"setter_message\")]set"), writer.ToString ());
}

[Test]
public void ObsoleteEntireProperty ()
{
var xml = @"<api>
<package name='java.lang' jni-name='java/lang'>
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
</package>
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
<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;'>
<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>
<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'>
<parameter name='count' type='int' jni-type='I'></parameter>
</method>
</class>
</package>
</api>";

var gens = ParseApiDefinition (xml);
var iface = gens.Single (g => g.Name == "MyClass");

generator.Context.ContextTypes.Push (iface);
generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
generator.Context.ContextTypes.Pop ();

// This should use [Obsolete] on the entire property because the getter and setter are both obsoleted with the same message
Assert.True (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete (@\"deprecated\")]public virtual unsafe int Count".NormalizeLineEndings ()), writer.ToString ());

// Ensure we don't write getter/setter attributes
Assert.False (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete(@\"deprecated\")]get"), writer.ToString ());
Assert.False (StripRegisterAttributes (writer.ToString ()).NormalizeLineEndings ().Contains ("[global::System.Obsolete(@\"deprecated\")]set"), writer.ToString ());
}

[Test]
[NonParallelizable] // We are setting a static property on Report
public void WarnIfTypeNameMatchesNamespace ()
{
var @class = new TestClass ("Object", "java.myclass.MyClass");
Expand Down Expand Up @@ -651,6 +797,20 @@ public void DontWarnIfNestedTypeNameMatchesNamespace ()
// The warning should not be raised if the nested type matches enclosing namespace
Assert.False (sb.ToString ().Contains ("warning BG8403"));
}

static string StripRegisterAttributes (string str)
{
// It is hard to test if the [Obsolete] is on the setter/etc due to the [Register], so remove all [Register]s
// [global::System.Obsolete (@"setter_message")]
// [Register ("setCount", "(I)V", "GetSetCount_IHandler")]
// set {
int index;

while ((index = str.IndexOf ("[Register", StringComparison.Ordinal)) > -1)
str = str.Substring (0, index) + str.Substring (str.IndexOf (']', index) + 1);

return str;
}
}

[TestFixture]
Expand Down
27 changes: 27 additions & 0 deletions tools/generator/Java.Interop.Tools.Generator.ObjectModel/Property.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,32 @@ public void AutoDetectEnumifiedOverrideProperties (AncestorDescendantCache cache
}

public string ExplicitInterface => Getter?.ExplicitInterface ?? Setter?.ExplicitInterface;

public bool IsWholePropertyDeprecated {
get {
// If the getter isn't deprecated then the property isn't
if (Getter?.Deprecated is null)
return false;

// If the getter is deprecated and there is no setter then the property is deprecated
if (Setter is null)
return true;

// If the setter isn't deprecated then the property isn't
if (Setter.Deprecated is null)
return false;

// If the getter/setter deprecation messages differ, don't use whole property deprecation
if (Getter.Deprecated != Setter.Deprecated)
return false;

// If the getter/setter deprecation versions differ, don't use whole property deprecation
if (Getter.DeprecatedSince != Setter.DeprecatedSince)
return false;

// Getter/Setter deprecation is the same, use whole property deprecation
return true;
}
}
}
}
17 changes: 12 additions & 5 deletions tools/generator/SourceWriters/BoundProperty.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,18 @@ public BoundProperty (GenBase gen, Property property, CodeGenerationOptions opt,
IsOverride = false;
}

// Unlike [Register], [Obsolete] cannot be put on property accessors, so we can apply them only under limited condition...
if (property.Getter.Deprecated != null && (property.Setter == null || property.Setter.Deprecated != null)) {
var message = property.Getter.Deprecated.Trim () + (property.Setter != null && property.Setter.Deprecated != property.Getter.Deprecated ? " " + property.Setter.Deprecated.Trim () : null);
var since = property.Getter?.DeprecatedSince ?? property.Setter?.DeprecatedSince;
SourceWriterExtensions.AddObsolete (Attributes, message, opt, deprecatedSince: since);
// Add [Obsolete] or [ObsoletedOSPlatform]
if (property.IsWholePropertyDeprecated) {
// This case applies [Obsolete] to the entire property
SourceWriterExtensions.AddObsolete (Attributes, property.Getter.Deprecated.Trim (), opt, deprecatedSince: property.Getter.DeprecatedSince);
} else {
// This case applies [Obsolete] to just the getter
if (property.Getter?.Deprecated != null)
SourceWriterExtensions.AddObsolete (GetterAttributes, property.Getter.Deprecated.Trim (), opt, deprecatedSince: property.Getter?.DeprecatedSince);

// This case applies [Obsolete] to just the setter
if (property.Setter?.Deprecated != null)
SourceWriterExtensions.AddObsolete (SetterAttributes, property.Setter.Deprecated.Trim (), opt, deprecatedSince: property.Setter?.DeprecatedSince);
}

SourceWriterExtensions.AddSupportedOSPlatform (Attributes, property.Getter, opt);
Expand Down