fix and test

uber · Nov 5, 2024 · b3eba82 · b3eba82
1 parent 17fc1ba
commit b3eba82
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 9 deletions.
diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java
@@ -1852,7 +1852,7 @@ private Description handleInvocation(
       }
       if (config.isJSpecifyMode()) {
         GenericsChecks.compareGenericTypeParameterNullabilityForCall(
-            formalParams, actualParams, varArgsMethod, this, state);
+            methodSymbol, tree, actualParams, varArgsMethod, this, state);
         if (!methodSymbol.getTypeParameters().isEmpty()) {
           GenericsChecks.checkGenericMethodCallTypeArguments(tree, state, this, config, handler);
         }

diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
@@ -595,29 +595,43 @@ public static void checkTypeParameterNullnessForConditionalExpression(
    * Checks that for each parameter p at a call, the type parameter nullability for p's type matches
    * that of the corresponding formal parameter. If a mismatch is found, report an error.
    *
-   * @param formalParams the formal parameters
-   * @param actualParams the actual parameters
+   * @param methodSymbol the symbol for the method being called
+   * @param tree the tree representing the method call
+   * @param actualParams the actual parameters at the call
    * @param isVarArgs true if the call is to a varargs method
    * @param analysis the analysis object
    * @param state the visitor state
    */
   public static void compareGenericTypeParameterNullabilityForCall(
-      List<Symbol.VarSymbol> formalParams,
+      Symbol.MethodSymbol methodSymbol,
+      Tree tree,
       List<? extends ExpressionTree> actualParams,
       boolean isVarArgs,
       NullAway analysis,
       VisitorState state) {
     if (!analysis.getConfig().isJSpecifyMode()) {
       return;
     }
-    int n = formalParams.size();
+    Type invokedMethodType = methodSymbol.type;
+    if (!methodSymbol.isStatic() && tree instanceof MethodInvocationTree) {
+      // TODO what if the receiver is `this`?
+      Type enclosingType =
+          getTreeType(
+              ((MemberSelectTree) ((MethodInvocationTree) tree).getMethodSelect()).getExpression(),
+              state);
+      if (enclosingType != null) {
+        invokedMethodType = state.getTypes().memberType(enclosingType, methodSymbol);
+      }
+    }
+    List<Type> formalParamTypes = invokedMethodType.getParameterTypes();
+    int n = formalParamTypes.size();
     if (isVarArgs) {
       // If the last argument is var args, don't check it now, it will be checked against
       // all remaining actual arguments in the next loop.
       n = n - 1;
     }
     for (int i = 0; i < n; i++) {
-      Type formalParameter = formalParams.get(i).type;
+      Type formalParameter = formalParamTypes.get(i);
       if (formalParameter.isRaw()) {
         // bail out of any checking involving raw types for now
         return;
@@ -630,11 +644,11 @@ public static void compareGenericTypeParameterNullabilityForCall(
         }
       }
     }
-    if (isVarArgs && !formalParams.isEmpty()) {
+    if (isVarArgs && !formalParamTypes.isEmpty()) {
       Type.ArrayType varargsArrayType =
-          (Type.ArrayType) formalParams.get(formalParams.size() - 1).type;
+          (Type.ArrayType) formalParamTypes.get(formalParamTypes.size() - 1);
       Type varargsElementType = varargsArrayType.elemtype;
-      for (int i = formalParams.size() - 1; i < actualParams.size(); i++) {
+      for (int i = formalParamTypes.size() - 1; i < actualParams.size(); i++) {
         Type actualParameterType = getTreeType(actualParams.get(i), state);
         // If the actual parameter type is assignable to the varargs array type, then the call site
         // is passing the varargs directly in an array, and we should skip our check.

diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java
@@ -938,6 +938,27 @@ public void parameterPassing() {
         .doTest();
   }
 
+  @Test
+  public void parameterPassingInstanceMethods() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "class Test {",
+            "  static class A<T extends @Nullable Object> {",
+            "    void foo(A<T> a) {}",
+            "  }",
+            "  static void test(A<@Nullable String> p, A<String> q) {",
+            "    // BUG: Diagnostic contains: Cannot pass parameter of type",
+            "    p.foo(q);",
+            "    // this one is fine",
+            "    p.foo(p);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
   @Test
   public void varargsParameter() {
     makeHelper()