Merge branch 'master' into cleanup-accesspathelement

.github/workflows/continuous-integration.yml

@@ -21,16 +21,16 @@ jobs:
             epVersion: 2.10.0
           - os: macos-latest
             java: 11
-            epVersion: 2.22.0
+            epVersion: 2.23.0
           - os: ubuntu-latest
             java: 11
-            epVersion: 2.22.0
+            epVersion: 2.23.0
           - os: windows-latest
             java: 11
-            epVersion: 2.22.0
+            epVersion: 2.23.0
           - os: ubuntu-latest
             java: 17
-            epVersion: 2.22.0
+            epVersion: 2.23.0
       fail-fast: false
     runs-on: ${{ matrix.os }}
     steps:
@@ -40,7 +40,7 @@ jobs:
         uses: actions/setup-java@v3
         with:
           java-version: |
-            21-ea
+            21
             17
             ${{ matrix.java }}
           distribution: 'temurin'
@@ -63,7 +63,7 @@ jobs:
         with:
           arguments: codeCoverageReport
         continue-on-error: true
-        if: runner.os == 'Linux' && matrix.java == '11' && matrix.epVersion == '2.22.0' && github.repository == 'uber/NullAway'
+        if: runner.os == 'Linux' && matrix.java == '11' && matrix.epVersion == '2.23.0' && github.repository == 'uber/NullAway'
       - name: Upload coverage reports to Codecov
         uses: codecov/codecov-action@v3
         with:
@@ -92,7 +92,7 @@ jobs:
         uses: actions/setup-java@v3
         with:
           java-version: |
-            21-ea
+            21
             11
           distribution: 'temurin'
       - name: 'Publish'

CHANGELOG.md

@@ -1,5 +1,23 @@
 Changelog
 =========
