On Thu, 21 Aug 2025 09:39:44 GMT, Volkan Yazici <vyaz...@openjdk.org> wrote:
>> `NoRepl`-suffixed `String` methods denote methods that do not replace >> invalid characters, but throw `CharacterCodingException` on encounter. This >> behavior cannot easily be derived from the method footprints, has been a >> source of confusion for maintainers, and is not uniformly adopted, e.g., >> `newStringUTF8NoRepl()` and `getBytesUTF8NoRepl()` does *not* throw `CCE`. >> This PR replaces the `NoRepl` suffix with `NoReplacement` in method names >> and consistently uses `throws CCE` in method footprints. > > Volkan Yazici has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 13 commits: > > - Javadoc fix > - Cosmetic improvements > - Merge remote-tracking branch 'upstream/master' into jlaNoRepl > - Remove redundant type parameters > - Simplify added null checks > - Avoid code duplication by sprinkling some generics magic > - Group `String` methods by `doReplace` argument > - Merge remote-tracking branch 'upstream/master' into jlaNoRepl > - Replace `requireNonNull` with implicit null checks > - Merge remote-tracking branch 'upstream/master' into jlaNoRepl > - ... and 3 more: https://git.openjdk.org/jdk/compare/a7c0f4b8...7af0f351 src/java.base/share/classes/java/lang/String.java line 890: > 888: > 889: private static <E extends Exception> byte[] encodeWithEncoder( > 890: Charset cs, byte coder, byte[] val, Class<E> > characterCodingException) The argument name `characterCodingException` reads more like an exception instance than an exception Class. In this use, it is referring to a class, an instance of which will be thrown. It could be just `exceptionClass`. src/java.base/share/classes/java/lang/String.java line 891: > 889: private static <E extends Exception> byte[] encodeWithEncoder( > 890: Charset cs, byte coder, byte[] val, Class<E> > characterCodingException) > 891: throws E { This is a very curious construct; clever in a way but also a bit magical. All the while enabling the caller (using null) to avoid having to declare the exception. If there were a concise explanation, that might be useful to future maintainers. src/java.base/share/classes/java/lang/String.java line 964: > 962: /** > 963: * {@return the sequence of bytes obtained by encoding the given > string in > 964: * the specified {@linkplain java.nio.charset.Charset charset}} Omit the link, for this private use, just say `Charset`. src/java.base/share/classes/java/lang/String.java line 1169: > 1167: > 1168: private static <E extends Exception> int decodeUTF8_UTF16( > 1169: byte[] src, int sp, int sl, byte[] dst, int dp, Class <E> > malformedInputException) Ditto comment about argument `malformedInputException` as an instance vs a class of exception, an instance of which will be thrown. src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java line 349: > 347: /** > 348: * {@return the sequence of bytes obtained by encoding the given > string in > 349: * the specified {@linkplain java.nio.charset.Charset charset}} (ok, not public javadoc) But Links in the Title line of a method are discouraged since they end up in multiple places. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26413#discussion_r2291092201 PR Review Comment: https://git.openjdk.org/jdk/pull/26413#discussion_r2291070853 PR Review Comment: https://git.openjdk.org/jdk/pull/26413#discussion_r2291083898 PR Review Comment: https://git.openjdk.org/jdk/pull/26413#discussion_r2291096063 PR Review Comment: https://git.openjdk.org/jdk/pull/26413#discussion_r2291055185
