Support AutoOneOf

[kennknowles]

AutoOneOf is a very nice autovalue extension for creating tagged unions in Java. It seems that it does not "just work" with checkerframework.

I have saved a repo to reproduce:

git clone --depth 1 --branch checker-crashes-and-autooneof https://github.com/kennknowles/beam
cd beam
./gradlew compileJava

The repo is configured to use checkerframework 3.5.0. I have upgraded to 3.7.0 and the result is the same.

This will produce a number of crashes in Checkerframework (which I will file next), but also:

/Users/klk/GitHub/apache/beam/sdks/java/core/build/generated/sources/annotationProcessor/java/main/org/apache/beam/sdk/schemas/AutoOneOf_FieldAccessDescriptor_FieldDescriptor_Qualifier.java:50: error: [override.param.invalid] Incompatible parameter type for x.
    public boolean equals(Object x) {
                                 ^
  Method
    @Initialized @NonNull boolean equals(@Initialized @NonNull Impl_list this, @Initialized @NonNull Object p0) in org.apache.beam.sdk.schemas.AutoOneOf_FieldAccessDescriptor_FieldDescriptor_Qualifier.Impl_list
  cannot override
    @Initialized @NonNull boolean equals(@Initialized @NonNull Object this, @Initialized @Nullable Object p0) in java.lang.Object
  found   : @Initialized @NonNull Object
  required: @Initialized @Nullable Object
/Users/klk/GitHub/apache/beam/sdks/java/core/build/generated/sources/annotationProcessor/java/main/org/apache/beam/sdk/schemas/AutoOneOf_FieldAccessDescriptor_FieldDescriptor_Qualifier.java:84: error: [override.param.invalid] Incompatible parameter type for x.
    public boolean equals(Object x) {
                                 ^
  Method
    @Initialized @NonNull boolean equals(@Initialized @NonNull Impl_map this, @Initialized @NonNull Object p0) in org.apache.beam.sdk.schemas.AutoOneOf_FieldAccessDescriptor_FieldDescriptor_Qualifier.Impl_map
  cannot override
    @Initialized @NonNull boolean equals(@Initialized @NonNull Object this, @Initialized @Nullable Object p0) in java.lang.Object
  found   : @Initialized @NonNull Object
  required: @Initialized @Nullable Object
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
7 errors
1 warning

> Task :sdks:java:core:compileJava FAILED

FAILURE: Build failed with an exception.

I have not found any way to annotate FieldAccessDescriptor.FieldDescriptor.Qualifier (the abstract base for the AutoOneOf) to make this work or to suppress the nullness checker temporarily.

[kennknowles]

I am also interested in any short-term workarounds. My current project is to enable checkerframework by default throughout the project and suppress at the class level (rather than the module level), so that fixing up individual classes can be starter bugs.

[mernst]

The equals method's argument may be null. So equals should be declared as

public boolean equals(@Nullable MyClass other) { ... }

It seems the problem is that AutoValue isn't generating the @Nullable annotation. (I can't blame it, because it doesn't know about the Nullness Checker.)

Is that your analysis of the problem as well?

Here are some ideas, off the cuff. Maybe other people can come up with better ideas.

Things you could do:

• Suppress type-checking of generated files. -AskipDefs is probably the easiest way to do this.
• Change AutoValue to output the @Nullable annotation.
• Post-process the Checker Framework output to remove these warnings.

Things the Checker Framework could do, to support cases like this where source code is compiled by the Checker Framework but for some reason the programmer is unable to edit the source code.

• Add a mechanism that automatically inherits annotations from superclasses; the linked manual section notes pros and cons of such a mechanism.
• Add a mechanism for injecting annotations into source code that is being compiled. This would be like a stub file, but stub files are ignored if .java files are available, so a mechanism somewhat like -AmergeStubsWithSource would be needed.
• Add a more elaborate warning suppression mechanism than -AsuppressWarnings, permitting command-line arguments to target specific classes and methods.

[kennknowles]

Yes, my analysis of the issue matches yours.

I notice that AutoValue generates equals without the @Nullable annotation even in cases without @AutoOneOf. Yet that does work, doesn't it? I thought it did based on my experience so far, hence thought that there was some special-casing of AutoValue present in the nullness checking, or skipping generated code based on the Generated annotation or some such. I do see in the docs and code that this special support is limited to Called Methods and Returns Receiver. So I don't understand how any AutoValue class passes nullness, or if I am mistaken in thinking that they do pass.

Thank you for the suggestion of -AskipDefs, as I do think that will unblock this case and others (I have protocol buffer generated code that also fails checker).

Separately, it does make sense for AutoValue to annotate generated classes. It does use @Nullable annotations to guide generation of the code, so in some sense has embraced such annotations. Perhaps such a change would be welcomed.

[mernst]

I don't know why AutoValue-generated code would typecheck without @AutoOneOf but fail to typecheck with @AutoOneOf. If you have a test case for that, it would be useful.

If AutoValue already uses @Nullable, then it should add @Nullable to equals as well. If you could make a pull request about that to AutoValue, you wouldn't need to use -AskipDefs, and others would be helped as well. Thanks!

[kennknowles]

I figured out why we did not hit errors in AutoValue-generated classes: apache/beam@1742154 :-)

As for Nullable in AutoValue: the framework uses Nullable annotations to guide their code gen (doing the necessary runtime checks) and propagates the user's annotations. But I think the problem is in this case they would need to synthesize a Nullable annotation. The ecosystem of annotations is splintered enough that there may be an issue with choosing which annotation to insert since the javax annotation is artificially limited. Typically autovalue-generated classes have no added dependencies.

I believe that this ticket is probably not worth tracking in light of the fact that my project already uses skipDefs for autovalue and that it is reasonable to push the issue upstream to autovalue and autooneof.