Check `equals()` implementations for `instanceof SomeUnrelatedType`

[Chordrain]

[cpovirk notes: The original issue title was: EqualsWrongThing: false negatives in equals() implementations of nested classes]

Opener

We are conducting a broader study on the behavior of static analyzers. As part of this, we tested Error Prone and found some cases where the diagnostics seemed surprising. We are not sure whether these are intended design decisions or potential issues, so we’d like to share them here for clarification. Sorry for the noise if it's a false alarm.

---

Summary

The EqualsWrongThing checker reports suspicious field and accessor comparisons in equals() methods of top-level classes, but fails to do so when the same comparisons appear in equals() implementations of nested classes or when such methods depend on members of nested classes. This leads to false negatives.

---

Reproducer Cases

Error Prone version: 2.41.0

Case 1a (diagnostic as expected ✅)

class Test {
  private int a;
  private int b;

  @Override
  public boolean equals(Object o) {
    Test that = (Test) o;
    // BUG: Diagnostic contains: comparison between `a` and `b`
    return a == that.b && b == that.b;
  }
}

Case 1b (false negative ❌)

class Test {
  private int a;
  private int b;

  class NestedClass {
    @Override
    public boolean equals(Object o) {
      Test that = (Test) o;
      // BUG: Diagnostic contains: comparison between `a` and `b`
      return a == that.b && b == that.b;
    }
  }
}

---

Case 2a (diagnostic as expected ✅)

class Test {
  private Object a;
  private Object b;

  @Override
  public boolean equals(Object o) {
    Test that = (Test) o;
    // BUG: Diagnostic contains: comparison between `a` and `b`
    return a.equals(that.b) && b == that.b;
  }
}

Case 2b (false negative ❌)

class Test {
  private Object a;
  private Object b;

  class NestedClass {
    @Override
    public boolean equals(Object o) {
      Test that = (Test) o;
      // BUG: Diagnostic contains: comparison between `a` and `b`
      return a.equals(that.b) && b == that.b;
    }
  }
}

---

Case 3a (diagnostic as expected ✅)

class Test {
  private int a;
  private int b;

  private int getA() { return a; }
  private int getB() { return b; }

  @Override
  public boolean equals(Object o) {
    Test that = (Test) o;
    // BUG: Diagnostic contains: comparison between `getA()` and `getB()`
    return getA() == that.getB() && getB() == that.getB();
  }
}

Case 3b (false negative ❌)

class Test {
  private int a;
  private int b;

  private int getB() { return b; }

  @Override
  public boolean equals(Object o) {
    Test that = (Test) o;
    // BUG: Diagnostic contains: comparison between `getA()` and `getB()`
    return getA() == that.getB() && getB() == that.getB();
  }

  class NestedClass {
    private int getA() { return a; }
  }
}

---

Observed Behavior

• Diagnostics are emitted for suspicious comparisons in top-level classes (1a, 2a, 3a).
• No diagnostics are emitted for equivalent code in nested class contexts (1b, 2b, 3b).

Expected Behavior

• Diagnostics should also be reported for nested class cases, since the comparisons remain semantically suspicious and equally indicative of copy–paste errors.

[cpovirk]

I haven't tested, but my guess would be that the difference here isn't whether the class is nested but whether the cast in the equals method is to an unrelated class: Most of the examples show NestedClass.equals with a cast to Test, which violates the equals contract. Perhaps Error Prone should check for that, too, but it's a different sort of problem.

[graememorgan]

I suspect Chris is right. We're also generally a lot more motivated by false negatives that have been seen in reality. A nested class comparing fields of its outer class in equals seems rather unusual.

[Chordrain]

Thank you for your reply! I’d like to ask whether you consider this finding worth attention, and if it will be addressed as a bug in Error Prone.

[cpovirk]

If we were to do something here, my guess is that it would be to write a new check that would look for equals implementations that check obj instanceof SomeUnrelatedClass. I could see how that could come up when someone copies and pastes an equals method from one class to another. I'll retitle this bug as a feature request.