NullAway icon indicating copy to clipboard operation
NullAway copied to clipboard

LibraryModels should model Objects.equals()

Open agrieve opened this issue 11 months ago • 3 comments

void method(@NonNull Object a, @Nullable Object b) {
  if (Objects.equals(a, b)) {
    // b must be non-null here.
  }
}

I think the contract for this is:

@Contract("!null, null -> false")
@Contract("null, !null -> false")

But I don't see a method in LibraryModels that aligns with this contract (I also haven't tested that this contract works).

agrieve avatar Jan 29 '25 16:01 agrieve

Checked the same example using a == b, and no warning in that case.

agrieve avatar Feb 19 '25 15:02 agrieve

@agrieve any chance you could test that the @Contract annotations you wrote work as expected if Object.equals were implemented in source? I agree that special-casing Object.equals is worthwhile and I can try to look into it.

msridhar avatar Feb 19 '25 15:02 msridhar

No problem:

This works:

import org.jspecify.annotations.Nullable;
import org.jspecify.annotations.NullMarked;
import org.chromium.build.annotations.Contract;

@NullMarked
public class Foo {

  static boolean isSame(@Nullable Object a, @Nullable Object b) {
    return a == b;
  }

  void method(Object a, @Nullable Object b) {
    if (isSame(a, b)) {
      b.hashCode();
    }
  }
}

Adding a second @Contract gives:

Foo.java:9: error: Contract is not a repeatable annotation interface
  @Contract("null, !null -> false")
  ^

If I change @Contract to be @Repeatable(Contracts.class), and add:

@Target(ElementType.METHOD)
@Retention(RetentionPolicy.CLASS)
public @interface Contracts {
    Contract[] value();
}

Then the multiple annotations have the same result as no annotations.

agrieve avatar Feb 19 '25 16:02 agrieve