RFR: 8157000: Do not generate javadoc for overridden method with no spec change

[Kumar Srinivasan]

Hi Jon,

Inspections of the specdiff pointed out some missing links, in the 
previous revision.

In this iteration:
1. All missing links have been purged.
2. Fixing the above exposed issues relating to:
     a. handling of javafx properties
    b. missing ids in class-use and index
    c.  latent bug in the initialization of enum methods (value/valueof) 
comments
    d. made some adjustments to VisibleMemberMap and else where.
    e. add some more test use-cases.

My qparent:
47320[qparent]:47318,47319   8b09673f7ede   2017-10-06 20:54 +0000   lana


Full webrev:
http://cr.openjdk.java.net/~ksrini/8157000/webrev.02/

The delta webrev is here:
http://cr.openjdk.java.net/~ksrini/8157000/webrev.02/webrev.delta/

Specdiff
http://cr.openjdk.java.net/~ksrini/8157000/specdiff.out/overview-summary.html

API docs before:
http://cr.openjdk.java.net/~ksrini/8157000/docs-before/api/overview-summary.html

API docs after:
http://cr.openjdk.java.net/~ksrini/8157000/docs-after/api/overview-summary.html

Thanks
Kumar



> Looks good to me.
> Thanks for the delta-webrev.
>
> -- Jon
>
> On 10/03/2017 02:47 PM, Kumar Srinivasan wrote:
>> Hi Jon,
>>
>> Thanks for the review, here is a new webrev:
>> http://cr.openjdk.java.net/~ksrini/8157000/webrev.01/index.html
>> and a delta webrev:
>> http://cr.openjdk.java.net/~ksrini/8157000/webrev.01/webrev.delta/
>>
>>> HtmlConfiguration.java
>>> Other public fields nearby have javadoc comments. It would be nice 
>>> to follow the precedent.
>>> That being said, see comments for Utils.java:1568. This option would 
>>> be better defined in
>>> BaseConfiguration, not HtmlConfiguration.
>>
>> Done
>>
>>>
>>> MethodWriterImpl.java 293
>>> No need to assign null. Better to leave it empty (just "Content 
>>> label;") so that the compiler
>>> will verify it is asigned correctly in the statements that follow.
>>>
>> Done
>>> MethodWriterImpl.java 295,296
>>> I acknowledge you've followed the style of 300,301, but as a comment 
>>> for general cleanup
>>> we should move away from legacy wrapper methods like 
>>> configuration.getText in favor
>>> if the more explicit underlying call resources.getText. Or we could 
>>> do that as part of a more
>>> comprehensive cleanup.
>>
>> Agreed. Will look into filing a bug.
>>
>>>
>>> resources/standard.properties:371
>>> <...> is normally used for "fill in your own value here", as in 
>>> <name>, <url>, etc
>>> Use (...|...) to indicate alternatives. See line 389 for an example.
>>>         (all|none|[-]<group>)
>>
>> Done
>>
>>>
>>> MemberSumaryBuilder.java:485
>>> Would be good to find a semantically helpful name instead of just 
>>> inhmembers0.
>>
>> Done,  renamed.
>>
>>>
>>> MemberSumaryBuilder.java:486,487,490,517
>>> The short names are sort-of OK, but "members" should be capitalized. 
>>> Also "class" in "inhclass"
>>
>> Done, renamed.
>>
>>>
>>> MemberSumaryBuilder.java:497, 517
>>> "footnote" is one word, not two. Also implies addSummaryFootnote
>>
>> Fixed
>>
>>>
>>> Utils.java:1566
>>> Since this is for methods only, it would be better if the parameter 
>>> type was
>>> ExecutableElement, not Element. That should be OK with the callsite 
>>> at MemberSummaryBuilder:498
>>> Also, the name is not ideal. How about isSimpleOverride?
>>
>> Done.
>>
>>>
>>> Utils.java:1568
>>> The need to cast to HtmlConfiguration is a big indication that 
>>> something is defined
>>> in the wrong place. summarizeOveriddenMethods is not HTML-specific 
>>> and so should
>>> be moved to BaseConfiguration.
>>
>> Done.
>>
>>>
>>> VisibleMemberMap.java:241,316
>>> Is the cast safe? You're iterating the members, but assuming the 
>>> member is an
>>> ExecutableElement
>>
>> Yes the cast  is safe!, what happens is the VMM is being built for 
>> each *kind* so in
>> the method getClassMembers, the member kinds are obtained on the
>> VMM kind.
>>
>>>
>>> Test files:
>>> I assume I'm seeing false/duplicate output from the recent repo 
>>> consolidation.
>>> The first group of 3 test files should not be here.
>>>
>>
>> Oops,  removed those extraneous files.
>>
>>
>>> TestOverrideMethods.java
>>> a) if you're going to have long strings in checkOutput, I suggest 
>>> blank lines to help separate the
>>> individual strings.
>>> b) Long strings will be annoying to maintain going forward. Ideally, 
>>> you should reduce the strings
>>> to the minimum necessary to focus on what you need to test, so that 
>>> unrelated work on nearby
>>> content doesn't break these tests.
>>
>> Ok I narrowed down the focus, and eliminated the long lines.
>>
>> Thanks
>>
>> Kumar
>>
>>>
>>> -- Jon
>>>
>>>
>>> On 09/21/2017 02:10 PM, Kumar Srinivasan wrote:
>>>> Hello,
>>>>
>>>> Please review for 8157000, please let me know if you have any
>>>> comments or ways I can improve it.
>>>>
>>>> The "before and after" test without the flag is identical.
>>>>
>>>> http://cr.openjdk.java.net/~ksrini/8157000/webrev.00/index.html
>>>>
>>>> Thanks
>>>> Kumar
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8157000
>>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8187386
>>>
>>
>