Bug 43579 - Generator emits invalid code when using the same method name (overload) in @delegates using events and C# delegates
Summary: Generator emits invalid code when using the same method name (overload) in @d...
Alias: None
Product: iOS
Classification: Xamarin
Component: General ()
Version: XI 9.99 (iOS 10 previews)
Hardware: PC Mac OS
: --- normal
Target Milestone: (C9)
Assignee: Alex Soto [MSFT]
Depends on:
Reported: 2016-08-22 02:04 UTC by Alex Soto [MSFT]
Modified: 2016-11-07 21:01 UTC (History)
6 users (show)

Is this bug a regression?: ---
Last known good build:

Test Case (5.28 KB, application/zip)
2016-08-22 02:04 UTC, Alex Soto [MSFT]
Fixed Test Case (8.21 KB, application/zip)
2016-09-15 19:21 UTC, Alex Soto [MSFT]

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.

Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:

Description Alex Soto [MSFT] 2016-08-22 02:04:01 UTC
Created attachment 17152 [details]
Test Case

Generator will emit invalid code when using the same name in a method inside a @delegate (protocol) that is decorated either with EventArgs or DelegateName like in the following example 

> interface IFooDelegate { }
> [BaseType (typeof (NSObject))]
> [Model]
> [Protocol]
> interface FooDelegate {
> 	// Events
> 	[Export ("FooDidEndEditing:"), EventArgs ("FooDelegateEnded"), EventName ("Ended")]
> 	void EditingEnded (Foo foo);
> 	[Export ("FooDidEndEditing:after:"), EventArgs ("FooDelegateEndedTime"), EventName ("Ended2")]
> 	void EditingEnded (Foo foo, double time);
> 	// Using Func
> 	[Export ("foo:dobar:"), DelegateName ("Func<Foo,nint,bool>"), DefaultValue ("true")]
> 	bool DoBar (Foo foo, nint bar);
> 	[Export ("foo:doBar:after:"), DelegateName ("Func<Foo,nint,double,bool>"), DefaultValue ("true")]
> 	bool DoBar (Foo foo, nint bar, double time);
> 	// Using an actual DelegateName
> 	[Export ("foo:doAnotherBar:"), DelegateName ("FooDelegateAnotherBar"), DefaultValue ("true")]
> 	bool DoAnotherBar (Foo foo, nint bar);
> 	[Export ("foo:doAnotherBar:after:"), DelegateName ("FooDelegateAnotherBarTime"), DefaultValue ("true")]
> 	bool DoAnotherBar (Foo foo, nint bar, double time);
> }
> [BaseType (typeof (NSObject), Delegates = new string [] { "Delegate" }, Events = new Type [] { typeof (FooDelegate) })]
> interface Foo {
> 	[Export ("delegate", ArgumentSemantic.Assign), NullAllowed]
> 	IFooDelegate Delegate { get; set; }
> }

attached test case
Comment 1 Sebastien Pouliot 2016-09-02 13:25:00 UTC
I have a fix for the first errors (duplicated internal field names) but the next part of this problem cannot totally be fixed - the case where [EventName] is used do work :)


	[Export ("foo:dobar:"), DelegateName ("Func<Foo,nint,bool>"), DefaultValue ("true")]
	bool DoBar (Foo foo, nint bar);

	[Export ("foo:doBar:after:"), DelegateName ("Func<Foo,nint,double,bool>"), DefaultValue ("true")]
	bool DoBar (Foo foo, nint bar, double time);

