Skip to content
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.

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

[generator] Allow a constant map to explicitly set deprecated-since for an enum value. #1070

Merged
merged 4 commits into from
Jan 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 7 additions & 3 deletions Documentation/EnumMappingFile.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ There is now a "v2" version of defining constants. This is a line-level change
and you can mix "v1" and "v2" lines in the same file for backwards compatibility,
but for consistency it's probably better to stick to one style.

A "v2" line contains up to 8 fields:
A "v2" line contains up to 9 fields:

* **Action** - The action to perform. This is what denotes a "v2" line, if the first
character is not one of the following it will be treated as "v1".
Expand All @@ -89,10 +89,14 @@ A "v2" line contains up to 8 fields:
* `keep` - Keeps the Java constant
* **Flags** - If this field contains `flags` the enum will be created with the
`[Flags]` attribute. (Any member will `flags` will make the whole enum `[Flags]`.)

* **Deprecated Since** - This is generally only used by `Mono.Android.dll` to denote
the Android level the constant was deprecated in. Specifying "-1" will add an obsolete
message to the effect of: "This value was incorrectly added to the enumeration and is
not a valid value". Leave blank if constant is not deprecated.

Full example:
```
E,10,android/view/Window.PROGRESS_START,0,Android.Views.WindowProgress,Start,remove,flags
E,10,android/view/Window.PROGRESS_START,0,Android.Views.WindowProgress,Start,remove,flags,30
```

[0]: https://docs.microsoft.com/en-us/xamarin/android/platform/binding-java-library/customizing-bindings/java-bindings-metadata#enumfieldsxml-and-enummethodsxml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public class ConstantEntry
public string? EnumMember { get; set; }
public FieldAction FieldAction { get; set; }
public bool IsFlags { get; set; }
public int? DeprecatedSince { get; set; }

public string EnumNamespace {
get {
Expand Down Expand Up @@ -133,7 +134,8 @@ static ConstantEntry FromVersion2String (CsvParser parser)
EnumFullType = parser.GetField (4),
EnumMember = parser.GetField (5),
FieldAction = FromFieldActionString (parser.GetField (6)),
IsFlags = parser.GetField (7).ToLowerInvariant () == "flags"
IsFlags = parser.GetField (7).ToLowerInvariant () == "flags",
DeprecatedSince = parser.GetFieldAsNullableInt32 (8)
};

entry.NormalizeJavaSignature ();
Expand Down Expand Up @@ -175,7 +177,8 @@ public string ToVersion2String ()
EnumFullType,
EnumMember,
ToConstantFieldActionString (FieldAction),
IsFlags ? "flags" : string.Empty
IsFlags ? "flags" : string.Empty,
DeprecatedSince.HasValue ? DeprecatedSince.Value.ToString () : string.Empty,
};

return string.Join (",", fields);
Expand Down
14 changes: 14 additions & 0 deletions src/Java.Interop.Tools.Generator/Enumification/ConstantsParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,20 @@ public static void SaveEnumMapCsv (List<ConstantEntry> constants, string filenam

public static void SaveEnumMapCsv (List<ConstantEntry> constants, TextWriter writer)
{
var column_names = new [] {
"Action",
"API Level",
"JNI Signature",
"Enum Value",
"C# Enum Type",
"C# Member Name",
"Field Action",
"Is Flags",
"Deprecated Since",
};

writer.WriteLine ("// " + string.Join (",", column_names));

foreach (var c in Sort (constants))
writer.WriteLine (c.ToVersion2String ());
}
Expand Down
10 changes: 10 additions & 0 deletions src/Java.Interop.Tools.Generator/Utilities/CsvParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,15 @@ public int GetFieldAsInt (int index)
{
return int.Parse (GetField (index));
}

public int? GetFieldAsNullableInt32 (int index)
{
var value = GetField (index);

if (int.TryParse (value, out var val))
return val;

return default;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ public void ParseEnumMapV2 ()
Assert.AreEqual ("Cdsect", entry.EnumMember);
Assert.AreEqual (FieldAction.Keep, entry.FieldAction);
Assert.False (entry.IsFlags);
Assert.IsNull (entry.DeprecatedSince);
}

[Test]
Expand All @@ -99,13 +100,14 @@ public void ParseAddEnumMapV2 ()
Assert.AreEqual ("Org.XmlPull.V1.XmlPullParserNode", entry.EnumFullType);
Assert.AreEqual ("Cdsect", entry.EnumMember);
Assert.AreEqual (FieldAction.None, entry.FieldAction);
Assert.IsNull (entry.DeprecatedSince);
Assert.False (entry.IsFlags);
}

