Complete support for [BindAs] - #2220
Conversation
… as NSObjects BindAsAttribute allows to bind NSValue and NSNumber into more accurate C# types lyke bool?, int? etc. so we must teach registrar about this.
Introspection will currently fail if BindAs is used, introspection will report that the incorrect type is registered so we need to skip this check if Nullable type is found in the signature
|
@spouliot / @rolfbjarne Please review so I can unblock @mandel-macaque on #2193 also feedback is super appreciated since this is my first time touching the registrar 😄 so pardon in advance is something is not right :p |
|
Build failure |
| } | ||
|
|
||
| // We use BindAsAttribute to wrap NSNumber/NSValue into more accurate Nullable<T> types | ||
| if (typeFullName != null && typeFullName.Contains ("Nullable")) |
There was a problem hiding this comment.
Contains is not safe enough
Startwith would be better
But it might be possible to check for generic earlier
| { | ||
| if (method is MethodInfo mf) { | ||
| if (IsNullableType (mf.ReturnType)) | ||
| return true; |
There was a problem hiding this comment.
We should not skip
Else the other check won't be done and we'll miss errors
Instead we could detect nullable and see the the return or parameters are something bindas support
…the type when Nullable type is encountered Introspection will currently fail if BindAs is used. Introspection will report that the incorrect type is registered so we need verify if a Nullable type is found in the signature and check against of a withelist of BindAs supported types
|
Sadly I can't use C# 7 heh anyways @spouliot I think I've addressed the introspection tests side, I think I'll need to wait for @rolfbjarne input since it seems I am missing something from the whole picture with the current fix according to registrar tests. |
|
Build failure |
|
So this is a lot more complex for the registrar. I'll have a look and get back to you. |
| return false; | ||
| } | ||
|
|
||
| protected HashSet<string> BindAsSupportedTypes = new HashSet<string> { |
There was a problem hiding this comment.
Love it. Worth having a bidi comment with the generator source to keep this in sync
|
@dalexsoto can you remove the registrar changes, so that we can get the introspection changes in? Then we can do the registrar changes (which are extensive) in a separate PR. |
…gistered as NSObjects" This reverts commit 911eab9.
|
@rolfbjarne / @spouliot done |
|
Build success |
* [registrar] BindAs uses Nullable types so allow them to be registered as NSObjects BindAsAttribute allows to bind NSValue and NSNumber into more accurate C# types lyke bool?, int? etc. so we must teach registrar about this. * [tests][introspection] Teach intro about BindAs and Nullable types Introspection will currently fail if BindAs is used, introspection will report that the incorrect type is registered so we need to skip this check if Nullable type is found in the signature * [introspection] Add better type checking instead of totally skipping the type when Nullable type is encountered Introspection will currently fail if BindAs is used. Introspection will report that the incorrect type is registered so we need verify if a Nullable type is found in the signature and check against of a withelist of BindAs supported types * Revert "[registrar] BindAs uses Nullable types so allow them to be registered as NSObjects" This reverts commit 911eab9. * [tests] Add comment about where to find BindAs types
* [registrar] BindAs uses Nullable types so allow them to be registered as NSObjects BindAsAttribute allows to bind NSValue and NSNumber into more accurate C# types lyke bool?, int? etc. so we must teach registrar about this. * [tests][introspection] Teach intro about BindAs and Nullable types Introspection will currently fail if BindAs is used, introspection will report that the incorrect type is registered so we need to skip this check if Nullable type is found in the signature * [introspection] Add better type checking instead of totally skipping the type when Nullable type is encountered Introspection will currently fail if BindAs is used. Introspection will report that the incorrect type is registered so we need verify if a Nullable type is found in the signature and check against of a withelist of BindAs supported types * Revert "[registrar] BindAs uses Nullable types so allow them to be registered as NSObjects" This reverts commit 911eab9. * [tests] Add comment about where to find BindAs types
5 participants
Introspection tests have an issue when verifying native signatures, since we are using
Nullable<T>and notNSValueorNSNumberit reports a false positive.Once we land this fix we can use it for Xcode 9 bindings and actually add to
monotouch-testssome API using this attribute. like in PR #2193