Skip to content

Commit

Permalink
Update handling of annotations on varargs argument (uber#1025)
Browse files Browse the repository at this point in the history
  • Loading branch information
@msridhar
msridhar authored Sep 1, 2024
1 parent 0d500cc commit b7756ea
Show file tree
Hide file tree
Showing 8 changed files with 1,202 additions and 31 deletions.
55 changes: 40 additions & 15 deletions nullaway/src/main/java/com/uber/nullaway/NullAway.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1705,8 +1704,9 @@ private Description handleInvocation(
List<? extends ExpressionTree> actualParams) {
List<VarSymbol> formalParams = methodSymbol.getParameters();

boolean varArgsMethod = methodSymbol.isVarArgs();
if (formalParams.size() != actualParams.size()
&& !methodSymbol.isVarArgs()
&& !varArgsMethod
&& !methodSymbol.isStatic()
&& methodSymbol.isConstructor()
&& methodSymbol.enclClass().isInner()) {
Expand Down Expand Up @@ -1751,7 +1751,7 @@ private Description handleInvocation(
}
if (config.isJSpecifyMode()) {
GenericsChecks.compareGenericTypeParameterNullabilityForCall(
formalParams, actualParams, methodSymbol.isVarArgs(), this, state);
formalParams, actualParams, varArgsMethod, this, state);
}
}

Expand All @@ -1764,30 +1764,55 @@ 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;
if (argPos == formalParams.size() - 1 && methodSymbol.isVarArgs()) {
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;
}
for (ExpressionTree arg : actualParams.subList(argPos, actualParams.size())) {
actual = arg;
mayActualBeNull = mayBeNullExpr(state, actual);
if (mayActualBeNull) {
break;
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);
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);
}
} 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;
}
// 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);
if (mayActualBeNull) {
break;
}
}
}
} else {
} else { // not the vararg position
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 =
"passing @Nullable parameter '"
Expand Down
8 changes: 7 additions & 1 deletion nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -278,7 +279,7 @@ public static Stream<? extends AnnotationMirror> getAllAnnotationsForParameter(
* Gets the type use annotations on a symbol, ignoring annotations on components of the type (type
* arguments, wildcards, etc.)
*/
private static Stream<? extends AnnotationMirror> getTypeUseAnnotations(
public static Stream<? extends AnnotationMirror> getTypeUseAnnotations(
Symbol symbol, Config config) {
Stream<Attribute.TypeCompound> rawTypeAttributes = symbol.getRawTypeAttributes().stream();
if (symbol instanceof Symbol.MethodSymbol) {
Expand Down Expand Up @@ -432,6 +433,11 @@ public static boolean isArrayElementNullable(Symbol arraySymbol, Config config)
}
}
}
// 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;
}
}
35 changes: 33 additions & 2 deletions nullaway/src/main/java/com/uber/nullaway/Nullness.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,16 @@ public static boolean hasNullableAnnotation(Symbol symbol, Config config) {
return hasNullableAnnotation(NullabilityUtil.getAllAnnotations(symbol, config), config);
}

private 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.
*
* <p>For a varargs parameter, this method returns true if <em>individual</em> arguments passed in
* the varargs positions can be null.
*/
public static boolean paramHasNullableAnnotation(
Symbol.MethodSymbol symbol, int paramInd, Config config) {
Expand All @@ -217,8 +224,16 @@ 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
&& !config.isLegacyAnnotationLocation()) {
// 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);
}
}

private static boolean isRecordEqualsParam(Symbol.MethodSymbol symbol, int paramInd) {
Expand Down Expand Up @@ -251,4 +266,20 @@ 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 varargsArrayIsNullable(Symbol paramSymbol, Config config) {
return hasNullableTypeUseAnnotation(paramSymbol, config)
|| (config.isLegacyAnnotationLocation()
&& hasNullableDeclarationAnnotation(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);
}
}
11 changes: 8 additions & 3 deletions nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.varargsArrayIsNullable(paramSymbol, config) ? NULLABLE : NONNULL;
} else {
assumed = Nullness.hasNullableAnnotation(paramSymbol, config) ? NULLABLE : NONNULL;
}
result.setInformation(AccessPath.fromLocal(param), assumed);
}
result = handler.onDataflowInitialStore(underlyingAST, parameters, result);
Expand Down
Loading

0 comments on commit b7756ea

Please sign in to comment.