From da233fe81f9d9e793461b1da07c9ca4f52a06a53 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Fri, 2 Dec 2022 16:18:10 +0100 Subject: [PATCH] Suggestions --- .../errorprone/bugpatterns/SpecifyLocale.java | 30 ++++++------- .../bugpatterns/SpecifyLocaleTest.java | 43 +++++++++++++------ 2 files changed, 43 insertions(+), 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 index e57e8b45f72..ac4a7e720dd 100644 --- 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 STRING_TO_UPPER_OR_LOWER_CASE = + private static final Matcher 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 index 15ddd602392..613a13947af 100644 --- 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);