Go-tools: Incorrect return simplification

[JensRantil]

func (t *trainer) Train(labels []string, description string) error {
...
    if _, err := fmt.Fprintln(t.bufTrainingFile); err != nil {
        return err
    }
    return nil
}

yields

$ gosimple *.go
trainer.go:89:2:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (megacheck)

but

func (t *trainer) Train(labels []string, description string) error {
...
    return fmt.Fprintln(t.bufTrainingFile)
}

would never work since fmt.Fprintln returns multiple values.

Additional info:

$ gosimple --version
gosimple 2017.2

[dominikh]

Can confirm, will fix.

[ryboe]

Is this really a false-positive? I think it should be simplified to this:

_, err := fmt.Fprintln(t.bufTrainingFile)
return err

[JensRantil]

Yes, but in that case I think we need a different error message.

[dmitshur]

@dominikh How are you planning on fixing this? I agree with above, this doesn’t seem like a false positive, just the message wording might need adjusting to make it more applicable.

Also worth pointing out that the message said it can be simplified to return err, not to return fmt.Fprintln(...).

[dominikh]

@shurcooL We'll see. That entire check has been a thorn in my side because I feel that it's too eager at times. It makes suggestions that only minimally improve code and that sometimes break consistency.

As a first step, I will likely restrict it to if conditions with no assignment, simply to reduce the number of times it makes the suggestion. In the future, I may further reduce the instances on which it makes the suggestion.

[dmitshur]

That’s unfortunate. :( I like this check and want it to trigger in all applicable cases. But I understand.