From 1569e530eb6dfec80ef49b7d752cc61614f00a2a Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 11 Dec 2024 14:14:10 -0800 Subject: [PATCH 1/3] fix --- .../com/uber/nullaway/NullabilityUtil.java | 31 ++++++++++++++----- .../java/com/uber/nullaway/VarargsTests.java | 16 ++++++++++ .../src/main/java/com/uber/lib/Varargs.java | 3 ++ 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java index fc93add428..e736aa1119 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java @@ -540,15 +540,30 @@ public static 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; - } - } - } + // TODO remove copy-paste!!!!! + if (getTypeUseAnnotations(arraySymbol, config, /* onlyDirect= */ false) + .anyMatch( + t -> { + for (TypeAnnotationPosition.TypePathEntry entry : t.position.location) { + if (entry.tag == TypeAnnotationPosition.TypePathEntryKind.ARRAY) { + if (Nullness.isNullableAnnotation(t.type.toString(), config)) { + return true; + } + } + } + return false; + })) { + return true; } + // 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 diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index 3f29e1e082..cfee867589 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/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 diff --git a/test-java-lib/src/main/java/com/uber/lib/Varargs.java b/test-java-lib/src/main/java/com/uber/lib/Varargs.java index a232d1c14a..81cefb9166 100644 --- a/test-java-lib/src/main/java/com/uber/lib/Varargs.java +++ b/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) {} } From 5ada6f0a6c485072b17b3c7adf4b087ad273f9e3 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 11 Dec 2024 16:46:04 -0800 Subject: [PATCH 2/3] cleanup and another fix --- .../com/uber/nullaway/NullabilityUtil.java | 71 +++++++++---------- 1 file changed, 34 insertions(+), 37 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java index e736aa1119..1bfe710c42 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java +++ b/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 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,13 +541,35 @@ public static T castToNonNull(@Nullable T obj) { * otherwise */ public static boolean isArrayElementNullable(Symbol arraySymbol, Config config) { - // TODO remove copy-paste!!!!! + return checkArrayElementAnnotations( + arraySymbol, + config, + Nullness::isNullableAnnotation, + Nullness::hasNullableDeclarationAnnotation); + } + + /** + * 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 typeUseCheck, + BiPredicate declarationCheck) { if (getTypeUseAnnotations(arraySymbol, config, /* onlyDirect= */ false) .anyMatch( t -> { for (TypeAnnotationPosition.TypePathEntry entry : t.position.location) { if (entry.tag == TypeAnnotationPosition.TypePathEntryKind.ARRAY) { - if (Nullness.isNullableAnnotation(t.type.toString(), config)) { + if (typeUseCheck.test(t.type.toString(), config)) { return true; } } @@ -555,20 +578,10 @@ public static boolean isArrayElementNullable(Symbol arraySymbol, Config config) })) { return true; } - // 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 + // 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.hasNullableDeclarationAnnotation(arraySymbol, config); + return declarationCheck.test(arraySymbol, config); } return false; } @@ -597,27 +610,11 @@ public static boolean nullableVarargsElementsForSourceOrBytecode( * otherwise */ public static boolean isArrayElementNonNull(Symbol arraySymbol, Config config) { - 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)) { - return true; - } - } - } - return false; - })) { - return true; - } - // For varargs symbols we also consider the elements to be @NonNull if there is a @NonNull - // 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.hasNonNullDeclarationAnnotation(arraySymbol, config); - } - return false; + return checkArrayElementAnnotations( + arraySymbol, + config, + Nullness::isNonNullAnnotation, + Nullness::hasNonNullDeclarationAnnotation); } /** From a2eef51d112c72d02811f089bd80282829ceda8b Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 11 Dec 2024 16:50:41 -0800 Subject: [PATCH 3/3] move method for cleaner diff --- .../com/uber/nullaway/NullabilityUtil.java | 76 +++++++++---------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java index 1bfe710c42..4df2807189 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java @@ -548,44 +548,6 @@ public static boolean isArrayElementNullable(Symbol arraySymbol, Config config) Nullness::hasNullableDeclarationAnnotation); } - /** - * 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 typeUseCheck, - BiPredicate declarationCheck) { - if (getTypeUseAnnotations(arraySymbol, config, /* onlyDirect= */ false) - .anyMatch( - t -> { - for (TypeAnnotationPosition.TypePathEntry entry : t.position.location) { - if (entry.tag == TypeAnnotationPosition.TypePathEntryKind.ARRAY) { - if (typeUseCheck.test(t.type.toString(), config)) { - return true; - } - } - } - return false; - })) { - return true; - } - // 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 declarationCheck.test(arraySymbol, config); - } - return false; - } - /** * Checks if the given varargs symbol has a {@code @Nullable} annotation for its elements. Works * for both source and bytecode. @@ -632,6 +594,44 @@ public static boolean nonnullVarargsElementsForSourceOrBytecode( || 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 typeUseCheck, + BiPredicate declarationCheck) { + if (getTypeUseAnnotations(arraySymbol, config, /* onlyDirect= */ false) + .anyMatch( + t -> { + for (TypeAnnotationPosition.TypePathEntry entry : t.position.location) { + if (entry.tag == TypeAnnotationPosition.TypePathEntryKind.ARRAY) { + if (typeUseCheck.test(t.type.toString(), config)) { + return true; + } + } + } + return false; + })) { + return true; + } + // 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 declarationCheck.test(arraySymbol, config); + } + return false; + } + /** * Does the given symbol have a JetBrains @NotNull declaration annotation? Useful for workarounds * in light of https://github.com/uber/NullAway/issues/720