uber / NullAway / #888 / 1
93%
master: 93%

DEFAULT BRANCH: master
Ran
Files 75
Run time 14s
Badge
Embed ▾
README BADGES
x

If you need to use a raster PNG badge, change the '.svg' to '.png' in the link

Markdown

Textile

RDoc

HTML

Rst
coverage: 92.552% (-0.01%) from 92.566%
#888.1

push

github-actions

web-flow 
Fix inconsistent treament of generated code in RestrictiveAnnotationHandler (#618)

Without this change, we have a bug whenever the following flags are
used together:

```
-XepOpt:NullAway:TreatGeneratedAsUnannotated=true
-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true
```

and a method inside `@Generated` code is explicitly marked as having a
`@Nullable` return.

In this case, the `RestrictiveAnnotationHandler` would treat the method
as definitely `@NonNull` when handling the `onOverrideMayBeNullExpr` extension
point, but as `@Nullable` for the purposes of `onDataflowVisitMethodInvocation`.

This is inconsistent on its own, but it produces even more undesirable behavior
if a `castToNonNull` method is configured (whether through library models or
`-XepOpt:NullAway:CastToNonNullMethod`).

If `foo` is such a `@Generated`+`@Nullable` method, then, in the above
configuration:

```
Object o = castToNonNull(foo());
o.toString();
```

Produces an error, claiming that a known `@NonNull` value is being passed to
the cast.

Yet the following code:
```
Object o = foo();
o.toString();
```

Causes NullAway to complain about a `@Nullable` dereference (due to dataflow
concluding `o` is `@Nullable`).

To fix this, we need to make sure that the behavior of `AcknowledgeRestrictiveAnnotations`
is consistent.

One option is to not care about `@Generated` vs non-generated code, or
about `TreatGeneratedAsUnannotated`, and always use restrictive annotations
if available. Unfortunately, this is a change that will increase the number of
errors from NullAway on existing codebases (internally, we use
`TreatGeneratedAsUnannotated` explicitly as a way to relax our checks around
thrift generated code which does have `@Nullable` annotations). Thus, the one
remaining answer seems to be that, in this configuration, generated code should
be treated optimistically for both `onOverrideMayBeNullExpr` and dataflow.

4859 of 5250 relevant lines covered (92.55%)

0.93 hits per line

Source Files on job #888.1