+Version 0.10.15
+---------------
+* [IMPORTANT] Update minimum Error Prone version and Guava version (#843)
+  NullAway now requires Error Prone 2.10.0 or later
+* Add Spring mock/testing annotations to excluded field annotation list (#757)
+* Update to Checker Framework 3.39.0 (#839) [Support for JDK 21 constructs]
+* Support for JSpecify's 0.3.0 annotation [experimental]
+  - Properly check generic method overriding in explicitly-typed anonymous classes (#808)
+  - JSpecify: handle incorrect method parameter nullability for method reference (#845)
+  - JSpecify: initial handling of generic enclosing types for inner classes (#837)
+* Build / CI tooling for NullAway itself:
+  - Update Gradle and a couple of plugin versions (#832)
+  - Run recent JDK tests on JDK 21 (#834)
+  - Fix which JDKs are installed on CI (#835)
+  - Update to Error Prone 2.22.0 (#833)
+  - Ignore code coverage for method executed non-deterministically in tests (#838 and #844)
+  - Build NullAway with JSpecify mode enabled (#841)
+
 Version 0.10.14
 ---------------
 IMPORTANT: This version introduces EXPERIMENTAL JDK21 support.

gradle.properties

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

gradle/dependencies.gradle

@@ -19,7 +19,7 @@ import org.gradle.util.VersionNumber
 // The oldest version of Error Prone that we support running on
 def oldestErrorProneVersion = "2.10.0"
 // Latest released Error Prone version that we've tested with
-def latestErrorProneVersion = "2.22.0"
+def latestErrorProneVersion = "2.23.0"
 // Default to using latest tested Error Prone version
 def defaultErrorProneVersion =  latestErrorProneVersion
 def errorProneVersionToCompileAgainst = defaultErrorProneVersion

gradle/wrapper/gradle-wrapper.properties

@@ -1,7 +1,7 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionSha256Sum=591855b517fc635b9e04de1d05d5e76ada3f89f5fc76f87978d1b245b4f69225
-distributionUrl=https\://services.gradle.org/distributions/gradle-8.3-bin.zip
+distributionSha256Sum=3e1af3ae886920c3ac87f7a91f816c0c7c436f276a6eefdb3da152100fef72ae
+distributionUrl=https\://services.gradle.org/distributions/gradle-8.4-bin.zip
 networkTimeout=10000
 validateDistributionUrl=true
 zipStoreBase=GRADLE_USER_HOME

gradlew

@@ -145,15 +145,15 @@ if ! "$cygwin" && ! "$darwin" && ! "$nonstop" ; then
     case $MAX_FD in #(
       max*)
         # In POSIX sh, ulimit -H is undefined. That's why the result is checked to see if it worked.
-        # shellcheck disable=SC3045
+        # shellcheck disable=SC2039,SC3045
         MAX_FD=$( ulimit -H -n ) ||
             warn "Could not query maximum file descriptor limit"
     esac
     case $MAX_FD in  #(
       '' | soft) :;; #(
       *)
         # In POSIX sh, ulimit -n is undefined. That's why the result is checked to see if it worked.
-        # shellcheck disable=SC3045
+        # shellcheck disable=SC2039,SC3045
         ulimit -n "$MAX_FD" ||
             warn "Could not set maximum file descriptor limit to $MAX_FD"
     esac
@@ -202,11 +202,11 @@ fi
 # Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
 DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"'
 
-# Collect all arguments for the java command;
-#   * $DEFAULT_JVM_OPTS, $JAVA_OPTS, and $GRADLE_OPTS can contain fragments of
-#     shell script including quotes and variable substitutions, so put them in
-#     double quotes to make sure that they get re-expanded; and
-#   * put everything else in single quotes, so that it's not re-expanded.
+# Collect all arguments for the java command:
+#   * DEFAULT_JVM_OPTS, JAVA_OPTS, JAVA_OPTS, and optsEnvironmentVar are not allowed to contain shell fragments,
+#     and any embedded shellness will be escaped.
+#   * For example: A user cannot expect ${Hostname} to be expanded, as it is an environment variable and will be
+#     treated as '${Hostname}' itself on the command line.
 
 set -- \
         "-Dorg.gradle.appname=$APP_BASE_NAME" \

jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/BytecodeAnnotator.java

@@ -15,6 +15,8 @@
  */
 package com.uber.nullaway.jarinfer;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import java.io.BufferedReader;
@@ -227,7 +229,7 @@ private static void copyAndAnnotateJarEntry(
     } else if (entryName.equals("META-INF/MANIFEST.MF")) {
       // Read full file
       StringBuilder stringBuilder = new StringBuilder();
-      BufferedReader br = new BufferedReader(new InputStreamReader(is, "UTF-8"));
+      BufferedReader br = new BufferedReader(new InputStreamReader(is, UTF_8));
       String currentLine;
       while ((currentLine = br.readLine()) != null) {
         stringBuilder.append(currentLine + "\n");
@@ -240,7 +242,7 @@ private static void copyAndAnnotateJarEntry(
         throw new SignedJarException(SIGNED_JAR_ERROR_MESSAGE);
       }
       jarOS.putNextEntry(new ZipEntry(jarEntry.getName()));
-      jarOS.write(manifestMinusDigests.getBytes("UTF-8"));
+      jarOS.write(manifestMinusDigests.getBytes(UTF_8));
     } else if (entryName.startsWith("META-INF/")
         && (entryName.endsWith(".DSA")
             || entryName.endsWith(".RSA")

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

@@ -702,11 +702,6 @@ public static void checkTypeParameterNullnessForMethodOverriding(
   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);
   }
 
@@ -738,8 +733,13 @@ private static Type getTypeForSymbol(Symbol symbol, VisitorState state) {
     }
   }
 
-  private static Nullness getGenericMethodReturnTypeNullness(
-      Symbol.MethodSymbol method, Type enclosingType, VisitorState state, Config config) {
+  static Nullness getGenericMethodReturnTypeNullness(
+      Symbol.MethodSymbol method, @Nullable Type enclosingType, VisitorState state, Config config) {
+    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;
+    }
     Type overriddenMethodType = state.getTypes().memberType(enclosingType, method);
     verify(
         overriddenMethodType instanceof ExecutableType,

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

@@ -957,7 +957,8 @@ 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, state)
+    if (overriddenMethodReturnsNonNull(
+            overriddenMethod, overridingMethod.owner, memberReferenceTree, state)
         && getMethodReturnNullness(overridingMethod, state, Nullness.NONNULL)
             .equals(Nullness.NULLABLE)
         && (memberReferenceTree == null
@@ -996,18 +997,30 @@ && getMethodReturnNullness(overridingMethod, state, Nullness.NONNULL)
   }
 
   private boolean overriddenMethodReturnsNonNull(
-      Symbol.MethodSymbol overriddenMethod, Symbol enclosingSymbol, VisitorState state) {
+      Symbol.MethodSymbol overriddenMethod,
+      Symbol enclosingSymbol,
+      @Nullable MemberReferenceTree memberReferenceTree,
+      VisitorState state) {
     Nullness methodReturnNullness =
         getMethodReturnNullness(overriddenMethod, state, Nullness.NULLABLE);
     if (!methodReturnNullness.equals(Nullness.NONNULL)) {
       return false;
     }
     // In JSpecify mode, for generic methods, we additionally need to check the return nullness
-    // using the type parameters from the type enclosing the overriding method
+    // using the type arguments from the type enclosing the overriding method
     if (config.isJSpecifyMode()) {
-      return GenericsChecks.getGenericMethodReturnTypeNullness(
-              overriddenMethod, enclosingSymbol, state, config)
-          .equals(Nullness.NONNULL);
+      if (memberReferenceTree != null) {
+        // For a method reference, we get generic type arguments from javac's inferred type for the
+        // tree, which properly preserves type-use annotations
+        return GenericsChecks.getGenericMethodReturnTypeNullness(
+                overriddenMethod, ASTHelpers.getType(memberReferenceTree), state, config)
+            .equals(Nullness.NONNULL);
+      } else {
+        // Use the enclosing class of the overriding method to find generic type arguments
+        return GenericsChecks.getGenericMethodReturnTypeNullness(
+                overriddenMethod, enclosingSymbol, state, config)
+            .equals(Nullness.NONNULL);
+      }
     }
     return true;
   }

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

@@ -484,6 +484,106 @@ public void testForMethodReferenceInAnAssignment() {
         .doTest();
   }
 
+  @Test
+  public void testForMethodReferenceForClassFieldAssignment() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "class Test {",
+            "  interface A<T1 extends @Nullable Object> {",
+            "    T1 function(Object o);",
+            "  }",
+            "  static @Nullable String foo(Object o) {",
+            "    return o.toString();",
+            "  }",
+            "  // BUG: Diagnostic contains: referenced method returns @Nullable",
+            "  A<String> positiveField = Test::foo;",
+            "  A<@Nullable String> negativeField = Test::foo;",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void testForMethodReferenceReturnTypeInAnAssignment() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "class Test {",
+            "  interface A<T1 extends @Nullable Object> {",
+            "    T1 function(Object o);",
+            "  }",
+            "  static @Nullable String foo(Object o) {",
+            "    return o.toString();",
+            "  }",
+            "  static void testPositive() {",
+            "    // BUG: Diagnostic contains: referenced method returns @Nullable",
+            "    A<String> p = Test::foo;",
+            "  }",
+            "  static void testNegative() {",
+            "    A<@Nullable String> p = Test::foo;",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void testForMethodReferenceWhenReturned() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "class Test {",
+            "  interface A<T1 extends @Nullable Object> {",
+            "    T1 function(Object o);",
+            "  }",
+            "  static @Nullable String foo(Object o) {",
+            "    return o.toString();",
+            "  }",
+            "  static A<String> testPositive() {",
+            "    // BUG: Diagnostic contains: referenced method returns @Nullable",
+            "    return Test::foo;",
+            "  }",
+            "  static A<@Nullable String> testNegative() {",
+            "    return Test::foo;",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void testForMethodReferenceAsMethodParameter() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "class Test {",
+            "  interface A<T1 extends @Nullable Object> {",
+            "    T1 function(Object o);",
+            "  }",
+            "  static @Nullable String foo(Object o) {",
+            "    return o.toString();",
+            "  }",
+            "  static void fooPositive(A<String> a) {",
+            "  }",
+            "  static void fooNegative(A<@Nullable String> a) {",
+            "  }",
+            "  static void testPositive() {",
+            "    // BUG: Diagnostic contains: referenced method returns @Nullable",
+            "    fooPositive(Test::foo);",
+            "  }",
+            "  static void testNegative() {",
+            "    fooNegative(Test::foo);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
   @Test
   public void testForLambdasInAnAssignment() {
     makeHelper()