On Wed, 12 Jul 2023 14:31:53 GMT, Glavo <d...@openjdk.org> wrote: >> src/java.base/share/classes/java/io/StreamTokenizer.java line 632: >> >>> 630: sval = String.copyValueOf(buf, 0, i); >>> 631: if (forceLower) >>> 632: sval = sval.toLowerCase(Locale.ROOT); >> >> I suspect this change to StreamTokenizer needs eyes. I think long standing >> behavior of the lowerCaseMode(true) has been to use the rules for the >> default locale so we need to be careful. > >> I suspect this change to StreamTokenizer needs eyes. I think long standing >> behavior of the lowerCaseMode(true) has been to use the rules for the >> default locale so we need to be careful. > > I investigated usage of this method on GitHub: > > https://github.com/search?q=%22lowerCaseMode%28true%29%22+language%3AJava&type=code > > In some of the use cases I investigated, it seems that no one wants to rely > on the default locale. > > However, while I think this corrects the behavior, this caused a change in > the behavior of the API, so a CSR may be required. I don't want to debate > this in this PR, so I'll revert this change and open a new PR in the future.
Maybe a small suggestion to make it clear whats wanted here. In other projects I am involved in (Apache Lucene/Solr, Apache TIKA, PostgresSQL JDBC, Checkstyle itsself, Elasticserach/Opensearch), which use the [forbiddenapis Maven/Gradle/Ant plugin](https://github.com/policeman-tools/forbidden-apis/), we forbid all calls to several Java APIs (including toLowerCase/toUpperCase case). All bytecode using this will build failure (FYI, we also disallow other stuff like relying of default timezone or characterset). To make it clear what is really intended, those projects agreed on having `toLowerCase(Locale.getDefault())`, so it is explicit what's wanted. Without that it could be that somebody else starts the discussion again. This is just a suggestion to be explicit as it makes maintaining the code easier. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14763#discussion_r1261404900
