We should align with Spring Framework (see https://github.com/spring-projects/spring-framework/issues/33708). We should also add a checkstyle rule (https://github.com/spring-projects/spring-framework/commit/0a645916cd78de333482cf68329864618562fb98)
Comment From: bclozel
I've had a look at creating a custom check for spring-javaformat - in the end, it's not really far from a "single line regexp check" because the AST doesn't give any type information about the instance nor arguments. Checking for the method name and argument count and heuristics is the best we can do.
Comment From: wilkinsona
We might be able to check more precisely using ArchUnit.
Comment From: nosan
I tried ArchUnit in my branch and it works fine.
private ArchRule noClassesShouldCallStringToLowerCaseWithoutLocale() {
return ArchRuleDefinition.noClasses()
.should()
.callMethod(String.class, "toLowerCase")
.because("String.toLowerCase(Locale.ROOT) should be used instead");
}
private ArchRule noClassesShouldCallStringToUpperCaseWithoutLocale() {
return ArchRuleDefinition.noClasses()
.should()
.callMethod(String.class, "toUpperCase")
.because("String.toUpperCase(Locale.ROOT) should be used instead");
}
Architecture Violation [Priority: MEDIUM] - Rule 'no classes should call method String.toUpperCase(), because String.toUpperCase(Locale.ROOT) should be used instead' was violated (1 times): Method
calls method in (ToUpperCase.java:23)
Comment From: rickie
Another tool that could've helped with flagging this issue is Error Prone. It's a compiler plugin that, in addition to pointing out problems, can also automatically apply the fixes (patch mode), so no manual change is required.
For this particular case, there is the StringCaseLocaleUsage check that flags such cases. To see the full list of checks you can go to the docs: https://errorprone.info/bugpatterns.
Is using Error Prone something the Spring team would find interesting to consider and explore?
Comment From: philwebb
Thanks @rickie, error prone has been on the radar for a while, but we haven't yet found the time to integrate it. We'll need it when we tackle #10712 and at that point will probably evaluate more checks.