proposal: cmd/vet: Not using the return value of a function that only produces an error should be flagged

[rski]

Proposal Details

I ran into a piece of code today that did

if something {
   fmt.Errorf("something happened: %s", someString)
}

if somethingElse {
  return fmt.Errorf("something else happened: %s", someOtherString)
}

Clearly the first one was a mistake, the intention was to return. I think it would be nice if vet flagged such uses of Errorf, Wrapf, errors.New as they are most definitely not intentional.

There is an existing tool, https://github.com/kisielk/errcheck which would have pointed this out. However, in my experience integrating it in existing codebases can be quite tedious and requires fixing many calls that might be fishy but not necessarily buggy.

[seankhliao]

As your experience points out, this wouldn't meet the accuracy bar for vet.

[rski]

sorry, I meant to contrast them. errcheck checks for every single possible function. I'm suggesting vet only check the ones from the standard library, which would be very accurate

[randall77]

What about io.Closer? Do we have to handle the error returned by any Close call in the stdlib?

[rski]

Close has side effects. Errorf doesn't, the whole point of it is to create an error, so doing nothing with it doesn't make sense. To me it's the same sort of check as not allowing unused variables.

[randall77]

Sure, but how do you differentiate Close from Errorf? They both return just an error.

[rski]

oh, by listing the relevant functions out I guess

[randall77]

Right, just depending on the return type as proposed is probably insufficient.
So I think this issue is asking for something more specific. Maybe just fmt.Errorf? Some more expansive list? Would it be opt-in or opt-out for new functions?
Something similar to #62729 for fmt?

[ianlancetaylor]

This is a subset of #65984.