[New rule] Avoid Ignoring Return Values

[vHanda]

Please describe what the rule should do:
We should never silently ignore a return value of a function.

If your rule is inspired by other please provide link to it:

• Swift has a similar feature
• Request for this in Typescript
• Java Jetbrains issue for such an annotation
• Golang proposal
• GoLang IDE (Goland by Jetbrains) implements such a warning
• Rust #must_use
• OCaml warns about unused return values by default. It provides an ignore function in the standard library.

I found out about this from this wonderful blog post by Joe Duffy (Warning: Super long)

What category of rule is this? (place an "X" next to just one item)

[X] Warns about a potential error (problem)
[ ] Suggests an alternate way of doing something (suggestion)
[ ] Other (please specify:)

Provide 2-3 code examples that this rule will warn about (it will be better if you can provide both good and bad examples):

int foo() {
    return 5;
}

void goo() {
    print('whatever');
}

void main() {
    goo(); // fine, no return value
    var t = foo(); // fine, return value is assigned
    foo(); // BAD: return value is silently ignored

    var str = "Hello there";
    str.substr(5); // BAD: Strings are immutable and the return value should be handled

    var date = new DateTime(2018, 1, 13);
    date.add(Duration(days: 1, hours: 23))); // BAD: Return value ignored, DateTime is immutable
}

The rationale behind the rule:

• Immutable classes: DateTime/String are immutable and operations on them return a new object. One sometimes forgets about this and bugs are introduced.

• Some people are using the Result class and avoiding throwing exceptions. Here each operation returns a union type with either the expected result or an error. Sometimes, certain operations only return an error. It would be nice to be reminded to handle that error.

  I've been using this pattern in dart-git where I have found it is critical to handle all errors. Exceptions haven't worked for me, as we don't have checked-exceptions.

Are you willing to submit a pull request to implement this rule? No

(maybe? I haven't ever implemented a rule before, and I'm not sure how long it will take me, though I'm willing to try - but I don't want to promise anything)

[incendial]

@vHanda thanks for suggesting this rule. It looks awesome and definitely nice to have, but I don't think that we can easily get the return type of an invocation while performing static analysis. I will take a closer look and return with the results.

Dart team have recently added a new annotation to meta package called useResult. I think that they try to solve the same problem, although it will require all libraries to add it to their source code.

[incendial]

Nope, I was wrong, we have a return type info! Seems that there are no blockers for implementing this rule.

[incendial]

@vHanda have you though about exceptions for this rule? I think that void, Never, Null, Future<void>, Future<Null> should be definitely ignored, but maybe there is more?

And how do you think we should handle methods/functions that mutate the data and return something (and you might not want to handle the returned value)?

[vHanda]

@vHanda have you though about exceptions for this rule? I think that void, Never, Null, Future<void>, Future<Null> should be definitely ignored, but maybe there is more?

I can't think of anything else, but lets just start with this. We can then add more.

And how do you think we should handle methods/functions that mutate the data and return something (and you might not want to handle the returned value)?

I would like the user to explicitly assign the value anyway.

var _ = operationWithReturnValue();

It makes the code a bit more verbose though. In my case, at least, I want to be super strict, and make a choice about each return value that I'm ignoring.

If possible, could you please point me towards a similar rule I could look at to implement this? I'm not sure when I will, but still.

[incendial]

If possible, could you please point me towards a similar rule I could look at to implement this?

I'm not sure that we have a similar one. But you need to implement visitMethodInvocation in a visitor and check if an invocation is not used in an assignment or cascade and not passed as an argument (maybe there are more edge cases). And have a not exceptional staticType.

[incendial]

@vHanda hi, do you still need this rule? I've opened a PR with the implementation and I think we'll release it in a week or so. Could you help us with running it on your codebase and providing feedback? If so, we can release it earlier as a part of a dev release.

[incendial]

Available in 4.2.0 release.

[vHanda]

Hi. I just tried it out.

It is wonderful. It has already caught a number of bugs. Thank you!!

[incendial]

Happy to hear that! 🎉