[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.

Author: jonpryor

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

@@ -26,6 +26,8 @@ public static class IdentifierValidator
 
 privateconststringIdentifier=IdentifierStartCharacter+"("+IdentifierPartCharacter+")";
 
+staticRegexIsValidIdentifierRegex=newRegex($"^[{IdentifierStartCharacter}][{IdentifierPartCharacter}]*$",RegexOptions.Compiled);
+
 // We use [^ ...] to detect any character that is NOT a match.
 staticRegexvalidIdentifier=newRegex($"[^{Identifier}]",RegexOptions.Compiled);
 
@@ -43,7 +45,7 @@ public static string CreateValidIdentifier (string identifier, bool useEncodedRe
 
 publicstaticboolIsValidIdentifier(stringidentifier)
 {
-return!validIdentifier.IsMatch(identifier);
+returnIsValidIdentifierRegex.IsMatch(identifier);
 }
 
 // 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

@@ -24,5 +24,18 @@ public void CreateValidIdentifier_Encoded ()
 Assert.AreEqual("my_x45_identifier_x36_test",IdentifierValidator.CreateValidIdentifier("my-identifier$test",true));
 Assert.AreEqual("myidentifier_x55357__x56842_test",IdentifierValidator.CreateValidIdentifier("myidentifier😊test",true));
 }
+
+[Test]
+publicvoidIsValidIdentifier()
+{
+Assert.IsTrue(IdentifierValidator.IsValidIdentifier("name"));
+Assert.IsTrue(IdentifierValidator.IsValidIdentifier("Name_With_Underscores"));
+
+// Yes, this is "wrong" -- keywords aren't identifiers -- but the keyword check is done elsewhere.
+Assert.IsTrue(IdentifierValidator.IsValidIdentifier("true"));
+
+Assert.IsFalse(IdentifierValidator.IsValidIdentifier("name-with-hyphens and spaces"));
+Assert.IsFalse(IdentifierValidator.IsValidIdentifier("123"));
+}
 }
 }

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

@@ -6,6 +6,10 @@
  <parametername="l"type="android.view.View.OnClickListener">
  </parameter>
  </method>
+ <methodabstract="false"deprecated="not deprecated"final="false"name="setOn123Listener"native="false"return="void"static="false"synchronized="false"visibility="public">
+ <parametername="l"type="android.view.View.OnClickListener">
+ </parameter>
+ </method>
  <methodabstract="false"deprecated="not deprecated"final="false"name="addTouchables"native="false"return="void"static="false"synchronized="false"visibility="public">
  <parametername="views"type="java.util.ArrayList&lt;android.view.View&gt;">
  </parameter>

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

@@ -179,6 +179,36 @@ public virtual unsafe void SetOnClickListener (Android.Views.View.IOnClickListen
 }
 }
 
+staticDelegatecb_setOn123Listener_Landroid_view_View_OnClickListener_;
+#pragma warning disable 0169
+staticDelegateGetSetOn123Listener_Landroid_view_View_OnClickListener_Handler()
+{
+if(cb_setOn123Listener_Landroid_view_View_OnClickListener_==null)
+cb_setOn123Listener_Landroid_view_View_OnClickListener_=JNINativeWrapper.CreateDelegate((Action<IntPtr,IntPtr,IntPtr>)n_SetOn123Listener_Landroid_view_View_OnClickListener_);
+returncb_setOn123Listener_Landroid_view_View_OnClickListener_;
+}
+
+staticvoidn_SetOn123Listener_Landroid_view_View_OnClickListener_(IntPtrjnienv,IntPtrnative__this,IntPtrnative_l)
+{
+Android.Views.View__this=global::Java.Lang.Object.GetObject<Android.Views.View>(jnienv,native__this,JniHandleOwnership.DoNotTransfer);
+Android.Views.View.IOnClickListenerl=(Android.Views.View.IOnClickListener)global::Java.Lang.Object.GetObject<Android.Views.View.IOnClickListener>(native_l,JniHandleOwnership.DoNotTransfer);
+__this.SetOn123Listener(l);
+}
+#pragma warning restore 0169
+
+// 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']]"
+[Register("setOn123Listener","(Landroid/view/View$OnClickListener;)V","GetSetOn123Listener_Landroid_view_View_OnClickListener_Handler")]
+publicvirtualunsafevoidSetOn123Listener(Android.Views.View.IOnClickListenerl)
+{
+conststring__id="setOn123Listener.(Landroid/view/View$OnClickListener;)V";
+try{
+JniArgumentValue*__args=stackallocJniArgumentValue[1];
+__args[0]=newJniArgumentValue((l==null)?IntPtr.Zero:((global::Java.Lang.Object)l).Handle);
+_members.InstanceMethods.InvokeVirtualVoidMethod(__id,this,__args);
+}finally{
+}
+}
+
 staticDelegatecb_addTouchables_Ljava_util_ArrayList_;
 #pragma warning disable 0169
 staticDelegateGetAddTouchables_Ljava_util_ArrayList_Handler()

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

