Skip to content

Add @checkReturnValue annotation that requires callers to use the return value #36653

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

Closed
parren-google opened this issue Apr 17, 2019 · 4 comments
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. P3 A lower priority bug or feature request

Comments

@parren-google
Copy link

It would be very helpful on the .build/.rebuild methods of BuiltValue, for example.
Otherwise, one can easily forget the _built= part in

set foo(int val) => _built = _built.rebuild((b) => b..val = val);

Dart VM version: 2.2.1-dev.4.2+google3-v2.2.1.dev.4.2 (google3) on "linux_x64"

@davidmorgan fyi

@vsmenon vsmenon added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Apr 17, 2019
@bwilkerson
Copy link
Member

Do you see this as being a generally useful annotation, or is this specifically to catch a common error in the way BuiltValue is used?

If it's the latter, then this might want to be part of the checking that the built_value plugin performs.

If it's the former, then could you be a bit more specific about what you mean by "check" or "use"? In the example it means "assigned to a field", but if this is a general annotation it should probably include something more than that.

@parren-google
Copy link
Author

This is more generic. For example, it can be used on methods that return error values which callers really should not ignore by accident. Similar to the discussion about not awaiting Future returns.

So there are two categories:

  • methods returning values that should not be ignored (like errors)
  • methods whose sole purpose is to produce a return value without side effects -> not using the return is always a strong smell

For the latter, I guess any pure method qualifies that doesn't use exceptions as a form of regular alternative exit. But I'm not suggesting that Dart auto-detect such methods. The author would have to annotate them. Such auto-detection would be a different thing to possibly look into.

@davidmorgan
Copy link
Contributor

Java has https://errorprone.info/bugpattern/CheckReturnValue which is the same idea.

@srawlins srawlins added the P3 A lower priority bug or feature request label Apr 11, 2021
@pq
Copy link
Member

pq commented Jun 2, 2021

See discussion in #58084; basic support for @useResult has landed.

Feedback (there) welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. P3 A lower priority bug or feature request
Projects
None yet
Development

No branches or pull requests

6 participants