Suggestions

PicnicSupermarket · Dec 2, 2022 · a43b876 · a43b876
1 parent fd70586
commit a43b876
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 20 deletions.
diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SpecifyLocale.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SpecifyLocale.java
@@ -1,8 +1,8 @@
 package tech.picnic.errorprone.bugpatterns;
 
 import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
-import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
-import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR;
+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;
 
@@ -20,19 +20,20 @@
 import tech.picnic.errorprone.bugpatterns.util.SourceCode;
 
 /**
- * A {@link BugChecker} that flags {@link String#toLowerCase()} or {@link String#toUpperCase()}
- * which do not specify a {@code Locale}.
+ * 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.
 @AutoService(BugChecker.class)
 @BugPattern(
     summary = "Specify a `Locale` when calling `String#to{Lower,Upper}Case`",
     link = BUG_PATTERNS_BASE_URL + "SpecifyLocale",
     linkType = CUSTOM,
-    severity = SUGGESTION,
-    tags = LIKELY_ERROR)
+    severity = WARNING,
+    tags = FRAGILE_CODE)
 public final class SpecifyLocale extends BugChecker implements MethodInvocationTreeMatcher {
   private static final long serialVersionUID = 1L;
-  private static final Matcher<ExpressionTree> STRING_TO_LOWER_OR_UPPER_CASE =
+  private static final Matcher<ExpressionTree> DEFAULT_LOCALE_CASE_CONVERSION =
       instanceMethod()
           .onExactClass(String.class.getName())
           .namedAnyOf("toLowerCase", "toUpperCase")
@@ -43,20 +44,20 @@ public SpecifyLocale() {}
 
   @Override
   public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
-    if (!STRING_TO_LOWER_OR_UPPER_CASE.matches(tree, state)) {
+    if (!DEFAULT_LOCALE_CASE_CONVERSION.matches(tree, state)) {
       return Description.NO_MATCH;
     }
 
     return buildDescription(tree)
-        .addFix(suggestLocale("Locale.ROOT", tree, state))
-        .addFix(suggestLocale("Locale.getDefault()", tree, state))
+        .addFix(suggestLocale(tree, "Locale.ROOT", state))
+        .addFix(suggestLocale(tree, "Locale.getDefault()", state))
         .build();
   }
 
-  private static Fix suggestLocale(String locale, MethodInvocationTree tree, VisitorState state) {
+  private static Fix suggestLocale(MethodInvocationTree tree, String locale, VisitorState state) {
     return SuggestedFix.builder()
         .addImport("java.util.Locale")
-        .replace(tree, SourceCode.treeToString(tree, state).replace("()", "(" + locale + ")"))
+        .replace(tree, SourceCode.treeToString(tree, state).replaceFirst("\\(", '(' + locale))
         .build();
   }
 }
diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SpecifyLocaleTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SpecifyLocaleTest.java
@@ -38,14 +38,12 @@ void identification() {
             "",
             "    // 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();",
@@ -60,10 +58,9 @@ void replacementFirstSuggestedFix() {
         .setFixChooser(FixChoosers.FIRST)
         .addInputLines(
             "A.java",
-            "",
             "class A {",
             "  void m() {",
-            "    \"a\".toLowerCase();",
+            "    \"a\".toLowerCase(/* Comment with parens: (). */ );",
             "    \"b\".toUpperCase();",
             "  }",
             "}")
@@ -73,7 +70,7 @@ void replacementFirstSuggestedFix() {
             "",
             "class A {",
             "  void m() {",
-            "    \"a\".toLowerCase(Locale.ROOT);",
+            "    \"a\".toLowerCase(Locale.ROOT /* Comment with parens: (). */);",
             "    \"b\".toUpperCase(Locale.ROOT);",
             "  }",
             "}")
@@ -86,11 +83,10 @@ void replacementSecondSuggestedFix() {
         .setFixChooser(FixChoosers.SECOND)
         .addInputLines(
             "A.java",
-            "",
             "class A {",
             "  void m() {",
             "    \"a\".toLowerCase();",
-            "    \"b\".toUpperCase();",
+            "    \"b\".toUpperCase(/* Comment with parens: (). */ );",
             "  }",
             "}")
         .addOutputLines(
@@ -100,7 +96,7 @@ void replacementSecondSuggestedFix() {
             "class A {",
             "  void m() {",
             "    \"a\".toLowerCase(Locale.getDefault());",
-            "    \"b\".toUpperCase(Locale.getDefault());",
+            "    \"b\".toUpperCase(Locale.getDefault() /* Comment with parens: (). */);",
             "  }",
             "}")
         .doTest(TestMode.TEXT_MATCH);