diff options
author | Manu Sridharan <msridhar@gmail.com> | 2024-01-25 09:49:35 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-01-25 09:49:35 -0800 |
commit | b94597a826d28dd36364f4dff903541910465d28 (patch) | |
tree | 01105329222bfeed020479070d16d6730b227f4e | |
parent | c1eb8cae63cd697e7b124b6bfa43130826c80bb6 (diff) | |
download | nullaway-b94597a826d28dd36364f4dff903541910465d28.tar.gz |
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.
-rw-r--r-- | nullaway/src/main/java/com/uber/nullaway/NullAway.java | 24 | ||||
-rw-r--r-- | nullaway/src/test/java/com/uber/nullaway/NullAwayCoreTests.java | 16 |
2 files changed, 36 insertions, 4 deletions
diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 3bf0a85..a23438a 100644 --- 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 static com.uber.nullaway.NullabilityUtil.castToNonNull; 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 class NullAway extends BugChecker 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 index dd6e0d7..3649de9 100644 --- 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 class NullAwayCoreTests extends NullAwayTestsBase { "}") .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(); + } } |