RFR: 4247235 (spec str) StringBuffer.insert(int, char[]) specification is inconsistent

[Mike Duigou]

AbstractStringBuilder probably needs the "Unless otherwise noted," blanket statement as well (Same as StringBuffer/StringBuilder)

You might want to move the Objects.requireNonNull(dst); in String.getBytes() to after the existing checks so as not to unnecessarily change the exception thrown for bad inputs. An exception will still be thrown but some bad code may anticipate StringIndexOutOfBoundsException rather than NPE. This is a very minor matter though.

Otherwise, it looks good.

Mike

On Jan 9 2013, at 14:46 , Jim Gish wrote:

> Please review the following:
> 
> Webrev: http://cr.openjdk.java.net/~jgish/Bug4247235-Add-Blanket-Null-Stmt/ <http://cr.openjdk.java.net/%7Ejgish/Bug4247235-Add-Blanket-Null-Stmt/>
> Here's a specdiff: http://cr.openjdk.java.net/~jgish/Bug4247235-string-specdiff/ <http://cr.openjdk.java.net/%7Ejgish/Bug4247235-string-specdiff/>
> 
> 
> Summary:  This change replaces java/lang/*String*.java javadoc, method-specific @throws NullPointerException clauses with  blanket null-handling statements like that currently in String.java
> 
> That was motivated by a discussion awhile back, strongly favoring a blanket statement over individual @throws clauses.
> 
> For String, the following blanket statement is already in place:
> 
> * <p> Unless otherwise noted, passing a <tt>null</tt> argument to a constructor
> * or method in this class will cause a {@link NullPointerException} to be
> * thrown.
> 
> 
> However, despite the blanket statement, the following methods also have @throws clauses:
> 
> public boolean contains(java.lang.CharSequence s)
> 
> Throws:
>   |java.lang.NullPointerException|- if|s|is|null|
> ---------------------------------------------------------------
> 
> public staticString  <http://cr.openjdk.java.net/%7Ejgish/Bug4247235-string-specdiff/java/lang/String.html>  format(String  <http://cr.openjdk.java.net/%7Ejgish/Bug4247235-string-specdiff/java/lang/String.html>  format,
>            java.lang.Object... args)
> 
> 
> Throws:
> |java.lang.NullPointerException|- If the|format|is|null
> |-----------------------------------------------------------------------
> ||
> 
> public staticString  <http://cr.openjdk.java.net/%7Ejgish/Bug4247235-string-specdiff/java/lang/String.html>  format(java.util.Locale l,
>            String  <http://cr.openjdk.java.net/%7Ejgish/Bug4247235-string-specdiff/java/lang/String.html>  format,
>            java.lang.Object... args)
> 
> Throws:
> |java.lang.NullPointerException|- If the|format|is|null
> |--------------------------------------------------------------------------
> All of the above are properly specified with the blanket statement or other parts of their spec (such as format w.r.t. null Locale) and the @throws can safely be removed.
> 
> Because the blanket statement is already in effect for String.java, I wrote tests for all methods/constructors to ensure compliance.  Running them revealed the following:
> 
> public void getBytes(int srcBegin, int srcEnd, byte dst[], int dstBegin)
> - I would expect an NPE to be thrown if dst == null.  However, this isn't always the case.  If dst isn't accessed because of the values of the other parameters (as in getBytes(1,2,(byte[]null,1), then no NPE is thrown.
> - This brings up the question as to the correctness of the blanket statement w.r.t. this method.  I think this method (and others like it) should use Objects.requireNonNull(dst).  (The correspoding method public void getChars(int srcBegin, int srcEnd, char dst[], int dstBegin), does always throw NPE for dst==null)
> 
> All other methods/constructors in StringBuffer and StringBuilder either correctly state null-handling behavior that differs from the blanket statement or are correct w.r.t. the blanket statement.
> 
> This has been reviewed by the JCK team.  It will require CCC approval (because of the new blanket statement, and the fact that some of the existing methods were previously silent on null handling, but all already conform to the blanket statement).
> 
> Thanks,
>    Jim Gish
> 
> 
> -- 
> Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304
> Oracle Java Platform Group | Core Libraries Team
> 35 Network Drive
> Burlington, MA 01803
> jim.gish at oracle.com
> 
> -- 
> Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304
> Oracle Java Platform Group | Core Libraries Team
> 35 Network Drive
> Burlington, MA 01803
> jim.gish at oracle.com
>