Skip to content

fix(runtime): Respect availability attributes for protocols and base classes #1086

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

Merged

Conversation

mbektchiev
Copy link
Contributor

@mbektchiev mbektchiev commented Feb 25, 2019

PR Checklist

  • Filter members which are unavailable in the current SDK

    • Add isImplementedInClass and isAvailableInClass methods in
      PropertyMeta and MethodMeta which additionally check whether a Class
      instance supports a given optional method

    • Use isAvailableInClass instead of isAvailable in getOwnPropertyNames
      of ObjCPrototype, ObjCConstructorBase, ObjCConstructorNative and
      in materializeProperties of ObjCConstructorNative

    • Filter via isAvailableInClass in BaseClassMeta's various
      method and property getters

    • Change MembersCollection to be a HashSet to avoid adding the same
      member more than once when collecting them from the inheritance chain

    • Remove unused functions from BaseClassMeta

    • Add findInterfaceMeta function with fallback to the base interface
      if the desired one is unavailable in the current SDK version (instead
      of the hardcoded NSObject so far)

    • Remove hacky patch from ObjCMethodWrapper::preInvocation which
      fixed the symptoms of Reading collisionBoundingPath on UIView results in Objective C exception #978 but not its root cause. Namely, that
      unimplemented optional methods were returned as available properties
      in getOwnPropertyNames.

    • Modify fixtures and add test cases for methods availability in
      InheritanceTests.js

    • Add test case for unavailable base class

    • Add test case for property with custom selector in ApiTests. The getter
      of secureTextEntry(isSecureText) overrides the default secureTextEntry

    • Add encodeVersion getMajorVersion and getMinorVersion helpers

  • Never check for availability of protocols
    Apple regularly create new protocols and move existing
    interface members there. E.g. iOS 12.0 introduced the
    UIFocusItemScrollableContainer protocol in UIKit which
    contained members that have existed in UIScrollView
    since iOS 2.0.

    • Create findProtocol function in GlobalTable
    • Use it instead of findMeta everwhere where a protocol is looked for
    • Add tests

refs #1084, #1085

@mbektchiev mbektchiev force-pushed the bektchiev/respect-availability-attributes-for-protocols branch from ae18b38 to 3ec5a02 Compare February 25, 2019 10:29
@mbektchiev
Copy link
Contributor Author

test package_version#latest

@etabakov
Copy link
Contributor

@cla-bot check

@cla-bot
Copy link

cla-bot bot commented Feb 25, 2019

The cla-bot has been summoned, and re-checked this pull request!

@cla-bot cla-bot bot added the cla: yes label Feb 25, 2019
@mbektchiev mbektchiev force-pushed the bektchiev/respect-availability-attributes-for-protocols branch from 3ec5a02 to 4bdbdfa Compare February 27, 2019 12:53
@cla-bot
Copy link

cla-bot bot commented Feb 27, 2019

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla.
CLA has not been signed by users: @Natalia-Hristova.
After signing the CLA, you can ask me to recheck this PR by posting @cla-bot check as a comment to the PR.

@cla-bot cla-bot bot removed the cla: yes label Feb 27, 2019
@mbektchiev mbektchiev changed the base branch from release to master February 27, 2019 13:34
@mbektchiev mbektchiev added this to the 5.3.0 milestone Feb 27, 2019
@mbektchiev
Copy link
Contributor Author

@cla-bot check

@cla-bot cla-bot bot added the cla: yes label Feb 27, 2019
@cla-bot
Copy link

cla-bot bot commented Feb 27, 2019

The cla-bot has been summoned, and re-checked this pull request!

@mbektchiev mbektchiev force-pushed the bektchiev/respect-availability-attributes-for-protocols branch 2 times, most recently from d2e119e to 8b81914 Compare February 28, 2019 08:11
* Add `isImplementedInClass` and `isAvailableInClass` methods in
`PropertyMeta` and `MethodMeta` which additionally check whether a Class
instance supports a given optional method

