Skip to content

Commit

Permalink
JSpecify mode: initial support for generic methods (with explicit typ…
Browse files Browse the repository at this point in the history 
…e arguments at calls) (#1053)

- [ ] Added support for generic identity functions with explicitly-typed
nullness.

  - [ ] Related to the issue #1035.

- [ ] void genericNonNullIdentityFunction() and void
genericNullAllowingIdentityFunction() unit tests are added to
GenericMethodTests.java file.

---------

Co-authored-by: Manu Sridharan <[email protected]>
  • Loading branch information
@haewiful @msridhar
haewiful and msridhar authored Oct 30, 2024
1 parent 238363a commit e9541e3
Show file tree
Hide file tree
Showing 3 changed files with 242 additions and 1 deletion.
3 changes: 3 additions & 0 deletions nullaway/src/main/java/com/uber/nullaway/NullAway.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1853,6 +1853,9 @@ private Description handleInvocation(
if (config.isJSpecifyMode()) {
GenericsChecks.compareGenericTypeParameterNullabilityForCall(
formalParams, actualParams, varArgsMethod, this, state);
if (!methodSymbol.getTypeParameters().isEmpty()) {
GenericsChecks.checkGenericMethodCallTypeArguments(tree, state, this, config, handler);
}
}
}

Expand Down
144 changes: 143 additions & 1 deletion nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,19 @@
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.TargetType;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.tree.JCTree;
import com.uber.nullaway.CodeAnnotationInfo;
import com.uber.nullaway.Config;
import com.uber.nullaway.ErrorBuilder;
import com.uber.nullaway.ErrorMessage;
import com.uber.nullaway.NullAway;
import com.uber.nullaway.Nullness;
import com.uber.nullaway.handlers.Handler;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import javax.lang.model.type.ExecutableType;
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeVariable;
Expand Down Expand Up @@ -145,6 +148,81 @@ private static boolean[] getTypeParamsWithNullableUpperBound(
return result;
}

/**
* Checks validity of type arguments at a generic method call. A {@code @Nullable} type argument
* can only be used for a type variable that has a {@code @Nullable} upper bound.
*
* @param tree the tree representing the instantiated type
* @param state visitor state
* @param analysis the analysis object
* @param config the analysis config
* @param handler the handler instance
*/
public static void checkGenericMethodCallTypeArguments(
Tree tree, VisitorState state, NullAway analysis, Config config, Handler handler) {
List<? extends Tree> typeArguments = ((MethodInvocationTree) tree).getTypeArguments();
if (typeArguments.isEmpty()) {
return;
}
// get Nullable annotated type arguments
MethodInvocationTree methodTree = (MethodInvocationTree) tree;
Map<Integer, Tree> nullableTypeArguments = new HashMap<>();
for (int i = 0; i < typeArguments.size(); i++) {
Tree curTypeArg = typeArguments.get(i);
if (curTypeArg instanceof AnnotatedTypeTree) {
AnnotatedTypeTree annotatedType = (AnnotatedTypeTree) curTypeArg;
for (AnnotationTree annotation : annotatedType.getAnnotations()) {
Type annotationType = ASTHelpers.getType(annotation);
if (annotationType != null
&& Nullness.isNullableAnnotation(annotationType.toString(), config)) {
nullableTypeArguments.put(i, curTypeArg);
break;
}
}
}
}
Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodTree);

// check if type variables are allowed to be Nullable
Type baseType = methodSymbol.asType();
List<Type> baseTypeVariables = baseType.getTypeArguments();
for (int i = 0; i < baseTypeVariables.size(); i++) {
if (nullableTypeArguments.containsKey(i)) {
Type typeVariable = baseTypeVariables.get(i);
Type upperBound = typeVariable.getUpperBound();
com.sun.tools.javac.util.List<Attribute.TypeCompound> annotationMirrors =
upperBound.getAnnotationMirrors();
boolean hasNullableAnnotation =
Nullness.hasNullableAnnotation(annotationMirrors.stream(), config)
|| handler.onOverrideTypeParameterUpperBound(baseType.tsym.toString(), i);
// if type variable's upper bound does not have @Nullable annotation then the instantiation
// is invalid
if (!hasNullableAnnotation) {
reportInvalidTypeArgumentError(
nullableTypeArguments.get(i), methodSymbol, typeVariable, state, analysis);
}
}
}
}

