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] The generator creates not compilable code when a selector has the word Handle in it. #18645

Closed
mandel-macaque opened this issue Aug 6, 2023 · 0 comments · Fixed by #18646
Assignees
Milestone

Comments

@mandel-macaque
Copy link
Member

Given the following example binding:

        [NoWatch, NoTV, Mac (14,0), iOS (17,0), MacCatalyst (17,0)]
	[Protocol]
	[BaseType (typeof(NSObject))]
	interface ASCredentialIdentity {

		[Abstract]
		[Export ("serviceIdentifier", ArgumentSemantic.Strong)]
		ASCredentialServiceIdentifier ServiceIdentifier { get; }

		[Abstract]
		[Export ("user")]
		string User { get; }

		[Abstract]
		[NullAllowed, Export ("recordIdentifier")]
		string RecordIdentifier { get; }

		[Abstract]
		[Export ("rank")]
		nint Rank { get; set; }
	}
        
        [NoWatch, NoTV, Mac (14,0), iOS (17,0), MacCatalyst (17,0)]
	[BaseType (typeof (NSObject))]
	[DisableDefaultCtor]
	interface ASPasskeyCredentialIdentity : NSCopying, NSSecureCoding, ASCredentialIdentity {
		[Export ("initWithRelyingPartyIdentifier:userName:credentialID:userHandle:recordIdentifier:")]
		NativeHandle Constructor (string relyingPartyIdentifier, string userName, NSData credentialID, NSData userHandle, [NullAllowed] string recordIdentifier);

		[Static]
		[Export ("identityWithRelyingPartyIdentifier:userName:credentialID:userHandle:recordIdentifier:")]
		ASPasskeyCredentialIdentity CreateIdentity (string relyingPartyIdentifier, string userName, NSData credentialID, NSData userHandle, [NullAllowed] string recordIdentifier);

		[Export ("relyingPartyIdentifier")]
		string RelyingPartyIdentifier { get; }

		[Export ("userName")]
		string UserName { get; }

		[Export ("credentialID", ArgumentSemantic.Copy)]
		NSData CredentialID { get; }

		[Export ("userHandle", ArgumentSemantic.Copy)]
		NSData UserHandle { get; }

		[NullAllowed, Export ("recordIdentifier")]
		new string RecordIdentifier { get; }

		[Export ("rank")]
		new nint Rank { get; set; }
	}

The generator will create two private fields in the generated class that use the same name resulting in the following error:

build/dotnet/macos/generated-sources/AuthenticationServices/ASPasskeyCredentialIdentity.g.cs(90,16): error CS0102: The type 'ASPasskeyCredentialIdentity' already contains a definition for 'selUserHandle'
make[1]: *** [build/dotnet/macos/ref/Microsoft.macOS.dll] Error 1
make[1]: *** Waiting for unfinished jobs....
^Cmake[1]: *** [build/dotnet/ios/ref/Microsoft.iOS.dll] Deleting file `build/dotnet/ios/ref/Microsoft.iOS.xml'
make[1]: *** [build/dotnet/ios/ref/Microsoft.iOS.dll] Deleting file `build/dotnet/ios/64/Microsoft.iOS.pdb'
make[1]: *** [build/dotnet/ios/ref/Microsoft.iOS.dll] Deleting file `build/dotnet/ios/64/Microsoft.iOS.dll'
make[1]: *** [build/dotnet/ios/ref/Microsoft.iOS.dll] Interrupt: 2
make[1]: *** Deleting intermediate file `build/tvos/reference/Xamarin.TVOS.dll'
make[1]: *** Deleting intermediate file `build/watch/reference/MonoTouch.NUnitLite.dll'
make[1]: *** Deleting intermediate file `build/watch/reference/Xamarin.WatchOS.dll'
make: *** [all-recurse] Interrupt: 2

The reason for this is that we have the word handle and the ned of the selector form the property. Because we do not keep track of the variable names in the scope, this results a duplicated variable name. There are 2 ways to fix this:

  1. Simplest way is to perform the same code we have now but add a prefix to the word handle that we know will probably not happened in a selector from Apple.
  2. Make the generator aware of the variables in the current context, and if a variable has already been used in the same context, add a postfix of some kind.

Approach 1 is easier for several reasons:

  1. The code to change is minimal.
  2. We do have manual code (in the Foundation directory) that uses the knowledge of the selector naming convention used by the Generator. Using approach 2 in which we add a postfix only if we found a variable with the same name makes the manual code much harder to write and fragile.
@mandel-macaque mandel-macaque self-assigned this Aug 6, 2023
mandel-macaque added a commit to mandel-macaque/xamarin-macios that referenced this issue Aug 6, 2023
Fixes xamarin#18645

The way the generator names private fields for selectors results on not
compilable code when there are two selectoors A and B such that selector
B == AHandle.

That is, if selector A is "user", selector B will be
"userHandle". This happens because Handle is quite a common postfix in
security when working with user accounts (userHandler and others) and
we need to ensure that the Handle postfix added by the generator is unique.

As stated in the bug we could try to fix this by keeping track of the variables in the context
in wich the generator is creating code. The problem with such approach
is that it will make very hard to predict the name of the fields making
the manual code harder to write. The PR contains examples of such manual
code.

To fix the situation we have moved from using Handle to XHandle as a
postfix in the field names whihc reduces the chances of finding a
similar corner case in the future.

A test has been added to show the case in which we found the bug.

Fixes xamarin#18645
mandel-macaque added a commit to mandel-macaque/xamarin-macios that referenced this issue Aug 6, 2023
Fixes xamarin#18645

The way the generator names private fields for selectors results on not
compilable code when there are two selectoors A and B such that selector
B == AHandle.

That is, if selector A is "user", selector B will be
"userHandle". This happens because Handle is quite a common postfix in
security when working with user accounts (userHandler and others) and
we need to ensure that the Handle postfix added by the generator is unique.

As stated in the bug we could try to fix this by keeping track of the variables in the context
in wich the generator is creating code. The problem with such approach
is that it will make very hard to predict the name of the fields making
the manual code harder to write. The PR contains examples of such manual
code.

To fix the situation we have moved from using Handle to XHandle as a
postfix in the field names whihc reduces the chances of finding a
similar corner case in the future.

A test has been added to show the case in which we found the bug.

Fixes xamarin#18645
@rolfbjarne rolfbjarne added this to the xcode15 milestone Aug 7, 2023
vs-mobiletools-engineering-service2 pushed a commit to vs-mobiletools-engineering-service2/xamarin-macios that referenced this issue Aug 8, 2023
Fixes xamarin#18645

The way the generator names private fields for selectors results on not
compilable code when there are two selectoors A and B such that selector
B == AHandle.

That is, if selector A is "user", selector B will be
"userHandle". This happens because Handle is quite a common postfix in
security when working with user accounts (userHandler and others) and
we need to ensure that the Handle postfix added by the generator is unique.

As stated in the bug we could try to fix this by keeping track of the variables in the context
in wich the generator is creating code. The problem with such approach
is that it will make very hard to predict the name of the fields making
the manual code harder to write. The PR contains examples of such manual
code.

To fix the situation we have moved from using Handle to XHandle as a
postfix in the field names whihc reduces the chances of finding a
similar corner case in the future.

A test has been added to show the case in which we found the bug.

Fixes xamarin#18645
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants