Properly check generic method overriding in explicitly-typed anonymous classes

nullaway/build.gradle

@@ -123,6 +123,8 @@ def jdk8Test = tasks.register("testJdk8", Test) {
     testClassesDirs = testTask.testClassesDirs
     jvmArgs "-Xbootclasspath/p:${configurations.errorproneJavac.asPath}"
     filter {
+        // JDK 8 does not support diamonds on anonymous classes
+        excludeTestsMatching "com.uber.nullaway.NullAwayJSpecifyGenericsTests.overrideDiamondAnonymousClass"
         // tests cannot run on JDK 8 since Mockito version no longer supports it
         excludeTestsMatching "com.uber.nullaway.NullAwaySerializationTest.initializationError"
         excludeTestsMatching "com.uber.nullaway.handlers.contract.ContractUtilsTest.getEmptyAntecedent"

nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java

@@ -1,5 +1,6 @@
 package com.uber.nullaway;
 
+import static com.google.common.base.Verify.verify;
 import static com.uber.nullaway.NullabilityUtil.castToNonNull;
 import static java.util.stream.Collectors.joining;
 
@@ -22,6 +23,7 @@
 import com.sun.source.tree.Tree;
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.SimpleTreeVisitor;
+import com.sun.source.util.TreePath;
 import com.sun.tools.javac.code.Attribute;
 import com.sun.tools.javac.code.BoundKind;
 import com.sun.tools.javac.code.Symbol;
@@ -686,14 +688,53 @@
    * String}.
    *
    * @param method the generic method
-   * @param enclosingType the enclosing type in which we want to know {@code method}'s return type
-   *     nullability
+   * @param enclosingSymbol the enclosing class in which we want to know {@code method}'s return
+   *     type nullability
    * @param state Visitor state
    * @param config The analysis config
    * @return nullability of the return type of {@code method} in the context of {@code
    *     enclosingType}
    */
   public static Nullness getGenericMethodReturnTypeNullness(
+      Symbol.MethodSymbol method, Symbol enclosingSymbol, VisitorState state, Config config) {
+    Type enclosingType = getTypeForSymbol(enclosingSymbol, state);
+    if (enclosingType == null) {
+      // we have no additional information from generics, so return NONNULL (presence of a @Nullable
+      // annotation should have been handled by the caller)
+      return Nullness.NONNULL;
+    }
+    return getGenericMethodReturnTypeNullness(method, enclosingType, state, config);
+  }
+
+  /**
+   * Get the type for the symbol, accounting for anonymous classes
+   *
+   * @param symbol the symbol
+   * @param state the visitor state
+   * @return the type for {@code symbol}
+   */
+  @Nullable
+  private static Type getTypeForSymbol(Symbol symbol, VisitorState state) {
+    if (symbol.isAnonymous()) {
+      // For anonymous classes, symbol.type does not contain annotations on generic type parameters.
+      // So, we get a correct type from the enclosing NewClassTree.
+      TreePath path = state.getPath();
+      NewClassTree newClassTree = ASTHelpers.findEnclosingNode(path, NewClassTree.class);
+      if (newClassTree == null) {
+        throw new RuntimeException(
+            "method should be inside a NewClassTree " + state.getSourceForNode(path.getLeaf()));
+      }
+      Type typeFromTree = getTreeType(newClassTree, state);
+      if (typeFromTree != null) {
+        verify(state.getTypes().isAssignable(symbol.type, typeFromTree));
+      }
+      return typeFromTree;
+    } else {
+      return symbol.type;
+    }
+  }
+
+  private static Nullness getGenericMethodReturnTypeNullness(
       Symbol.MethodSymbol method, Type enclosingType, VisitorState state, Config config) {
     Type overriddenMethodType = state.getTypes().memberType(enclosingType, method);
     if (!(overriddenMethodType instanceof Type.MethodType)) {
@@ -743,7 +784,7 @@
     }
     Type methodReceiverType =
         castToNonNull(
-            ASTHelpers.getType(((MemberSelectTree) tree.getMethodSelect()).getExpression()));
+            getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state));
     return getGenericMethodReturnTypeNullness(
         invokedMethodSymbol, methodReceiverType, state, config);
   }
@@ -791,11 +832,11 @@
     if (!(tree.getMethodSelect() instanceof MemberSelectTree)) {
       return Nullness.NONNULL;
     }
