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

Allow overriding C# method modifiers #681

Closed
jonpryor opened this issue Jul 22, 2020 · 2 comments · Fixed by #701
Closed

Allow overriding C# method modifiers #681

jonpryor opened this issue Jul 22, 2020 · 2 comments · Fixed by #701
Assignees
Labels
enhancement Proposed change to current functionality generator Issues binding a Java library (generator, class-parse, etc.)

Comments

@jonpryor
Copy link
Member

A C# method modifier are the keywords which can be applied to a method, such as visibility (public, private, …).

Some of these modifiers can be overridden via metadata, e.g. setting //method/@visibility will be inserted as-is into the C# binding.

What is desired is a way to override the default logic for emitting virtual & override.

Sometimes when binding a library, the default logic causes virtual to be emitted when override is needed for the code to compile. (TODO: Insert concrete example here.)

The current workaround is to use metadata to rename the "offending" method and then use a partial class to properly override the method.

What we would like is e.g. a managedOverride attribute in metadata/on //method.

Question: what should the semantics be? Would a "boolean" attribute be desirable, or a string?

<!-- boolean? -->
<attr path="//method[@name='example']" name="managedOverride">true</attr>

<!-- string? -->
<attr path="//method[@name='example']" name="managedOverride">override</attr>
@jonpryor jonpryor added enhancement Proposed change to current functionality generator Issues binding a Java library (generator, class-parse, etc.) labels Jul 22, 2020
@jonpryor
Copy link
Member Author

Example of what we want to improve

https://github.com/xamarin/FacebookComponents/blob/70e0b91a24a247f4fe0d317d371441a3278a3623/Facebook.Android/source/facebook-core/transforms/Metadata.xml#L32

https://github.com/xamarin/FacebookComponents/blob/70e0b91a24a247f4fe0d317d371441a3278a3623/Facebook.Android/source/facebook-core/additions/additions.cs#L15

A common "source" of the problem tends to be Java generics, in which e.g. FileDownloadTask.doInBackground() has java.lang.String parameter types, which doesn't match the base class. Metadata could be used to "fix" the parameter types to be java.lang.Object, allowing the parameter types to match the base class, but generator wouldn't realize this.

This is the same scenario as: #586

@jpobst
Copy link
Contributor

jpobst commented Jul 22, 2020

It feels like this maybe needs to go both ways: changing virtual to override, or override to virtual. Given that, a string would work better:

<attr path="//method[@name='example']" name="managedOverride">override</attr>
<attr path="//method[@name='example']" name="managedOverride">virtual</attr>

jonpryor pushed a commit that referenced this issue Aug 28, 2020
Fixes: #681

There are scenarios where `generator` does not correctly find
overridden methods, often due to generic parameters.  When these issues
happen, the fix is to `<remove-node/>` the offending method and add a
fully hand-bound version of it as an `Addition`.

This is a very manual fix to simply add `override` to a method.

While we would like to eventually fix all these scenarios to "do the
right thing", that will be a complex fix, and perhaps we will never be
able to handle all cases.

For now, we can at least alleviate the pain by adding a `metadata`
attribute that allows the user to override our detection, avoiding the
hand-binding.

This new attribute is called `managedOverride` and allows the user to
specify the values `virtual` or `override`, forcing us to use the
requested modifier instead of `generator`'s usual logic.

	<attr path="//method[@name='example']" name="managedOverride">override</attr>
	<attr path="//method[@name='example']" name="managedOverride">virtual</attr>
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Proposed change to current functionality generator Issues binding a Java library (generator, class-parse, etc.)
Projects
None yet
2 participants