Skip to content

Commit

Permalink
Generics checks for parameter passing (#746)
Browse files Browse the repository at this point in the history
For method calls, the nullability annotations of the type parameters for formal parameter types and actual parameter types should be exactly the same. 
This pull request adds code to compare nullability annotations of actual and formal parameters of generic type and reports an error if the annotations don't match. example:
  ```
    static class A<T extends @nullable Object> { }
    static A<String> sampleMethod(A<A<String>> a1, A<String> a2) {
       return a2;
    }
    static void methodCall(A<A<@nullable String>> a1, A<String> a2) {

   // **_here the code will report an error as the annotations of formal and actual parameters don't match_**

         A<String> a = sampleMethod(a1, a2);

    }
```
  • Loading branch information
NikitaAware committed Mar 20, 2023
1 parent 07a3847 commit a1d1eed
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 0 deletions.
1 change: 1 addition & 0 deletions nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java
Expand Up @@ -55,6 +55,7 @@ public enum MessageTypes {
TYPE_PARAMETER_CANNOT_BE_NULLABLE,
ASSIGN_GENERIC_NULLABLE,
RETURN_NULLABLE_GENERIC,
PASS_NULLABLE_GENERIC,
}

public String getMessage() {
Expand Down
75 changes: 75 additions & 0 deletions nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java
Expand Up @@ -168,6 +168,26 @@ private static void reportMismatchedTypeForTernaryOperator(
errorMessage, analysis.buildDescription(tree), state, null));
}

private void reportInvalidParametersNullabilityError(
Type formalParameterType,
Type actualParameterType,
ExpressionTree paramExpression,
VisitorState state,
NullAway analysis) {
ErrorBuilder errorBuilder = analysis.getErrorBuilder();
ErrorMessage errorMessage =
new ErrorMessage(
ErrorMessage.MessageTypes.PASS_NULLABLE_GENERIC,
"Cannot pass parameter of type "
+ actualParameterType
+ ", as formal parameter has type "
+ formalParameterType
+ ", which has mismatched type parameter nullability");
state.reportMatch(
errorBuilder.createErrorDescription(
errorMessage, analysis.buildDescription(paramExpression), state, null));
}

/**
* This method returns the type of the given tree, including any type use annotations.
*
Expand Down Expand Up @@ -429,4 +449,59 @@ public void checkTypeParameterNullnessForConditionalExpression(ConditionalExpres
}
}
}

/**
* Checks that for each parameter p at a call, the type parameter nullability for p's type matches
* that of the corresponding formal parameter. If a mismatch is found, report an error.
*
* @param formalParams the formal parameters
* @param actualParams the actual parameters
* @param isVarArgs true if the call is to a varargs method
*/
public void compareGenericTypeParameterNullabilityForCall(
List<Symbol.VarSymbol> formalParams,
List<? extends ExpressionTree> actualParams,
boolean isVarArgs) {
if (!config.isJSpecifyMode()) {
return;
}
int n = formalParams.size();
if (isVarArgs) {
// If the last argument is var args, don't check it now, it will be checked against
// all remaining actual arguments in the next loop.
n = n - 1;
}
for (int i = 0; i < n - 1; i++) {
Type formalParameter = formalParams.get(i).type;
if (!formalParameter.getTypeArguments().isEmpty()) {
Type actualParameter = getTreeType(actualParams.get(i));
if (formalParameter instanceof Type.ClassType
&& actualParameter instanceof Type.ClassType) {
if (!compareNullabilityAnnotations(
(Type.ClassType) formalParameter, (Type.ClassType) actualParameter)) {
reportInvalidParametersNullabilityError(
formalParameter, actualParameter, actualParams.get(i), state, analysis);
}
}
}
}
if (isVarArgs && !formalParams.isEmpty()) {
Type.ArrayType varargsArrayType =
(Type.ArrayType) formalParams.get(formalParams.size() - 1).type;
Type varargsElementType = varargsArrayType.elemtype;
if (varargsElementType.getTypeArguments().size() > 0) {
for (int i = formalParams.size() - 1; i < actualParams.size(); i++) {
Type actualParameter = getTreeType(actualParams.get(i));
if (varargsElementType instanceof Type.ClassType
&& actualParameter instanceof Type.ClassType) {
if (!compareNullabilityAnnotations(
(Type.ClassType) varargsElementType, (Type.ClassType) actualParameter)) {
reportInvalidParametersNullabilityError(
varargsElementType, actualParameter, actualParams.get(i), state, analysis);
}
}
}
}
}
}
}
3 changes: 3 additions & 0 deletions nullaway/src/main/java/com/uber/nullaway/NullAway.java
Expand Up @@ -1624,6 +1624,9 @@ private Description handleInvocation(
: Nullness.NONNULL;
}
}
new GenericsChecks(state, config, this)
.compareGenericTypeParameterNullabilityForCall(
formalParams, actualParams, methodSymbol.isVarArgs());
}

// Allow handlers to override the list of non-null argument positions
Expand Down
Expand Up @@ -662,6 +662,62 @@ public void ternaryMismatchedAssignmentContext() {
.doTest();
}

@Test
public void parameterPassing() {
makeHelper()
.addSourceLines(
"Test.java",
"package com.uber;",
"import org.jspecify.annotations.Nullable;",
"class Test {",
"static class A<T extends @Nullable Object> { }",
" static A<String> sampleMethod1(A<A<String>> a1, A<String> a2) {",
" return a2;",
" }",
" static A<String> sampleMethod2(A<A<@Nullable String>> a1, A<String> a2) {",
" return a2;",
" }",
" static void testPositive1(A<A<@Nullable String>> a1, A<String> a2) {",
" // BUG: Diagnostic contains: Cannot pass parameter of type",
" A<String> a = sampleMethod1(a1, a2);",
" }",
" static void testPositive2(A<A<String>> a1, A<String> a2) {",
" // BUG: Diagnostic contains: Cannot pass parameter of type",
" A<String> a = sampleMethod2(a1, a2);",
" }",
" static void testNegative(A<A<String>> a1, A<String> a2) {",
" A<String> a = sampleMethod1(a1, a2);",
" }",
"}")
.doTest();
}

@Test
public void varargsParameter() {
makeHelper()
.addSourceLines(
"Test.java",
"package com.uber;",
"import org.jspecify.annotations.Nullable;",
"class Test {",
" static class A<T extends @Nullable Object> { }",
" static A<@Nullable String> sampleMethodWithVarArgs(A<String>... args) {",
" return new A<@Nullable String>();",
" }",
" static void testPositive(A<@Nullable String> a1, A<String> a2) {",
" // BUG: Diagnostic contains: Cannot pass parameter of type",
" A<@Nullable String> b = sampleMethodWithVarArgs(a1);",
" // BUG: Diagnostic contains: Cannot pass parameter of type",
" A<@Nullable String> b2 = sampleMethodWithVarArgs(a2, a1);",
" }",
" static void testNegative(A<String> a1, A<String> a2) {",
" A<@Nullable String> b = sampleMethodWithVarArgs(a1);",
" A<@Nullable String> b2 = sampleMethodWithVarArgs(a2, a1);",
" }",
"}")
.doTest();
}

private CompilationTestHelper makeHelper() {
return makeTestHelperWithArgs(
Arrays.asList(
Expand Down

0 comments on commit a1d1eed

Please sign in to comment.