-    Type methodReceiverType =
+    Type enclosingType =
         castToNonNull(
-            ASTHelpers.getType(((MemberSelectTree) tree.getMethodSelect()).getExpression()));
+            getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state));
     return getGenericMethodParameterNullness(
-        paramIndex, invokedMethodSymbol, methodReceiverType, state, config);
+        paramIndex, invokedMethodSymbol, enclosingType, state, config);
   }
 
   /**
@@ -822,6 +863,35 @@
    *
    * @param parameterIndex index of the parameter
    * @param method the generic method
+   * @param enclosingSymbol the enclosing symbol in which we want to know {@code method}'s parameter
+   *     type nullability
+   * @param state the visitor state
+   * @param config the config
+   * @return nullability of the relevant parameter type of {@code method} in the context of {@code
+   *     enclosingSymbol}
+   */
+  public static Nullness getGenericMethodParameterNullness(
+      int parameterIndex,
+      Symbol.MethodSymbol method,
+      Symbol enclosingSymbol,
+      VisitorState state,
+      Config config) {
+    Type enclosingType = getTypeForSymbol(enclosingSymbol, state);
+    if (enclosingType == null) {
+      // we have no additional information from generics, so return NONNULL (presence of a @Nullable
+      // annotation should have been handled by the caller)
+      return Nullness.NONNULL;
+    }
+    return getGenericMethodParameterNullness(parameterIndex, method, enclosingType, state, config);
+  }
+
+  /**
+   * Just like {@link #getGenericMethodParameterNullness(int, Symbol.MethodSymbol, Symbol,
+   * VisitorState, Config)}, but takes the enclosing {@code Type} rather than the enclosing {@code
+   * Symbol}.
+   *
+   * @param parameterIndex index of the parameter
+   * @param method the generic method
    * @param enclosingType the enclosing type in which we want to know {@code method}'s parameter
    *     type nullability
    * @param state the visitor state

nullaway/src/main/java/com/uber/nullaway/NullAway.java

@@ -738,7 +738,7 @@ private Description checkParamOverriding(
                     ? GenericsChecks.getGenericMethodParameterNullness(
                         i,
                         overriddenMethod,
-                        overridingParamSymbols.get(i).owner.owner.type,
+                        overridingParamSymbols.get(i).owner.owner,
                         state,
                         config)
                     : Nullness.NONNULL);
@@ -944,7 +944,7 @@ private Description checkOverriding(
     // if the super method returns nonnull, overriding method better not return nullable
     // Note that, for the overriding method, the permissive default is non-null,
     // but it's nullable for the overridden one.
-    if (overriddenMethodReturnsNonNull(overriddenMethod, overridingMethod.owner.type, state)
+    if (overriddenMethodReturnsNonNull(overriddenMethod, overridingMethod.owner, state)
         && getMethodReturnNullness(overridingMethod, state, Nullness.NONNULL)
             .equals(Nullness.NULLABLE)
         && (memberReferenceTree == null
@@ -983,7 +983,7 @@ && getMethodReturnNullness(overridingMethod, state, Nullness.NONNULL)
   }
 
   private boolean overriddenMethodReturnsNonNull(
-      Symbol.MethodSymbol overriddenMethod, Type overridingMethodType, VisitorState state) {
+      Symbol.MethodSymbol overriddenMethod, Symbol enclosingSymbol, VisitorState state) {
     Nullness methodReturnNullness =
         getMethodReturnNullness(overriddenMethod, state, Nullness.NULLABLE);
     if (!methodReturnNullness.equals(Nullness.NONNULL)) {
@@ -993,7 +993,7 @@ private boolean overriddenMethodReturnsNonNull(
     // using the type parameters from the type enclosing the overriding method
     if (config.isJSpecifyMode()) {
       return GenericsChecks.getGenericMethodReturnTypeNullness(
-              overriddenMethod, overridingMethodType, state, config)
+              overriddenMethod, enclosingSymbol, state, config)
           .equals(Nullness.NONNULL);
     }
     return true;

nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java

@@ -2,6 +2,7 @@
 
 import com.google.errorprone.CompilationTestHelper;
 import java.util.Arrays;
+import org.junit.Ignore;
 import org.junit.Test;
 
 public class NullAwayJSpecifyGenericsTests extends NullAwayTestsBase {
@@ -974,6 +975,163 @@ public void overrideParameterType() {
         .doTest();
   }
 
+  @Test
+  public void overrideExplicitlyTypedAnonymousClass() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "class Test {",
+            "  interface Fn<P extends @Nullable Object, R extends @Nullable Object> {",
+            "    R apply(P p);",
+            "  }",
+            "  static abstract class FnClass<P extends @Nullable Object, R extends @Nullable Object> {",
+            "    abstract R apply(P p);",
+            "  }",
+            "  static void anonymousClasses() {",
+            "    Fn<@Nullable String, String> fn1 = new Fn<@Nullable String, String>() {",
+            "      // BUG: Diagnostic contains: parameter s is @NonNull, but parameter in superclass method",
+            "      public String apply(String s) { return s; }",
+            "    };",
+            "    FnClass<String, String> fn2 = new FnClass<String, String>() {",
+            "      // BUG: Diagnostic contains: method returns @Nullable, but superclass method",
+            "      public @Nullable String apply(String s) { return null; }",
+            "    };",
+            "    Fn<String, @Nullable String> fn3 = new Fn<String, @Nullable String>() {",
+            "      public @Nullable String apply(String s) { return null; }",
+            "    };",
+            "    FnClass<@Nullable String, String> fn4 = new FnClass<@Nullable String, String>() {",
+            "      public String apply(@Nullable String s) { return \"hello\"; }",
+            "    };",
+            "  }",
+            "  static void anonymousClassesFullName() {",
+            "    Test.Fn<@Nullable String, String> fn1 = new Test.Fn<@Nullable String, String>() {",
+            "      // BUG: Diagnostic contains: parameter s is @NonNull, but parameter in superclass method",
+            "      public String apply(String s) { return s; }",
+            "    };",
+            "    Test.FnClass<String, String> fn2 = new Test.FnClass<String, String>() {",
+            "      // BUG: Diagnostic contains: method returns @Nullable, but superclass method",
+            "      public @Nullable String apply(String s) { return null; }",
+            "    };",
+            "    Test.Fn<String, @Nullable String> fn3 = new Test.Fn<String, @Nullable String>() {",
+            "      public @Nullable String apply(String s) { return null; }",
+            "    };",
+            "    Test.FnClass<@Nullable String, String> fn4 = new Test.FnClass<@Nullable String, String>() {",
+            "      public String apply(@Nullable String s) { return \"hello\"; }",
+            "    };",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Ignore("https://github.com/uber/NullAway/issues/836")
+  @Test
+  public void overrideAnonymousNestedClass() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "class Test {",
+            "  class Wrapper<P extends @Nullable Object> {",
+            "    abstract class Fn<R extends @Nullable Object> {",
+            "      abstract R apply(P p);",
+            "    }",
+            "  }",
+            "  void anonymousNestedClasses() {",
+            "    Wrapper<@Nullable String>.Fn<String> fn1 = (this.new Wrapper<@Nullable String>()).new Fn<String>() {",
+            "      // BUG: Diagnostic contains: parameter s is @NonNull, but parameter in superclass method",
+            "      public String apply(String s) { return s; }",
+            "    };",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void explicitlyTypedAnonymousClassAsReceiver() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "class Test {",
+            "  interface Fn<P extends @Nullable Object, R extends @Nullable Object> {",
+            "    R apply(P p);",
+            "  }",
+            "  static abstract class FnClass<P extends @Nullable Object, R extends @Nullable Object> {",
+            "    abstract R apply(P p);",
+            "  }",
+            "  static void anonymousClasses() {",
+            "    String s1 = (new Fn<String, @Nullable String>() {",
+            "      public @Nullable String apply(String s) { return null; }",
+            "    }).apply(\"hi\");",
+            "    // BUG: Diagnostic contains: dereferenced expression s1",
+            "    s1.hashCode();",
+            "    String s2 = (new FnClass<String, @Nullable String>() {",
+            "      public @Nullable String apply(String s) { return null; }",
+            "    }).apply(\"hi\");",
+            "    // BUG: Diagnostic contains: dereferenced expression s2",
+            "    s2.hashCode();",
+            "    (new Fn<String, String>() {",
+            "      public String apply(String s) { return \"hi\"; }",
+            "    // BUG: Diagnostic contains: passing @Nullable parameter",
+            "    }).apply(null);",
+            "    (new FnClass<String, String>() {",
+            "      public String apply(String s) { return \"hi\"; }",
+            "    // BUG: Diagnostic contains: passing @Nullable parameter",
+            "    }).apply(null);",
+            "    (new Fn<@Nullable String, String>() {",
+            "      public String apply(@Nullable String s) { return \"hi\"; }",
+            "    }).apply(null);",
+            "    (new FnClass<@Nullable String, String>() {",
+            "      public String apply(@Nullable String s) { return \"hi\"; }",
+            "    }).apply(null);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  /** Diamond anonymous classes are not supported yet; tests are for future reference */
+  @Test
+  public void overrideDiamondAnonymousClass() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "class Test {",
+            "  interface Fn<P extends @Nullable Object, R extends @Nullable Object> {",
+            "    R apply(P p);",
+            "  }",
+            "  static abstract class FnClass<P extends @Nullable Object, R extends @Nullable Object> {",
+            "    abstract R apply(P p);",
+            "  }",
+            "  static void anonymousClasses() {",
+            "    Fn<@Nullable String, String> fn1 = new Fn<>() {",
+            "      // TODO: should report a bug here",
+            "      public String apply(String s) { return s; }",
+            "    };",
+            "    FnClass<@Nullable String, String> fn2 = new FnClass<>() {",
+            "      // TODO: should report a bug here",
+            "      public String apply(String s) { return s; }",
+            "    };",
+            "    Fn<String, @Nullable String> fn3 = new Fn<>() {",
+            "      // TODO: this is a false positive",
+            "      // BUG: Diagnostic contains: method returns @Nullable, but superclass method",
+            "      public @Nullable String apply(String s) { return null; }",
+            "    };",
+            "    FnClass<String, @Nullable String> fn4 = new FnClass<>() {",
+            "      // TODO: this is a false positive",
+            "      // BUG: Diagnostic contains: method returns @Nullable, but superclass method",
+            "      public @Nullable String apply(String s) { return null; }",
+            "    };",
+            "  }",
+            "}")
+        .doTest();
+  }
+
   @Test
   public void nullableGenericTypeVariableReturnType() {
     makeHelper()