From 9b2c2a9fc8dcea7c40ec6574c6325dc82f534abd Mon Sep 17 00:00:00 2001 From: ghm Date: Wed, 27 Mar 2024 05:18:37 -0700 Subject: [PATCH] Yoda conditions: consider static fields which are in `SHOUTY_CASE` to be constant. Motivated by `Boolean.TRUE`, but that seemed a bit special-casey. PiperOrigin-RevId: 619497318 --- .../errorprone/bugpatterns/YodaCondition.java | 10 ++++- .../bugpatterns/YodaConditionTest.java | 44 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/YodaCondition.java b/core/src/main/java/com/google/errorprone/bugpatterns/YodaCondition.java index 6ccfcb2cedb..7a2055d6932 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/YodaCondition.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/YodaCondition.java @@ -25,6 +25,7 @@ import static com.google.errorprone.util.ASTHelpers.getNullnessValue; import static com.google.errorprone.util.ASTHelpers.getReceiver; import static com.google.errorprone.util.ASTHelpers.getSymbol; +import static com.google.errorprone.util.ASTHelpers.isStatic; import static java.lang.String.format; import com.google.errorprone.BugPattern; @@ -41,6 +42,7 @@ import com.sun.source.tree.Tree; import com.sun.tools.javac.code.Symbol.VarSymbol; import java.util.Objects; +import java.util.regex.Pattern; /** See the summary. */ @BugPattern( @@ -119,6 +121,12 @@ private static boolean seemsConstant(Tree tree) { return true; } var symbol = getSymbol(tree); - return symbol instanceof VarSymbol && symbol.isEnum(); + if (!(symbol instanceof VarSymbol)) { + return false; + } + return symbol.isEnum() + || (isStatic(symbol) && CONSTANT_CASE.matcher(symbol.getSimpleName().toString()).matches()); } + + private static final Pattern CONSTANT_CASE = Pattern.compile("[A-Z0-9_]+"); } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/YodaConditionTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/YodaConditionTest.java index 3a6cc6e4bc8..eaf08537c49 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/YodaConditionTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/YodaConditionTest.java @@ -53,6 +53,50 @@ public void primitive() { .doTest(); } + @Test + public void boxedBoolean() { + refactoring + .addInputLines( + "Test.java", + "class Test {", + " boolean yoda(Boolean a) {", + " return Boolean.TRUE.equals(a);", + " }", + "}") + .addOutputLines( + "Test.java", + "import java.util.Objects;", + "class Test {", + " boolean yoda(Boolean a) {", + " return Objects.equals(a, Boolean.TRUE);", + " }", + "}") + .doTest(); + } + + @Test + public void boxedVsUnboxedBoolean() { + refactoring + .addInputLines( + "Test.java", + "class Test {", + " boolean yoda(boolean a) {", + " return Boolean.TRUE.equals(a);", + " }", + "}") + .addOutputLines( + "Test.java", + "class Test {", + " boolean yoda(boolean a) {", + // NOTE: this is a broken fix! We could detect this if it turns out to be an issue in + // practice. + " return a.equals(Boolean.TRUE);", + " }", + "}") + .allowBreakingChanges() + .doTest(); + } + @Test public void enums() { refactoring