private static void reportInvalidTypeArgumentError(
Tree tree,
Symbol.MethodSymbol methodSymbol,
Type typeVariable,
VisitorState state,
NullAway analysis) {
ErrorBuilder errorBuilder = analysis.getErrorBuilder();
ErrorMessage errorMessage =
new ErrorMessage(
ErrorMessage.MessageTypes.TYPE_PARAMETER_CANNOT_BE_NULLABLE,
String.format(
"Type argument cannot be @Nullable, as method %s's type variable %s is not @Nullable",
methodSymbol.toString(), typeVariable.tsym.toString()));
state.reportMatch(
errorBuilder.createErrorDescription(
errorMessage, analysis.buildDescription(tree), state, null));
}

private static void reportInvalidInstantiationError(
Tree tree, Type baseType, Type baseTypeVariable, VisitorState state, NullAway analysis) {
ErrorBuilder errorBuilder = analysis.getErrorBuilder();
Expand Down Expand Up @@ -682,7 +760,8 @@ public static Nullness getGenericMethodReturnTypeNullness(
/**
* Computes the nullness of the return of a generic method at an invocation, in the context of the
* declared type of its receiver argument. If the return type is a type variable, its nullness
* depends on the nullability of the corresponding type parameter in the receiver's type.
* depends on the nullability of the corresponding type parameter in the receiver's type or the
* type argument of the method call.
*
* <p>Consider the following example:
*
Expand Down Expand Up @@ -715,6 +794,29 @@ public static Nullness getGenericReturnNullnessAtInvocation(
MethodInvocationTree tree,
VisitorState state,
Config config) {
// If generic method invocation
if (!invokedMethodSymbol.getTypeParameters().isEmpty()) {
List<? extends Tree> typeArgumentTrees = tree.getTypeArguments();
com.sun.tools.javac.util.List<Type> explicitTypeArgs =
convertTreesToTypes(typeArgumentTrees); // Convert to Type objects
Type.ForAll forAllType = (Type.ForAll) invokedMethodSymbol.type;
// Extract the underlying MethodType (the actual signature)
Type.MethodType methodTypeInsideForAll = (Type.MethodType) forAllType.asMethodType();
// Substitute type arguments inside the return type
// NOTE: if the return type it not a type variable of the method itself, or if
// explicitTypeArgs is empty, this is a noop.
Type substitutedReturnType =
state
.getTypes()
.subst(methodTypeInsideForAll.restype, forAllType.tvars, explicitTypeArgs);
// If this condition evaluates to false, we fall through to the subsequent logic, to handle
// type variables declared on the enclosing class
if (substitutedReturnType != null
&& Objects.equals(getTypeNullness(substitutedReturnType, config), Nullness.NULLABLE)) {
return Nullness.NULLABLE;
}
}

if (!(tree.getMethodSelect() instanceof MemberSelectTree) || invokedMethodSymbol.isStatic()) {
return Nullness.NONNULL;
}
Expand All @@ -728,6 +830,18 @@ public static Nullness getGenericReturnNullnessAtInvocation(
}
}

private static com.sun.tools.javac.util.List<Type> convertTreesToTypes(
List<? extends Tree> typeArgumentTrees) {
List<Type> types = new ArrayList<>();
for (Tree tree : typeArgumentTrees) {
if (tree instanceof JCTree.JCExpression) {
JCTree.JCExpression expression = (JCTree.JCExpression) tree;
types.add(expression.type); // Retrieve the Type
}
}
return com.sun.tools.javac.util.List.from(types);
}

/**
* Computes the nullness of a formal parameter of a generic method at an invocation, in the
* context of the declared type of its receiver argument. If the formal parameter's type is a type
Expand Down Expand Up @@ -768,6 +882,34 @@ public static Nullness getGenericParameterNullnessAtInvocation(
MethodInvocationTree tree,
VisitorState state,
Config config) {
// If generic method invocation
if (!invokedMethodSymbol.getTypeParameters().isEmpty()) {
List<? extends Tree> typeArgumentTrees = tree.getTypeArguments();
com.sun.tools.javac.util.List<Type> explicitTypeArgs =
convertTreesToTypes(typeArgumentTrees); // Convert to Type objects

Type.ForAll forAllType = (Type.ForAll) invokedMethodSymbol.type;
// Extract the underlying MethodType (the actual signature)
Type.MethodType methodTypeInsideForAll = (Type.MethodType) forAllType.qtype;
// Substitute the argument types within the MethodType
// NOTE: if explicitTypeArgs is empty, this is a noop
List<Type> substitutedParamTypes =
state
.getTypes()
.subst(
methodTypeInsideForAll.argtypes,
forAllType.tvars, // The type variables from the ForAll
explicitTypeArgs // The actual type arguments from the method invocation
);
// If this condition evaluates to false, we fall through to the subsequent logic, to handle
// type variables declared on the enclosing class
if (substitutedParamTypes != null
&& Objects.equals(
getTypeNullness(substitutedParamTypes.get(paramIndex), config), Nullness.NULLABLE)) {
return Nullness.NULLABLE;
}
}

if (!(tree.getMethodSelect() instanceof MemberSelectTree) || invokedMethodSymbol.isStatic()) {
return Nullness.NONNULL;
}
Expand Down
96 changes: 96 additions & 0 deletions nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,102 @@

public class GenericMethodTests extends NullAwayTestsBase {

@Test
public void genericNonNullIdentityFunction() {
makeHelper()
.addSourceLines(
"Test.java",
"package com.uber;",
"import org.jspecify.annotations.Nullable;",
"class Test {",
" static <T> T nonNullIdentity(T t) {",
" return t;",
" }",
" static void test() {",
" // legal",
" nonNullIdentity(new Object()).toString();",
" // BUG: Diagnostic contains: passing @Nullable parameter 'null'",
" nonNullIdentity(null);",
" // BUG: Diagnostic contains: Type argument cannot be @Nullable",
" Test.<@Nullable Object>nonNullIdentity(new Object());",
" }",
"}")
.doTest();
}

@Test
public void genericNullAllowingIdentityFunction() {
makeHelper()
.addSourceLines(
"Test.java",
"package com.uber;",
"import org.jspecify.annotations.Nullable;",
"class Test {",
" static <T extends @Nullable Object> T identity(T t) {",
" return t;",
" }",
" static void test() {",
" // legal",
" identity(new Object()).toString();",
" // also legal",
" Test.<@Nullable Object>identity(null);",
" // BUG: Diagnostic contains: dereferenced expression",
" Test.<@Nullable Object>identity(null).toString();",
" }",
"}")
.doTest();
}

@Test
public void multipleTypeVariablesMethodCall() {
makeHelper()
.addSourceLines(
"Test.java",
"package com.uber;",
"import org.jspecify.annotations.Nullable;",
"import org.jspecify.annotations.NonNull;",
"class Test {",
" public static <T extends @Nullable Object, U> void twoTypeVars(T first, U second) {}",
" static void test() {",
" // legal",
" Test.<@NonNull Object, Object>twoTypeVars(new Object(), new Object());",
" // legal",
" Test.<@Nullable Object, Object>twoTypeVars(null, new Object());",
" // BUG: Diagnostic contains: Type argument cannot be @Nullable",
" Test.<@Nullable Object, @Nullable Object>twoTypeVars(null, null);",
" }",
"}")
.doTest();
}

@Test
public void genericInstanceMethods() {
makeHelper()
.addSourceLines(
"Test.java",
"package com.uber;",
"import org.jspecify.annotations.Nullable;",
"class Test {",
" abstract class Foo<T extends @Nullable String, S> {",
" public abstract <U> T test1(U u);",
" public abstract <U> S test2(U u);",
" public abstract <U extends @Nullable Object> void test3(U u);",
" }",
" public void run(Foo<@Nullable String, Character> f) {",
" String s = f.<Integer>test1(3);",
" // BUG: Diagnostic contains: dereferenced expression",
" s.toString();",
" Character c = f.<Integer>test2(3);",
" // legal, Type S is @NonNull",
" c.toString();",
" // BUG: Diagnostic contains: passing @Nullable parameter 'null'",
" f.<Integer>test3(null);",
" f.<@Nullable Integer>test3(null);",
" }",
"}")
.doTest();
}

@Test
@Ignore("requires generic method support")
public void genericMethodAndVoidType() {
Expand Down

0 comments on commit e9541e3

Please sign in to comment.