Handle null != CONST_CASE in YodaCondition

cushon · Error Prone Team · commit 13be411511ab · 2024-05-22T14:07:33.000-07:00
PiperOrigin-RevId: 636287602
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/YodaCondition.java b/core/src/main/java/com/google/errorprone/bugpatterns/YodaCondition.java
@@ -95,7 +95,7 @@ private Description fix(
       ExpressionTree rhs,
       boolean provideNullSafeFix,
       VisitorState state) {
-    if (seemsConstant(lhs) && !seemsConstant(rhs)) {
+    if (yodaCondition(lhs, rhs)) {
       var description = buildDescription(lhs);
       if (provideNullSafeFix
           && !getNullnessValue(rhs, state, NullnessAnalysis.instance(state.context))
@@ -116,19 +116,40 @@ private Description fix(
     return NO_MATCH;
   }
 
-  private static boolean seemsConstant(Tree tree) {
+  private static boolean yodaCondition(ExpressionTree lhs, ExpressionTree rhs) {
+    ConstantKind l = seemsConstant(lhs);
+    ConstantKind r = seemsConstant(rhs);
+    return l.constness > r.constness;
+  }
+
+  enum ConstantKind {
+    NULL(3),
+    CONSTANT(2),
+    CONSTANT_VARIABLE(1),
+    NON_CONSTANT(0);
+
+    final int constness;
+
+    ConstantKind(int constness) {
+      this.constness = constness;
+    }
+  }
+
+  private static ConstantKind seemsConstant(Tree tree) {
     if (constValue(tree) != null) {
-      return true;
+      return ConstantKind.CONSTANT;
     }
     if (tree.getKind().equals(Tree.Kind.NULL_LITERAL)) {
-      return true;
+      return ConstantKind.NULL;
     }
     var symbol = getSymbol(tree);
-    if (!(symbol instanceof VarSymbol)) {
-      return false;
+    if (symbol instanceof VarSymbol
+        && (symbol.isEnum()
+            || (isStatic(symbol)
+                && CONSTANT_CASE.matcher(symbol.getSimpleName().toString()).matches()))) {
+      return ConstantKind.CONSTANT_VARIABLE;
     }
-    return symbol.isEnum()
-        || (isStatic(symbol) && CONSTANT_CASE.matcher(symbol.getSimpleName().toString()).matches());
+    return ConstantKind.NON_CONSTANT;
   }
 
   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
@@ -223,4 +223,32 @@ public void nullableConstant() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void nullableYodaCondition() {
+    refactoring
+        .addInputLines(
+            "Test.java",
+            "class Test {",
+            "  private static final String CONST = \"hello\";",
+            "  public static boolean f(String foo) {",
+            "    return null != foo;",
+            "  }",
+            "  public static boolean g() {",
+            "    return null != CONST;",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "class Test {",
+            "  private static final String CONST = \"hello\";",
+            "  public static boolean f(String foo) {",
+            "    return foo != null;",
+            "  }",
+            "  public static boolean g() {",
+            "    return CONST != null;",
+            "  }",
+            "}")
+        .doTest();
+  }
 }
