Suggestions

PicnicSupermarket · Dec 2, 2022 · da233fe · da233fe
1 parent 79ce593
commit da233fe
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 30 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,7 +1,7 @@
 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.SeverityLevel.SUGGESTION;
 import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR;
 import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
 import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
@@ -21,19 +21,18 @@
 
 /**
  * A {@link BugChecker} that flags {@link String#toLowerCase()} or {@link String#toUpperCase()}
- * which do not specify a Locale.
+ * which do not specify a {@code Locale}.
  */
 @AutoService(BugChecker.class)
 @BugPattern(
-    summary =
-        "Specify `Locale.ROOT` or `Locale.getDefault()` when calling `String#to{Lower,Upper}Case` without a specific Locale",
+    summary = "Specify a `Locale` when calling `String#to{Lower,Upper}Case`",
     link = BUG_PATTERNS_BASE_URL + "SpecifyLocale",
     linkType = CUSTOM,
-    severity = WARNING,
+    severity = SUGGESTION,
     tags = LIKELY_ERROR)
 public final class SpecifyLocale extends BugChecker implements MethodInvocationTreeMatcher {
   private static final long serialVersionUID = 1L;
-  private static final Matcher<ExpressionTree> STRING_TO_UPPER_OR_LOWER_CASE =
+  private static final Matcher<ExpressionTree> STRING_TO_LOWER_OR_UPPER_CASE =
       instanceMethod()
           .onExactClass(String.class.getName())
           .namedAnyOf("toLowerCase", "toUpperCase")
@@ -44,21 +43,20 @@ public SpecifyLocale() {}
 
   @Override
   public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
-    if (STRING_TO_UPPER_OR_LOWER_CASE.matches(tree, state)) {
-      return buildDescription(tree)
-          .addFix(buildFix("Locale.ROOT", tree, state))
-          .addFix(buildFix("Locale.getDefault()", tree, state))
-          .build();
+    if (!STRING_TO_LOWER_OR_UPPER_CASE.matches(tree, state)) {
+      return Description.NO_MATCH;
     }
-    return Description.NO_MATCH;
+
+    return buildDescription(tree)
+        .addFix(suggestLocale("Locale.ROOT", tree, state))
+        .addFix(suggestLocale("Locale.getDefault()", tree, state))
+        .build();
   }
 
-  private static Fix buildFix(
-      String localeToSpecify, MethodInvocationTree tree, VisitorState state) {
+  private static Fix suggestLocale(String locale, MethodInvocationTree tree, VisitorState state) {
     return SuggestedFix.builder()
-        .replace(
-            tree, SourceCode.treeToString(tree, state).replace("()", "(" + localeToSpecify + ")"))
         .addImport("java.util.Locale")
+        .replace(tree, SourceCode.treeToString(tree, state).replace("()", "(" + 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
@@ -19,21 +19,36 @@ 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:",
-            "    \"a\".toUpperCase();",
+            "    \"f\".toLowerCase();",
             "",
             "    // BUG: Diagnostic contains:",
-            "    \"b\".toLowerCase();",
+            "    \"g\".toUpperCase();",
             "",
-            "    String c = \"c\";",
+            "    String h = \"h\";",
             "    // BUG: Diagnostic contains:",
-            "    c.toUpperCase();",
+            "    h.toLowerCase();",
             "",
-            "    String d = \"d\";",
+            "    String i = \"i\";",
             "    // BUG: Diagnostic contains:",
-            "    d.toLowerCase();",
+            "    i.toUpperCase();",
             "  }",
             "}")
         .doTest();
@@ -48,8 +63,8 @@ void replacementFirstSuggestedFix() {
             "",
             "class A {",
             "  void m() {",
-            "    \"a\".toUpperCase();",
-            "    \"b\".toLowerCase();",
+            "    \"a\".toLowerCase();",
+            "    \"b\".toUpperCase();",
             "  }",
             "}")
         .addOutputLines(
@@ -58,8 +73,8 @@ void replacementFirstSuggestedFix() {
             "",
             "class A {",
             "  void m() {",
-            "    \"a\".toUpperCase(Locale.ROOT);",
-            "    \"b\".toLowerCase(Locale.ROOT);",
+            "    \"a\".toLowerCase(Locale.ROOT);",
+            "    \"b\".toUpperCase(Locale.ROOT);",
             "  }",
             "}")
         .doTest(TestMode.TEXT_MATCH);
@@ -74,8 +89,8 @@ void replacementSecondSuggestedFix() {
             "",
             "class A {",
             "  void m() {",
-            "    \"a\".toUpperCase();",
-            "    \"b\".toLowerCase();",
+            "    \"a\".toLowerCase();",
+            "    \"b\".toUpperCase();",
             "  }",
             "}")
         .addOutputLines(
@@ -84,8 +99,8 @@ void replacementSecondSuggestedFix() {
             "",
             "class A {",
             "  void m() {",
-            "    \"a\".toUpperCase(Locale.getDefault());",
-            "    \"b\".toLowerCase(Locale.getDefault());",
+            "    \"a\".toLowerCase(Locale.getDefault());",
+            "    \"b\".toUpperCase(Locale.getDefault());",
             "  }",
             "}")
         .doTest(TestMode.TEXT_MATCH);