New rule: Warn on ignored return value

[bbaldino]

The rule or changes you're looking for might have already been suggested!
Please search in the issues before creating a new one.

Expected Behavior of the rule

If a method which returns a value is called and the return value is ignored, detekt should issue a warning.

Example:

fun Byte.putBit(bitPos: Int, isSet: Boolean): Byte {
    return if (isSet) {
        (this.toInt() or (0b10000000 ushr bitPos)).toByte()
    } else {
        (this.toInt() and (0b10000000 ushr bitPos).inv()).toByte()
    }
}

It would be easy for someone to call myByte.putBit(1, true) and think it would be modified in place, but actually it returns the new value. A failure to handle this return value will almost certainly result in a bug.

Context

In many (most?) cases, the failure to observe a value returned by a function will result in a bug, but it can be an easy thing for a developer to forget. A warning will help catch instances where the return value should be handled. Cases which don't fall under this condition can suppress the warning (which also serves to explicitly document that it was intended).

I plan on taking a look at implementing this rule myself when I get a chance.

[BraisGabin]

Related topics: The annotation @CheckReturnValue does that more less. There was a thread about this here: #209 and there is an implementation for android lint and error-prone

I think that force to handle all the returns values is overkill. It works perfect for a pure-function world but in the real world you are going to add far to much @Supress. I agree that it could be usefull for some use cases but I'm not sure of it as a core rule.

On the other hand, a rule that checks the @CheckReturnValue and reports the unused results would be great. Because here is the owner of the function which is saying that you should handle the return value and if you don't we should point it out. This would need type and symbol resolution but I think that it should be a good rule.

[3flex]

Thanks @bbaldino for the rule suggestion (and first contribution PR!) and @BraisGabin for your comments, which I 100% agree with.

Limiting this to functions annotated with @CheckReturnValue is best, as that's already a convention used in several projects. We might also want to introduce a detekt-specific annotation for those who want to enforce CheckReturnValue but don't already have that annotation from another provider on the classpath - this is what error-prone does. But that could be a future enhancement.

[bbaldino]

Of course I'll defer to what you guys think is best, but I'll admit I actually think more often than not a function which returns something whose value is ignored is a bug, though I don't have any data to back that up. I'm doing some tests against the my actual project to try and see what the results are like, but the rule logic still needs further refinement.