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] Fix exception caused by incorrect nested type name. #1267

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Oct 18, 2024

Context: #1266

While the issue described in #1266 is definitely an issue (types should not be added to the generator symbol table with a JNI type specification), that example was merely setting up the scenario needed to expose another bug.

When a JLO type has a public or internal field whose type is a nested type:

namespace Com.Mypackage;

[Register ("com/mypackage/FieldClass")]
public class FieldClass : Java.Lang.Object
{
	public NestedFieldClass field;

	public class NestedFieldClass : Java.Lang.Object { }
}

We correctly import the Field.TypeName as Com.Mypackage.FieldClass.NestedFieldClass, however we also create Field.SetterParameter with the same type, but we do not replace the / nested type separator with a period, resulting in Com.Mypackage.FieldClass/NestedFieldClass.

Later, when validating the Field, we successfully find Field.TypeName in the symbol table, but fail to find Field.SetterParameter as the symbol table expects a period nested type separator. (Note this only happens because NestedFieldClass does not have a [Register] attribute, thus it gets added to the symbol table with its managed name rather than its Java name.)

This causes generator to crash with:

System.NotSupportedException: Unable to generate setter parameter list in method Bar in managed type Com.Example.Foo.

Fix this by applying the same FullNameCorrected () logic that is applied to Field.TypeName.

@jpobst jpobst marked this pull request as ready for review October 18, 2024 21:02
@jpobst jpobst requested a review from jonpryor October 18, 2024 21:02
@jonpryor
Copy link
Member

I think that there is a second related issue here, though I'm not certain how much of an issue there is. Consider the original example:

	[Register ("com/mypackage/FieldClass")]
	public class FieldClass : Java.Lang.Object {
	    public NestedFieldClass field;

	    public class NestedFieldClass : Java.Lang.Object {
	    }
	}

The Java name of FieldClass.NestedFieldClass would be com.mypackage.FieldClass.NestedFieldClass. What actually winds up in the symbol table is the managed name, Com.Mypackage.FieldClass.NestedFieldClass. This doesn't seem right, but I'm not sure if it's a problem either.

@jonpryor
Copy link
Member

Context: https://github.com/dotnet/java-interop/pull/1266

While the issue described in dotnet/java-interop#1266 is definitely
an issue, that example was merely setting up the scenario needed to
expose another bug:

When a `Java.Lang.Object` or `Java.Interop.JavaObject` subclass
has a `public` or `internal` field whose type is a nested type:

	// C#
	namespace Com.Mypackage;

	[Register ("com/mypackage/FieldClass")]
	public class FieldClass : Java.Lang.Object {
	    public NestedFieldClass field;

	    public class NestedFieldClass : Java.Lang.Object {
	    }
	}

We correctly import the `Field.TypeName` as
`Com.Mypackage.FieldClass.NestedFieldClass`, however we also create
`Field.SetterParameter` with the "same" type, but we do not replace
the `/` nested type separator with a period, resulting in
`Com.Mypackage.FieldClass/NestedFieldClass`.

Later, when validating the `Field`, we successfully find
`Field.TypeName` in the symbol table, but fail to find
`Field.SetterParameter` as the symbol table expects a period nested
type separator.  (Note this only happens because `NestedFieldClass`
does not have a `[Register]` attribute, thus it gets added to the
symbol table with its managed name rather than its Java name.)

This causes `generator` to crash with:

	System.NotSupportedException: Unable to generate setter parameter list in managed type Com.Mypackage.FieldClass
	   at MonoDroid.Generation.Field.Validate(CodeGenerationOptions opt, GenericParameterDefinitionList type_params, CodeGeneratorContext context)
	   at MonoDroid.Generation.GenBase.OnValidate(CodeGenerationOptions opt, GenericParameterDefinitionList type_params, CodeGeneratorContext context)
	   at MonoDroid.Generation.ClassGen.OnValidate(CodeGenerationOptions opt, GenericParameterDefinitionList type_params, CodeGeneratorContext context)
	   at MonoDroid.Generation.GenBase.Validate(CodeGenerationOptions opt, GenericParameterDefinitionList type_params, CodeGeneratorContext context)

Fix this by applying the same `FullNameCorrected()` logic that is
applied to `Field.TypeName`.

@jonpryor jonpryor merged commit 56cfab9 into main Oct 21, 2024
4 checks passed
@jonpryor jonpryor deleted the field-fullname-type branch October 21, 2024 19:42
jonpryor added a commit to dotnet/android that referenced this pull request Oct 22, 2024
Changes: dotnet/java-interop@9d99723...2a1e180

  * dotnet/java-interop@2a1e1800: [generator] Fix StackOverflow when copying DIM via private interfaces (dotnet/java-interop#1261)
  * dotnet/java-interop@f863351e: [generator] Only use `[JniTypeSignatureAttribute]` for `JavaInterop1` (dotnet/java-interop#1266)
  * dotnet/java-interop@56cfab93: [generator] Fix exception caused by incorrect nested type name. (dotnet/java-interop#1267)
  * dotnet/java-interop@b656f7f5: [generator] Add support for `skipInterfaceMethods` (dotnet/java-interop#1265)
jonpryor added a commit to dotnet/android that referenced this pull request Oct 22, 2024
Changes: dotnet/java-interop@9d99723...2a1e180

  * dotnet/java-interop@2a1e1800: [generator] Fix StackOverflow when copying DIM via private interfaces (dotnet/java-interop#1261)
  * dotnet/java-interop@f863351e: [generator] Only use `[JniTypeSignatureAttribute]` for `JavaInterop1` (dotnet/java-interop#1266)
  * dotnet/java-interop@56cfab93: [generator] Fix exception caused by incorrect nested type name. (dotnet/java-interop#1267)
  * dotnet/java-interop@b656f7f5: [generator] Add support for `skipInterfaceMethods` (dotnet/java-interop#1265)
@github-actions github-actions bot locked and limited conversation to collaborators Nov 21, 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