RFR: JDK-8189778: Jshell crash on tab for StringBuilder.append(

[Jan Lahoda]

Hi,

A webrev updated to the current state and with some further fixes is here:
http://cr.openjdk.java.net/~jlahoda/8189778/webrev.02/

On 2.11.2017 13:35, Kumar Srinivasan wrote:
>
> Hi Jan,
>
> Generally it looks fine to me, a few nits...
>
> JavadocHelper.java
>
> 1.
>
> -                        //which will be resolve in visitInheritDoc:
> suggest....
> +                        //which will be resolved in visitInheritDoc:

Done.

>
> 2. Suggest adding a space after /** or /* makes it a little easier
> to read the comments.

Done.

>
> 3.
>
> - //insertPos (as future missing elements should be inserted behind
> + //insertPos (as future missing elements should be inserted before

The missing elements are inserted behind the tree that was visited by 
the method.

>
> 4.
> -  //if there is a newline immediatelly after this tree, insert after
> +   //if there is a newline immediately after this tree, insert after

Done. Also changed "after" to "behind".

>
> 5. Several private methods have have javadoc comments /**, do you
> run javadoc -private on jshell sources ?

Changed to comments to /*.

Any feedback is welcome!

Thanks,
    Jan

>
> You may want to get Jon's review on this, he is much more familiar
> with these things, than I.
>
> Thanks
> Kumar
>
>> I've updated to patch to have some comments at places that seemed
>> important:
>> http://cr.openjdk.java.net/~jlahoda/8189778/webrev.01/
>>
>> Jan
>>
>> On 26.10.2017 11:35, Jan Lahoda wrote:
>>> Hi,
>>>
>>> Typing:
>>> jshell> StringBuilder sb = new StringBuilder();
>>> jshell> sb.append(<tab><tab>
>>>
>>> Will lead to an exception:
>>> jshell> sb.append(Exception in thread "main"
>>> java.lang.StringIndexOutOfBoundsException: start -59, end -59, length
>>> 238
>>> [snip]
>>> jdk.compiler/jdk.internal.shellsupport.doc.JavadocHelper$OnDemandJavadocHelper.getResolvedDocComment(JavadocHelper.java:481)
>>>
>>>
>>>
>>> The reason is that the javadoc for StringBuilder.append(CharSequence,
>>> int, int) is:
>>>      /**
>>>       * @throws     IndexOutOfBoundsException {@inheritDoc}
>>>       */
>>>
>>> The JavadocHelper tries to fill in the missing javadoc entries, but that
>>> fails because it tries to insert the preceding entries at an
>>> uninitialized place.
>>>
>>> The proposed patch:
>>> -fixes the above
>>> -adds a test that runs the JavadocHelper on all
>>> methods/fields/constructors of top-level types in all exported packages
>>> (this runs for a considerable time, and we may need to disable this test
>>> if it proves to be too heavyweight)
>>> -adds a few more test cases for problems found by the above test
>>> -fixes the javadoc resolution to treat missing javadoc body as
>>> {@inheritDoc} (so that the overridden method's javadoc text is used if
>>> missing in this method)
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8189778
>>> Webrev: http://cr.openjdk.java.net/~jlahoda/8189778/webrev.00/
>>>
>>> Feedback is welcome.
>>>
>>> Thanks,
>>>      Jan
>