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

Add isVisibleFrom and isVisibleFromSubclass to Visibility #227

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tbroyer
Copy link
Contributor

@tbroyer tbroyer commented Apr 12, 2015

Fixes #226

@tbroyer
Copy link
Contributor Author

tbroyer commented Apr 12, 2015

I didn't know where to put those 2 methods, so I put them on Visibility, but I could move them to MoreElements if you prefer.

@gk5885
Copy link
Contributor

gk5885 commented Apr 13, 2015

So, the problem is that these methods guarantee way more than they actually provide. For classes, you have to account for nesting and the visibility of enclosing types. For members, protected has to worry about inheritance. In #226 you pointed to the relevant portions of the JLS, but AFAICT didn't account for most of the bullets therein - or at least not in a way that makes this generally applicable to Element.

@tbroyer
Copy link
Contributor Author

tbroyer commented Apr 13, 2015

The visibility of the enclosing type is handled with effectiveVisibilityOfElement.

For protected, the decision is left at the discretion of the caller of the API.

TL;DR: I believe the methods, as proposed here, cover 99% of use-cases. Maybe we could enhance the javadoc to be more explicit about the unsupported edge-cases? (also note that the second argument is a PackageElement for good reasons: if it were an Element then people could assume that inheritance checks would be done; with PackageElement, we're assuming that they did their homework in picking the appropriate method for their use-case)

Let's review the bullets one by one if you don't mind:

  • A package is always accessible.

effectiveVisibilityOfElement returns PUBLIC for packages, which let's us fall through to the next bullet:

  • If a class or interface type is declared public, then it may be accessed by any code, provided that the compilation unit (§7.3) in which it is declared is observable.

That's the return true part.

If a class or interface type is declared with package access, then it may be accessed only from within the package in which it is declared.

This is the MoreElements.getPackage(element).equals(from); part.

  • An array type is accessible if and only if its element type is accessible.

Array types aren't represented as Elements.

  • A member (class, interface, field, or method) of a reference type, or a constructor of a class type, is accessible only if the type is accessible and the member or constructor is declared to permit access:
    • If the member or constructor is declared public, then access is permitted.

return true

  • Otherwise, if the member or constructor is declared protected, then access is permitted only when one of the following is true:
    • Access to the member or constructor occurs from within the package containing the class in which the protected member or constructor is declared.
    • Access is correct as described in §6.6.2.

This is the MoreElements.getPackage(element).equals(from); part, and the reason we have 2 methods. More about that below.

  • Otherwise, if the member or constructor is declared with package access, then access is permitted only when the access occurs from within the package in which the type is declared.

Same as above.

  • Otherwise, the member or constructor is declared private, and access is permitted if and only if it occurs within the body of the top level class (§7.6) that encloses the declaration of the member or constructor.

This case isn't handled, but should never happen in practice in an annotation processor: you cannot have an Element for a class that you're generating and Java doesn't support "partial class declarations", so you should never be in a situation where you have an Element for a private member and want/have to check that it's visible from somewhere else within the same top level class.

So now for the protected specifics:

A protected member or constructor of an object may be accessed from outside the package in which it is declared only by code that is responsible for the implementation of that object.

This is the rule that users of the API should follow to determine which one of the two methods they should call. All the bullets in §6.6.2 would help the user determine which method to call depending on their use-case, as access will depend on which code you're generating that would possibly reference the target element.

For example, when you have an ExecutableElement representing a constructor, if you want to simply instantiate the class then use isVisibleFrom, but if you want to generate a subclass (whether anonymous or not), then use isVisibleFromSubclass.

In many cases, you won't have an Element representing the exact type (class) that would have to be checked for inheritance, as you'll be in the process of generating it.

* Returns whether the given element is visible from another element in the given package,
* assuming that other element is a subclass (or a member of a subclass) of the former (or
* its enclosing type).
Copy link
Member

Choose a reason for hiding this comment

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

Being a subclass doesn't guarantee protected access. If the element being accessed is a constructor or an instance method, it is only accessible from within the same instance. I'm not sure how to capture that here. Maybe just give in and actually mention protected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AutoCommon: check visibility of one method (or type or field) from another.
5 participants