Skip to content

Commit 1a086ff

Browse files
jonpryorjonpryor
authored
[Java.Interop.Tools.JavaCallableWrappers] Fix IsValidIdentifier() (#610)
Context: dotnet/android#4431 Commit e7c5f54 added `IdentifierValidator.IsValidIdentifier()`, replacing use of `CSharpCodeProvider.IsValidIdentifier()`, and this change caused a unit test failure within xamarin-android: // Xamarin.Android.Build.Tests.BuildTest.GeneratorValidateEventName obj/Debug/generated/src/Com.Xamarin.Testing.Test.cs(350,29,350,32): error CS1001: Identifier expected The line in question? public event EventHandler 123 { Oops. It Turns Out™ that `IdentifierValidator.IsValidIdentifier("123")` returned True, so we attempted to create an event named `123`, which the C# compiler Does Not Like. Fix `IdentifierValidator.IsValidIdentifier()` by using a new `IsValidIdentifierRegex` regex to match against identifiers, instead of attempting to reuse the `validIdentifier` regex, which matches for *invalid* characters (it negates the match via `[^...]`), and thus cannot differentiate between "starting" characters and everything else, which is fine for `IdentifierValidator.CreateValidIdentifier()`, but not for `IdentifierValidator.IsValidIdentifier()`. Add unit tests for `IdentifierValidator.IsValidIdentifier()`. Update `make run-test-generator-core` tests so that we add a `View.setOn123Listener()` method, which prior to this commit would add a `View.123` event: partial class View { public event EventHandler 123 { add { … } remove { … } } } With a corrected `IdentifierValidator.IsValidIdentifier()`, this event is no longer generated.
1 parent f7ea4ed commit 1a086ff

File tree

5 files changed

+86
-1
lines changed

5 files changed

+86
-1
lines changed

‎src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/IdentifierValidator.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ public static class IdentifierValidator
2626

2727
private const string Identifier = IdentifierStartCharacter + "(" + IdentifierPartCharacter + ")";
2828

29+
static Regex IsValidIdentifierRegex = new Regex ($"^[{IdentifierStartCharacter}][{IdentifierPartCharacter}]*$", RegexOptions.Compiled);
30+
2931
// We use [^ ...] to detect any character that is NOT a match.
3032
static Regex validIdentifier = new Regex ($"[^{Identifier}]", RegexOptions.Compiled);
3133

@@ -43,7 +45,7 @@ public static string CreateValidIdentifier (string identifier, bool useEncodedRe
4345

4446
public static bool IsValidIdentifier (string identifier)
4547
{
46-
return !validIdentifier.IsMatch (identifier);
48+
return IsValidIdentifierRegex.IsMatch (identifier);
4749
}
4850

4951
// Makes uglier but unique identifiers by encoding each invalid character with its character value

‎tests/Java.Interop.Tools.JavaCallableWrappers-Tests/Java.Interop.Tools.JavaCallableWrappers/IdentifierValidatorTests.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,18 @@ public void CreateValidIdentifier_Encoded ()
2424
Assert.AreEqual ("my_x45_identifier_x36_test", IdentifierValidator.CreateValidIdentifier ("my-identifier$test", true));
2525
Assert.AreEqual ("myidentifier_x55357__x56842_test", IdentifierValidator.CreateValidIdentifier ("myidentifier😊test", true));
2626
}
27+
28+
[Test]
29+
public void IsValidIdentifier ()
30+
{
31+
Assert.IsTrue (IdentifierValidator.IsValidIdentifier ("name"));
32+
Assert.IsTrue (IdentifierValidator.IsValidIdentifier ("Name_With_Underscores"));
33+
34+
// Yes, this is "wrong" -- keywords aren't identifiers -- but the keyword check is done elsewhere.
35+
Assert.IsTrue (IdentifierValidator.IsValidIdentifier ("true"));
36+
37+
Assert.IsFalse (IdentifierValidator.IsValidIdentifier ("name-with-hyphens and spaces"));
38+
Assert.IsFalse (IdentifierValidator.IsValidIdentifier ("123"));
39+
}
2740
}
2841
}

‎tests/generator-Tests/Tests-Core/api.xml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
<parameter name="l" type="android.view.View.OnClickListener">
77
</parameter>
88
</method>
9+
<method abstract="false" deprecated="not deprecated" final="false" name="setOn123Listener" native="false" return="void" static="false" synchronized="false" visibility="public">
10+
<parameter name="l" type="android.view.View.OnClickListener">
11+
</parameter>
12+
</method>
913
<method abstract="false" deprecated="not deprecated" final="false" name="addTouchables" native="false" return="void" static="false" synchronized="false" visibility="public">
1014
<parameter name="views" type="java.util.ArrayList&lt;android.view.View&gt;">
1115
</parameter>

‎tests/generator-Tests/Tests-Core/expected.ji/Android.Views.View.cs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,36 @@ public virtual unsafe void SetOnClickListener (Android.Views.View.IOnClickListen
179179
}
180180
}
181181

