Case-insensitive comparisons may be adversely affected by the user's locale

[philwebb]

We should align with Spring Framework (see spring-projects/spring-framework#33708). We should also add a checkstyle rule (spring-projects/spring-framework@0a64591)

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

[wilkinsona]

We might be able to check more precisely using ArchUnit.

[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 <org.springframework.boot.build.architecture.string.toUpperCase.ToUpperCase.exampleMethod()> calls method <java.lang.String.toUpperCase()> in (ToUpperCase.java:23)

[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?

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