Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discourage APIs locale-dependent APIs like String.to{Lower,Upper}Case #3809

Closed
cushon opened this issue Mar 8, 2023 · 4 comments
Closed

Comments

@cushon
Copy link
Collaborator

cushon commented Mar 8, 2023

Inspired by bazelbuild/bazel#17687

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).

We have an internal refaster template that suggests com.google.common.base.Ascii, so we'd get this for free from #649

@fmeum
Copy link

fmeum commented Mar 8, 2023

@cushon Given that #649 has an unclear future, would you accept contributions for this?

@cushon
Copy link
Collaborator Author

cushon commented Mar 8, 2023

Yes, absolutely

@rickie
Copy link
Contributor

rickie commented Mar 8, 2023

In Error Prone Support we have the StringCaseLocaleUsage check (link) exactly for this purpose.

We made it an Error Prone check, instead of a Refaster template, to show both Locale.ROOT and Locale.getDefault() in the suggested fix. We don't use the option to have multiple after-templates per Refaster rule.

We are definitely open to contributing this rule upstream (possibly with some tweaks to the BugPattern for e.g. the order of suggested fixes?). Would that be an option?

(Look here to see some extra documentation on the check.)

rickie added a commit to PicnicSupermarket/error-prone that referenced this issue Mar 9, 2023
rickie added a commit to PicnicSupermarket/error-prone that referenced this issue Mar 9, 2023
@rickie
Copy link
Contributor

rickie commented Mar 9, 2023

Filed #3813.

copybara-service bot pushed a commit that referenced this issue Mar 10, 2023
Fixes #3809.

Fixes #3813

FUTURE_COPYBARA_INTEGRATE_REVIEW=#3813 from PicnicSupermarket:rossendrijver/StringCaseLocaleUsage 59abfd4
PiperOrigin-RevId: 515721685
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants