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

[Kumar Srinivasan]

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:
   

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

3.

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

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

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

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