Enforce cuddling of error checks

[JDiPierro]

I'd love to be able to enforce that error checking is cuddled with the error assignment.

For example, right now this doesn't trigger the linter:

thing, err := db.GetThing()

if err != nil {
  return err
}

But I'd like it to :)

Thanks for the awesome linter!

[bombsimon]

Thanks for the issue!

Yeah that would be cool! I agree that the recommended way is to cuddle the error with the assignment in your example (and that's why it's allowed/supported).

I started working on fixing the errors and noticed that I had some design flaws I wanted to address first. I lost track a bit but I intend to pick up where I left and improve on this linter. When I do I'll add this feature as well!

If someone wants to create a PR in the meanwhile that's very welcomed!

[bombsimon]

Thanks to contributions from @JDiPierro this is now implemented and merged, thanks!

@JDiPierro Since you're the one who also implemented this I'll let you close the issue if/when you're ready!

[JDiPierro]

I ran the error cuddling enforcement on a large codebase this morning and found a case that generates a false-positive:

if err = ProduceError(); err != nil {
	return err
}

I'll look into supporting that tonight. Other than that it ran great! Once that's fixed up I can put together a PR to golangci-lint with the new configuration option.

[bombsimon]

Ah, thanks! Should've seen that and also another example of bad test coverage. :( I thought I've written tests but I guess I'm pretty biased since I wrote the linter. 😄

Please do, no rush since nothing is tagged, see master as development.

[bombsimon]

@JDiPierro Have you tried out the master branch more this week? Do you think we can close this issue?

[JDiPierro]

I haven't run it on any repos beside the one project I've already integrated wsl with. I'd expect it to throw many many errors on any other projects :P I can probably give it a shot though just to see if the errors it throws make sense. The repo that I have tested it with is 10k LoC according to SonarQube so it was a decent workout.

I was thinking of leaving this open until the golangci-lint changes get in for it and it's generally available. If you'd prefer to close it since the change is in master that's cool too :)

I have a start to the golanci-lint changes in the wsl__error_enforcement branch of the fork on my profile, here's the commit to add the new config: JDiPierro/golangci-lint@d37945a

But I believe before that can be merged WSL needs to cut a release so we can update the go.mod, vendor the new version, and then the new config would be valid. As it is now the wsl.Configuration is invalid because the new fields don't exist.

LMK how you'd like to proceed with that.

[bombsimon]

Alright, no worries, just curious! No rush, just wanted to know if everything seems OK. Feel free to leave this open!

I've drafted a new release (v2.1.0) on a new branch with the same name.

I don't really like the idea of basing releases of specific branches but I think this is alright for this time. I think I'll probably merge v2.1.0 into master and the next release will be built directly on master. It's just easier to keep track of everything that way.

[bombsimon]

PR in golangci-lint. Will close this issue when merged.

[bombsimon]

Merged to master in golangci-lint, will be in next release.