182+
static Delegate cb_setOn123Listener_Landroid_view_View_OnClickListener_;
183+
#pragma warning disable 0169
184+
static Delegate GetSetOn123Listener_Landroid_view_View_OnClickListener_Handler ()
185+
{
186+
if (cb_setOn123Listener_Landroid_view_View_OnClickListener_ == null)
187+
cb_setOn123Listener_Landroid_view_View_OnClickListener_ = JNINativeWrapper.CreateDelegate ((Action<IntPtr, IntPtr, IntPtr>) n_SetOn123Listener_Landroid_view_View_OnClickListener_);
188+
return cb_setOn123Listener_Landroid_view_View_OnClickListener_;
189+
}
190+
191+
static void n_SetOn123Listener_Landroid_view_View_OnClickListener_ (IntPtr jnienv, IntPtr native__this, IntPtr native_l)
192+
{
193+
Android.Views.View __this = global::Java.Lang.Object.GetObject<Android.Views.View> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
194+
Android.Views.View.IOnClickListener l = (Android.Views.View.IOnClickListener)global::Java.Lang.Object.GetObject<Android.Views.View.IOnClickListener> (native_l, JniHandleOwnership.DoNotTransfer);
195+
__this.SetOn123Listener (l);
196+
}
197+
#pragma warning restore 0169
198+
199+
// Metadata.xml XPath method reference: path="/api/package[@name='android.view']/class[@name='View']/method[@name='setOn123Listener' and count(parameter)=1 and parameter[1][@type='android.view.View.OnClickListener']]"
200+
[Register ("setOn123Listener", "(Landroid/view/View$OnClickListener;)V", "GetSetOn123Listener_Landroid_view_View_OnClickListener_Handler")]
201+
public virtual unsafe void SetOn123Listener (Android.Views.View.IOnClickListener l)
202+
{
203+
const string __id = "setOn123Listener.(Landroid/view/View$OnClickListener;)V";
204+
try {
205+
JniArgumentValue* __args = stackalloc JniArgumentValue [1];
206+
__args [0] = new JniArgumentValue ((l == null) ? IntPtr.Zero : ((global::Java.Lang.Object) l).Handle);
207+
_members.InstanceMethods.InvokeVirtualVoidMethod (__id, this, __args);
208+
} finally {
209+
}
210+
}
211+
182212
static Delegate cb_addTouchables_Ljava_util_ArrayList_;
183213
#pragma warning disable 0169
184214
static Delegate GetAddTouchables_Ljava_util_ArrayList_Handler ()

‎tests/generator-Tests/Tests-Core/expected/Android.Views.View.cs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,42 @@ public virtual unsafe void SetOnClickListener (Android.Views.View.IOnClickListen
172172
}
173173
}
174174

175+
static Delegate cb_setOn123Listener_Landroid_view_View_OnClickListener_;
176+
#pragma warning disable 0169
177+
static Delegate GetSetOn123Listener_Landroid_view_View_OnClickListener_Handler ()
178+
{
179+
if (cb_setOn123Listener_Landroid_view_View_OnClickListener_ == null)
180+
cb_setOn123Listener_Landroid_view_View_OnClickListener_ = JNINativeWrapper.CreateDelegate ((Action<IntPtr, IntPtr, IntPtr>) n_SetOn123Listener_Landroid_view_View_OnClickListener_);
181+
return cb_setOn123Listener_Landroid_view_View_OnClickListener_;
182+
}
183+
184+
static void n_SetOn123Listener_Landroid_view_View_OnClickListener_ (IntPtr jnienv, IntPtr native__this, IntPtr native_l)
185+
{
186+
Android.Views.View __this = global::Java.Lang.Object.GetObject<Android.Views.View> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
187+
Android.Views.View.IOnClickListener l = (Android.Views.View.IOnClickListener)global::Java.Lang.Object.GetObject<Android.Views.View.IOnClickListener> (native_l, JniHandleOwnership.DoNotTransfer);
188+
__this.SetOn123Listener (l);
189+
}
190+
#pragma warning restore 0169
191+
192+
static IntPtr id_setOn123Listener_Landroid_view_View_OnClickListener_;
193+
// Metadata.xml XPath method reference: path="/api/package[@name='android.view']/class[@name='View']/method[@name='setOn123Listener' and count(parameter)=1 and parameter[1][@type='android.view.View.OnClickListener']]"
194+
[Register ("setOn123Listener", "(Landroid/view/View$OnClickListener;)V", "GetSetOn123Listener_Landroid_view_View_OnClickListener_Handler")]
195+
public virtual unsafe void SetOn123Listener (Android.Views.View.IOnClickListener l)
196+
{
197+
if (id_setOn123Listener_Landroid_view_View_OnClickListener_ == IntPtr.Zero)
198+
id_setOn123Listener_Landroid_view_View_OnClickListener_ = JNIEnv.GetMethodID (class_ref, "setOn123Listener", "(Landroid/view/View$OnClickListener;)V");
199+
try {
200+
JValue* __args = stackalloc JValue [1];
201+
__args [0] = new JValue (l);
202+
203+
if (((object) this).GetType () == ThresholdType)
204+
JNIEnv.CallVoidMethod (((global::Java.Lang.Object) this).Handle, id_setOn123Listener_Landroid_view_View_OnClickListener_, __args);
205+
else
206+
JNIEnv.CallNonvirtualVoidMethod (((global::Java.Lang.Object) this).Handle, ThresholdClass, JNIEnv.GetMethodID (ThresholdClass, "setOn123Listener", "(Landroid/view/View$OnClickListener;)V"), __args);
207+
} finally {
208+
}
209+
}
210+
175211
static Delegate cb_addTouchables_Ljava_util_ArrayList_;
176212
#pragma warning disable 0169
177213
static Delegate GetAddTouchables_Ljava_util_ArrayList_Handler ()

0 commit comments

Comments
 (0)