Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

golangci-lint: update to v1.49.0 #44089

Merged
merged 1 commit into from Sep 27, 2022
Merged

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Sep 3, 2022

Remove the "deadcode", "structcheck", and "varcheck" linters, as they are
deprecated:

WARN [runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused.
WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused.
WARN [runner] The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused.
WARN [linters context] structcheck is disabled because of generics. You can track the evolution of the generics support by following the https://github.com/golangci/golangci-lint/issues/2649.

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah
Copy link
Member Author

The remaining ones appear to be legit, but we should review what timeouts we should (and can) set; details about these checks can be found below;

libnetwork/diagnostic/server.go:96:10: G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (gosec)
    srv := &http.Server{
        Addr:    net.JoinHostPort(ip, strconv.Itoa(port)),
        Handler: s,
    }
api/server/server.go:60:10: G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (gosec)
            srv: &http.Server{
                Addr: addr,
            },
daemon/metrics_unix.go:34:13: G114: Use of net/http serve function that has no support for setting timeouts (gosec)
        if err := http.Serve(l, mux); err != nil && !strings.Contains(err.Error(), "use of closed network connection") {
                  ^
cmd/dockerd/metrics.go:27:13: G114: Use of net/http serve function that has no support for setting timeouts (gosec)
        if err := http.Serve(l, mux); err != nil && !strings.Contains(err.Error(), "use of closed network connection") {
                  ^

@thaJeztah
Copy link
Member Author

Oh, fun; bug in the linter? I tried to enable all revive rules, and got this;

INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 423.369909ms 
panic: not enough arguments for cognitive-complexity rule, expected 1, got 0. Please check the rule's documentation

goroutine 7368 [running]:
github.com/mgechev/revive/rule.checkNumberOfArguments(...)
	/go/pkg/mod/github.com/mgechev/revive@v1.2.3/rule/utils.go:171
github.com/mgechev/revive/rule.(*CognitiveComplexityRule).configure(0xc004ac5aa0?, {0x0?, 0x8f0000000131b94f?, 0x1919360?})
	/go/pkg/mod/github.com/mgechev/revive@v1.2.3/rule/cognitive-complexity.go:22 +0x1ad
github.com/mgechev/revive/rule.(*CognitiveComplexityRule).Apply(0xc0003d5ab0, 0xc004d05410?, {0x0?, 0x14?, 0x0?})
	/go/pkg/mod/github.com/mgechev/revive@v1.2.3/rule/cognitive-complexity.go:35 +0x35
github.com/mgechev/revive/lint.(*File).lint(0xc0019457c0, {0xc003926800, 0x44, 0x80}, {0x0, 0x3fe999999999999a, {0x16a6eea, 0x7}, 0x1, 0xc004d05410, ...}, ...)
	/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/file.go:105 +0x19b
github.com/mgechev/revive/lint.(*Package).lint.func1(0xc0030f9060?)
	/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/package.go:185 +0x85
created by github.com/mgechev/revive/lint.(*Package).lint
	/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/package.go:184 +0xac
make: *** [Makefile:251: validate-golangci-lint] Error 2

@thaJeztah thaJeztah force-pushed the update_golangci_lint branch 2 times, most recently from 858a134 to 520281f Compare September 23, 2022 21:30
Remove the "deadcode", "structcheck", and "varcheck" linters, as they are
deprecated:

    WARN [runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused.
    WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused.
    WARN [runner] The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused.
    WARN [linters context] structcheck is disabled because of generics. You can track the evolution of the generics support by following the golangci/golangci-lint#2649.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Copy link
Member

@neersighted neersighted left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM -- is there a lint that can enforce a reason being added to inline linter exclusions?

@thaJeztah
Copy link
Member Author

is there a lint that can enforce a reason being added to inline linter exclusions?

Hmm.. good one; maybe there is 🤔. Actually, I think nolintlint may be able to check for that; https://github.com/golangci/golangci-lint/blob/master/pkg/golinters/nolintlint/README.md

There's a sh*tload of linters in GolangCI-lint https://golangci-lint.run/usage/linters/, and revive itself is becoming more of a meta-linter as well; not all of them are useful to run by default (some very opinionated; good suggestions, but "it depends", so those may be good to run periodically to manually fix up some things).

I'm interested in exportloopref (https://github.com/kyoh86/exportloopref), but I tried it recently, and the specific case where I was hoping it would produce an error, it ... didn't. So I need to look into why not (I think revive now also has some checks for that, so we need to look at overlaps)

@thaJeztah thaJeztah merged commit 762fc76 into moby:master Sep 27, 2022
@thaJeztah thaJeztah deleted the update_golangci_lint branch September 27, 2022 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants