chore: fix errors reported by golangci-lint

[galargh]

This PR prepares go-libp2p for the addition of native golangci-lint support to UCI. I fix a bunch of errors reported by golangci-lint in this codebase and disable errcheck as it produces a huge number of errors and #3269 mentioned that there was some doubt whether it was going to be useful going forward.

Why the errors fixed here weren't reported by the current implementation of golangci-lint in CI?

This is because the added action was configured to check only the diff from the pull request i.e. it never runs checks over the entire codebase.

Unlike the default golangci-lint action, I propose the UCI implementation to run checks over the entire codebase in these cases:

• when the workflow is triggered by the push event
• when the pull request modifies either of
  • go.mod
  • .github/workflows/go-check.yml
  • .golangci-lint.yaml

Otherwise, it will behave the same as the action we use now and check only the diff.

Are there any other differences between the UCI implementation of golangci-lint and the action we use now?

Yes! The action we use now does not run checks in subpackages such as examples. The UCI implementation does.

Another difference is that the UCI implementation runs staticcheck and govet separately to golangci-lint. And when we run golangci-lint in CI, we exclude these linters. When you run golangci-lint without these exclusions locally, you might get slightly different results due to different versions of these tools being used. In the future, I'd like to explore using staticcheck and govet from golangci-lint to bridge this gap.

Finally, the UCI implementation of golangci-lint will only run the checks on linux. It won't run them with different GOOS settings. Is that OK?

How will UCI implementation of golangci-lint get enabled?

After UCI version with that feature is released, go-check will start running the golangci-lint step on ubuntu-latest if it finds a golangci-lint configuration file in the repository. We'll be able to remove the jobs that currently run golangci-lint here then.

[sukunrt]

why is this disabled? This was the entire reason why we included revive.

See: #3269 (comment)

[galargh]

It produces a lot of errors at the moment. See https://github.com/libp2p/go-libp2p/actions/runs/15387329399/job/43288668727, for example (btw, 50 is the default cap on the number of reported issues). Do we want all these occurrences to be replaced with _? If so, I can take care of it and reenable the rule.

[sukunrt]

I'd prefer enabling this particular check. It is useful.

I'm also happy with having the "only new changes" option for linting as we currently do.

[galargh]

"only new changes" makes sense in the context of a PR but I think that on a push to the default branch, we should check the entire codebase. Otherwise, one cannot really check if their PR is OK before pushing the changes because golangci-lint would always fail locally due to a huge number of pre-existing failures.

If you don't mind, I'll try to fix the errors reported by the errcheck in the entire codebase then and then I'll reenable the check here.

[sukunrt]

If you don't mind, I'll try to fix the errors reported by the errcheck in the entire codebase then and then I'll reenable the check here.

That works too. Thanks!

[galargh]

I meant revive, of course, not errcheck, as errcheck is already disabled on the master branch 🤦 So I fixed all the issues reported by revive now 🥳 This should be ready for a re-review. Thank you for the feedback 🙇

[sukunrt]

squash and merge :(