Skip to content

Commit

Permalink
Fix false positive involving type parameter @nullable annotations.
Browse files Browse the repository at this point in the history
NullAway doesn't currently support `@Nullable` annotations on type
parameters. However, we had an error where a `@Nullable` annotation
in a type parameter of the return type of a method was being
interpreted as applying to the method return itself.

For example:

```
public static <R> Foo<@nullable R> bar() { ... }

public static void baz() {
  bar().toString();
}
```

would mistakenly be considered a null dereference.

We introduce an additional check to `NullabilityUtil.getTypeUseAnnotations(...)`
which will retrieve the method return annotation for the following cases:

```
@nullable public static <R> Foo<R> bar()
@nullable public static <R> Foo<@nullable R> bar()
public static <R> @nullable Foo<R> bar()
```

Yet ignore the annotation inside the type parameter of `Foo`.

It is possible we might need to refactor this check when we actually start
handling nullability of type parameters.
  • Loading branch information
lazaroclapp committed Jun 10, 2022
1 parent cea4fb8 commit abaa1f9
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 2 deletions.
10 changes: 8 additions & 2 deletions nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -271,11 +271,17 @@ public static Stream<? extends AnnotationMirror> getAllAnnotationsForParameter(
&& t.position.parameter_index == paramInd));
}

private static Stream<? extends AnnotationMirror> getTypeUseAnnotations(Symbol symbol) {
public static Stream<? extends AnnotationMirror> getTypeUseAnnotations(Symbol symbol) {
Stream<Attribute.TypeCompound> rawTypeAttributes = symbol.getRawTypeAttributes().stream();
if (symbol instanceof Symbol.MethodSymbol) {
// for methods, we want the type-use annotations on the return type
return rawTypeAttributes.filter((t) -> t.position.type.equals(TargetType.METHOD_RETURN));
return rawTypeAttributes.filter(
(t) ->
// location is a list of TypePathEntry objects, indicating whether the annotation is
// on an array, inner type, wildcard, or type argument. If it's empty, then the
// annotation
// is directly on the return type.
t.position.type.equals(TargetType.METHOD_RETURN) && t.position.location.isEmpty());
}
return rawTypeAttributes;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package com.uber.nullaway;

import org.junit.Test;

public class NullAwayTypeUseAnnotationTests extends NullAwayTestsBase {
@Test
public void typeParameterAnnotationIsDistinctFromMethodReturnAnnotation() {
defaultCompilationHelper
.addSourceLines(
"Test.java",
"package com.uber;",
"import org.jspecify.nullness.Nullable;",
"import java.util.function.Supplier;",
"public class Test {",
" public static <R> @Nullable Supplier<@Nullable R> getNullableSupplierOfNullable() {",
" return new Supplier<R>() {",
" @Nullable",
" public R get() { return null; }",
" };",
" }",
" public static <R> Supplier<@Nullable R> getNonNullSupplierOfNullable() {",
" return new Supplier<R>() {",
" @Nullable",
" public R get() { return null; }",
" };",
" }",
" public static String test1() {",
" // BUG: Diagnostic contains: dereferenced expression getNullableSupplierOfNullable() is @Nullable",
" return getNullableSupplierOfNullable().toString();",
" }",
" public static String test2() {",
" // The supplier contains null, but isn't itself nullable. Check against a past FP",
" return getNonNullSupplierOfNullable().toString();",
" }",
"}")
.doTest();
}
}

0 comments on commit abaa1f9

Please sign in to comment.