From a72b4a602d8b1a7061d60a9ce263d48fd9a07088 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Fri, 20 Oct 2023 11:16:18 -0700 Subject: [PATCH 1/6] changes related to the test testForLambdasInAnAssignment --- .../dataflow/CoreNullnessStoreInitializer.java | 18 ++++++++++++++++-- .../NullAwayJSpecifyGenericsTests.java | 3 +-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java index 5a5eabcc73..aeac6ce7b6 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java @@ -3,10 +3,12 @@ import static com.uber.nullaway.Nullness.NONNULL; import static com.uber.nullaway.Nullness.NULLABLE; +import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.ClassTree; import com.sun.source.tree.LambdaExpressionTree; import com.sun.source.tree.VariableTree; import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Types; import com.sun.tools.javac.util.Context; import com.uber.nullaway.CodeAnnotationInfo; @@ -92,13 +94,25 @@ private static NullnessStore lambdaInitialStore( Symbol.MethodSymbol fiMethodSymbol = NullabilityUtil.getFunctionalInterfaceMethod(code, types); com.sun.tools.javac.util.List fiMethodParameters = fiMethodSymbol.getParameters(); + // This obtains the Types of the functional interface method with preserved annotations in case + // of generic types + List overridenMethodParamTypeList = + types.memberType(ASTHelpers.getType(code), fiMethodSymbol).getParameterTypes(); // If fiArgumentPositionNullness[i] == null, parameter position i is unannotated Nullness[] fiArgumentPositionNullness = new Nullness[fiMethodParameters.size()]; final boolean isFIAnnotated = !codeAnnotationInfo.isSymbolUnannotated(fiMethodSymbol, config); if (isFIAnnotated) { for (int i = 0; i < fiMethodParameters.size(); i++) { - fiArgumentPositionNullness[i] = - Nullness.hasNullableAnnotation(fiMethodParameters.get(i), config) ? NULLABLE : NONNULL; + if (Nullness.hasNullableAnnotation(fiMethodParameters.get(i), config)) { + // Get the Nullness if the Annotation is directly written with the parameter + fiArgumentPositionNullness[i] = NULLABLE; + } else if (Nullness.hasNullableAnnotation( + overridenMethodParamTypeList.get(i).getAnnotationMirrors().stream(), config)) { + // Get the Nullness if the Annotation is indirectly applied through a generic type + fiArgumentPositionNullness[i] = NULLABLE; + } else { + fiArgumentPositionNullness[i] = NONNULL; + } } } fiArgumentPositionNullness = diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index dd70683380..7d445cedd4 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -596,8 +596,7 @@ public void testForLambdasInAnAssignment() { " String function(T1 o);", " }", " static void testPositive() {", - " // TODO: we should report an error here, since the lambda cannot take", - " // a @Nullable parameter. we don't catch this yet", + " // BUG: Diagnostic contains: dereferenced expression o is @Nullable", " A<@Nullable Object> p = o -> o.toString();", " }", " static void testNegative() {", From b6b1c502e9457c5d96005b0adc3c858dd2f89309 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Sat, 21 Oct 2023 14:29:17 -0700 Subject: [PATCH 2/6] Adding JSpecify check --- .../nullaway/dataflow/CoreNullnessStoreInitializer.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java index aeac6ce7b6..786e46895b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java @@ -106,9 +106,11 @@ private static NullnessStore lambdaInitialStore( if (Nullness.hasNullableAnnotation(fiMethodParameters.get(i), config)) { // Get the Nullness if the Annotation is directly written with the parameter fiArgumentPositionNullness[i] = NULLABLE; - } else if (Nullness.hasNullableAnnotation( - overridenMethodParamTypeList.get(i).getAnnotationMirrors().stream(), config)) { - // Get the Nullness if the Annotation is indirectly applied through a generic type + } else if (config.isJSpecifyMode() + && Nullness.hasNullableAnnotation( + overridenMethodParamTypeList.get(i).getAnnotationMirrors().stream(), config)) { + // Get the Nullness if the Annotation is indirectly applied through a generic type if we + // are in JSpecify mode fiArgumentPositionNullness[i] = NULLABLE; } else { fiArgumentPositionNullness[i] = NONNULL; From 5f545de16f44aa23befd6f1407919766a28116da Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Sat, 21 Oct 2023 15:04:02 -0700 Subject: [PATCH 3/6] Adding a test case for lambdas without JSpecify mode --- .../NullAwayJSpecifyGenericsTests.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 7d445cedd4..0d6bcd0093 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -606,6 +606,28 @@ public void testForLambdasInAnAssignment() { .doTest(); } + @Test + public void testForLambdasInAnAssignmentWithoutJSpecifyMode() { + makeHelperWithoutJSpecifyMode() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " interface A {", + " String function(T1 o);", + " }", + " static void testPositive() {", + " // Using outside JSpecify Mode So we don't get a bug here", + " A<@Nullable Object> p = o -> o.toString();", + " }", + " static void testNegative() {", + " A p = o -> o.toString();", + " }", + "}") + .doTest(); + } + @Test public void testForDiamondInAnAssignment() { makeHelper() @@ -1426,4 +1448,8 @@ private CompilationTestHelper makeHelper() { Arrays.asList( "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:JSpecifyMode=true")); } + + private CompilationTestHelper makeHelperWithoutJSpecifyMode() { + return makeTestHelperWithArgs(Arrays.asList("-XepOpt:NullAway:AnnotatedPackages=com.uber")); + } } From e3ba9381a3feacbd9b07af813908b0c79200d1da Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Sun, 22 Oct 2023 11:05:59 -0700 Subject: [PATCH 4/6] comment tweak --- .../uber/nullaway/dataflow/CoreNullnessStoreInitializer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java index 786e46895b..b0b77c7b53 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java @@ -94,8 +94,8 @@ private static NullnessStore lambdaInitialStore( Symbol.MethodSymbol fiMethodSymbol = NullabilityUtil.getFunctionalInterfaceMethod(code, types); com.sun.tools.javac.util.List fiMethodParameters = fiMethodSymbol.getParameters(); - // This obtains the Types of the functional interface method with preserved annotations in case - // of generic types + // This obtains the Types of the functional interface method parameters with preserved + // annotations in case of generic types List overridenMethodParamTypeList = types.memberType(ASTHelpers.getType(code), fiMethodSymbol).getParameterTypes(); // If fiArgumentPositionNullness[i] == null, parameter position i is unannotated From 5d7c120dc80b6a8955e8807ceeac6ec47c2a5e6a Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 23 Oct 2023 11:54:32 -0700 Subject: [PATCH 5/6] tweak comment --- .../uber/nullaway/dataflow/CoreNullnessStoreInitializer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java index b0b77c7b53..2dfce9fc74 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java @@ -94,8 +94,8 @@ private static NullnessStore lambdaInitialStore( Symbol.MethodSymbol fiMethodSymbol = NullabilityUtil.getFunctionalInterfaceMethod(code, types); com.sun.tools.javac.util.List fiMethodParameters = fiMethodSymbol.getParameters(); - // This obtains the Types of the functional interface method parameters with preserved - // annotations in case of generic types + // This obtains the types of the functional interface method parameters with preserved + // annotations in case of generic type arguments. Only used in JSpecify mode. List overridenMethodParamTypeList = types.memberType(ASTHelpers.getType(code), fiMethodSymbol).getParameterTypes(); // If fiArgumentPositionNullness[i] == null, parameter position i is unannotated From 86c2d6b50ea11530ad78e1e51a361ada6080b82f Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Mon, 23 Oct 2023 15:57:54 -0700 Subject: [PATCH 6/6] Adding test case for lambda that has two params --- .../NullAwayJSpecifyGenericsTests.java | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 0d6bcd0093..73737b8ca6 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -585,7 +585,7 @@ public void testForMethodReferenceAsMethodParameter() { } @Test - public void testForLambdasInAnAssignment() { + public void testForLambdasInAnAssignmentWithSingleParam() { makeHelper() .addSourceLines( "Test.java", @@ -606,6 +606,28 @@ public void testForLambdasInAnAssignment() { .doTest(); } + @Test + public void testForLambdasInAnAssignmentWithMultipleParams() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " interface A {", + " String function(T1 o1,T2 o2);", + " }", + " static void testPositive() {", + " // BUG: Diagnostic contains: dereferenced expression o1 is @Nullable", + " A<@Nullable Object,Object> p = (o1,o2) -> o1.toString();", + " }", + " static void testNegative() {", + " A<@Nullable Object,Object> p = (o1,o2) -> o2.toString();", + " }", + "}") + .doTest(); + } + @Test public void testForLambdasInAnAssignmentWithoutJSpecifyMode() { makeHelperWithoutJSpecifyMode()