Open Bananeweizen opened 2 years ago
I am not sure this would be all that helpful. Client code calls the declared abstract methods, not their concrete implementations. So even if we added @CheckReturnValue
to the latter, I don't think it would have any effect.
User writes:
@AutoValue
public abstract class Foo {
public abstract int bar();
public static Foo of(int bar) {
return new AutoValue_Foo(bar);
}
}
We generate:
class AutoValue_Foo extends Foo {
...
@Override
@CheckReturnValue
public int bar() {...}
}
Client code writes:
Foo foo = Foo.of(23);
foo.bar(); // forgot to use return value
I think as far as ErrorProne is concerned, we're not calling a @CheckReturnValue
method, so nothing is flagged.
Incidentally we're slowly moving towards a world where @CheckReturnValue
will be the default and (with few exceptions) you have to mark a non-void method explicitly if you want to be able to call it without using its return value. In that world, the question here will be moot.
It would be useful to have the
@CheckReturnValue
annotation automatically be added to the AutoValue generated getters and builder methods. Calling these without using their return value (i.e. writing them as statement) is probably always an error (e.g. configuring a builder, but never actually building the final object). The CheckReturnValue annotation is validated at least by errorprone and spotbugs. To my knowledge both tools care only about the simple class name, therefore the annotation could be cloned into the AutoValue annotations namespace, to not create a dependency to any of those tools.Currently the developer can add the annotation to the AutoValue fields and they will be copied (so there is a workaround for the getters). However, it seems impossible to have the annotation at the generated builder methods. With our without using
@AutoValue.CopyAnnotations
they are missing from the generated code.