golangci-lint icon indicating copy to clipboard operation
golangci-lint copied to clipboard

Published 20 hours ago • verified publisher icon golangci

New linter: flag unnecessary calls to `Finish` on `gomock.Controller`s in Go 1.14+

Open jamietanna opened this issue 1 year ago • 3 comments

Your feature request related to a problem? Please describe.

As highlighted in https://pkg.go.dev/github.com/golang/mock/gomock#NewController:

New in go1.14+, if you are passing a *testing.T into this function you no longer need to call ctrl.Finish() in your test methods.

Describe the solution you'd like.

A linting rule that can flag when we're unnecessarily calling .Finish, when using a Go version of 1.14+

Describe alternatives you've considered.

N/A

Additional context.

No response

jamietanna avatar Sep 16 '22 13:09 jamietanna

Hello,

FYI golangci-lint is a meta-linter, there are no internal linting rules.

ldez avatar Sep 16 '22 13:09 ldez

That's fair! Are you happy me leaving this here in case someone else wants to pick it up and contribute it?

jamietanna avatar Sep 16 '22 13:09 jamietanna

I flagged it as a linter idea, so yes we can keep it. Can you improve the title of your issue?

ldez avatar Sep 16 '22 13:09 ldez

Hi can I try implementing a linter based on this idea?

daikidev111 avatar Nov 21 '22 18:11 daikidev111

Yes please, happy for you to go for it ☺

jamietanna avatar Nov 21 '22 18:11 jamietanna

Sorry for a bit of delay as I was busy with my uni work. I have finished the implementation part and I will do more testing to ensure that it really works. Once done with it, I will throw a PR.

daikidev111 avatar Dec 02 '22 03:12 daikidev111

@daikidev111, how is it going? :)

Pay attention that github.com/golang/mock is archived and moved under Uber support (github.com/uber-go/mock).

Antonboom avatar Oct 01 '23 09:10 Antonboom

@Antonboom I've completed 90% of the implementation. However, due to the package having different support now, I'm unsure whether I should finish the implementation and submit a pull request or not. Is it possible to create a linter for a package that isn't natively supported by Go?

daikidev111 avatar Oct 02 '23 15:10 daikidev111

Is it possible to create a linter for a package that isn't natively supported by Go?

It has never been supported natively by Go, it was related to a library.

The libraries inside the golang organization are not related to Go itself.

ldez avatar Oct 02 '23 17:10 ldez

Thank you for the clarification. I will resume working on this!

daikidev111 avatar Oct 07 '23 14:10 daikidev111

Hi, look like the interested user still doesn't move forward with this

so I created the linter here https://github.com/hendrywiranto/gomockcontrollerfinish will open a PR soon to integrate it also if you guys have other idea for the name I'm still open for it, maybe gomocklint might be a more suitable name

hendrywiranto avatar Nov 13 '23 15:11 hendrywiranto

For the workaround see https://github.com/golangci/golangci-lint/pull/4202#issuecomment-1824345449

alexandear avatar Nov 23 '23 12:11 alexandear

@ldez maybe we should close this issue because it's possible to detect Finish calls with forbidigo?

alexandear avatar Nov 25 '23 16:11 alexandear