Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

generate @CheckReturnValue annotations #1316

Open
Bananeweizen opened this issue May 4, 2022 · 1 comment
Open

generate @CheckReturnValue annotations #1316

Bananeweizen opened this issue May 4, 2022 · 1 comment

Comments

@Bananeweizen
Copy link

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.

@eamonnmcmanus
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants