Skip to content

Commit 846f211

Browse files
authored
[generator] Remove buggy annotation loose match (#886)
Fixes: #885 Context: 1e8f513 Context: 3f12cd2 Attempting to build the xamarin/xamarin-android repo with a xamarin/Java.Interop between commits [1e8f513, 3f12cd2), exclusive -- as commit 3f12cd2 partially reverted commit 1e8f513 -- would result in a build failure during `make framework-assemblies`: # in xamarin-android… % make prepare && make all && make framework-assemblies … …/src/Mono.Android/Mono.Android.targets(247,5): error : CompatApi command: $HOME/.nuget/packages/microsoft.dotnet.apicompat/5.0.0-beta.20181.7/tools/net472/Microsoft.DotNet.ApiCompat.exe "…/bin/Debug/lib/xamarin.android/xbuild-frameworks/MonoAndroid/v4.4.87/Mono.Android.dll" -i "…/bin/Debug/lib/xamarin.android/xbuild-frameworks/MonoAndroid/v5.0" --allow-default-interface-methods --baseline "…/tests/api-compatibility/acceptable-breakages-v5.0.txt" --validate-baseline --exclude-attributes "…/tests/api-compatibility/api-compat-exclude-attributes.txt" …/src/Mono.Android/Mono.Android.targets(247,5): error : CheckApiCompatibility found nonacceptable Api breakages for ApiLevel: v5.0. …/src/Mono.Android/Mono.Android.targets(247,5): error : Compat issues with assembly Mono.Android: …/src/Mono.Android/Mono.Android.targets(247,5): error : CannotChangeAttribute : Attribute 'Android.Runtime.RequiresPermissionAttribute' on 'Android.Net.ConnectivityManager.GetNetworkInfo(Android.Net.ConnectivityType)' changed from '[RequiresPermissionAttribute("quot;android.permission.ACCESS_NETWORK_STATE&quot")]' in the contract to '[RequiresPermissionAttribute("quot;android.permission.ACCESS_NETWORK_STATE&quot")]' in the implementation. …/src/Mono.Android/Mono.Android.targets(247,5): error : CannotChangeAttribute : Attribute 'Android.Runtime.RequiresPermissionAttribute' on 'Android.OS.Vibrator.Vibrate(System.Int64[], System.Int32)' changed from '[RequiresPermissionAttribute("quot;android.permission.VIBRATE&quot")]' in the contract to '[RequiresPermissionAttribute("quot;android.permission.VIBRATE&quot")]' in the implementation. …/src/Mono.Android/Mono.Android.targets(247,5): error : Total Issues: 2 The cause of the error was twofold: 1. The `annotations.zip` within `platform-tools/api/annotations.zip` for the Android SDK Platform-tools r30 package contained annotations in the `androidx.annotation` package, which commit 1e8f513 added support for, and 2. The [method finding code][0] responsible for associating the method declared within `annotations.zip` to the `IMethodBase` which was responsible for C# codegen was "too loose", and could add annotations to methods which shouldn't get them. Consider this XML snippet from `android/os/annotations.xml` within `annotations.zip`, for two overloads of `Vibrator.vibrate()`: <item name="android.os.Vibrator void vibrate(android.os.VibrationEffect)"> <annotation name="androidx.annotation.RequiresPermission"> <val name="value" val="&quot;android.permission.VIBRATE&quot;" /> </annotation> </item> <item name="android.os.Vibrator void vibrate(long)"> <annotation name="androidx.annotation.RequiresPermission"> <val name="value" val="&quot;android.permission.VIBRATE&quot;" /> </annotation> </item> which corresponds to the C# bindings: partial class Vibrator { public virtual void Vibrate (Android.OS.VibrationEffect vibe); public virtual void Vibrate (long milliseconds); } However, the `Vibrator.Vibrate(VibrationEffect)` overload wasn't added until API-26. When binding API-21, only `Vibrator.Vibrate(long)` exists. The [method finding code][0] only checks for *number* of parameters, *not* parameter type, and thus would add the annotation for `vibrate(VibrationEffect)` to `vibrate(long)`, resulting in the generated C# binding: namespace Android.OS { public abstract partial class Vibrator : Java.Lang.Object { [global::Android.Runtime.RequiresPermission ("quot;android.permission.VIBRATE&quot")] [global::Android.Runtime.RequiresPermission ("quot;android.permission.VIBRATE&quot")] public abstract void Vibrate (long milliseconds); } } i.e. `Vibrator.Vibrate(long)` had *two* `[RequiresPermission]` attributes, *both* with the same value. Fix the bug by updating `ManagedTypeFinderGeneratorTypeSystem.GetMethod()` to try only a strict method overload match. Finally, "revert the partial revert" of 3f12cd2, and reintroduce support for the `androidx.annotation` package for Annotations. [0]: https://github.com/xamarin/java.interop/blob/c936d09aec8171fa99a37fd99dba253e41fec05d/tools/generator/ManagedTypeFinderGeneratorTypeSystem.cs#L113
1 parent c936d09 commit 846f211

File tree

2 files changed

+10
-23
lines changed

2 files changed

+10
-23
lines changed

src/Xamarin.Android.Tools.AnnotationSupport/Objects/AnnotationData.cs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,21 @@ namespace Xamarin.AndroidTools.AnnotationSupport
77
{
88
public class AnnotationData : AnnotationObject
99
{
10+
static readonly string[] Prefixes = new[] {
11+
"android.support.annotation.",
12+
"androidx.annotation.",
13+
};
14+
1015
public AnnotationData (XElement e)
1116
{
1217
var a = e.Attribute ("name");
1318
Name = a == null ? null : a.Value;
14-
string predef = "android.support.annotation.";
15-
if (Name.StartsWith (predef, StringComparison.Ordinal))
19+
foreach (var predef in Prefixes) {
20+
if (!Name.StartsWith (predef, StringComparison.Ordinal))
21+
continue;
1622
Name = Name.Substring (predef.Length);
23+
break;
24+
}
1725
Values = e.Elements ("val").Select (c => new AnnotationValue (c)).ToArray ();
1826
}
1927

tools/generator/ManagedTypeFinderGeneratorTypeSystem.cs

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -119,27 +119,6 @@ public override IMethodBase GetMethod (IType iType, AnnotatedItem item)
119119
m.Parameters.Count == item.Arguments.Length)
120120
.ToArray ();
121121

122-
// First, try loose match.
123-
124-
MethodBase candidate = null;
125-
bool overloaded = false;
126-
foreach (var m in methods) {
127-
if (overloaded)
128-
break;
129-
if (candidate != null) {
130-
overloaded = true;
131-
break;
132-
} else
133-
candidate = m;
134-
}
135-
if (!overloaded) {
136-
if (candidate == null)
137-
Errors.Add ("warning: method with matching argument count not found: " + t.FullName + " member: " + item.FormatMember ());
138-
return candidate.Wrap ();
139-
}
140-
141-
// Second, try strict match.
142-
143122
var argTypeLists = methods.Select (m => new { Method = m, Jni = m is Ctor ? ((Ctor) m).JniSignature : ((Method) m).JniSignature})
144123
.Select (p => new { Method = p.Method, Arguments = p.Jni == null ? null : ParseJniMethodArgumentsSignature (p.Jni)})
145124
.ToArray ();

0 commit comments

Comments
 (0)