diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index db5b9585b6..94e7911b4c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -2083,15 +2083,20 @@ private Description handleInvocation( } } - // perform generics checks for calls to annotated methods in JSpecify mode - if (config.isJSpecifyMode()) { - genericsChecks.compareGenericTypeParameterNullabilityForCall(methodSymbol, tree, state); - if (!methodSymbol.getTypeParameters().isEmpty()) { - genericsChecks.checkGenericMethodCallTypeArguments(tree, state); - } + // perform generics checks for type arguments on calls to annotated methods in JSpecify mode + if (config.isJSpecifyMode() && !methodSymbol.getTypeParameters().isEmpty()) { + genericsChecks.checkGenericMethodCallTypeArguments(tree, state); } } + // Perform generics checks for calls in JSpecify mode, including calls to @NullUnmarked + // methods. For @NullUnmarked methods, only restrictive (explicit) nullness annotations + // on type parameters are enforced (see #1488). + if (config.isJSpecifyMode()) { + genericsChecks.compareGenericTypeParameterNullabilityForCall( + methodSymbol, tree, state, isMethodAnnotated); + } + // Allow handlers to override the list of non-null argument positions. For a varargs parameter, // this array holds the nullness of individual varargs arguments; the case of a varargs array is // handled separately diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java index bb8f9a604a..eb628fe29a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java @@ -5,6 +5,7 @@ import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Types; import com.uber.nullaway.Config; +import com.uber.nullaway.Nullness; import java.util.List; import javax.lang.model.type.NullType; import javax.lang.model.type.TypeKind; @@ -18,12 +19,22 @@ public class CheckIdenticalNullabilityVisitor extends Types.DefaultTypeVisitor { - if (!subtypeParameterNullability(supertype, subtype, state)) { + if (!subtypeParameterNullability( + supertype, subtype, state, isUnannotated)) { if (relationKind == MethodRefTypeRelationKind.RETURN) { reportInvalidMethodReferenceReturnTypeError( memberReferenceTree, supertype, subtype, state); @@ -1646,7 +1665,8 @@ public void compareGenericTypeParameterNullabilityForCall( false, false); } - if (!subtypeParameterNullability(formalParameter, actualParameterType, state)) { + if (!subtypeParameterNullability( + formalParameter, actualParameterType, state, !isMethodAnnotated)) { reportInvalidParametersNullabilityError( formalParameter, actualParameterType, currentActualParam, state); } @@ -1929,8 +1949,12 @@ private Type substituteTypeArgsInGenericMethodType( return TypeSubstitutionUtils.updateMethodTypeWithInferredNullability( methodTypeAtCallSite, methodType, successResult.typeVarNullability, state, config); } else { - // inference failed; just return the method type at the call site with no substitutions - return methodTypeAtCallSite; + // inference failed; restore explicit nullability annotations from the original method + // type to the call-site type, so restrictive annotations (e.g. @NonNull on type + // arguments) are still enforced + return TypeSubstitutionUtils.restoreExplicitNullabilityAnnotations( + methodType, methodTypeAtCallSite, config, Collections.emptyMap()) + .asMethodType(); } } return TypeSubstitutionUtils.subst( diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/NullMarkednessTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/NullMarkednessTests.java index 2345fb50b6..feea2da5d7 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/NullMarkednessTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/NullMarkednessTests.java @@ -1031,6 +1031,78 @@ public static void caller() { .doTest(); } + @Test + public void nullUnmarkedRestrictiveAnnotationsOnTypeVariables() { + makeTestHelperWithArgs( + JSpecifyJavacConfig.withJSpecifyModeArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:OnlyNullMarked=true"))) + .addSourceLines( + "Test.java", + """ + package com.uber; + import org.jspecify.annotations.NonNull; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.NullUnmarked; + import org.jspecify.annotations.Nullable; + @NullMarked + class Test { + interface Foo {} + @NullUnmarked + static void takeFoo(Foo<@NonNull String> f) {} + @NullUnmarked + static T identity(Foo<@NonNull T> t) { + throw new RuntimeException("not implemented"); + } + void test(Foo<@Nullable String> s, Foo nonNullFoo) { + // Error: passing Foo<@Nullable String> where Foo<@NonNull String> is required + // BUG: Diagnostic contains: incompatible types + takeFoo(s); + // OK: passing Foo where Foo<@NonNull String> is required + takeFoo(nonNullFoo); + // Error: passing Foo<@Nullable String> to generic identity method + // with restrictive @NonNull on type parameter + // BUG: Diagnostic contains: incompatible types + String x = identity(s); + // OK: passing Foo where Foo<@NonNull String> is expected + String y = identity(nonNullFoo); + } + } + """) + .doTest(); + } + + @Test + public void nullUnmarkedUnannotatedTypeVariableNoError() { + makeTestHelperWithArgs( + JSpecifyJavacConfig.withJSpecifyModeArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:OnlyNullMarked=true"))) + .addSourceLines( + "Test.java", + """ + package com.uber; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.NullUnmarked; + import org.jspecify.annotations.Nullable; + import java.util.List; + @NullMarked + class Test { + @NullUnmarked + static void bar(List list) {} + void test(List<@Nullable String> s) { + // No error: T is unannotated in @NullUnmarked code, so unconstrained + bar(s); + } + } + """) + .doTest(); + } + @Test public void nullUnmarkedOuterMethodLevelWithLocalClass() { defaultCompilationHelper