Run more linters with golangci-lint

[oxddr]

Currently we run only golint and gofmt. We should at least enable linters which are enabled by default in golangci-lint:

$ golangci-lint help linters
Enabled by default linters:
deadcode: Finds unused code [fast: true, auto-fix: false]
errcheck: Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases [fast: true, auto-fix: false]
gosimple (megacheck): Linter for Go source code that specializes in simplifying a code [fast: true, auto-fix: false]
govet (vet, vetshadow): Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string [fast: true, auto-fix: false]
ineffassign: Detects when assignments to existing variables are not used [fast: true, auto-fix: false]
staticcheck (megacheck): Staticcheck is a go vet on steroids, applying a ton of static analysis checks [fast: true, auto-fix: false]
structcheck: Finds unused struct fields [fast: true, auto-fix: false]
typecheck: Like the front-end of a Go compiler, parses and type-checks Go code [fast: true, auto-fix: false]
unused (megacheck): Checks Go code for unused constants, variables, functions and types [fast: false, auto-fix: false]
varcheck: Finds unused global variables and constants [fast: true, auto-fix: false]

Enabling some of them (I've checked errcheck and deadcode) would require fixing outstanding issues.

Work here should be done incrementally, one-by-one.

/good-first-issue
/help-wanted

[k8s-ci-robot]

@oxddr:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

Currently we run only golint and gofmt. We should at least enable linters which are enabled by default in golangci-lint:

$ golangci-lint help linters
Enabled by default linters:
deadcode: Finds unused code [fast: true, auto-fix: false]
errcheck: Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases [fast: true, auto-fix: false]
gosimple (megacheck): Linter for Go source code that specializes in simplifying a code [fast: true, auto-fix: false]
govet (vet, vetshadow): Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string [fast: true, auto-fix: false]
ineffassign: Detects when assignments to existing variables are not used [fast: true, auto-fix: false]
staticcheck (megacheck): Staticcheck is a go vet on steroids, applying a ton of static analysis checks [fast: true, auto-fix: false]
structcheck: Finds unused struct fields [fast: true, auto-fix: false]
typecheck: Like the front-end of a Go compiler, parses and type-checks Go code [fast: true, auto-fix: false]
unused (megacheck): Checks Go code for unused constants, variables, functions and types [fast: false, auto-fix: false]
varcheck: Finds unused global variables and constants [fast: true, auto-fix: false]

Enabling some of them (I've checked errcheck and deadcode) would require fixing outstanding issues.

Work here should be done incrementally, one-by-one.

/good-first-issue
/help-wanted

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[bitcodr]

@oxddr I'd like to work on it

[oxddr]

@bitcodr that's great to hear. I think the best way to proceed is to pick up one of the linters from above (e.g. deadcode of errcheck), enable it and fix all reported issue and send me a PR for a review.

[wfernandes]

Hey @bitcodr Are you perhaps still working on this? If so feel free to /assign this issue. I was just looking for places to contribute and didn't know if this issue was taken or not. Thanks.

[satishbellapu]

/assign

[wojtek-t]

Adding gofmt would be especially useful.

[satishbellapu]

@oxddr Cleared errors for few of the modules, need some help on "clusterloader2" on few errors,

ERRO Running error: deadcode: analysis skipped: errors in package: [.../perf-tests/clusterloader2/pkg/chaos/nodes.go:109:52: cannot use k.client (variable of type kubernetes.Interface) as kubernetes.Interface value in argument to util.GetSchedulableUntainedNodes: wrong type for method AdmissionregistrationV1 .../perf-tests/clusterloader2/pkg/chaos/nodes.go:114:52

[wojtek-t]

@satishbellapu - this looks strange.
But let's work iteratively on it. Feel free to send a PR that is fixing just some of the errors.
And then the pattern should be:

• extend the presubmit to check gofmt, but make it not fail the test (just informative)
• we will be working on fixing it while having it running
• once we fix everything, we will make it blocking

[kushthedude]

Hey @wojtek-t, I would like to fix it! Is this up for grabs?

[wojtek-t]

I didn't see any actions from @satishbellapu for last month, so it seems fine.

[kushthedude]

/assign

[kushthedude]

Errcheck has been added, Deadcode linter on the way!

[kushthedude]

Hi @wojtek-t with #1448, we will have six outstanding linters running with golangci-lint. Apart from them I would like to suggest staticcheck & goimport to be enabled, what do you suggest do we need them or we are fine without them?

[wojtek-t]

I think we're fine without them (at least now). We can add them later...

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[wojtek-t]

/remove-lifecycle stale

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[wojtek-t]

/remove-lifecycle stale

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[wojtek-t]

/remove-lifecycle stale
/lifecycle frozen