as it would generate two identically named events

	public Func<Foo,nint,bool> DoBar {
	public Func<Foo,nint,double,bool> DoBar {

which is not allowed by C# (only the return type is different). 

If you have to change the name then it's not an issue anymore (for the generator).

@Alex what was the original case (in the bindings) ? I want to see if we can avoid the `2` suffix.
Comment 3 Alex Soto [MSFT] 2016-09-07 03:53:20 UTC
Sebastien's patch: https://gist.github.com/spouliot/a5477c4cf19b33bf3682216e3ea30316

Once we fix this bug we need to go to uikit.cs and fix the following members

- UITextFieldDelegate.EditingEnded2 (UITextField textField, UITextFieldDidEndEditingReason reason)

- UITextViewDelegate.ShouldInteractWithUrl2 (UITextView textView, NSUrl url, NSRange characterRange, UITextItemInteraction interaction)

- UITextViewDelegate.ShouldInteractWithTextAttachment2 (UITextView textView, NSTextAttachment textAttachment, NSRange characterRange, UITextItemInteraction interaction) 

We do need to find a better name for the above events ^
Comment 4 Alex Soto [MSFT] 2016-09-13 06:57:43 UTC
Propposed PR: https://github.com/xamarin/xamarin-macios/pull/817
Comment 5 Sebastien Pouliot 2016-09-14 13:24:34 UTC
Merged in xamarin-macios/master e397849108d9e019369d52538f61226689d32372

@Alex please re-enable the API that were commented (on master) then create a trello card to backport both the generator and the API fixes for SR1. Thanks!

[1] https://trello.com/b/ddOX7IOt/platform-cycle-8-service-release-1
Comment 6 Alex Soto [MSFT] 2016-09-14 21:56:13 UTC
Resolved and fixed in: 

xamarin-macios/master e397849108d9e019369d52538f61226689d32372


xamarin-macios/master 1da975dbf468bf0e1a22e74c1f613db1ab525cc5
Comment 7 Mohit Kheterpal 2016-09-15 10:53:01 UTC
As per comment 6, this issue has been fixed for Alex Soto.

So, closing this issue by marking it as Verified Fixed.

Comment 8 Mohit Kheterpal 2016-09-15 10:59:17 UTC
I am able to reproduce this issue with test sample provided in bug description with stable builds of cycle 8.

I am observing a different error with fixed build of master xamarin.ios-

Error : /OverloadsBug/OverloadsBug/BTOUCH: Error BI1043: btouch: Repeated overload DoAnotherBar and no [DelegateApiNameAttribute] provided to generate property name on host class. (BI1043) (OverloadsBug)

But I am not sure about the expected behaviour, could you please let me know is this the expected behaviour? 

so that I can verify this issue accordingly.

As of now keeping this bug in Resolved fixed state.

Comment 9 Alex Soto [MSFT] 2016-09-15 19:21:42 UTC
Created attachment 17515 [details]
Fixed Test Case

This test case adds the new required DelegateApiName where it is needed so the binding builds successfully
Comment 10 Alex Soto [MSFT] 2016-09-15 19:33:09 UTC
Hello Mohit

The behaviour you are seeing is now the expected, whenever we get wrong bindings like the ones in the test case now instead of generating invalid code we will issue an error  like BI1043 so what you are seeing is correct because the bindings lack the DelegateApiName attribute.

I have attached a fixed test case where the DelegateApiNameAttribute is present and the bindings are generated correctly.

Comment 11 Mohit Kheterpal 2016-09-16 10:14:51 UTC
Thanks @Alex for the test case.

Now I have checked it with the test case provided by you in comment 9 and it builds successfully.

Using latest build of master xamarin.ios-

Screencast : http://www.screencast.com/t/ks0PndRsUhjn

Hence I am closing this issue by marking it as Verified fixed.

Comment 12 Brendan Zagaeski (Xamarin Team, assistant) 2016-10-20 02:04:27 UTC
Apologies in advance for using the "REOPENED" status as the best available option to raise a question about release inclusion.

Comment 5 indicates that there was a plan to backport this change to Cycle 8 for Cycle 8 – Service Release 1.  But at the moment, the changes have not yet been backported.

If it has been decided to leave this change until Cycle 9, then feel free to just update the target milestone accordingly and reset the resolution/verification status.

Comment 13 Sebastien Pouliot 2016-10-20 02:10:05 UTC
Good catch, milestone was not updated when the bug was closed