diff --git a/guava-recent-unit-tests/build.gradle b/guava-recent-unit-tests/build.gradle index ccc5bb00a2..b6c853d17c 100644 --- a/guava-recent-unit-tests/build.gradle +++ b/guava-recent-unit-tests/build.gradle @@ -33,7 +33,7 @@ dependencies { exclude group: "junit", module: "junit" } testImplementation deps.test.jsr305Annotations - testImplementation "com.google.guava:guava:31.1-jre" + testImplementation "com.google.guava:guava:33.4.5-jre" errorProneOldest deps.build.errorProneCheckApiOld errorProneOldest(deps.build.errorProneTestHelpersOld) { diff --git a/guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java b/guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java index 156ea04e16..ad78431995 100644 --- a/guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java +++ b/guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java @@ -19,10 +19,12 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ +package com.uber.nullaway.guava; import com.google.errorprone.CompilationTestHelper; import com.uber.nullaway.NullAway; import java.util.Arrays; +import org.junit.Assume; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -33,6 +35,8 @@ public class NullAwayGuavaParametricNullnessTests { private CompilationTestHelper defaultCompilationHelper; + private CompilationTestHelper jspecifyCompilationHelper; + @Before public void setup() { defaultCompilationHelper = @@ -43,6 +47,14 @@ public void setup() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber,com.google.common", "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.nullaway.[a-zA-Z0-9.]+.unannotated")); + jspecifyCompilationHelper = + CompilationTestHelper.newInstance(NullAway.class, getClass()) + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:OnlyNullMarked=true", + "-XepOpt:NullAway:JSpecifyMode=true")); } @Test @@ -70,6 +82,33 @@ public void testFutureCallbackParametricNullness() { .doTest(); } + @Test + public void jspecifyFutureCallback() { + // to ensure javac reads proper generic types from the Guava jar + Assume.assumeTrue(Runtime.version().feature() >= 23); + jspecifyCompilationHelper + .addSourceLines( + "Test.java", + "import com.google.common.util.concurrent.FutureCallback;", + "import org.jspecify.annotations.*;", + "@NullMarked", + "class Test {", + " public static FutureCallback<@Nullable T> wrapFutureCallback(FutureCallback<@Nullable T> futureCallback) {", + " return new FutureCallback<@Nullable T>() {", + " @Override", + " public void onSuccess(@Nullable T result) {", + " futureCallback.onSuccess(result);", + " }", + " @Override", + " public void onFailure(Throwable throwable) {", + " futureCallback.onFailure(throwable);", + " }", + " };", + " }", + "}") + .doTest(); + } + @Test public void testIterableParametricNullness() { defaultCompilationHelper @@ -91,6 +130,59 @@ public void testIterableParametricNullness() { .doTest(); } + @Test + public void jspecifyIterables() { + // to ensure javac reads proper generic types from the Guava jar + Assume.assumeTrue(Runtime.version().feature() >= 23); + jspecifyCompilationHelper + .addSourceLines( + "Test.java", + "import com.google.common.collect.ImmutableList;", + "import com.google.common.collect.Iterables;", + "import org.jspecify.annotations.*;", + "@NullMarked", + "class Test {", + " public static String test1() {", + " // BUG: Diagnostic contains: returning @Nullable expression", + " return Iterables.<@Nullable String>getFirst(ImmutableList.of(), null);", + " }", + " public static @Nullable String test2() {", + " return Iterables.<@Nullable String>getFirst(ImmutableList.of(), null);", + " }", + " public static String test3() {", + " return Iterables.getOnlyElement(ImmutableList.of(\"hi\"));", + " }", + " public static String test4() {", + " // BUG: Diagnostic contains: returning @Nullable expression", + " return Iterables.<@Nullable String>getOnlyElement(ImmutableList.of(\"hi\"));", + " }", + "}") + .doTest(); + } + + @Test + public void jspecifyComparators() { + // to ensure javac reads proper generic types from the Guava jar + Assume.assumeTrue(Runtime.version().feature() >= 23); + jspecifyCompilationHelper + .addSourceLines( + "Test.java", + "import com.google.common.collect.Comparators;", + "import java.util.Comparator;", + "import org.jspecify.annotations.*;", + "@NullMarked", + "class Test {", + " public static String test1(String t1, String t2, Comparator cmp) {", + " return Comparators.min(t1, t2, cmp);", + " }", + " public static String test2(@Nullable String t1, @Nullable String t2, Comparator cmp) {", + " // BUG: Diagnostic contains: returning @Nullable expression", + " return Comparators.<@Nullable String>min(t1, t2, cmp);", + " }", + "}") + .doTest(); + } + @Test public void testCloserParametricNullness() { defaultCompilationHelper diff --git a/nullaway/src/main/java/com/uber/nullaway/Nullness.java b/nullaway/src/main/java/com/uber/nullaway/Nullness.java index 7db7abf50a..3190ed6cdb 100644 --- a/nullaway/src/main/java/com/uber/nullaway/Nullness.java +++ b/nullaway/src/main/java/com/uber/nullaway/Nullness.java @@ -183,12 +183,14 @@ public static boolean isNullableAnnotation(String annotName, Config config) { || annotName.endsWith(".checkerframework.checker.nullness.compatqual.NullableDecl") // matches javax.annotation.CheckForNull and edu.umd.cs.findbugs.annotations.CheckForNull || annotName.endsWith(".CheckForNull") - // matches any of the multiple @ParametricNullness annotations used within Guava - // (see https://github.com/google/guava/issues/6126) - // We check the simple name first and the package prefix second for boolean short - // circuiting, as Guava includes - // many annotations - || (annotName.endsWith(".ParametricNullness") && annotName.startsWith("com.google.common.")) + // matches any of the multiple @ParametricNullness annotations used within Guava (see + // https://github.com/google/guava/issues/6126). We check the simple name first and the + // package prefix second for boolean short circuiting, as Guava includes many annotations. + // We do _not_ match @ParametricNullness annotations in JSpecify mode and rely on generics + // checking instead. + || (!config.isJSpecifyMode() + && annotName.endsWith(".ParametricNullness") + && annotName.startsWith("com.google.common.")) || (config.acknowledgeAndroidRecent() && annotName.equals("androidx.annotation.RecentlyNullable")) || config.isCustomNullableAnnotation(annotName); diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/StreamNullabilityPropagator.java b/nullaway/src/main/java/com/uber/nullaway/handlers/StreamNullabilityPropagator.java index c411522da7..eab49f50d3 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/StreamNullabilityPropagator.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/StreamNullabilityPropagator.java @@ -22,6 +22,8 @@ * THE SOFTWARE. */ +import static com.uber.nullaway.NullabilityUtil.castToNonNull; + import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -377,7 +379,7 @@ private void handleChainFromFilter( } else if (collectCallToRecordsAndInnerMethodsOrLambdas.containsKey(outerCallInChain)) { // Update mapOrCollectRecordToFilterMap for all relevant methods / lambdas for (CollectRecordAndInnerMethod collectRecordAndInnerMethod : - collectCallToRecordsAndInnerMethodsOrLambdas.get(outerCallInChain)) { + collectCallToRecordsAndInnerMethodsOrLambdas.get(castToNonNull(outerCallInChain))) { MapOrCollectMethodToFilterInstanceRecord record = new MapOrCollectMethodToFilterInstanceRecord( collectRecordAndInnerMethod.getCollectLikeMethodRecord(), filterMethodOrLambda);