RFR JDK-8164836: fix jdeprscan violations of tools/all/RunCodingRules.java

[Jonathan Gibbons]

Looks OK to me.

-- Jon

On 08/28/2016 01:31 PM, Jan Lahoda wrote:
> On 26.8.2016 21:02, Jonathan Gibbons wrote:
>> Arguably, we should only check @DefinedBy in java.compiler and
>> jdk.compiler modules.
>
> Updated version of the patch where the DefinedByAnalyzer only checks 
> the java.compiler and jdk.compiler modules, and the @DefinedBy 
> annotation is removed from the other modules is here:
> http://cr.openjdk.java.net/~jlahoda/8164836/webrev.01/
>
> Jan
>
>>
>> -- Jon
>>
>> On 08/26/2016 08:36 AM, Stuart Marks wrote:
>>> Hi Jan,
>>>
>>> Thanks for picking this up.
>>>
>>> As you point out, the API usage in jdeprscan, and its containing
>>> module jdk.jdeps, really isn't the target of @DefinedBy. It's
>>> "implementing" the APIs, but for the purpose of getting callbacks from
>>> the annotation processing framework, not for the purpose of providing
>>> an implementation of the API to the general public. In that sense
>>> @DefinedBy doesn't really apply, so it makes sense to adjust the
>>> DefinedByAnalyzer to be more specific in what it's checking.
>>>
>>> But does special-casing jdk.jdeps make sense? Or should there be some
>>> kind of finer-grained logic to determine when @DefinedBy should and
>>> shouldn't apply? I wouldn't be surprised if there are other usages of
>>> @DefinedBy outside of jdk.jdeps that are in a similar situation. See
>>> jdk.jshell for instance.
>>>
>>> I've prepared a patch -- not yet posted anywhere -- that simply adds
>>> the right @DefinedBy annotations in jdeprscan. This is simple enough
>>> to do, even though it might not be the right thing.
>>>
>>>
>>>
>>> s'marks
>>>
>>>
>>>
>>>
>>> On 8/26/16 5:00 AM, Jan Lahoda wrote:
>>>> Hi,
>>>>
>>>> The RunCodingRules test is broken after 871b60b0c091, and has been
>>>> disabled. My
>>>> proposal to fix this is to disable the DefinedByAnalyzer for classes
>>>> from the
>>>> jdk.jdeps module. This module is not AFAIK implementing the APIs, and
>>>> so is not
>>>> a primary target of the analyzer/@DefinedBy annotation. One notable
>>>> alternative
>>>> would be to disable the crules completely for jdk.jdeps, but I opted
>>>> rather for
>>>> a more limited change. (Another alternative would be to add the
>>>> @DefinedBy
>>>> annotations to classes jdk.jdeps, but that does not seem quite right
>>>> to me.)
>>>>
>>>> Bug:
>>>> https://bugs.openjdk.java.net/browse/JDK-8164836
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~jlahoda/8164836/webrev.00/
>>>>
>>>> Jan
>>