Fix bug with implicit equals() methods in interfaces (#898)

Fixes #897 This fixes a regression in our handling of implicit `equals()` methods in interfaces when building on JDK 21. I see this as an interim fix, until we can fix NullAway to properly always assume / enforce that the parameter of `equals()` is `@Nullable`. But, I think we should fix the regression in the short term.
uber · Jan 25, 2024 · b94597a · b94597a
1 parent c1eb8ca
commit b94597a
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 4 deletions.
diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java
@@ -36,6 +36,7 @@
 import com.google.auto.service.AutoService;
 import com.google.auto.value.AutoValue;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Verify;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.ImmutableSet;
@@ -399,16 +400,31 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
     if (!withinAnnotatedCode(state)) {
       return Description.NO_MATCH;
     }
-    final Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(tree);
-    if (methodSymbol == null) {
-      throw new RuntimeException("not expecting unresolved method here");
-    }
+    Symbol.MethodSymbol methodSymbol = getSymbolForMethodInvocation(tree, state);
     handler.onMatchMethodInvocation(this, tree, state, methodSymbol);
     // assuming this list does not include the receiver
     List<? extends ExpressionTree> actualParams = tree.getArguments();
     return handleInvocation(tree, state, methodSymbol, actualParams);
   }
 
+  private static Symbol.MethodSymbol getSymbolForMethodInvocation(
+      MethodInvocationTree tree, VisitorState state) {
+    Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(tree);
+    Verify.verify(methodSymbol != null, "not expecting unresolved method here");
+    // For interface methods, if the method is an implicit method corresponding to a method from
+    // java.lang.Object, use the symbol for the java.lang.Object method instead.  We do this to
+    // properly treat the method as unannotated, which is particularly important for equals()
+    // methods.  This is an adaptation to a change in JDK 18; see
+    // https://bugs.openjdk.org/browse/JDK-8272564
+    if (methodSymbol.owner.isInterface()) {
+      Symbol.MethodSymbol baseSymbol = (Symbol.MethodSymbol) methodSymbol.baseSymbol();
+      if (baseSymbol != methodSymbol && baseSymbol.owner == state.getSymtab().objectType.tsym) {
+        methodSymbol = baseSymbol;
+      }
+    }
+    return methodSymbol;
+  }
+
   @Override
   public Description matchNewClass(NewClassTree tree, VisitorState state) {
     if (!withinAnnotatedCode(state)) {

diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayCoreTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayCoreTests.java
@@ -985,4 +985,20 @@ public void cfgConstructionSymbolCompletionFailure() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void testDefaultEqualsInInterfaceTakesNullable() {
+    defaultCompilationHelper
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import javax.annotation.Nullable;",
+            "class Test {",
+            "  public interface AnInterface {}",
+            "  public static boolean foo(AnInterface a, @Nullable AnInterface b) {",
+            "    return a.equals(b);",
+            "  }",
+            "}")
+        .doTest();
+  }
 }