Skip to content

Conversation

@jonpryor
Copy link
Contributor

jonpryor added a commit to jonpryor/java.interop that referenced this pull request Mar 20, 2020
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 `CreateValidIdentifier()`, but not for
`IsValidIdentifier()`.

Add unit tests for `IsValidIdentifier()`.
jonpryor added a commit to jonpryor/java.interop that referenced this pull request Mar 20, 2020
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.
jonpryor added a commit to jonpryor/java.interop that referenced this pull request Mar 20, 2020
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.
jpobst pushed a commit to dotnet/java-interop that referenced this pull request Mar 20, 2020
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.
Changes: dotnet/java-interop@cedf4d0...1a086ff

  * dotnet/java-interop@1a086ff: [Java.Interop.Tools.JavaCallableWrappers] Fix IsValidIdentifier() (dotnet#610)
  * dotnet/java-interop@f7ea4ed: [Xamarin.Android.Tools.Bytecode-Tests] Remove obsolete files (dotnet#603)
  * dotnet/java-interop@e070ce3: [build] Remove <Imports/> now covered by Directory.Build.props (dotnet#607)
  * dotnet/java-interop@e7c5f54: [generator] Add netcoreapp3.1 support (dotnet#606)
  * dotnet/java-interop@95f698b: [build] Build additional `$(TargetFramework)` values. (dotnet#605)
@jonpryor jonpryor force-pushed the jonp-bump-ji-f7ea4ed branch from 3955ac2 to b6f2d04 Compare March 20, 2020 18:52
@jonpryor jonpryor merged commit 5cc0552 into dotnet:master Mar 21, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants