From 145f0e35e49dd68717752dea6245689cb93ab7e8 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 14 Feb 2023 06:56:14 -0800 Subject: [PATCH 01/29] WIP --- .../main/java/com/uber/nullaway/Nullness.java | 9 +++++ .../CoreNullnessStoreInitializer.java | 11 ++++-- .../java/com/uber/nullaway/VarargsTests.java | 37 ++++++++++++++++++- 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/Nullness.java b/nullaway/src/main/java/com/uber/nullaway/Nullness.java index 0ed694b60d..fbd0da70a0 100644 --- a/nullaway/src/main/java/com/uber/nullaway/Nullness.java +++ b/nullaway/src/main/java/com/uber/nullaway/Nullness.java @@ -251,4 +251,13 @@ public static boolean paramHasNonNullAnnotation( return hasNonNullAnnotation( NullabilityUtil.getAllAnnotationsForParameter(symbol, paramInd, config), config); } + + /** + * Is the varargs parameter {@code paramSymbol} have a {@code @Nullable} annotation indicating + * that the argument array passed at a call site can be {@code null}? Syntactically, this would be + * written as {@code foo(Object @Nullable... args}} + */ + public static boolean varargsParamIsNullable(Symbol paramSymbol, Config config) { + return false; + } } 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 0726cd87f5..9f0389cdc3 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java @@ -7,6 +7,7 @@ import com.sun.source.tree.ClassTree; import com.sun.source.tree.LambdaExpressionTree; import com.sun.source.tree.VariableTree; +import com.sun.tools.javac.code.Flags; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Types; @@ -64,9 +65,13 @@ private static NullnessStore methodInitialStore( NullnessStore envStore = getEnvNullnessStoreForClass(classTree, context); NullnessStore.Builder result = envStore.toBuilder(); for (LocalVariableNode param : parameters) { - Element element = param.getElement(); - Nullness assumed = - Nullness.hasNullableAnnotation((Symbol) element, config) ? NULLABLE : NONNULL; + Symbol paramSymbol = (Symbol) param.getElement(); + Nullness assumed; + if ((paramSymbol.flags() & Flags.VARARGS) != 0) { + assumed = Nullness.varargsParamIsNullable(paramSymbol, config) ? NULLABLE : NONNULL; + } else { + assumed = Nullness.hasNullableAnnotation(paramSymbol, config) ? NULLABLE : NONNULL; + } result.setInformation(AccessPath.fromLocal(param), assumed); } result = handler.onDataflowInitialStore(underlyingAST, parameters, result); diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index f404668695..28eb51c189 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -46,11 +46,9 @@ public void testNullableVarargs() { "public class Utilities {", " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", " String s = o.toString() + \" \";", - " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", // Shouldn't be an error! " for (Object other : others) {", " s += (other == null) ? \"(null) \" : other.toString() + \" \";", " }", - " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", // Shouldn't be an error! " for (Object other : others) {", " s += other.toString();", // SHOULD be an error! " }", @@ -177,4 +175,39 @@ public void testSkipJetbrainsNotNullOnVarArgsFromThirdPartyJars() { "}") .doTest(); } + + @Test + public void nullableVarargsArray() { + defaultCompilationHelper + .addSourceLines( + "Utilities.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Utilities {", + " public static String takesNullableVarargsArray(Object o, Object @Nullable... others) {", + " String s = o.toString() + \" \";", + " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", + " for (Object other : others) {", + " s += (other == null) ? \"(null) \" : other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Test {", + " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", + " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", + " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed + " // BUG: Diagnostic contains: passing @Nullable parameter", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", + " // this is fine!", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", + " }", + "}") + .doTest(); + } } From 50862eba66876c495f198428ea1b2e2ae9e25f6c Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 2 Aug 2024 17:39:10 -0700 Subject: [PATCH 02/29] WIP --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 5 +++-- nullaway/src/main/java/com/uber/nullaway/Nullness.java | 2 +- nullaway/src/test/java/com/uber/nullaway/VarargsTests.java | 2 ++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index e481729e07..41945c0558 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1764,12 +1764,13 @@ private Description handleInvocation( // NOTE: the case of an invocation on a possibly-null reference // is handled by matchMemberSelect() for (int argPos = 0; argPos < argumentPositionNullness.length; argPos++) { - if (!Objects.equals(Nullness.NONNULL, argumentPositionNullness[argPos])) { + boolean varargPosition = argPos == formalParams.size() - 1 && methodSymbol.isVarArgs(); + if (!varargPosition && !Objects.equals(Nullness.NONNULL, argumentPositionNullness[argPos])) { continue; } ExpressionTree actual = null; boolean mayActualBeNull = false; - if (argPos == formalParams.size() - 1 && methodSymbol.isVarArgs()) { + if (varargPosition) { // Check all vararg actual arguments for nullability if (actualParams.size() <= argPos) { continue; diff --git a/nullaway/src/main/java/com/uber/nullaway/Nullness.java b/nullaway/src/main/java/com/uber/nullaway/Nullness.java index fbd0da70a0..6cdfa744bf 100644 --- a/nullaway/src/main/java/com/uber/nullaway/Nullness.java +++ b/nullaway/src/main/java/com/uber/nullaway/Nullness.java @@ -258,6 +258,6 @@ public static boolean paramHasNonNullAnnotation( * written as {@code foo(Object @Nullable... args}} */ public static boolean varargsParamIsNullable(Symbol paramSymbol, Config config) { - return false; + return hasNullableAnnotation(paramSymbol, config); } } diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index 28eb51c189..1d0d5f72ee 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -44,6 +44,8 @@ public void testNullableVarargs() { "package com.uber;", "import javax.annotation.Nullable;", "public class Utilities {", + // TODO a declaration annotation at the top level should apply to the individual + // arguments, not the variable itself " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", " String s = o.toString() + \" \";", " for (Object other : others) {", From fbd131e34aa6144c3020297c99f1024f8b9f4b2d Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 2 Aug 2024 17:39:41 -0700 Subject: [PATCH 03/29] format --- nullaway/src/test/java/com/uber/nullaway/VarargsTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index 1d0d5f72ee..a8ecd27d53 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -45,7 +45,7 @@ public void testNullableVarargs() { "import javax.annotation.Nullable;", "public class Utilities {", // TODO a declaration annotation at the top level should apply to the individual - // arguments, not the variable itself + // arguments, not the variable itself " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", " String s = o.toString() + \" \";", " for (Object other : others) {", From db9c3b1b629ae77d59349a13006793c47961b9d8 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 17 Aug 2024 17:28:23 -0700 Subject: [PATCH 04/29] test case --- .../java/com/uber/nullaway/VarargsTests.java | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index a8ecd27d53..9d5834cd49 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -212,4 +212,43 @@ public void nullableVarargsArray() { "}") .doTest(); } + + @Test + public void typeUseBeforeDots() { + defaultCompilationHelper + .addSourceLines( + "Nullable.java", + "package com.uber;", + "import java.lang.annotation.ElementType;", + "import java.lang.annotation.Target;", + "@Target({ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE, ElementType.TYPE_USE})", + "public @interface Nullable {}") + .addSourceLines( + "Utilities.java", + "package com.uber;", + "public class Utilities {", + " public static String takesNullableVarargs(@Nullable Object @Nullable... others) {", + " String s = \"\";", + " for (Object other : others) {", + " s += other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Test {", + " public void testNullableVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNullableVarargs(o1, o2, o3, o4);", + " Utilities.takesNullableVarargs(o1);", // Empty var args passed + " Utilities.takesNullableVarargs(o1, o4);", + " Utilities.takesNullableVarargs(o1, (java.lang.Object) null);", + // SHOULD be an error! + " Utilities.takesNullableVarargs(o1, (java.lang.Object[]) null);", + " }", + "}") + .doTest(); + } } From d25d350299a0857b47011cc452d602e6d73a80f3 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 17 Aug 2024 17:35:35 -0700 Subject: [PATCH 05/29] tweak test --- .../src/test/java/com/uber/nullaway/VarargsTests.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index 9d5834cd49..8ffe76de13 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -44,16 +44,8 @@ public void testNullableVarargs() { "package com.uber;", "import javax.annotation.Nullable;", "public class Utilities {", - // TODO a declaration annotation at the top level should apply to the individual - // arguments, not the variable itself " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", - " String s = o.toString() + \" \";", - " for (Object other : others) {", - " s += (other == null) ? \"(null) \" : other.toString() + \" \";", - " }", - " for (Object other : others) {", - " s += other.toString();", // SHOULD be an error! - " }", + " String s = o.toString() + \" \" + others.toString();", " return s;", " }", "}") From a466b27c7fc124c6db39f208067352cc145776fe Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 17 Aug 2024 20:30:14 -0700 Subject: [PATCH 06/29] WIP --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 1 + nullaway/src/test/java/com/uber/nullaway/VarargsTests.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 41945c0558..68da94fdf8 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -2496,6 +2496,7 @@ private Description matchDereference( return Description.NO_MATCH; } } + // (baseExpressionSymbol.flags() & Flags.VARARGS) != 0 if (mayBeNullExpr(state, baseExpression)) { String message = "dereferenced expression " + state.getSourceForNode(baseExpression) + " is @Nullable"; diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index 8ffe76de13..000a5ef2e8 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -59,7 +59,7 @@ public void testNullableVarargs() { " Utilities.takesNullableVarargs(o1);", // Empty var args passed " Utilities.takesNullableVarargs(o1, o4);", " Utilities.takesNullableVarargs(o1, (java.lang.Object) null);", - // SHOULD be an error! + // TODO SHOULD be an error! " Utilities.takesNullableVarargs(o1, (java.lang.Object[]) null);", " }", "}") From 9f37626e5d433be2b7b698c3269c5c387dd55be3 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 18 Aug 2024 07:54:09 -0700 Subject: [PATCH 07/29] WIP --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 4 ++-- .../src/main/java/com/uber/nullaway/NullabilityUtil.java | 2 +- nullaway/src/main/java/com/uber/nullaway/Nullness.java | 6 +++++- nullaway/src/test/java/com/uber/nullaway/VarargsTests.java | 5 +++-- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 68da94fdf8..15962a7042 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1764,12 +1764,12 @@ private Description handleInvocation( // NOTE: the case of an invocation on a possibly-null reference // is handled by matchMemberSelect() for (int argPos = 0; argPos < argumentPositionNullness.length; argPos++) { - boolean varargPosition = argPos == formalParams.size() - 1 && methodSymbol.isVarArgs(); - if (!varargPosition && !Objects.equals(Nullness.NONNULL, argumentPositionNullness[argPos])) { + if (!Objects.equals(Nullness.NONNULL, argumentPositionNullness[argPos])) { continue; } ExpressionTree actual = null; boolean mayActualBeNull = false; + boolean varargPosition = methodSymbol.isVarArgs() && argPos == formalParams.size() - 1; if (varargPosition) { // Check all vararg actual arguments for nullability if (actualParams.size() <= argPos) { diff --git a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java index c8c41bdfce..87f37fc602 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java @@ -278,7 +278,7 @@ public static Stream getAllAnnotationsForParameter( * Gets the type use annotations on a symbol, ignoring annotations on components of the type (type * arguments, wildcards, etc.) */ - private static Stream getTypeUseAnnotations( + public static Stream getTypeUseAnnotations( Symbol symbol, Config config) { Stream rawTypeAttributes = symbol.getRawTypeAttributes().stream(); if (symbol instanceof Symbol.MethodSymbol) { diff --git a/nullaway/src/main/java/com/uber/nullaway/Nullness.java b/nullaway/src/main/java/com/uber/nullaway/Nullness.java index 6cdfa744bf..cc9e573a02 100644 --- a/nullaway/src/main/java/com/uber/nullaway/Nullness.java +++ b/nullaway/src/main/java/com/uber/nullaway/Nullness.java @@ -206,6 +206,10 @@ public static boolean hasNullableAnnotation(Symbol symbol, Config config) { return hasNullableAnnotation(NullabilityUtil.getAllAnnotations(symbol, config), config); } + public static boolean hasNullableTypeUseAnnotation(Symbol symbol, Config config) { + return hasNullableAnnotation(NullabilityUtil.getTypeUseAnnotations(symbol, config), config); + } + /** * Does the parameter of {@code symbol} at {@code paramInd} have a {@code @Nullable} declaration * or type-use annotation? This method works for methods defined in either source or class files. @@ -258,6 +262,6 @@ public static boolean paramHasNonNullAnnotation( * written as {@code foo(Object @Nullable... args}} */ public static boolean varargsParamIsNullable(Symbol paramSymbol, Config config) { - return hasNullableAnnotation(paramSymbol, config); + return hasNullableTypeUseAnnotation(paramSymbol, config); } } diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index 000a5ef2e8..f8343f12ff 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -59,8 +59,9 @@ public void testNullableVarargs() { " Utilities.takesNullableVarargs(o1);", // Empty var args passed " Utilities.takesNullableVarargs(o1, o4);", " Utilities.takesNullableVarargs(o1, (java.lang.Object) null);", - // TODO SHOULD be an error! - " Utilities.takesNullableVarargs(o1, (java.lang.Object[]) null);", + " Object[] x = null;", + " // BUG: Diagnostic contains: passing @Nullable parameter", + " Utilities.takesNullableVarargs(o1, x);", " }", "}") .doTest(); From c45b44cd684dd02922dfa369d3a9188c678ad06a Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 18 Aug 2024 09:04:13 -0700 Subject: [PATCH 08/29] more --- .../main/java/com/uber/nullaway/NullAway.java | 43 +++++++++++++------ .../java/com/uber/nullaway/VarargsTests.java | 2 +- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 15962a7042..43cf795a0c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -36,7 +36,6 @@ import com.google.auto.service.AutoService; import com.google.auto.value.AutoValue; -import com.google.common.base.Preconditions; import com.google.common.base.Verify; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMultimap; @@ -1705,8 +1704,9 @@ private Description handleInvocation( List actualParams) { List formalParams = methodSymbol.getParameters(); + boolean varArgsMethod = methodSymbol.isVarArgs(); if (formalParams.size() != actualParams.size() - && !methodSymbol.isVarArgs() + && !varArgsMethod && !methodSymbol.isStatic() && methodSymbol.isConstructor() && methodSymbol.enclClass().isInner()) { @@ -1751,7 +1751,7 @@ private Description handleInvocation( } if (config.isJSpecifyMode()) { GenericsChecks.compareGenericTypeParameterNullabilityForCall( - formalParams, actualParams, methodSymbol.isVarArgs(), this, state); + formalParams, actualParams, varArgsMethod, this, state); } } @@ -1764,30 +1764,47 @@ private Description handleInvocation( // NOTE: the case of an invocation on a possibly-null reference // is handled by matchMemberSelect() for (int argPos = 0; argPos < argumentPositionNullness.length; argPos++) { - if (!Objects.equals(Nullness.NONNULL, argumentPositionNullness[argPos])) { + boolean varargPosition = varArgsMethod && argPos == formalParams.size() - 1; + boolean argIsNonNull = Objects.equals(Nullness.NONNULL, argumentPositionNullness[argPos]); + if (!varargPosition && !argIsNonNull) { continue; } - ExpressionTree actual = null; + ExpressionTree actual; boolean mayActualBeNull = false; - boolean varargPosition = methodSymbol.isVarArgs() && argPos == formalParams.size() - 1; if (varargPosition) { // Check all vararg actual arguments for nullability if (actualParams.size() <= argPos) { continue; } - for (ExpressionTree arg : actualParams.subList(argPos, actualParams.size())) { - actual = arg; - mayActualBeNull = mayBeNullExpr(state, actual); - if (mayActualBeNull) { - break; + actual = actualParams.get(argPos); + Type.ArrayType varargsArrayType = + (Type.ArrayType) formalParams.get(formalParams.size() - 1).type; + Type actualParameterType = ASTHelpers.getType(actual); + if (actualParameterType != null + && state.getTypes().isAssignable(actualParameterType, varargsArrayType)) { + Verify.verify(actualParams.size() == argPos + 1); + // If varargs array itself is not @Nullable, cannot pass @Nullable array + if (!Nullness.varargsParamIsNullable(formalParams.get(argPos), config)) { + mayActualBeNull = mayBeNullExpr(state, actual); + } + } else { + if (!argIsNonNull) { + continue; + } + // TODO report multiple errors for each violating vararg + for (ExpressionTree arg : actualParams.subList(argPos, actualParams.size())) { + actual = arg; + mayActualBeNull = mayBeNullExpr(state, actual); + if (mayActualBeNull) { + break; + } } } + } else { actual = actualParams.get(argPos); mayActualBeNull = mayBeNullExpr(state, actual); } - // This statement should be unreachable without assigning actual beforehand: - Preconditions.checkNotNull(actual); // make sure we are passing a non-null value if (mayActualBeNull) { String message = diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index f8343f12ff..323f7c66b0 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -60,7 +60,7 @@ public void testNullableVarargs() { " Utilities.takesNullableVarargs(o1, o4);", " Utilities.takesNullableVarargs(o1, (java.lang.Object) null);", " Object[] x = null;", - " // BUG: Diagnostic contains: passing @Nullable parameter", + " // BUG: Diagnostic contains: passing @Nullable parameter 'x'", " Utilities.takesNullableVarargs(o1, x);", " }", "}") From 3d086f3d486aa7ce17ffd502a5eac3c2e64dabfc Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 23 Aug 2024 13:13:32 -0700 Subject: [PATCH 09/29] WIP --- .../main/java/com/uber/nullaway/NullAway.java | 1 + .../main/java/com/uber/nullaway/Nullness.java | 22 +++++++++++++++++-- .../java/com/uber/nullaway/VarargsTests.java | 3 ++- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 43cf795a0c..d2a75b3a5f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1788,6 +1788,7 @@ private Description handleInvocation( mayActualBeNull = mayBeNullExpr(state, actual); } } else { + // TODO need to update this check to see if the vararg array contents is @Nullable if (!argIsNonNull) { continue; } diff --git a/nullaway/src/main/java/com/uber/nullaway/Nullness.java b/nullaway/src/main/java/com/uber/nullaway/Nullness.java index cc9e573a02..aa1b3fce91 100644 --- a/nullaway/src/main/java/com/uber/nullaway/Nullness.java +++ b/nullaway/src/main/java/com/uber/nullaway/Nullness.java @@ -213,6 +213,9 @@ public static boolean hasNullableTypeUseAnnotation(Symbol symbol, Config config) /** * Does the parameter of {@code symbol} at {@code paramInd} have a {@code @Nullable} declaration * or type-use annotation? This method works for methods defined in either source or class files. + * + *

For a varargs parameter, this method returns true if individual arguments passed in + * the varargs positions can be null. */ public static boolean paramHasNullableAnnotation( Symbol.MethodSymbol symbol, int paramInd, Config config) { @@ -221,8 +224,12 @@ public static boolean paramHasNullableAnnotation( if (isRecordEqualsParam(symbol, paramInd)) { return true; } - return hasNullableAnnotation( - NullabilityUtil.getAllAnnotationsForParameter(symbol, paramInd, config), config); + if (symbol.isVarArgs() && paramInd == symbol.getParameters().size() - 1) { + return individualVarargsParamsAreNullable(symbol.getParameters().get(paramInd), config); + } else { + return hasNullableAnnotation( + NullabilityUtil.getAllAnnotationsForParameter(symbol, paramInd, config), config); + } } private static boolean isRecordEqualsParam(Symbol.MethodSymbol symbol, int paramInd) { @@ -264,4 +271,15 @@ public static boolean paramHasNonNullAnnotation( public static boolean varargsParamIsNullable(Symbol paramSymbol, Config config) { return hasNullableTypeUseAnnotation(paramSymbol, config); } + + private static boolean individualVarargsParamsAreNullable(Symbol paramSymbol, Config config) { + // declaration annotation, or type-use annotation on the elements + // TODO update and test for legacy mode + return hasNullableDeclarationAnnotation(paramSymbol, config) + || NullabilityUtil.isArrayElementNullable(paramSymbol, config); + } + + private static boolean hasNullableDeclarationAnnotation(Symbol symbol, Config config) { + return hasNullableAnnotation(symbol.getRawAttributes().stream(), config); + } } diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index 323f7c66b0..48b1754d58 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -157,7 +157,8 @@ public void testSkipJetbrainsNotNullOnVarArgsFromThirdPartyJars() { " ThirdParty.takesNullableVarargs(o1);", // Empty var args passed " ThirdParty.takesNullableVarargs(o1, o4);", " ThirdParty.takesNullableVarargs(o1, (Object) null);", - " ThirdParty.takesNullableVarargs(o1, (Object[]) null);", // SHOULD be an error! + " // BUG: Diagnostic contains: passing @Nullable parameter '(Object[]) null' where @NonNull", + " ThirdParty.takesNullableVarargs(o1, (Object[]) null);", " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", " ThirdParty.takesNullableVarargs(o4);", // First arg is not varargs. " }", From e75c938f6aa42c1f7f63e44e2a4489e898271001 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 23 Aug 2024 13:24:30 -0700 Subject: [PATCH 10/29] WIP --- .../java/com/uber/nullaway/VarargsTests.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index 48b1754d58..e0f3398cae 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -67,6 +67,36 @@ public void testNullableVarargs() { .doTest(); } + public void nullableTypeUseVarargs() { + defaultCompilationHelper + .addSourceLines( + "Utilities.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Utilities {", + " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", + " String s = o.toString() + \" \" + others.toString();", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Test {", + " public void testNonNullVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNullableVarargs(o1, o2, o3, o4);", + " Utilities.takesNullableVarargs(o1);", // Empty var args passed + " Utilities.takesNullableVarargs(o1, o4);", + " Utilities.takesNullableVarargs(o1, (java.lang.Object) null);", + " Object[] x = null;", + " // BUG: Diagnostic contains: passing @Nullable parameter 'x'", + " Utilities.takesNullableVarargs(o1, x);", + " }", + "}") + .doTest(); + } + @Test public void testNonNullVarargsFromHandler() { String generatedAnnot = From 98b3d12f573c87dc7641e33e3a98b1959bcdd150 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 23 Aug 2024 13:37:12 -0700 Subject: [PATCH 11/29] WIP --- .../java/com/uber/nullaway/VarargsTests.java | 135 ++++++++++++++++-- 1 file changed, 122 insertions(+), 13 deletions(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index e0f3398cae..4f77d1473c 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -67,12 +67,13 @@ public void testNullableVarargs() { .doTest(); } + @Test public void nullableTypeUseVarargs() { defaultCompilationHelper .addSourceLines( "Utilities.java", "package com.uber;", - "import javax.annotation.Nullable;", + "import org.checkerframework.checker.nullness.qual.Nullable;", "public class Utilities {", " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", " String s = o.toString() + \" \" + others.toString();", @@ -238,7 +239,40 @@ public void nullableVarargsArray() { } @Test - public void typeUseBeforeDots() { + public void nullableVarargsArrayAndElements() { + defaultCompilationHelper + .addSourceLines( + "Utilities.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Utilities {", + " public static String takesNullableVarargsArray(Object o, @Nullable Object @Nullable... others) {", + " String s = o.toString() + \" \";", + " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", + " for (Object other : others) {", + " s += (other == null) ? \"(null) \" : other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Test {", + " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", + " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", + " // this is fine!", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", + " }", + "}") + .doTest(); + } + + @Test + public void typeUseAndDeclarationBeforeDots() { defaultCompilationHelper .addSourceLines( "Nullable.java", @@ -251,10 +285,11 @@ public void typeUseBeforeDots() { "Utilities.java", "package com.uber;", "public class Utilities {", - " public static String takesNullableVarargs(@Nullable Object @Nullable... others) {", - " String s = \"\";", + " public static String takesNullableVarargsArray(Object o, Object @Nullable... others) {", + " String s = o.toString() + \" \";", + " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", " for (Object other : others) {", - " s += other.toString() + \" \";", + " s += (other == null) ? \"(null) \" : other.toString() + \" \";", " }", " return s;", " }", @@ -262,15 +297,89 @@ public void typeUseBeforeDots() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", "public class Test {", - " public void testNullableVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", - " Utilities.takesNullableVarargs(o1, o2, o3, o4);", - " Utilities.takesNullableVarargs(o1);", // Empty var args passed - " Utilities.takesNullableVarargs(o1, o4);", - " Utilities.takesNullableVarargs(o1, (java.lang.Object) null);", - // SHOULD be an error! - " Utilities.takesNullableVarargs(o1, (java.lang.Object[]) null);", + " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", + " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", + " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed + " // BUG: Diagnostic contains: passing @Nullable parameter", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", + " // this is fine!", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", + " }", + "}") + .doTest(); + } + + @Test + public void typeUseAndDeclarationOnElements() { + defaultCompilationHelper + .addSourceLines( + "Nullable.java", + "package com.uber;", + "import java.lang.annotation.ElementType;", + "import java.lang.annotation.Target;", + "@Target({ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE, ElementType.TYPE_USE})", + "public @interface Nullable {}") + .addSourceLines( + "Utilities.java", + "package com.uber;", + "public class Utilities {", + " public static String takesNullableVarargsArray(Object o, @Nullable Object... others) {", + " String s = o.toString() + \" \";", + " for (Object other : others) {", + " s += (other == null) ? \"(null) \" : other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "public class Test {", + " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", + " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", + " // BUG: Diagnostic contains: passing @Nullable parameter '(java.lang.Object[]) null' where @NonNull", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", + " }", + "}") + .doTest(); + } + + @Test + public void typeUseAndDeclarationOnBoth() { + defaultCompilationHelper + .addSourceLines( + "Nullable.java", + "package com.uber;", + "import java.lang.annotation.ElementType;", + "import java.lang.annotation.Target;", + "@Target({ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE, ElementType.TYPE_USE})", + "public @interface Nullable {}") + .addSourceLines( + "Utilities.java", + "package com.uber;", + "public class Utilities {", + " public static String takesNullableVarargsArray(Object o, @Nullable Object @Nullable... others) {", + " String s = o.toString() + \" \";", + " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", + " for (Object other : others) {", + " s += (other == null) ? \"(null) \" : other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "public class Test {", + " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", + " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", " }", "}") .doTest(); From f32dd87b3ad305a014fa2596c230ddbab161cd87 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 23 Aug 2024 14:06:28 -0700 Subject: [PATCH 12/29] fix for nullable varargs arrays --- .../main/java/com/uber/nullaway/NullAway.java | 4 +-- .../java/com/uber/nullaway/VarargsTests.java | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index d2a75b3a5f..e8d07b3cef 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1781,8 +1781,8 @@ private Description handleInvocation( (Type.ArrayType) formalParams.get(formalParams.size() - 1).type; Type actualParameterType = ASTHelpers.getType(actual); if (actualParameterType != null - && state.getTypes().isAssignable(actualParameterType, varargsArrayType)) { - Verify.verify(actualParams.size() == argPos + 1); + && state.getTypes().isAssignable(actualParameterType, varargsArrayType) + && actualParams.size() == argPos + 1) { // If varargs array itself is not @Nullable, cannot pass @Nullable array if (!Nullness.varargsParamIsNullable(formalParams.get(argPos), config)) { mayActualBeNull = mayBeNullExpr(state, actual); diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index 4f77d1473c..226296373b 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -384,4 +384,30 @@ public void typeUseAndDeclarationOnBoth() { "}") .doTest(); } + + /** testing for no crash */ + @Test + public void varargsPassArrayAndOtherArgs() { + defaultCompilationHelper + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Test {", + " static void takesVarargs(Object... args) {}", + " static void test(Object o) {", + " Object[] x = new Object[10];", + " takesVarargs(x, o);", + " }", + " static void takesNullableVarargsArray(Object @Nullable... args) {}", + " static void test2(Object o) {", + " Object[] x = null;", + " // can pass x as the varargs array itself, but not along with other args", + " takesNullableVarargsArray(x);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'x'", + " takesNullableVarargsArray(x, o);", + " }", + "}") + .doTest(); + } } From 65dd597f4f23a20a425d82812899ff3948de01fd Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 23 Aug 2024 14:26:20 -0700 Subject: [PATCH 13/29] JSpecify mode tests --- .../java/com/uber/nullaway/VarargsTests.java | 5 +- ...rrayTests.java => JSpecifyArrayTests.java} | 2 +- .../jspecify/JSpecifyVarargsTests.java | 427 ++++++++++++++++++ 3 files changed, 432 insertions(+), 2 deletions(-) rename nullaway/src/test/java/com/uber/nullaway/jspecify/{ArrayTests.java => JSpecifyArrayTests.java} (99%) create mode 100644 nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index 226296373b..dd06880687 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -77,6 +77,10 @@ public void nullableTypeUseVarargs() { "public class Utilities {", " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", " String s = o.toString() + \" \" + others.toString();", + " for (Object other : others) {", + " // no error since we do not reason about array element nullability", + " s += other.toString();", + " }", " return s;", " }", "}") @@ -385,7 +389,6 @@ public void typeUseAndDeclarationOnBoth() { .doTest(); } - /** testing for no crash */ @Test public void varargsPassArrayAndOtherArgs() { defaultCompilationHelper diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyArrayTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java rename to nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyArrayTests.java index eee8b80969..35560acae2 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyArrayTests.java @@ -27,7 +27,7 @@ import java.util.Arrays; import org.junit.Test; -public class ArrayTests extends NullAwayTestsBase { +public class JSpecifyArrayTests extends NullAwayTestsBase { @Test public void arrayTopLevelAnnotationDereference() { diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java new file mode 100644 index 0000000000..31be7f1c0a --- /dev/null +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java @@ -0,0 +1,427 @@ +package com.uber.nullaway.jspecify; + +import com.google.errorprone.CompilationTestHelper; +import com.uber.nullaway.NullAwayTestsBase; +import java.util.Arrays; +import org.junit.Test; + +public class JSpecifyVarargsTests extends NullAwayTestsBase { + + @Test + public void testNonNullVarargs() { + makeHelper() + .addSourceLines( + "Utilities.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Utilities {", + " public static String takesNonNullVarargs(Object o, Object... others) {", + " String s = o.toString() + \" \";", + " for (Object other : others) {", + " s += other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Test {", + " public void testNonNullVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNonNullVarargs(o1, o2, o3);", + " Utilities.takesNonNullVarargs(o1);", // Empty var args passed + " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", + " Utilities.takesNonNullVarargs(o1, o4);", + " }", + "}") + .doTest(); + } + + @Test + public void testNullableVarargs() { + makeHelper() + .addSourceLines( + "Utilities.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Utilities {", + " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", + " String s = o.toString() + \" \" + others.toString();", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Test {", + " public void testNonNullVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNullableVarargs(o1, o2, o3, o4);", + " Utilities.takesNullableVarargs(o1);", // Empty var args passed + " Utilities.takesNullableVarargs(o1, o4);", + " Utilities.takesNullableVarargs(o1, (java.lang.Object) null);", + " Object[] x = null;", + " // BUG: Diagnostic contains: passing @Nullable parameter 'x'", + " Utilities.takesNullableVarargs(o1, x);", + " }", + "}") + .doTest(); + } + + @Test + public void nullableTypeUseVarargs() { + makeHelper() + .addSourceLines( + "Utilities.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Utilities {", + " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", + " String s = o.toString() + \" \" + others.toString();", + " for (Object other : others) {", + " // BUG: Diagnostic contains: dereferenced expression other is @Nullable", + " s += other.toString();", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Test {", + " public void testNonNullVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNullableVarargs(o1, o2, o3, o4);", + " Utilities.takesNullableVarargs(o1);", // Empty var args passed + " Utilities.takesNullableVarargs(o1, o4);", + " Utilities.takesNullableVarargs(o1, (java.lang.Object) null);", + " Object[] x = null;", + " // BUG: Diagnostic contains: passing @Nullable parameter 'x'", + " Utilities.takesNullableVarargs(o1, x);", + " }", + "}") + .doTest(); + } + + @Test + public void testNonNullVarargsFromHandler() { + String generatedAnnot = + (Double.parseDouble(System.getProperty("java.specification.version")) >= 11) + ? "@javax.annotation.processing.Generated" + : "@javax.annotation.Generated"; + makeTestHelperWithArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:TreatGeneratedAsUnannotated=true", + "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true")) + .addSourceLines( + "Generated.java", + "package com.uber;", + "import javax.annotation.Nonnull;", + generatedAnnot + "(\"foo\")", + "public class Generated {", + " public static String takesNonNullVarargs(@Nonnull Object o, @Nonnull Object... others) {", + " String s = o.toString() + \" \";", + " for (Object other : others) {", + " s += other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Test {", + " public void testNonNullVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Generated.takesNonNullVarargs(o1, o2, o3);", + " Generated.takesNonNullVarargs(o1);", // Empty var args passed + " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", + " Generated.takesNonNullVarargs(o1, o4);", + " }", + "}") + .doTest(); + } + + // This is required for compatibility with kotlinc generated jars + // See https://github.com/uber/NullAway/issues/720 + @Test + public void testSkipJetbrainsNotNullOnVarArgsFromThirdPartyJars() { + makeTestHelperWithArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.nullaway.[a-zA-Z0-9.]+.unannotated", + "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true")) + .addSourceLines( + "ThirdParty.java", + "package com.uber.nullaway.lib.unannotated;", + "import org.jetbrains.annotations.NotNull;", + "public class ThirdParty {", + " public static String takesNullableVarargs(@NotNull Object o, @NotNull Object... others) {", + " String s = o.toString() + \" \";", + " for (Object other : others) {", + " s += (other == null) ? \"(null) \" : other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "FirstParty.java", + "package com.uber;", + "import org.jetbrains.annotations.NotNull;", + "public class FirstParty {", + " public static String takesNonNullVarargs(@NotNull Object o, @NotNull Object... others) {", + " String s = o.toString() + \" \";", + " for (Object other : others) {", + " s += other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "import com.uber.nullaway.lib.unannotated.ThirdParty;", + "public class Test {", + " public void testNullableVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " ThirdParty.takesNullableVarargs(o1, o2, o3, o4);", + " ThirdParty.takesNullableVarargs(o1);", // Empty var args passed + " ThirdParty.takesNullableVarargs(o1, o4);", + " ThirdParty.takesNullableVarargs(o1, (Object) null);", + " // BUG: Diagnostic contains: passing @Nullable parameter '(Object[]) null' where @NonNull", + " ThirdParty.takesNullableVarargs(o1, (Object[]) null);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", + " ThirdParty.takesNullableVarargs(o4);", // First arg is not varargs. + " }", + " public void testNonNullVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " FirstParty.takesNonNullVarargs(o1, o2, o3);", + " FirstParty.takesNonNullVarargs(o1);", // Empty var args passed + " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", + " FirstParty.takesNonNullVarargs(o1, o4);", + " }", + "}") + .doTest(); + } + + @Test + public void nullableVarargsArray() { + makeHelper() + .addSourceLines( + "Utilities.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Utilities {", + " public static String takesNullableVarargsArray(Object o, Object @Nullable... others) {", + " String s = o.toString() + \" \";", + " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", + " for (Object other : others) {", + " s += (other == null) ? \"(null) \" : other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Test {", + " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", + " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", + " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed + " // BUG: Diagnostic contains: passing @Nullable parameter", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", + " // this is fine!", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", + " }", + "}") + .doTest(); + } + + @Test + public void nullableVarargsArrayAndElements() { + makeHelper() + .addSourceLines( + "Utilities.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Utilities {", + " public static String takesNullableVarargsArray(Object o, @Nullable Object @Nullable... others) {", + " String s = o.toString() + \" \";", + " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", + " for (Object other : others) {", + " // BUG: Diagnostic contains: dereferenced expression other is @Nullable", + " s += other.toString();", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Test {", + " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", + " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", + " // this is fine!", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", + " }", + "}") + .doTest(); + } + + @Test + public void typeUseAndDeclarationBeforeDots() { + makeHelper() + .addSourceLines( + "Nullable.java", + "package com.uber;", + "import java.lang.annotation.ElementType;", + "import java.lang.annotation.Target;", + "@Target({ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE, ElementType.TYPE_USE})", + "public @interface Nullable {}") + .addSourceLines( + "Utilities.java", + "package com.uber;", + "public class Utilities {", + " public static String takesNullableVarargsArray(Object o, Object @Nullable... others) {", + " String s = o.toString() + \" \";", + " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", + " for (Object other : others) {", + " s += (other == null) ? \"(null) \" : other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "public class Test {", + " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", + " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", + " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed + " // BUG: Diagnostic contains: passing @Nullable parameter", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", + " // this is fine!", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", + " }", + "}") + .doTest(); + } + + @Test + public void typeUseAndDeclarationOnElements() { + makeHelper() + .addSourceLines( + "Nullable.java", + "package com.uber;", + "import java.lang.annotation.ElementType;", + "import java.lang.annotation.Target;", + "@Target({ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE, ElementType.TYPE_USE})", + "public @interface Nullable {}") + .addSourceLines( + "Utilities.java", + "package com.uber;", + "public class Utilities {", + " public static String takesNullableVarargsArray(Object o, @Nullable Object... others) {", + " String s = o.toString() + \" \";", + " for (Object other : others) {", + " // BUG: Diagnostic contains: dereferenced expression other is @Nullable", + " s += other.toString();", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "public class Test {", + " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", + " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", + " // BUG: Diagnostic contains: passing @Nullable parameter '(java.lang.Object[]) null' where @NonNull", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", + " }", + "}") + .doTest(); + } + + @Test + public void typeUseAndDeclarationOnBoth() { + makeHelper() + .addSourceLines( + "Nullable.java", + "package com.uber;", + "import java.lang.annotation.ElementType;", + "import java.lang.annotation.Target;", + "@Target({ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE, ElementType.TYPE_USE})", + "public @interface Nullable {}") + .addSourceLines( + "Utilities.java", + "package com.uber;", + "public class Utilities {", + " public static String takesNullableVarargsArray(Object o, @Nullable Object @Nullable... others) {", + " String s = o.toString() + \" \";", + " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", + " for (Object other : others) {", + " // BUG: Diagnostic contains: dereferenced expression other is @Nullable", + " s += other.toString();", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "public class Test {", + " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", + " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", + " }", + "}") + .doTest(); + } + + @Test + public void varargsPassArrayAndOtherArgs() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Test {", + " static void takesVarargs(Object... args) {}", + " static void test(Object o) {", + " Object[] x = new Object[10];", + " takesVarargs(x, o);", + " }", + " static void takesNullableVarargsArray(Object @Nullable... args) {}", + " static void test2(Object o) {", + " Object[] x = null;", + " // can pass x as the varargs array itself, but not along with other args", + " takesNullableVarargsArray(x);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'x'", + " takesNullableVarargsArray(x, o);", + " }", + "}") + .doTest(); + } + + private CompilationTestHelper makeHelper() { + return makeTestHelperWithArgs( + Arrays.asList( + "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:JSpecifyMode=true")); + } +} From c85fec14a6c19c301fb6ef0a325d814537127f1e Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 23 Aug 2024 14:28:18 -0700 Subject: [PATCH 14/29] initial legacy mode tests --- .../com/uber/nullaway/LegacyVarargsTests.java | 424 ++++++++++++++++++ 1 file changed, 424 insertions(+) create mode 100644 nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java diff --git a/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java new file mode 100644 index 0000000000..c0c2469438 --- /dev/null +++ b/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java @@ -0,0 +1,424 @@ +package com.uber.nullaway; + +import com.google.errorprone.CompilationTestHelper; +import java.util.Arrays; +import org.junit.Test; + +public class LegacyVarargsTests extends NullAwayTestsBase { + + @Test + public void testNonNullVarargs() { + makeHelper() + .addSourceLines( + "Utilities.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Utilities {", + " public static String takesNonNullVarargs(Object o, Object... others) {", + " String s = o.toString() + \" \";", + " for (Object other : others) {", + " s += other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Test {", + " public void testNonNullVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNonNullVarargs(o1, o2, o3);", + " Utilities.takesNonNullVarargs(o1);", // Empty var args passed + " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", + " Utilities.takesNonNullVarargs(o1, o4);", + " }", + "}") + .doTest(); + } + + @Test + public void testNullableVarargs() { + makeHelper() + .addSourceLines( + "Utilities.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Utilities {", + " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", + " String s = o.toString() + \" \" + others.toString();", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Test {", + " public void testNonNullVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNullableVarargs(o1, o2, o3, o4);", + " Utilities.takesNullableVarargs(o1);", // Empty var args passed + " Utilities.takesNullableVarargs(o1, o4);", + " Utilities.takesNullableVarargs(o1, (java.lang.Object) null);", + " Object[] x = null;", + " // BUG: Diagnostic contains: passing @Nullable parameter 'x'", + " Utilities.takesNullableVarargs(o1, x);", + " }", + "}") + .doTest(); + } + + @Test + public void nullableTypeUseVarargs() { + makeHelper() + .addSourceLines( + "Utilities.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Utilities {", + " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", + " String s = o.toString() + \" \" + others.toString();", + " for (Object other : others) {", + " // no error since we do not reason about array element nullability", + " s += other.toString();", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Test {", + " public void testNonNullVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNullableVarargs(o1, o2, o3, o4);", + " Utilities.takesNullableVarargs(o1);", // Empty var args passed + " Utilities.takesNullableVarargs(o1, o4);", + " Utilities.takesNullableVarargs(o1, (java.lang.Object) null);", + " Object[] x = null;", + " // BUG: Diagnostic contains: passing @Nullable parameter 'x'", + " Utilities.takesNullableVarargs(o1, x);", + " }", + "}") + .doTest(); + } + + @Test + public void testNonNullVarargsFromHandler() { + String generatedAnnot = + (Double.parseDouble(System.getProperty("java.specification.version")) >= 11) + ? "@javax.annotation.processing.Generated" + : "@javax.annotation.Generated"; + makeTestHelperWithArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:TreatGeneratedAsUnannotated=true", + "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true")) + .addSourceLines( + "Generated.java", + "package com.uber;", + "import javax.annotation.Nonnull;", + generatedAnnot + "(\"foo\")", + "public class Generated {", + " public static String takesNonNullVarargs(@Nonnull Object o, @Nonnull Object... others) {", + " String s = o.toString() + \" \";", + " for (Object other : others) {", + " s += other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Test {", + " public void testNonNullVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Generated.takesNonNullVarargs(o1, o2, o3);", + " Generated.takesNonNullVarargs(o1);", // Empty var args passed + " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", + " Generated.takesNonNullVarargs(o1, o4);", + " }", + "}") + .doTest(); + } + + // This is required for compatibility with kotlinc generated jars + // See https://github.com/uber/NullAway/issues/720 + @Test + public void testSkipJetbrainsNotNullOnVarArgsFromThirdPartyJars() { + makeTestHelperWithArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.nullaway.[a-zA-Z0-9.]+.unannotated", + "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true")) + .addSourceLines( + "ThirdParty.java", + "package com.uber.nullaway.lib.unannotated;", + "import org.jetbrains.annotations.NotNull;", + "public class ThirdParty {", + " public static String takesNullableVarargs(@NotNull Object o, @NotNull Object... others) {", + " String s = o.toString() + \" \";", + " for (Object other : others) {", + " s += (other == null) ? \"(null) \" : other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "FirstParty.java", + "package com.uber;", + "import org.jetbrains.annotations.NotNull;", + "public class FirstParty {", + " public static String takesNonNullVarargs(@NotNull Object o, @NotNull Object... others) {", + " String s = o.toString() + \" \";", + " for (Object other : others) {", + " s += other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "import com.uber.nullaway.lib.unannotated.ThirdParty;", + "public class Test {", + " public void testNullableVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " ThirdParty.takesNullableVarargs(o1, o2, o3, o4);", + " ThirdParty.takesNullableVarargs(o1);", // Empty var args passed + " ThirdParty.takesNullableVarargs(o1, o4);", + " ThirdParty.takesNullableVarargs(o1, (Object) null);", + " // BUG: Diagnostic contains: passing @Nullable parameter '(Object[]) null' where @NonNull", + " ThirdParty.takesNullableVarargs(o1, (Object[]) null);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", + " ThirdParty.takesNullableVarargs(o4);", // First arg is not varargs. + " }", + " public void testNonNullVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " FirstParty.takesNonNullVarargs(o1, o2, o3);", + " FirstParty.takesNonNullVarargs(o1);", // Empty var args passed + " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", + " FirstParty.takesNonNullVarargs(o1, o4);", + " }", + "}") + .doTest(); + } + + @Test + public void nullableVarargsArray() { + makeHelper() + .addSourceLines( + "Utilities.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Utilities {", + " public static String takesNullableVarargsArray(Object o, Object @Nullable... others) {", + " String s = o.toString() + \" \";", + " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", + " for (Object other : others) {", + " s += (other == null) ? \"(null) \" : other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Test {", + " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", + " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", + " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed + " // BUG: Diagnostic contains: passing @Nullable parameter", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", + " // this is fine!", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", + " }", + "}") + .doTest(); + } + + @Test + public void nullableVarargsArrayAndElements() { + makeHelper() + .addSourceLines( + "Utilities.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Utilities {", + " public static String takesNullableVarargsArray(Object o, @Nullable Object @Nullable... others) {", + " String s = o.toString() + \" \";", + " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", + " for (Object other : others) {", + " s += (other == null) ? \"(null) \" : other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Test {", + " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", + " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", + " // this is fine!", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", + " }", + "}") + .doTest(); + } + + @Test + public void typeUseAndDeclarationBeforeDots() { + makeHelper() + .addSourceLines( + "Nullable.java", + "package com.uber;", + "import java.lang.annotation.ElementType;", + "import java.lang.annotation.Target;", + "@Target({ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE, ElementType.TYPE_USE})", + "public @interface Nullable {}") + .addSourceLines( + "Utilities.java", + "package com.uber;", + "public class Utilities {", + " public static String takesNullableVarargsArray(Object o, Object @Nullable... others) {", + " String s = o.toString() + \" \";", + " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", + " for (Object other : others) {", + " s += (other == null) ? \"(null) \" : other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "public class Test {", + " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", + " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", + " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed + " // BUG: Diagnostic contains: passing @Nullable parameter", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", + " // this is fine!", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", + " }", + "}") + .doTest(); + } + + @Test + public void typeUseAndDeclarationOnElements() { + makeHelper() + .addSourceLines( + "Nullable.java", + "package com.uber;", + "import java.lang.annotation.ElementType;", + "import java.lang.annotation.Target;", + "@Target({ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE, ElementType.TYPE_USE})", + "public @interface Nullable {}") + .addSourceLines( + "Utilities.java", + "package com.uber;", + "public class Utilities {", + " public static String takesNullableVarargsArray(Object o, @Nullable Object... others) {", + " String s = o.toString() + \" \";", + " for (Object other : others) {", + " s += (other == null) ? \"(null) \" : other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "public class Test {", + " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", + " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", + " // BUG: Diagnostic contains: passing @Nullable parameter '(java.lang.Object[]) null' where @NonNull", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", + " }", + "}") + .doTest(); + } + + @Test + public void typeUseAndDeclarationOnBoth() { + makeHelper() + .addSourceLines( + "Nullable.java", + "package com.uber;", + "import java.lang.annotation.ElementType;", + "import java.lang.annotation.Target;", + "@Target({ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE, ElementType.TYPE_USE})", + "public @interface Nullable {}") + .addSourceLines( + "Utilities.java", + "package com.uber;", + "public class Utilities {", + " public static String takesNullableVarargsArray(Object o, @Nullable Object @Nullable... others) {", + " String s = o.toString() + \" \";", + " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", + " for (Object other : others) {", + " s += (other == null) ? \"(null) \" : other.toString() + \" \";", + " }", + " return s;", + " }", + "}") + .addSourceLines( + "Test.java", + "package com.uber;", + "public class Test {", + " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", + " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", + " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", + " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", + " }", + "}") + .doTest(); + } + + @Test + public void varargsPassArrayAndOtherArgs() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class Test {", + " static void takesVarargs(Object... args) {}", + " static void test(Object o) {", + " Object[] x = new Object[10];", + " takesVarargs(x, o);", + " }", + " static void takesNullableVarargsArray(Object @Nullable... args) {}", + " static void test2(Object o) {", + " Object[] x = null;", + " // can pass x as the varargs array itself, but not along with other args", + " takesNullableVarargsArray(x);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'x'", + " takesNullableVarargsArray(x, o);", + " }", + "}") + .doTest(); + } + + private CompilationTestHelper makeHelper() { + return makeTestHelperWithArgs( + Arrays.asList( + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:LegacyAnnotationLocations=true")); + } +} From 0878e5b6be2a05230f7a2380c641434e93cbf321 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 23 Aug 2024 18:57:43 -0700 Subject: [PATCH 15/29] WIP --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 2 +- nullaway/src/main/java/com/uber/nullaway/Nullness.java | 6 ++++-- .../nullaway/dataflow/CoreNullnessStoreInitializer.java | 2 +- .../src/test/java/com/uber/nullaway/LegacyVarargsTests.java | 1 + 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index e8d07b3cef..f7eb9bf8bc 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1784,7 +1784,7 @@ private Description handleInvocation( && state.getTypes().isAssignable(actualParameterType, varargsArrayType) && actualParams.size() == argPos + 1) { // If varargs array itself is not @Nullable, cannot pass @Nullable array - if (!Nullness.varargsParamIsNullable(formalParams.get(argPos), config)) { + if (!Nullness.varargsArrayIsNullable(formalParams.get(argPos), config)) { mayActualBeNull = mayBeNullExpr(state, actual); } } else { diff --git a/nullaway/src/main/java/com/uber/nullaway/Nullness.java b/nullaway/src/main/java/com/uber/nullaway/Nullness.java index aa1b3fce91..ff9d2dd0bc 100644 --- a/nullaway/src/main/java/com/uber/nullaway/Nullness.java +++ b/nullaway/src/main/java/com/uber/nullaway/Nullness.java @@ -268,8 +268,10 @@ public static boolean paramHasNonNullAnnotation( * that the argument array passed at a call site can be {@code null}? Syntactically, this would be * written as {@code foo(Object @Nullable... args}} */ - public static boolean varargsParamIsNullable(Symbol paramSymbol, Config config) { - return hasNullableTypeUseAnnotation(paramSymbol, config); + public static boolean varargsArrayIsNullable(Symbol paramSymbol, Config config) { + return hasNullableTypeUseAnnotation(paramSymbol, config) + || (config.isLegacyAnnotationLocation() + && hasNullableDeclarationAnnotation(paramSymbol, config)); } private static boolean individualVarargsParamsAreNullable(Symbol paramSymbol, Config config) { 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 9f0389cdc3..a10b0829e4 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java @@ -68,7 +68,7 @@ private static NullnessStore methodInitialStore( Symbol paramSymbol = (Symbol) param.getElement(); Nullness assumed; if ((paramSymbol.flags() & Flags.VARARGS) != 0) { - assumed = Nullness.varargsParamIsNullable(paramSymbol, config) ? NULLABLE : NONNULL; + assumed = Nullness.varargsArrayIsNullable(paramSymbol, config) ? NULLABLE : NONNULL; } else { assumed = Nullness.hasNullableAnnotation(paramSymbol, config) ? NULLABLE : NONNULL; } diff --git a/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java index c0c2469438..0ddb051a0c 100644 --- a/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java @@ -46,6 +46,7 @@ public void testNullableVarargs() { "import javax.annotation.Nullable;", "public class Utilities {", " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", + " // BUG: Diagnostic contains: [NullAway] dereferenced expression others is @Nullable", " String s = o.toString() + \" \" + others.toString();", " return s;", " }", From 91fe3fed685f3a8c07e0142ed3de3aca7d0ada04 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 23 Aug 2024 19:11:58 -0700 Subject: [PATCH 16/29] fixes --- .../src/main/java/com/uber/nullaway/NullAway.java | 1 - .../src/main/java/com/uber/nullaway/Nullness.java | 4 +++- .../java/com/uber/nullaway/LegacyVarargsTests.java | 13 +++---------- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index f7eb9bf8bc..5801798dd6 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1788,7 +1788,6 @@ private Description handleInvocation( mayActualBeNull = mayBeNullExpr(state, actual); } } else { - // TODO need to update this check to see if the vararg array contents is @Nullable if (!argIsNonNull) { continue; } diff --git a/nullaway/src/main/java/com/uber/nullaway/Nullness.java b/nullaway/src/main/java/com/uber/nullaway/Nullness.java index ff9d2dd0bc..9ec0383638 100644 --- a/nullaway/src/main/java/com/uber/nullaway/Nullness.java +++ b/nullaway/src/main/java/com/uber/nullaway/Nullness.java @@ -224,7 +224,9 @@ public static boolean paramHasNullableAnnotation( if (isRecordEqualsParam(symbol, paramInd)) { return true; } - if (symbol.isVarArgs() && paramInd == symbol.getParameters().size() - 1) { + if (symbol.isVarArgs() + && paramInd == symbol.getParameters().size() - 1 + && !config.isLegacyAnnotationLocation()) { return individualVarargsParamsAreNullable(symbol.getParameters().get(paramInd), config); } else { return hasNullableAnnotation( diff --git a/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java index 0ddb051a0c..18e1a795be 100644 --- a/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java @@ -62,7 +62,6 @@ public void testNullableVarargs() { " Utilities.takesNullableVarargs(o1, o4);", " Utilities.takesNullableVarargs(o1, (java.lang.Object) null);", " Object[] x = null;", - " // BUG: Diagnostic contains: passing @Nullable parameter 'x'", " Utilities.takesNullableVarargs(o1, x);", " }", "}") @@ -78,6 +77,7 @@ public void nullableTypeUseVarargs() { "import org.checkerframework.checker.nullness.qual.Nullable;", "public class Utilities {", " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", + " // BUG: Diagnostic contains: [NullAway] dereferenced expression others is @Nullable", " String s = o.toString() + \" \" + others.toString();", " for (Object other : others) {", " // no error since we do not reason about array element nullability", @@ -97,7 +97,6 @@ public void nullableTypeUseVarargs() { " Utilities.takesNullableVarargs(o1, o4);", " Utilities.takesNullableVarargs(o1, (java.lang.Object) null);", " Object[] x = null;", - " // BUG: Diagnostic contains: passing @Nullable parameter 'x'", " Utilities.takesNullableVarargs(o1, x);", " }", "}") @@ -232,10 +231,8 @@ public void nullableVarargsArray() { "import org.checkerframework.checker.nullness.qual.Nullable;", "public class Test {", " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", - " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed - " // BUG: Diagnostic contains: passing @Nullable parameter", " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", " // this is fine!", " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", @@ -305,12 +302,9 @@ public void typeUseAndDeclarationBeforeDots() { "package com.uber;", "public class Test {", " public void testNullableVarargsArray(Object o1, Object o2, Object o3, @Nullable Object o4) {", - " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed - " // BUG: Diagnostic contains: passing @Nullable parameter", " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", - " // this is fine!", " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", " }", "}") @@ -333,6 +327,7 @@ public void typeUseAndDeclarationOnElements() { "public class Utilities {", " public static String takesNullableVarargsArray(Object o, @Nullable Object... others) {", " String s = o.toString() + \" \";", + " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", " for (Object other : others) {", " s += (other == null) ? \"(null) \" : other.toString() + \" \";", " }", @@ -347,7 +342,6 @@ public void typeUseAndDeclarationOnElements() { " Utilities.takesNullableVarargsArray(o1, o2, o3, o4);", " Utilities.takesNullableVarargsArray(o1);", // Empty var args passed " Utilities.takesNullableVarargsArray(o1, (java.lang.Object) null);", - " // BUG: Diagnostic contains: passing @Nullable parameter '(java.lang.Object[]) null' where @NonNull", " Utilities.takesNullableVarargsArray(o1, (java.lang.Object[]) null);", " }", "}") @@ -407,9 +401,8 @@ public void varargsPassArrayAndOtherArgs() { " static void takesNullableVarargsArray(Object @Nullable... args) {}", " static void test2(Object o) {", " Object[] x = null;", - " // can pass x as the varargs array itself, but not along with other args", " takesNullableVarargsArray(x);", - " // BUG: Diagnostic contains: passing @Nullable parameter 'x'", + " // in legacy mode the annotation allows for individual arguments to be nullable", " takesNullableVarargsArray(x, o);", " }", "}") From c524432c8cfe6ae33915b6efaddbfbe406590994 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 23 Aug 2024 20:14:00 -0700 Subject: [PATCH 17/29] comments --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 5801798dd6..851fcc987e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1777,6 +1777,7 @@ private Description handleInvocation( continue; } actual = actualParams.get(argPos); + // check if the varargs arguments are being passed as an array Type.ArrayType varargsArrayType = (Type.ArrayType) formalParams.get(formalParams.size() - 1).type; Type actualParameterType = ASTHelpers.getType(actual); @@ -1787,11 +1788,11 @@ private Description handleInvocation( if (!Nullness.varargsArrayIsNullable(formalParams.get(argPos), config)) { mayActualBeNull = mayBeNullExpr(state, actual); } - } else { + } else { // varargs are being passed individually if (!argIsNonNull) { continue; } - // TODO report multiple errors for each violating vararg + // TODO report an error for each violating vararg for (ExpressionTree arg : actualParams.subList(argPos, actualParams.size())) { actual = arg; mayActualBeNull = mayBeNullExpr(state, actual); @@ -1801,11 +1802,10 @@ private Description handleInvocation( } } - } else { + } else { // not the vararg position actual = actualParams.get(argPos); mayActualBeNull = mayBeNullExpr(state, actual); } - // make sure we are passing a non-null value if (mayActualBeNull) { String message = "passing @Nullable parameter '" From 252e6c32178843c95e8b8b4cd50195eac0a2f29d Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 23 Aug 2024 20:17:11 -0700 Subject: [PATCH 18/29] tweaks --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 2 -- nullaway/src/main/java/com/uber/nullaway/Nullness.java | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 851fcc987e..8af2ca2fe2 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1801,7 +1801,6 @@ private Description handleInvocation( } } } - } else { // not the vararg position actual = actualParams.get(argPos); mayActualBeNull = mayBeNullExpr(state, actual); @@ -2513,7 +2512,6 @@ private Description matchDereference( return Description.NO_MATCH; } } - // (baseExpressionSymbol.flags() & Flags.VARARGS) != 0 if (mayBeNullExpr(state, baseExpression)) { String message = "dereferenced expression " + state.getSourceForNode(baseExpression) + " is @Nullable"; diff --git a/nullaway/src/main/java/com/uber/nullaway/Nullness.java b/nullaway/src/main/java/com/uber/nullaway/Nullness.java index 9ec0383638..d2fc224713 100644 --- a/nullaway/src/main/java/com/uber/nullaway/Nullness.java +++ b/nullaway/src/main/java/com/uber/nullaway/Nullness.java @@ -206,7 +206,7 @@ public static boolean hasNullableAnnotation(Symbol symbol, Config config) { return hasNullableAnnotation(NullabilityUtil.getAllAnnotations(symbol, config), config); } - public static boolean hasNullableTypeUseAnnotation(Symbol symbol, Config config) { + private static boolean hasNullableTypeUseAnnotation(Symbol symbol, Config config) { return hasNullableAnnotation(NullabilityUtil.getTypeUseAnnotations(symbol, config), config); } @@ -278,7 +278,6 @@ public static boolean varargsArrayIsNullable(Symbol paramSymbol, Config config) private static boolean individualVarargsParamsAreNullable(Symbol paramSymbol, Config config) { // declaration annotation, or type-use annotation on the elements - // TODO update and test for legacy mode return hasNullableDeclarationAnnotation(paramSymbol, config) || NullabilityUtil.isArrayElementNullable(paramSymbol, config); } From 925265da4871a530fdf767190f70fdbd8019f5d1 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 24 Aug 2024 07:49:41 -0700 Subject: [PATCH 19/29] clarify comment --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 8af2ca2fe2..8c6c0b544e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1792,7 +1792,8 @@ private Description handleInvocation( if (!argIsNonNull) { continue; } - // TODO report an error for each violating vararg + // TODO report all varargs errors in a single build; this code only reports the first + // error for (ExpressionTree arg : actualParams.subList(argPos, actualParams.size())) { actual = arg; mayActualBeNull = mayBeNullExpr(state, actual); From 58ece61ebb0d9a79fb7ef30192a44a0296a98ffe Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 27 Aug 2024 22:03:58 -0700 Subject: [PATCH 20/29] comments --- .../src/test/java/com/uber/nullaway/LegacyVarargsTests.java | 4 ++++ nullaway/src/test/java/com/uber/nullaway/VarargsTests.java | 1 + .../com/uber/nullaway/jspecify/JSpecifyVarargsTests.java | 5 +++++ 3 files changed, 10 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java index 18e1a795be..115b20557c 100644 --- a/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java @@ -4,6 +4,10 @@ import java.util.Arrays; import org.junit.Test; +/** + * Tests for handling of varargs annotations with legacy annotation locations enabled. Based on + * {@link VarargsTests}, with tests and assertions modified appropriately. + */ public class LegacyVarargsTests extends NullAwayTestsBase { @Test diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index dd06880687..b0eb05a490 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -3,6 +3,7 @@ import java.util.Arrays; import org.junit.Test; +/** Tests for handling of varargs annotations in NullAway's default mode */ public class VarargsTests extends NullAwayTestsBase { @Test diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java index 31be7f1c0a..c19da96e12 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java @@ -2,9 +2,14 @@ import com.google.errorprone.CompilationTestHelper; import com.uber.nullaway.NullAwayTestsBase; +import com.uber.nullaway.VarargsTests; import java.util.Arrays; import org.junit.Test; +/** + * Tests for handling of varargs annotations in JSpecify mode. Based on {@link VarargsTests}, with + * tests and assertions modified appropriately. + */ public class JSpecifyVarargsTests extends NullAwayTestsBase { @Test From 4068568696107d1be886888582b9e5bb5d2a0be4 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 31 Aug 2024 17:49:48 -0700 Subject: [PATCH 21/29] suggested comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Lázaro Clapp --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 8c6c0b544e..a10573a615 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1773,6 +1773,8 @@ private Description handleInvocation( boolean mayActualBeNull = false; if (varargPosition) { // Check all vararg actual arguments for nullability + // This is the case where no actual parameter is passed for the var args parameter + // (i.e. it defaults to an empty array) if (actualParams.size() <= argPos) { continue; } From fe92b7d85c2024eb309edb0478571fe38f3695d9 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 31 Aug 2024 17:50:14 -0700 Subject: [PATCH 22/29] comment suggestion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Lázaro Clapp --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 1 + 1 file changed, 1 insertion(+) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index a10573a615..1e583ec4d7 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1786,6 +1786,7 @@ private Description handleInvocation( if (actualParameterType != null && state.getTypes().isAssignable(actualParameterType, varargsArrayType) && actualParams.size() == argPos + 1) { + // This is the case where an array is explicitly passed in the position of the var args parameter // If varargs array itself is not @Nullable, cannot pass @Nullable array if (!Nullness.varargsArrayIsNullable(formalParams.get(argPos), config)) { mayActualBeNull = mayBeNullExpr(state, actual); From 62a7df5b1ff9d9bf2d4b425a914988663be31d83 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 31 Aug 2024 17:51:59 -0700 Subject: [PATCH 23/29] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Lázaro Clapp --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 5 ++++- nullaway/src/test/java/com/uber/nullaway/VarargsTests.java | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 1e583ec4d7..51ade16cbe 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1791,7 +1791,10 @@ private Description handleInvocation( if (!Nullness.varargsArrayIsNullable(formalParams.get(argPos), config)) { mayActualBeNull = mayBeNullExpr(state, actual); } - } else { // varargs are being passed individually + } else { + // This is the case were varargs are being passed individually, as 1 or more actual + // arguments starting at the position of the var args formal. + // If the formal var args accepts `@Nullable`, then there is nothing for us to check. if (!argIsNonNull) { continue; } diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index b0eb05a490..eab8d8ed4f 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -209,7 +209,7 @@ public void testSkipJetbrainsNotNullOnVarArgsFromThirdPartyJars() { } @Test - public void nullableVarargsArray() { + public void typeUseNullableVarargsArray() { defaultCompilationHelper .addSourceLines( "Utilities.java", @@ -244,7 +244,7 @@ public void nullableVarargsArray() { } @Test - public void nullableVarargsArrayAndElements() { + public void typeUseNullableVarargsArrayAndElements() { defaultCompilationHelper .addSourceLines( "Utilities.java", From 893fabe15ae3c9d99710b9bd5961e6f5092ad383 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 31 Aug 2024 17:52:44 -0700 Subject: [PATCH 24/29] formatting --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 51ade16cbe..ee9236d959 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1773,8 +1773,8 @@ private Description handleInvocation( boolean mayActualBeNull = false; if (varargPosition) { // Check all vararg actual arguments for nullability - // This is the case where no actual parameter is passed for the var args parameter - // (i.e. it defaults to an empty array) + // This is the case where no actual parameter is passed for the var args parameter + // (i.e. it defaults to an empty array) if (actualParams.size() <= argPos) { continue; } @@ -1786,12 +1786,13 @@ private Description handleInvocation( if (actualParameterType != null && state.getTypes().isAssignable(actualParameterType, varargsArrayType) && actualParams.size() == argPos + 1) { - // This is the case where an array is explicitly passed in the position of the var args parameter + // This is the case where an array is explicitly passed in the position of the var args + // parameter // If varargs array itself is not @Nullable, cannot pass @Nullable array if (!Nullness.varargsArrayIsNullable(formalParams.get(argPos), config)) { mayActualBeNull = mayBeNullExpr(state, actual); } - } else { + } else { // This is the case were varargs are being passed individually, as 1 or more actual // arguments starting at the position of the var args formal. // If the formal var args accepts `@Nullable`, then there is nothing for us to check. From fee21e49ce52672dbd8ad62c092caa97a0c34ebc Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 31 Aug 2024 17:55:36 -0700 Subject: [PATCH 25/29] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Lázaro Clapp --- .../src/test/java/com/uber/nullaway/LegacyVarargsTests.java | 4 ++-- .../java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java index 115b20557c..f97b964143 100644 --- a/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java @@ -213,7 +213,7 @@ public void testSkipJetbrainsNotNullOnVarArgsFromThirdPartyJars() { } @Test - public void nullableVarargsArray() { + public void typeUseNullableVarargsArray() { makeHelper() .addSourceLines( "Utilities.java", @@ -246,7 +246,7 @@ public void nullableVarargsArray() { } @Test - public void nullableVarargsArrayAndElements() { + public void typeUseNullableVarargsArrayAndElements() { makeHelper() .addSourceLines( "Utilities.java", diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java index c19da96e12..7e233891f7 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java @@ -215,7 +215,7 @@ public void testSkipJetbrainsNotNullOnVarArgsFromThirdPartyJars() { } @Test - public void nullableVarargsArray() { + public void typeUseNullableVarargsArrayAndElements() { makeHelper() .addSourceLines( "Utilities.java", @@ -250,7 +250,7 @@ public void nullableVarargsArray() { } @Test - public void nullableVarargsArrayAndElements() { + public void typeUseNullableVarargsArrayAndElements() { makeHelper() .addSourceLines( "Utilities.java", From 71301a386e6285716f9a8d07c62fe62ce8a476ab Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 31 Aug 2024 18:00:16 -0700 Subject: [PATCH 26/29] more tests of passing null varargs array --- .../src/test/java/com/uber/nullaway/LegacyVarargsTests.java | 3 +++ nullaway/src/test/java/com/uber/nullaway/VarargsTests.java | 3 +++ .../com/uber/nullaway/jspecify/JSpecifyVarargsTests.java | 5 ++++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java index f97b964143..038c37baa8 100644 --- a/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/LegacyVarargsTests.java @@ -36,6 +36,9 @@ public void testNonNullVarargs() { " Utilities.takesNonNullVarargs(o1);", // Empty var args passed " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", " Utilities.takesNonNullVarargs(o1, o4);", + " Object[] x = null;", + " // BUG: Diagnostic contains: passing @Nullable parameter 'x'", + " Utilities.takesNonNullVarargs(o1, x);", " }", "}") .doTest(); diff --git a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index eab8d8ed4f..c6638a8f3f 100644 --- a/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -32,6 +32,9 @@ public void testNonNullVarargs() { " Utilities.takesNonNullVarargs(o1);", // Empty var args passed " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", " Utilities.takesNonNullVarargs(o1, o4);", + " Object[] x = null;", + " // BUG: Diagnostic contains: passing @Nullable parameter 'x'", + " Utilities.takesNonNullVarargs(o1, x);", " }", "}") .doTest(); diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java index 7e233891f7..50cc48740f 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java @@ -38,6 +38,9 @@ public void testNonNullVarargs() { " Utilities.takesNonNullVarargs(o1);", // Empty var args passed " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", " Utilities.takesNonNullVarargs(o1, o4);", + " Object[] x = null;", + " // BUG: Diagnostic contains: passing @Nullable parameter 'x'", + " Utilities.takesNonNullVarargs(o1, x);", " }", "}") .doTest(); @@ -215,7 +218,7 @@ public void testSkipJetbrainsNotNullOnVarArgsFromThirdPartyJars() { } @Test - public void typeUseNullableVarargsArrayAndElements() { + public void typeUseNullableVarargsArray() { makeHelper() .addSourceLines( "Utilities.java", From bc46f2e448887afa05f03cb879a403faf2013325 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 31 Aug 2024 18:08:35 -0700 Subject: [PATCH 27/29] another test --- .../java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java index 50cc48740f..c13dae81d0 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java @@ -56,6 +56,10 @@ public void testNullableVarargs() { "public class Utilities {", " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", " String s = o.toString() + \" \" + others.toString();", + " for (Object other : others) {", + " // no error here; requires a type-use annotation on the elements", + " s += other.toString();", + " }", " return s;", " }", "}") From 6b622c7d855c9464d599bf90eefa6a9426d79af0 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 31 Aug 2024 18:42:02 -0700 Subject: [PATCH 28/29] @Nullable declaration annotation makes varargs array elements @Nullable in JSpecify mode --- .../src/main/java/com/uber/nullaway/NullabilityUtil.java | 6 ++++++ nullaway/src/main/java/com/uber/nullaway/Nullness.java | 3 ++- .../com/uber/nullaway/jspecify/JSpecifyVarargsTests.java | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java index 87f37fc602..9bf78f796c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java @@ -38,6 +38,7 @@ import com.sun.source.tree.VariableTree; import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Attribute; +import com.sun.tools.javac.code.Flags; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.TargetType; import com.sun.tools.javac.code.Type; @@ -432,6 +433,11 @@ public static boolean isArrayElementNullable(Symbol arraySymbol, Config config) } } } + // In JSpecify mode, for varargs symbols we also consider the elements to be @Nullable if there + // is a @Nullable declaration annotation on the parameter + if (config.isJSpecifyMode() && (arraySymbol.flags() & Flags.VARARGS) != 0) { + return Nullness.hasNullableDeclarationAnnotation(arraySymbol, config); + } return false; } } diff --git a/nullaway/src/main/java/com/uber/nullaway/Nullness.java b/nullaway/src/main/java/com/uber/nullaway/Nullness.java index d2fc224713..157632df33 100644 --- a/nullaway/src/main/java/com/uber/nullaway/Nullness.java +++ b/nullaway/src/main/java/com/uber/nullaway/Nullness.java @@ -282,7 +282,8 @@ private static boolean individualVarargsParamsAreNullable(Symbol paramSymbol, Co || NullabilityUtil.isArrayElementNullable(paramSymbol, config); } - private static boolean hasNullableDeclarationAnnotation(Symbol symbol, Config config) { + /** Checks if the symbol has a {@code @Nullable} declaration annotation */ + public static boolean hasNullableDeclarationAnnotation(Symbol symbol, Config config) { return hasNullableAnnotation(symbol.getRawAttributes().stream(), config); } } diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java index c13dae81d0..6ff2f2d754 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java @@ -57,7 +57,7 @@ public void testNullableVarargs() { " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", " String s = o.toString() + \" \" + others.toString();", " for (Object other : others) {", - " // no error here; requires a type-use annotation on the elements", + " // BUG: Diagnostic contains: dereferenced expression other is @Nullable", " s += other.toString();", " }", " return s;", From 535af4f60c9cb0e616d77ae0cd0730a6414ca49d Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 1 Sep 2024 08:24:28 -0700 Subject: [PATCH 29/29] logic simplification --- .../main/java/com/uber/nullaway/NullabilityUtil.java | 6 +++--- nullaway/src/main/java/com/uber/nullaway/Nullness.java | 10 +++------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java index 9bf78f796c..56d54512cf 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java @@ -433,9 +433,9 @@ public static boolean isArrayElementNullable(Symbol arraySymbol, Config config) } } } - // In JSpecify mode, for varargs symbols we also consider the elements to be @Nullable if there - // is a @Nullable declaration annotation on the parameter - if (config.isJSpecifyMode() && (arraySymbol.flags() & Flags.VARARGS) != 0) { + // For varargs symbols we also consider the elements to be @Nullable if there is a @Nullable + // declaration annotation on the parameter + if ((arraySymbol.flags() & Flags.VARARGS) != 0) { return Nullness.hasNullableDeclarationAnnotation(arraySymbol, config); } return false; diff --git a/nullaway/src/main/java/com/uber/nullaway/Nullness.java b/nullaway/src/main/java/com/uber/nullaway/Nullness.java index 157632df33..56cf28dcba 100644 --- a/nullaway/src/main/java/com/uber/nullaway/Nullness.java +++ b/nullaway/src/main/java/com/uber/nullaway/Nullness.java @@ -227,7 +227,9 @@ public static boolean paramHasNullableAnnotation( if (symbol.isVarArgs() && paramInd == symbol.getParameters().size() - 1 && !config.isLegacyAnnotationLocation()) { - return individualVarargsParamsAreNullable(symbol.getParameters().get(paramInd), config); + // individual arguments passed in the varargs positions can be @Nullable if the array element + // type of the parameter is @Nullable + return NullabilityUtil.isArrayElementNullable(symbol.getParameters().get(paramInd), config); } else { return hasNullableAnnotation( NullabilityUtil.getAllAnnotationsForParameter(symbol, paramInd, config), config); @@ -276,12 +278,6 @@ public static boolean varargsArrayIsNullable(Symbol paramSymbol, Config config) && hasNullableDeclarationAnnotation(paramSymbol, config)); } - private static boolean individualVarargsParamsAreNullable(Symbol paramSymbol, Config config) { - // declaration annotation, or type-use annotation on the elements - return hasNullableDeclarationAnnotation(paramSymbol, config) - || NullabilityUtil.isArrayElementNullable(paramSymbol, config); - } - /** Checks if the symbol has a {@code @Nullable} declaration annotation */ public static boolean hasNullableDeclarationAnnotation(Symbol symbol, Config config) { return hasNullableAnnotation(symbol.getRawAttributes().stream(), config);