No way to mark a type whose equals() is the wrong comparison, so ReferenceEquality suggests the buggy fix

[vlsi]

In Apache Calcite, SqlCollation.equals compares only the collation name and ignores coercibility, so SqlCollation.IMPLICIT.equals(SqlCollation.COERCIBLE) returns true. Code that has to tell the two apart must compare collations with ==, and apache/calcite#5281 does that in BasicSqlType.createWithCharsetAndCollation. I want the build to flag a later edit that turns that == into equals or Objects.equals. Error Prone 2.50.0 does the opposite: ReferenceEquality flags the == and suggests Objects.equals(a, b), which is exactly the edit that introduces the bug. Nothing flags equals, Objects.equals, or assertEquals on the type.

Reproducer

A stand-in for SqlCollation, with no Calcite dependency:

package demo;

/** Two collations are equal when their names match; coercibility is ignored. */
public final class Collation {
  public enum Coercibility { IMPLICIT, COERCIBLE }

  public static final Collation IMPLICIT = new Collation("en_US", Coercibility.IMPLICIT);
  public static final Collation COERCIBLE = new Collation("en_US", Coercibility.COERCIBLE);

  private final String name;
  private final Coercibility coercibility;

  public Collation(String name, Coercibility coercibility) {
    this.name = name;
    this.coercibility = coercibility;
  }

  public Coercibility coercibility() {
    return coercibility;
  }

  @Override
  public boolean equals(Object o) {
    return o instanceof Collation that && that.name.equals(name);
  }

  @Override
  public int hashCode() {
    return name.hashCode();
  }
}

package demo;

import java.util.Objects;
import javax.lang.model.type.TypeMirror;

class Usage {
  static boolean identity(Collation a, Collation b) {
    return a == b;
  }

  static boolean instanceEquals(Collation a, Collation b) {
    return a.equals(b);
  }

  static boolean staticEquals(Collation a, Collation b) {
    return Objects.equals(a, b);
  }

  static void junitAssert(Collation a, Collation b) {
    org.junit.Assert.assertEquals(a, b);
  }

  static boolean typeMirrorStaticEquals(TypeMirror a, TypeMirror b) {
    return Objects.equals(a, b);
  }

  static void typeMirrorAssert(TypeMirror a, TypeMirror b) {
    org.junit.Assert.assertEquals(a, b);
  }

  public static void main(String[] args) {
    System.out.println(Collation.IMPLICIT.equals(Collation.COERCIBLE));
  }
}

Compiled with Error Prone 2.50.0 on JDK 21.0.9, through the net.ltgt.errorprone Gradle plugin 4.3.0 with default check settings and JUnit 4.13.2 on the classpath:

src/main/java/demo/Usage.java:8: warning: [ReferenceEquality] Comparison using reference equality instead of value equality
    return a == b;
             ^
    (see https://errorprone.info/bugpattern/ReferenceEquality)
  Did you mean 'return Objects.equals(a, b);' or 'return a.equals(b);'?
src/main/java/demo/Usage.java:24: warning: [TypeEquals] TypeMirror should be compared using Types#isSameType, not equality operators or equals().
    return Objects.equals(a, b);
                         ^
    (see https://errorprone.info/bugpattern/TypeEquals)
2 warnings

java -cp build/classes/java/main demo.Usage prints true, so both suggested replacements for line 8 change the result. Lines 12, 16, and 20 compare Collation with equals and produce no finding.

The pattern exists already, for types chosen by Error Prone

Several checks already say "this type's equals is not the comparison you want, use X instead". Each one has its types written into the checker:

Check	Type	Use instead
TypeEquals	javax.lang.model.type.TypeMirror	Types#isSameType
BigDecimalEquals	java.math.BigDecimal	compareTo
ArrayEquals	arrays	Arrays.equals
UndefinedEquals, CollectionUndefinedEquality	the list in TypesWithUndefinedEquality: Collection, Iterable, Multimap, CharSequence, Future, Date, and others	a Truth containsExactly… assertion, or toString() for CharSequence

Each check also matches its own set of call shapes. In the run above, TypeEquals reports Objects.equals on TypeMirror (line 24) and does not report assertEquals (line 28), while UndefinedEquals reports the same assertEquals call on two Collection arguments and also matches Truth's isEqualTo. BigDecimalEquals skips calls inside an equals implementation, and the others do not.

A library type can have the same property. Guava's Range.equals compares the representation, so its Javadoc notes that [3..3) and (3..3] are unequal.

What I am asking for

A way for a library to declare on its own type that equals is not the comparison callers should use, and what to use instead. The name and shape are yours to choose. One possible shape:

@ComparedBy(value = "==", explanation = "equals() ignores coercibility")
public class SqlCollation { ... }

It would work if, for a type carrying the annotation:

1. a.equals(b), Objects.equals(a, b), JUnit assertEquals(a, b), and Truth assertThat(a).isEqualTo(b) produce a finding whose message names the declared replacement, when either operand's static type is the annotated type.
2. ReferenceEquality does not report a == b when the declared replacement is reference comparison.
3. Calls inside the type's own equals and hashCode are not reported, as BigDecimalEquals does today.

With that in place, TypeEquals, BigDecimalEquals, and TypesWithUndefinedEquality could become built-in entries of the same mechanism and share one set of call shapes. Whether to migrate them is a separate decision.

Not in scope: equals calls made by hash-based collections and Map.get, and any change to what ReferenceEquality reports for types without the annotation.

Prior discussion

• Introduce @ReferenceEqualitySafe class annotation to avoid triggering ReferenceEquality check #2818 asked for @ReferenceEqualitySafe to silence ReferenceEquality on classes whose instances are controlled, and was closed. The reply asked whether there is a reason to prefer reference equality. This case gives one: equals returns the wrong answer.
• Calling equals() on a reference of exact static type Iterable, Collection, Queue or Multimap #340 introduced UndefinedEquals with a fixed list of types.

I searched the issues for identity equals annotation, ReferenceEquality annotation, UndefinedEquals configurable, TypeEquals, BigDecimalEquals, and TypesWithUndefinedEquality, and found no request to make these checks extensible.

Alternatives

• Change SqlCollation.equals to include coercibility. That is for the Calcite maintainers to decide, since type canonicalization may depend on the current behavior, and I have not checked whether it does. It also does not help a type whose equals is fixed by a published contract.
• @SuppressWarnings("ReferenceEquality") on the method with the ==. It removes the wrong suggestion at that one place. The next equals call on the type elsewhere is still not flagged.
• @RestrictedApi on the equals override. I have not tried it. At best it catches direct a.equals(b) calls, because Objects.equals and assertEquals call Object.equals.
• A project-local BugChecker. It works for the project that writes it. Each project then writes the same checker, and ReferenceEquality still reports the correct ==.

[Gautam-aman]

I'd like to work on this issue. I plan to explore the existing equality checkers and implement an annotation-based solution with appropriate diagnostics and regression tests. Would it be okay if I took this up?