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

consider "package protected" methods and fields in autoclass #522

Merged
merged 3 commits into from
Apr 26, 2020

Conversation

hx2A
Copy link
Member

@hx2A hx2A commented Apr 25, 2020

In addition to public, protected, and private visibility, methods and fields can be "package protected".

https://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html

Package protected is the visibility setting when none of public, protected, and private are used (and therefore do not have any of those modifiers). The end result is the visibility depends on if the code is in the same package as the class package.

Rather than make a new autoclass parameter called include_package_protected, I made it look at the package names as it goes up the class hierarchy and follow include_protected if the package name is not the same as the starting package name. @tshirtman @cmacdonald I believe this is the better choice but am not 100% sure and want to hear what you have to say.

This PR also adds some unit test assert statements that should have been added in #500.

@hx2A
Copy link
Member Author

hx2A commented Apr 25, 2020

This might be better.

If a field or method is package protected:
if include_protected is True and the package name is the same as the starting package name, include it. If include_protected is False, leave it out.
if the package name is not the same, include it if include_private is True.

…ude_protected or include_private if the class is or is not in the same package as the autoclass start package
@hx2A
Copy link
Member Author

hx2A commented Apr 25, 2020

OK, now I am confident in what I have checked in.

For the input values (include_protected, include_private), here is how package protected fields and method are handled:

  • (True, True) => always include in class
  • (True, False) => compare package of class in the hierarchy with the package autoclass started with, and only include if they are the same
  • (False, False) => always exclude

Now all three possible outcomes are possible with different values for (include_protected, include_private). None of the other proposed ideas can do that.

This should be considered a bug because right now the package protected fields and methods are always included, no matter what (include_protected, include_private) are set to.

Copy link
Member

@tshirtman tshirtman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks good, i'm just unsure if i like the isPackageProtected method, otherwise it looks good, and the tests looks thorough!

jnius/reflect.py Outdated Show resolved Hide resolved
@tshirtman tshirtman merged commit d339fe7 into kivy:master Apr 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants