Skip to content

Commit

Permalink
Introduce StringCaseLocaleUsage check (#376)
Browse files Browse the repository at this point in the history
  • Loading branch information
mlrprananta authored Dec 5, 2022
1 parent 066591c commit 4f9aba8
Show file tree
Hide file tree
Showing 2 changed files with 174 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package tech.picnic.errorprone.bugpatterns;

import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.FRAGILE_CODE;
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;

import com.google.auto.service.AutoService;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.fixes.Fix;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;

/**
* A {@link BugChecker} that flags calls to {@link String#toLowerCase()} and {@link
* String#toUpperCase()}, as these methods implicitly rely on the environment's default locale.
*/
// XXX: Also flag `String::toLowerCase` and `String::toUpperCase` method references. For these cases
// the suggested fix should introduce a lambda expression with a parameter of which the name does
// not coincide with the name of an existing variable name. Such functionality should likely be
// introduced in a utility class.
@AutoService(BugChecker.class)
@BugPattern(
summary = "Specify a `Locale` when calling `String#to{Lower,Upper}Case`",
link = BUG_PATTERNS_BASE_URL + "StringCaseLocaleUsage",
linkType = CUSTOM,
severity = WARNING,
tags = FRAGILE_CODE)
public final class StringCaseLocaleUsage extends BugChecker implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> DEFAULT_LOCALE_CASE_CONVERSION =
instanceMethod()
.onExactClass(String.class.getName())
.namedAnyOf("toLowerCase", "toUpperCase")
.withNoParameters();

/** Instantiates a new {@link StringCaseLocaleUsage} instance. */
public StringCaseLocaleUsage() {}

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!DEFAULT_LOCALE_CASE_CONVERSION.matches(tree, state)) {
return Description.NO_MATCH;
}

return buildDescription(tree)
.addFix(suggestLocale(tree, "Locale.ROOT", state))
.addFix(suggestLocale(tree, "Locale.getDefault()", state))
.build();
}

private static Fix suggestLocale(MethodInvocationTree tree, String locale, VisitorState state) {
// XXX: The logic that replaces the first parenthesis assumes that `tree` does not have a source
// code representation such as `str.toLowerCase/* Some comment with parens (). */()`. In such a
// case the comment, rather than the method invocation arguments, will be modified. Implement a
// generic solution for this.
return SuggestedFix.builder()
.addImport("java.util.Locale")
.replace(tree, SourceCode.treeToString(tree, state).replaceFirst("\\(", '(' + locale))
.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package tech.picnic.errorprone.bugpatterns;

import static com.google.errorprone.BugCheckerRefactoringTestHelper.newInstance;

import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers;
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;

final class StringCaseLocaleUsageTest {
private final CompilationTestHelper compilationTestHelper =
CompilationTestHelper.newInstance(StringCaseLocaleUsage.class, getClass());
private final BugCheckerRefactoringTestHelper refactoringTestHelper =
newInstance(StringCaseLocaleUsage.class, getClass());

@Test
void identification() {
compilationTestHelper
.addSourceLines(
"A.java",
"import static java.util.Locale.ROOT;",
"",
"import java.util.Locale;",
"",
"class A {",
" void m() {",
" \"a\".toLowerCase(Locale.ROOT);",
" \"a\".toUpperCase(Locale.ROOT);",
" \"b\".toLowerCase(ROOT);",
" \"b\".toUpperCase(ROOT);",
" \"c\".toLowerCase(Locale.getDefault());",
" \"c\".toUpperCase(Locale.getDefault());",
" \"d\".toLowerCase(Locale.ENGLISH);",
" \"d\".toUpperCase(Locale.ENGLISH);",
" \"e\".toLowerCase(new Locale(\"foo\"));",
" \"e\".toUpperCase(new Locale(\"foo\"));",
"",
" // BUG: Diagnostic contains:",
" \"f\".toLowerCase();",
" // BUG: Diagnostic contains:",
" \"g\".toUpperCase();",
"",
" String h = \"h\";",
" // BUG: Diagnostic contains:",
" h.toLowerCase();",
" String i = \"i\";",
" // BUG: Diagnostic contains:",
" i.toUpperCase();",
" }",
"}")
.doTest();
}

@Test
void replacementFirstSuggestedFix() {
refactoringTestHelper
.setFixChooser(FixChoosers.FIRST)
.addInputLines(
"A.java",
"class A {",
" void m() {",
" \"a\".toLowerCase(/* Comment with parens: (). */ );",
" \"b\".toUpperCase();",
" }",
"}")
.addOutputLines(
"A.java",
"import java.util.Locale;",
"",
"class A {",
" void m() {",
" \"a\".toLowerCase(Locale.ROOT /* Comment with parens: (). */);",
" \"b\".toUpperCase(Locale.ROOT);",
" }",
"}")
.doTest(TestMode.TEXT_MATCH);
}

@Test
void replacementSecondSuggestedFix() {
refactoringTestHelper
.setFixChooser(FixChoosers.SECOND)
.addInputLines(
"A.java",
"class A {",
" void m() {",
" \"a\".toLowerCase();",
" \"b\".toUpperCase(/* Comment with parens: (). */ );",
" }",
"}")
.addOutputLines(
"A.java",
"import java.util.Locale;",
"",
"class A {",
" void m() {",
" \"a\".toLowerCase(Locale.getDefault());",
" \"b\".toUpperCase(Locale.getDefault() /* Comment with parens: (). */);",
" }",
"}")
.doTest(TestMode.TEXT_MATCH);
}
}

0 comments on commit 4f9aba8

Please sign in to comment.