Specify an explicit locale for all to{Lower,Upper}Case calls

[fmeum]

Java's String#to{Lower,Upper}Case() is locale-dependent, which can
lead to unexpected results in locales with special case mappings in the
ASCII range (e.g. in a Turkish locale, a capital ASCII I lowercases to
a non-ASCII variant of i).

This is prevented by specifying a local without such case mappings. This
commit uses Locale.ROOT as the canonical choice with the same case
mapping behavior as other common locales such as Locale.ENGLISH or
Locale.US.

Follow-up changes could use Guava's Ascii.to{Lower,Upper}Case instead,
but whether this is safe may depend on the context, which makes this
replacement unsuitable to perform across the repo.

Fixes #17541

[fmeum]

@meteorcloudy Could you review this? It's a global change that mostly affects OSS users. Turns out Bazel was very broken with a Turkish locale.

[fmeum]

@bazel-io flag

[meteorcloudy]

Looks a nice fix to me.
@lberki Can you confirm if this is a safe change to make?

Follow-up changes could use Guava's Ascii.to{Lower,Upper}Case instead,
but whether this is safe may depend on the context, which makes this
replacement unsuitable to perform across the repo.

What context specifically? I can see Ascii.to{Lower,Upper}Case is already used in many places.

[fmeum]

@meteorcloudy As far as I understand, Ascii#to{Lower,Upper}Case only changes the case of ASCII characters, whereas String#to{Lower,Upper}Case also applies Unicode case mapping rules (but now a fixed set of those).

I would be surprised if Ascii#to{Lower,Upper}Case didn't end up being the correct choice in essentially all locations I touched, but it wouldn't be as obvious that the change is correct and I certainly wouldn't feel like cherry-picking it.

[meteorcloudy]

@fmeum Understood, thanks!

[lberki]

I'm somewhat worried that even though this change fixes the issue at hand any many others that stem from the meaning of to{Lower,Upper}Case() being different depending on the locale, it's easy to introduce such a mistake again.

WDYT about sanitizing the environment of the Bazel server instead so that as far as the JVM is concerned, the locale is always the same, regardless of the locale the user sets? (I thought that we already do that, but we apparently don't)

[fmeum]

@lberki Char locales are fixed for Java actions and also by the Java stub template, but I don't think Bazel itself is subject to any locale cleaning.

While forcing a locale makes sense from the point of view of hermeticity, there are certain places in which having locale-dependent output may actually be desirable (e.g. String#format rendering of floating point numbers and decimal separators). Not saying that it wouldn't be worth it, but I can see a cost in terms of usability.

An alternative I have thought about was to add an ErrorProne check for locale-less calls to to{Lower,Upper}Case and enable it for Bazel. But maybe it doesn't clear the "obviously wrong" bar?

[lberki]

That also works (@meteorcloudy do we have ErrorProne set up on our public CI?)

I think consistent formatting of floats and the like (to the extent that Bazel emits floats, I don't think it happens a lot) is a positive development, isn't it? And it would remove a whole class of possible bugs; output is one thing, but I'm worried that we are also parsing things in a locale-dependent way right now which isn't great.

Ultimately, I'm fine with either approach, it's just that sanitizing the server environment seems to bring the most back for the buck in terms of bugs fixed and possible bugs eliminated.

[fmeum]

@cushon Has an ErrorProne check for locale-less String#to{Lower,Upper}Case() been considered in the past?

[cushon]

@cushon Has an ErrorProne check for locale-less String#to{Lower,Upper}Case() been considered in the past?

I filed google/error-prone#3809

[keertk]

@bazel-io fork 6.2.0

[fmeum]

Superseded by #17702