[Test]
public void ParseRemoveEnumMapV2 ()
{
var csv = "R,10,I:org/xmlpull/v1/XmlPullParser.CDSECT,5,,,remove";
var csv = "R,10,I:org/xmlpull/v1/XmlPullParser.CDSECT,5,,,remove,,33";
var entry = ConstantEntry.FromString (csv);

Assert.AreEqual (ConstantAction.Remove, entry.Action);
Expand All @@ -116,6 +118,7 @@ public void ParseRemoveEnumMapV2 ()
Assert.AreEqual (string.Empty, entry.EnumMember);
Assert.AreEqual (FieldAction.Remove, entry.FieldAction);
Assert.False (entry.IsFlags);
Assert.AreEqual (33, entry.DeprecatedSince.Value);
}
}
}
54 changes: 54 additions & 0 deletions tests/generator-Tests/Unit-Tests/EnumGeneratorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,60 @@ public void ObsoleteFieldButNotEnumAttributeSupport ()
Assert.False (writer.ToString ().NormalizeLineEndings ().Contains ("[global::System.Obsolete(@\"deprecated\")]WithExcluded=1"), writer.ToString ());
}

[Test]
public void ObsoletedOSPlatformAttributeOverrideSupport ()
{
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='android.app' jni-name='android/app'>
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='ActivityManager' static='false' visibility='public' jni-signature='Landroid/app/ActivityManager;'>
<field deprecated='not deprecated' final='true' name='RECENT_IGNORE_UNAVAILABLE' jni-signature='Ljava/lang/String;' static='true' transient='false' type='java.lang.String' type-generic-aware='java.lang.String' value='&quot;android.permission.BIND_CHOOSER_TARGET_SERVICE&quot;' visibility='public' volatile='false' api-since='30' />
</class>
</package>
</api>";

options.UseObsoletedOSPlatformAttributes = true;

var enu = CreateEnum ();
enu.Value.Members.Single (m => m.EnumMember == "WithExcluded").DeprecatedSince = 33;

var gens = ParseApiDefinition (xml);

generator.WriteEnumeration (options, enu, gens.ToArray ());

// The field itself is not deprecated, but [ObsoletedOSPlatform] should be written because we set `DeprecatedSince` on the enum map
Assert.True (writer.ToString ().NormalizeLineEndings ().Contains ("[global::System.Runtime.Versioning.ObsoletedOSPlatform(\"android33.0\")]WithExcluded=1"), writer.ToString ());
}

[Test]
public void ObsoleteAccidentalAddition ()
{
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='android.app' jni-name='android/app'>
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='ActivityManager' static='false' visibility='public' jni-signature='Landroid/app/ActivityManager;'>
<field deprecated='not deprecated' final='true' name='RECENT_IGNORE_UNAVAILABLE' jni-signature='Ljava/lang/String;' static='true' transient='false' type='java.lang.String' type-generic-aware='java.lang.String' value='&quot;android.permission.BIND_CHOOSER_TARGET_SERVICE&quot;' visibility='public' volatile='false' api-since='30' />
</class>
</package>
</api>";

options.UseObsoletedOSPlatformAttributes = true;

var enu = CreateEnum ();
enu.Value.Members.Single (m => m.EnumMember == "WithExcluded").DeprecatedSince = -1;

var gens = ParseApiDefinition (xml);

generator.WriteEnumeration (options, enu, gens.ToArray ());

// "-1" is a "magic" API level which adds a message indicating the enum value shouldn't even exist
Assert.True (writer.ToString ().NormalizeLineEndings ().Contains ("[global::System.Obsolete(@\"Thisvaluewasincorrectlyaddedtotheenumerationandisnotavalidvalue\")]WithExcluded=1"), writer.ToString ());
}

protected new string GetExpected (string testName)
{
var root = Path.GetDirectoryName (Assembly.GetExecutingAssembly ().Location);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,16 @@ EnumWriter CreateWriter (CodeGenerationOptions opt, KeyValuePair<string, EnumDes

SourceWriterExtensions.AddSupportedOSPlatform (m.Attributes, member.ApiLevel, opt);

// Some of our source fields may have been marked with:
// "This constant will be removed in the future version. Use XXX enum directly instead of this field."
// We don't want this message to propogate to the enum.
if (managedMember != null && managedMember.Value.Field?.DeprecatedComment?.Contains ("enum directly instead of this field") == false)
if (member.DeprecatedSince.HasValue) {
// If we've manually marked it as obsolete in map.csv, that takes precedence
var msg = member.DeprecatedSince.Value == -1 ? "This value was incorrectly added to the enumeration and is not a valid value" : "deprecated";
SourceWriterExtensions.AddObsolete (m.Attributes, msg, opt, deprecatedSince: member.DeprecatedSince);
} else if (managedMember != null && managedMember.Value.Field?.DeprecatedComment?.Contains ("enum directly instead of this field") == false) {
// Some of our source fields may have been marked with:
// "This constant will be removed in the future version. Use XXX enum directly instead of this field."
// We don't want this message to propogate to the enum.
SourceWriterExtensions.AddObsolete (m.Attributes, managedMember.Value.Field.DeprecatedComment, opt, deprecatedSince: managedMember.Value.Field.DeprecatedSince);
}

enoom.Members.Add (m);
}
Expand Down