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

[Jonathan Gibbons]

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.