RFR: 8311943: Cleanup usages of toLowerCase() and toUpperCase() in java.base [v2]

[Remi Forax]

----- Original Message -----
> From: "Uwe Schindler" <uschindler at openjdk.org>
> To: "core-libs-dev" <core-libs-dev at openjdk.org>, net-dev at openjdk.org, nio-dev at openjdk.org, security-dev at openjdk.org
> Sent: Wednesday, July 12, 2023 6:08:17 PM
> Subject: Re: RFR: 8311943: Cleanup usages of toLowerCase() and toUpperCase() in java.base [v2]

> On Wed, 12 Jul 2023 14:31:53 GMT, Glavo <duke at 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.

One solution is to deprecate String.toLowerCase()/toUpperCase(), forcing users to explicitly use the variants that takes a Locale.
Obviously, I'm talking about a simple deprecation not a deprecation for removal.

Rémi

> 
> -------------
> 
> PR Review Comment: https://git.openjdk.org/jdk/pull/14763#discussion_r1261404900