@@ -172,6 +172,42 @@ public virtual unsafe void SetOnClickListener (Android.Views.View.IOnClickListen
 }
 }
 
+staticDelegatecb_setOn123Listener_Landroid_view_View_OnClickListener_;
+#pragma warning disable 0169
+staticDelegateGetSetOn123Listener_Landroid_view_View_OnClickListener_Handler()
+{
+if(cb_setOn123Listener_Landroid_view_View_OnClickListener_==null)
+cb_setOn123Listener_Landroid_view_View_OnClickListener_=JNINativeWrapper.CreateDelegate((Action<IntPtr,IntPtr,IntPtr>)n_SetOn123Listener_Landroid_view_View_OnClickListener_);
+returncb_setOn123Listener_Landroid_view_View_OnClickListener_;
+}
+
+staticvoidn_SetOn123Listener_Landroid_view_View_OnClickListener_(IntPtrjnienv,IntPtrnative__this,IntPtrnative_l)
+{
+Android.Views.View__this=global::Java.Lang.Object.GetObject<Android.Views.View>(jnienv,native__this,JniHandleOwnership.DoNotTransfer);
+Android.Views.View.IOnClickListenerl=(Android.Views.View.IOnClickListener)global::Java.Lang.Object.GetObject<Android.Views.View.IOnClickListener>(native_l,JniHandleOwnership.DoNotTransfer);
+__this.SetOn123Listener(l);
+}
+#pragma warning restore 0169
+
+staticIntPtrid_setOn123Listener_Landroid_view_View_OnClickListener_;
+// 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']]"
+[Register("setOn123Listener","(Landroid/view/View$OnClickListener;)V","GetSetOn123Listener_Landroid_view_View_OnClickListener_Handler")]
+publicvirtualunsafevoidSetOn123Listener(Android.Views.View.IOnClickListenerl)
+{
+if(id_setOn123Listener_Landroid_view_View_OnClickListener_==IntPtr.Zero)
+id_setOn123Listener_Landroid_view_View_OnClickListener_=JNIEnv.GetMethodID(class_ref,"setOn123Listener","(Landroid/view/View$OnClickListener;)V");
+try{
+JValue*__args=stackallocJValue[1];
+__args[0]=newJValue(l);
+
+if(((object)this).GetType()==ThresholdType)
+JNIEnv.CallVoidMethod(((global::Java.Lang.Object)this).Handle,id_setOn123Listener_Landroid_view_View_OnClickListener_,__args);
+else
+JNIEnv.CallNonvirtualVoidMethod(((global::Java.Lang.Object)this).Handle,ThresholdClass,JNIEnv.GetMethodID(ThresholdClass,"setOn123Listener","(Landroid/view/View$OnClickListener;)V"),__args);
+}finally{
+}
+}
+
 staticDelegatecb_addTouchables_Ljava_util_ArrayList_;
 #pragma warning disable 0169
 staticDelegateGetAddTouchables_Ljava_util_ArrayList_Handler()