Merge branch 'master' into remove-inferred-jar-models-handler

CHANGELOG.md

@@ -1,5 +1,25 @@
 Changelog
 =========
+Version 0.12.2
+---------------
+* Fix reading of JSpecify @nullable annotations from varargs parameter in bytecode (#1089)
+* Fix JarInfer handling of generic types (#1078)
+* Fix another JSpecify mode crash involving raw types (#1086)
+* Fix bugs in handling of valueOf calls for map keys (#1085)
+* Suggest correct fix when array component of non-nullable array is made null. (#1087)
+* Substitute type arguments when checking type parameter nullability at call site (#1070)
+* Fix JarInfer parameter indexes for instance methods (#1071)
+* JSpecify mode: initial support for generic methods (with explicit type arguments at calls) (#1053)
+* Maintenance
+  - Update to latest Error Prone and Error Prone Gradle plugin (#1064)
+  - Refactor serialization adapter retrieval by version (#1066)
+  - Remove fixes.tsv serialization from NullAway serialization service (#1063)
+  - Enable javac -parameters flag (#1069)
+  - Update to Gradle 8.11 (#1073)
+  - Add test for issue 1035 (#1074)
+  - remove use of deprecated Gradle API (#1076)
+  - Update to Error Prone 2.36.0 (#1077)
+
 Version 0.12.1
 ---------------
 * Add library model for Apache Commons CollectionUtils.isNotEmpty (#932) (#1062)

gradle.properties

@@ -12,7 +12,7 @@ org.gradle.caching=true
 org.gradle.jvmargs=-Xmx2g -XX:MaxMetaspaceSize=512m
 
 GROUP=com.uber.nullaway
-VERSION_NAME=0.12.2-SNAPSHOT
+VERSION_NAME=0.12.3-SNAPSHOT
 
 POM_DESCRIPTION=A fast annotation-based null checker for Java
 

gradle/dependencies.gradle

@@ -48,7 +48,7 @@ def versions = [
     // The version of Error Prone that NullAway is compiled and tested against
     errorProneApi          : errorProneVersionToCompileAgainst,
     support                : "27.1.1",
-    wala                   : "1.6.8",
+    wala                   : "1.6.9",
     commonscli             : "1.4",
     autoValue              : "1.10.2",
     autoService            : "1.1.1",

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

@@ -51,6 +51,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.BiPredicate;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import javax.lang.model.element.AnnotationMirror;
@@ -328,13 +329,13 @@ private static Stream<Attribute.TypeCompound> getTypeUseAnnotations(
       return rawTypeAttributes.filter(
           (t) ->
               t.position.type.equals(TargetType.METHOD_RETURN)
-                  && isDirectTypeUseAnnotation(t, symbol, config));
+                  && (!onlyDirect || isDirectTypeUseAnnotation(t, symbol, config)));
     } else {
       // filter for annotations directly on the type
       return rawTypeAttributes.filter(
           t ->
               targetTypeMatches(symbol, t.position)
-                  && (!onlyDirect || NullabilityUtil.isDirectTypeUseAnnotation(t, symbol, config)));
+                  && (!onlyDirect || isDirectTypeUseAnnotation(t, symbol, config)));
     }
   }
 
@@ -540,22 +541,11 @@ public static <T> T castToNonNull(@Nullable T obj) {
    *     otherwise
    */
   public static boolean isArrayElementNullable(Symbol arraySymbol, Config config) {
-    for (Attribute.TypeCompound t : arraySymbol.getRawTypeAttributes()) {
-      for (TypeAnnotationPosition.TypePathEntry entry : t.position.location) {
-        if (entry.tag == TypeAnnotationPosition.TypePathEntryKind.ARRAY) {
-          if (Nullness.isNullableAnnotation(t.type.toString(), config)) {
-            return true;
-          }
-        }
-      }
-    }
-    // For varargs symbols we also consider the elements to be @Nullable if there is a @Nullable
-    // declaration annotation on the parameter
-    // NOTE this flag check does not work for the varargs parameter of a method defined in bytecodes
-    if ((arraySymbol.flags() & Flags.VARARGS) != 0) {
-      return Nullness.hasNullableDeclarationAnnotation(arraySymbol, config);
-    }
-    return false;
+    return checkArrayElementAnnotations(
+        arraySymbol,
+        config,
+        Nullness::isNullableAnnotation,
+        Nullness::hasNullableDeclarationAnnotation);
   }
 
   /**
@@ -582,12 +572,50 @@ public static boolean nullableVarargsElementsForSourceOrBytecode(
    *     otherwise
    */
   public static boolean isArrayElementNonNull(Symbol arraySymbol, Config config) {
+    return checkArrayElementAnnotations(
+        arraySymbol,
+        config,
+        Nullness::isNonNullAnnotation,
+        Nullness::hasNonNullDeclarationAnnotation);
+  }
+
+  /**
+   * Checks if the given varargs symbol has a {@code @NonNull} annotation for its elements. Works
+   * for both source and bytecode.
+   *
+   * @param varargsSymbol the symbol of the varargs parameter
+   * @param config NullAway configuration
+   * @return true if the varargs symbol has a {@code @NonNull} annotation for its elements, false
+   *     otherwise
+   */
+  public static boolean nonnullVarargsElementsForSourceOrBytecode(
+      Symbol varargsSymbol, Config config) {
+    return isArrayElementNonNull(varargsSymbol, config)
+        || Nullness.hasNonNullDeclarationAnnotation(varargsSymbol, config);
+  }
+
+  /**
+   * Checks if the annotations on the elements of some array symbol satisfy some predicate.
+   *
+   * @param arraySymbol the array symbol
+   * @param config NullAway configuration
+   * @param typeUseCheck the predicate to check the type-use annotations
+   * @param declarationCheck the predicate to check the declaration annotations (applied only to
+   *     varargs symbols)
+   * @return true if the annotations on the elements of the array symbol satisfy the given
+   *     predicates, false otherwise
+   */
+  private static boolean checkArrayElementAnnotations(
+      Symbol arraySymbol,
+      Config config,
+      BiPredicate<String, Config> typeUseCheck,
+      BiPredicate<Symbol, Config> declarationCheck) {
     if (getTypeUseAnnotations(arraySymbol, config, /* onlyDirect= */ false)
         .anyMatch(
             t -> {
               for (TypeAnnotationPosition.TypePathEntry entry : t.position.location) {
                 if (entry.tag == TypeAnnotationPosition.TypePathEntryKind.ARRAY) {
-                  if (Nullness.isNonNullAnnotation(t.type.toString(), config)) {
+                  if (typeUseCheck.test(t.type.toString(), config)) {
                     return true;
                   }
                 }
@@ -596,30 +624,14 @@ public static boolean isArrayElementNonNull(Symbol arraySymbol, Config config) {
             })) {
       return true;
     }
-    // For varargs symbols we also consider the elements to be @NonNull if there is a @NonNull
-    // declaration annotation on the parameter
+    // For varargs symbols we also check for declaration annotations on the parameter
     // NOTE this flag check does not work for the varargs parameter of a method defined in bytecodes
     if ((arraySymbol.flags() & Flags.VARARGS) != 0) {
-      return Nullness.hasNonNullDeclarationAnnotation(arraySymbol, config);
+      return declarationCheck.test(arraySymbol, config);
     }
     return false;
   }
 
-  /**
-   * Checks if the given varargs symbol has a {@code @NonNull} annotation for its elements. Works
-   * for both source and bytecode.
-   *
-   * @param varargsSymbol the symbol of the varargs parameter
-   * @param config NullAway configuration
-   * @return true if the varargs symbol has a {@code @NonNull} annotation for its elements, false
-   *     otherwise
-   */
-  public static boolean nonnullVarargsElementsForSourceOrBytecode(
-      Symbol varargsSymbol, Config config) {
-    return isArrayElementNonNull(varargsSymbol, config)
-        || Nullness.hasNonNullDeclarationAnnotation(varargsSymbol, config);
-  }
-
   /**
    * Does the given symbol have a JetBrains @NotNull declaration annotation? Useful for workarounds
    * in light of https://github.com/uber/NullAway/issues/720

nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java

@@ -28,6 +28,8 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.errorprone.VisitorState;
+import com.google.errorprone.suppliers.Supplier;
+import com.google.errorprone.suppliers.Suppliers;
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.LiteralTree;
 import com.sun.source.tree.MethodInvocationTree;
@@ -75,6 +77,12 @@
  */
 public final class AccessPath implements MapKey {
 
+  private static final Supplier<Type> INTEGER_TYPE_SUPPLIER =
+      Suppliers.typeFromString("java.lang.Integer");
+
+  private static final Supplier<Type> LONG_TYPE_SUPPLIER =
+      Suppliers.typeFromString("java.lang.Long");
+
   /**
    * A prefix added for elements appearing in method invocation APs which represent fields that can
    * be proven to be class-initialization time constants (i.e. static final fields of a type known
@@ -278,14 +286,15 @@ private static Node stripCasts(Node node) {
         return new NumericMapKey(((LongLiteralNode) argument).getValue());
       case METHOD_INVOCATION:
         MethodAccessNode target = ((MethodInvocationNode) argument).getTarget();
-        Node receiver = stripCasts(target.getReceiver());
         List<Node> arguments = ((MethodInvocationNode) argument).getArguments();
         // Check for int/long boxing.
         if (target.getMethod().getSimpleName().toString().equals("valueOf")
-            && arguments.size() == 1
-            && castToNonNull(receiver.getTree()).getKind().equals(Tree.Kind.IDENTIFIER)
-            && (receiver.toString().equals("Integer") || receiver.toString().equals("Long"))) {
-          return argumentToMapKeySpecifier(arguments.get(0), state, apContext);
+            && arguments.size() == 1) {
+          Type ownerType = ((Symbol.MethodSymbol) target.getMethod()).owner.type;
+          if (ASTHelpers.isSameType(ownerType, INTEGER_TYPE_SUPPLIER.get(state), state)
+              || ASTHelpers.isSameType(ownerType, LONG_TYPE_SUPPLIER.get(state), state)) {
+            return argumentToMapKeySpecifier(arguments.get(0), state, apContext);
+          }
         }
       // Fine to fallthrough:
       default:

nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java

@@ -35,10 +35,10 @@ public Boolean visitClassType(Type.ClassType lhsType, Type rhsType) {
     // The base type of rhsType may be a subtype of lhsType's base type.  In such cases, we must
     // compare lhsType against the supertype of rhsType with a matching base type.
     Type rhsTypeAsSuper = types.asSuper(rhsType, lhsType.tsym);
-    // This is impossible, considering the fact that standard Java subtyping succeeds before
-    // running NullAway
     if (rhsTypeAsSuper == null) {
-      throw new RuntimeException("Did not find supertype of " + rhsType + " matching " + lhsType);
+      // Surprisingly, this can in fact occur, in cases involving raw types.  See, e.g.,
+      // GenericsTests#issue1082 and https://github.com/uber/NullAway/pull/1086. Bail out.
+      return true;
     }
     // bail out of checking raw types for now
     if (rhsTypeAsSuper.isRaw()) {

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

@@ -434,4 +434,58 @@ public void testAccessUsingExplicitThis() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void mapKeysFromValueOf() {
+    defaultCompilationHelper
+        .addSourceLines(
+            "Foo.java",
+            "package com.uber;",
+            "import java.util.HashMap;",
+            "import java.util.Map;",
+            "public class Foo {",
+            "  private final Map<Integer, Object> map = new HashMap<>();",
+            "  private final Map<Long, Object> longMap = new HashMap<>();",
+            "  static Integer valueOf(int i) { return 0; }",
+            "  static Integer valueOf(int i, int j) { return i+j; }",
+            "  public void putThenGetIntegerValueOf() {",
+            "    map.put(Integer.valueOf(10), new Object());",
+            "    map.get(Integer.valueOf(10)).toString();",
+            "  }",
+            "  public void putThenGetLongValueOf() {",
+            "    longMap.put(Long.valueOf(10), new Object());",
+            "    longMap.get(Long.valueOf(10)).toString();",
+            "  }",
+            "  public void putThenGetFooValueOf() {",
+            "    map.put(valueOf(10), new Object());",
+            "    // Unknown valueOf method so we report a warning",
+            "    // BUG: Diagnostic contains: dereferenced expression map.get(valueOf(10)) is @Nullable",
+            "    map.get(valueOf(10)).toString();",
+            "    map.put(valueOf(10,20), new Object());",
+            "    // Unknown valueOf method so we report a warning",
+            "    // BUG: Diagnostic contains: dereferenced expression map.get(valueOf(10,20)) is @Nullable",
+            "    map.get(valueOf(10,20)).toString();",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void mapKeyFromIntegerValueOfStaticImport() {
+    defaultCompilationHelper
+        .addSourceLines(
+            "Foo.java",
+            "package com.uber;",
+            "import java.util.HashMap;",
+            "import java.util.Map;",
+            "import static java.lang.Integer.valueOf;",
+            "public class Foo {",
+            "  private final Map<Integer, Object> map = new HashMap<>();",
+            "  public void putThenGet() {",
+            "    map.put(valueOf(10), new Object());",
+            "    map.get(valueOf(10)).toString();",
+            "  }",
+            "}")
+        .doTest();
+  }
 }

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

@@ -106,6 +106,22 @@ public void nullableTypeUseVarArgsFromBytecode() {
         .doTest();
   }
 
+  @Test
+  public void nullableVarArgsFromBytecodeJSpecify() {
+    defaultCompilationHelper
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import com.uber.lib.Varargs;",
+            "public class Test {",
+            "  public void testTypeUse() {",
+            "    String x = null;",
+            "    Varargs.typeUseNullableElementsJSpecify(x);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
   @Test
   public void nullableTypeUseVarargs() {
     defaultCompilationHelper

nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java

@@ -2002,6 +2002,31 @@ public void issue1019() {
         .doTest();
   }
 
+  @Test
+  public void issue1082() {
+    makeHelper()
+        .addSourceLines(
+            "Main.java",
+            "package com.uber;",
+            "import java.util.Optional;",
+            "public class Main {",
+            "  public interface Factory<T> {",
+            "    T create();",
+            "  }",
+            "  public interface Expiry<K, V> {}",
+            "  static class Config<K, V> {",
+            "    Config<K, V> setFactory(Optional<Factory<? extends Expiry<K, V>>> factory) {",
+            "      return this;",
+            "    }",
+            "  }",
+            "  static void caller(Config config) {",
+            "    // checking that we don't crash",
+            "    config.setFactory(Optional.<Object>empty());",
+            "  }",
+            "}")
+        .doTest();
+  }
+
   private CompilationTestHelper makeHelper() {
     return makeTestHelperWithArgs(
         Arrays.asList(

test-java-lib/src/main/java/com/uber/lib/Varargs.java

@@ -7,4 +7,7 @@ public class Varargs {
   public Varargs(@Nullable String... args) {}
 
   public static void typeUse(String @org.jspecify.annotations.Nullable ... args) {}
+
+  public static void typeUseNullableElementsJSpecify(
+      @org.jspecify.annotations.Nullable String... args) {}
 }