* Use `isAvailableInClass` instead of `isAvailable` in `getOwnPropertyNames`
of `ObjCPrototype`, `ObjCConstructorBase`, `ObjCConstructorNative` and
in `materializeProperties` of `ObjCConstructorNative`

* Filter via `isAvailableInClass` in `BaseClassMeta`'s various
method and property getters

* Change `MembersCollection` to be a `HashSet` to avoid adding the same
member more than once when collecting them from the inheritance chain

* Remove unused functions from `BaseClassMeta`

* Add `findInterfaceMeta` function with fallback to the base interface
if the desired one is unavailable in the current SDK version (instead
of the hardcoded `NSObject` so far)

* Remove hacky patch from `ObjCMethodWrapper::preInvocation` which
fixed the symptoms of #978 but not its root cause. Namely, that
unimplemented optional methods were returned as available properties
in `getOwnPropertyNames`.

* Modify fixtures and add test cases for methods availability in
`InheritanceTests.js`

* Add test case for unavailable base class

* Add test case for property with custom selector in ApiTests. The getter
of `secureTextEntry`(`isSecureText`) overrides the default `secureTextEntry`

* Add `encodeVersion` `getMajorVersion` and `getMinorVersion` helpers

refs #1085
Apple regularly create new protocols and move existing
interface members there. E.g. iOS 12.0 introduced the
UIFocusItemScrollableContainer protocol in UIKit which
contained members that have existed in UIScrollView
since iOS 2.0.

* Create `findProtocol` function in `GlobalTable`
* Use it instead of `findMeta` everwhere where a
protocol is looked for
* Add tests

refs #1084
@mbektchiev mbektchiev force-pushed the bektchiev/respect-availability-attributes-for-protocols branch from 8b81914 to 5bfffab Compare February 28, 2019 09:25
* Ensure that the module providing the method is loaded before checking
* Add check for dynamic selectors
* As a last resort `alloc` an instance and send `respondsToSelector` message
* Cache each class' instances to minimize the number of allocations and deallocations
performed
* Improve diagnostics messages in the `ApiIterator` test
Do not break the loop at the first encountered unavailable initializer
* Add not implemented initializers
* Check that implemented initializers will be called
* Check that not implemented initializers will not be used
If we don't have this special treatment we enter an endless recursion.
* Remove redundant `static_cast`
@mbektchiev mbektchiev self-assigned this Mar 5, 2019
@mbektchiev mbektchiev added the bug label Mar 5, 2019
@mbektchiev mbektchiev merged commit cc88c41 into master Mar 5, 2019
@ghost ghost removed the bug label Mar 5, 2019
@mbektchiev mbektchiev deleted the bektchiev/respect-availability-attributes-for-protocols branch March 5, 2019 13:22
mbektchiev added a commit that referenced this pull request Mar 11, 2019
Fix issue with missing `imageInsets`, `landscapeImagePhone`, `title` properties of UITabBarItem.
These properties are declared in `UIBarItem` but it actually doesn't implement them. Some of its
successors like `UITabBarItem` however do, and we need to expose them there.

This regression has been introduced with #1086.
mbektchiev added a commit that referenced this pull request Mar 12, 2019
Fix issue with missing `imageInsets`, `landscapeImagePhone`, `title` properties of UITabBarItem.
These properties are declared in `UIBarItem` but it actually doesn't implement them. Some of its
successors like `UITabBarItem` however do, and we need to expose them there.

This regression has been introduced with #1086.
mbektchiev added a commit that referenced this pull request Mar 12, 2019
Fix issue with missing `imageInsets`, `landscapeImagePhone`, `title` properties of UITabBarItem.
These properties are declared in `UIBarItem` but it actually doesn't implement them. Some of its
successors like `UITabBarItem` however do, and we need to expose them there.

This regression has been introduced with #1086.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants