JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

[Jan Lahoda]

Thanks for the comments/suggestion, and apologies for a belated reply.

I've updated the patch to use -Xlint:exports, which I agree is more 
consistent with the existing lint keys.

The updated patch for the first phase is here:
http://cr.openjdk.java.net/~jlahoda/8153362/langtools.02-phase1/

And for the second phase is here:
http://cr.openjdk.java.net/~jlahoda/8153362/langtools.02-phase2/

Updated patch for the top-level repository is here:
http://cr.openjdk.java.net/~jlahoda/8153362/top-level.02/

I've also filled JDK-8161376:
https://bugs.openjdk.java.net/browse/JDK-8161376

For the first phase.

Thanks!

Jan

On 27.6.2016 21:10, Jonathan Gibbons wrote:
> Since you have already admitted to using multiple concatenated lower
> case words in unexportedinapi, and since "unexported" is not a real
> word, can I suggest notexportedinapi for the category, and
> NOT_EXPORTED_IN_API for the Lint category.   Or else just simply all the
> was down to "exports" and "EXPORTS", by analogy with other language
> features like "static", "cast", etc
>
> Code changes look OK.  Long lines in final test are easy enough to read
> in a new file (as here) but will be harder to read in futire
> side-by-side webrevs.
>
> If it helps there is a ModuleBuilder class in the toolbox package in the
> langtools/test/lib directory.
>
> -- Jon
>
>
> In javac.properties, I think the text
>
>   217     Warn about use of types not visible to clients in exported API
>
> would read slightly better as
>
>   217     Warn about use of types in exported API that are not visible
> to clients
>
>
> Similarly , I think the wording of the messages in compiler.properties
> could be improved somewhat:
>
> For example, change
>
> 2846 compiler.warn.inaccessible.in.api=\
> 2847     inaccessible type referenced in exported API
>
> to
>
>       # 0: symbol kind, 1: symbol, 2:symbol
> 2846 compiler.warn.inaccessible.in.api=\
> 2847     {0} {1} in the exported API for module {2} is not accessible
>
> but even that may not be right.  What exactly does "referenced in
> exported API" mean? Can we get rid of the phrase altogether, as in
>
>
> 2846 compiler.warn.inaccessible.in.api=\
>
>       # 0: symbol kind, 1: symbol, 2:symbol
> 2847{0} {1} in module {2} is not accessible
>       # 0: symbol kind, 1: symbol, 2:symbol
> 2848 compiler.warn.unexported.in.api=\
> 2849{0} {1} in module {2} is not exported     # 0: symbol kind, 1:
> symbol, 2:symbol
> 2850 compiler.warn.unexported.in.api.not.required.public=\
> 2851{0} {1} in module {2}is not indirectly exported using 'requires
> public'     # 0: symbol kind, 1: symbol, 2:symbol
> 2852 compiler.warn.unexported.in.api.qualified=\
> 2853{0} {1} in module {2}  may not be visible to all clients that
> require this module
>
> -- Jon
>
> On 06/17/2016 07:18 AM, Jan Lahoda wrote:
>> Hi,
>>
>> I've updated the patches, reflecting the feedback so far.
>>
>> The langtools change is now split into two parts, one is only adding
>> the new lint key (but no checks are actually performed):
>> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.01-phase1/
>>
>> And the second part is adding the checks:
>> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.01-phase2/
>>
>> We could push the first part first, and the second one together with
>> other changes later, so that the repositories don't have to be updated
>> in a lockstep.
>>
>> In addition to the langtools changes, only the top-level repository
>> needs to be changed now, to disable the checks in some modules:
>> http://cr.openjdk.java.net/~jlahoda/8153362/top-level.01/
>>
>> Any feedback is welcome!
>>
>> Thanks,
>>     Jan
>>
>> On 14.6.2016 14:29, Jan Lahoda wrote:
>>> Hi Alan,
>>>
>>> On 14.6.2016 12:57, Alan Bateman wrote:
>>>> On 13/06/2016 17:12, Jan Lahoda wrote:
>>>>
>>>>> Hello,
>>>>>
>>>>> There is:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8153362
>>>>>
>>>>> which is about a new warning that should be produced by javac when
>>>>> exported API refers to types not exported/accessible to the API
>>>>> clients.
>>>>>
>>>>> I've put my current javac change here:
>>>>> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.00/
>>>> Did you have a short list of names for the lint option before deciding
>>>> on "unexportedinapi"? If time has already been put into this and
>>>> this is
>>>
>>> I had a few (e.g. "publishingunexported"), but none of them particularly
>>> nice.
>>>
>>>> the best of a bad bunch then ignore my mail. I bring it up because it
>>>> feels more like a "potentiallynotaccessible" or "notaccessible" or
>>>> "leaksnotaccessible". For the cases where we have ended up with
>>>
>>> I like "leaksnotaccessible". Unless there would be better ideas or
>>> objections, I'd go with that. Thanks for the ideas!
>>>
>>>> protected fields in public classes but the field type is
>>>> package-private
>>>> then the field is never accessible. For the JSObject.getWindow case
>>>> then
>>>> consumers will need to require java.desktop to use this method.
>>>>
>>>> Related is the description:
>>>>
>>>> javac.opt.Xlint.desc.unexportedinapi=\
>>>>      Warn about use of types not visible to clients in exported API
>>>>
>>>> Shouldn't get say something about the type potentially not accessible
>>>> rather than visible?
>>>
>>> Yes, it should. I'll fix that. Thanks for catching that.
>>>
>>> Jan
>>>
>>>>
>>>> -Alan
>>>>
>>>> PS: You asked about the JVMCI classes in the hotspot repo. While this
>>>> might look strange then it is intentional. The "framework" uses the
>>>> reflective APIs to export the otherwise internal packages to the JVMCI
>>>> implementation when it is located and loaded.
>