Improve TruthConstantAsserts to detect more constant-like expressions.

This reuses `ConstantExpressions`, but excludes non-static identifiers, given it's pretty darn common to assert on, say, a variable.

PiperOrigin-RevId: 828437937

Author: graememorgan

core/src/main/java/com/google/errorprone/bugpatterns/TruthConstantAsserts.java

@@ -25,13 +25,18 @@
 import com.google.errorprone.BugPattern.StandardTags;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
+import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions.ConstantExpressionVisitor;
 import com.google.errorprone.fixes.SuggestedFix;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodInvocationTree;
-import java.util.regex.Pattern;
+import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Symbol.MethodSymbol;
+import java.util.concurrent.atomic.AtomicBoolean;
+import javax.inject.Inject;
 
 /**
  * Points out if Truth Library assert is called on a constant.
@@ -47,14 +52,19 @@ public class TruthConstantAsserts extends BugChecker implements MethodInvocation
   private static final Matcher<ExpressionTree> ASSERT_THAT =
       staticMethod().onClass("com.google.common.truth.Truth").named("assertThat");
 
-  private static final Pattern EQ_NEQ = Pattern.compile("isEqualTo|isNotEqualTo");
-
   private static final Matcher<ExpressionTree> TRUTH_SUBJECT_CALL =
       instanceMethod()
           .onDescendantOf("com.google.common.truth.Subject")
-          .withNameMatching(EQ_NEQ)
+          .namedAnyOf("isEqualTo", "isNotEqualTo")
           .withParameters("java.lang.Object");
 
+  private final ConstantExpressions constantExpressions;
+
+  @Inject
+  TruthConstantAsserts(ConstantExpressions constantExpressions) {
+    this.constantExpressions = constantExpressions;
+  }
+
   @Override
   public Description matchMethodInvocation(
       MethodInvocationTree methodInvocationTree, VisitorState state) {
@@ -76,15 +86,35 @@ public Description matchMethodInvocation(
       return Description.NO_MATCH;
     }
     // check that argument of assertThat is a constant
-    if (ASTHelpers.constValue(expr) == null) {
+    if (!constantIsh(expr, state)) {
       return Description.NO_MATCH;
     }
     // check that expectation isn't a constant
     ExpressionTree expectation = getOnlyElement(methodInvocationTree.getArguments());
-    if (ASTHelpers.constValue(expectation) != null) {
+    if (constantIsh(expectation, state)) {
       return Description.NO_MATCH;
     }
     SuggestedFix fix = SuggestedFix.swap(expr, expectation, state);
     return describeMatch(methodInvocationTree, fix);
   }
+
+  private boolean constantIsh(ExpressionTree tree, VisitorState state) {
+    var constant = constantExpressions.constantExpression(tree, state).orElse(null);
+    if (constant == null) {
+      return false;
+    }
+    // Identifiers can be considered constants, but they're exactly what we usually assert on! So
+    // don't consider them to be constants in this context.
+    AtomicBoolean involvesIdentifiers = new AtomicBoolean();
+    constant.accept(
+        new ConstantExpressionVisitor() {
+          @Override
+          public void visitIdentifier(Symbol identifier) {
+            if (!(identifier instanceof MethodSymbol)) {
+              involvesIdentifiers.set(true);
+            }
+          }
+        });
+    return !involvesIdentifiers.get();
+  }
 }

core/src/test/java/com/google/errorprone/bugpatterns/TruthConstantAssertsTest.java

@@ -109,4 +109,24 @@ private static TruthConstantAssertsNegativeCases getObject() {
             """)
         .doTest();
   }
+
+  @Test
+  public void positiveCase_viaImmutableList() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            """
+            import static com.google.common.truth.Truth.assertThat;
+
+            import com.google.common.collect.ImmutableList;
+
+            public class Test {
+              public void test(Object a) {
+                // BUG: Diagnostic contains:
+                assertThat(ImmutableList.of(1, 2, 3)).isEqualTo(a);
+              }
+            }
+            """)
+        .doTest();
+  }
 }