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

Add additional golang linters #407

Closed
wants to merge 1 commit into from
Closed

Conversation

maggie44
Copy link
Contributor

Detect issues like unhandled errors: #365

@nhooyr
Copy link
Owner

nhooyr commented Oct 3, 2023

Hey @maggie44 I appreciate the PR but I'm not a fan of a lot of the automated linters beyond go vet and golint. I find they force you to write code that isn't any more correct just to satisfy the linter and so the code ends up being less clear and more verbose. So I'd prefer to not add anything beyond what I already have.

@nhooyr nhooyr closed this Oct 3, 2023
@nhooyr
Copy link
Owner

nhooyr commented Oct 3, 2023

And the cited issue would not have been detected by a linter because I was explicitly ignoring the error with _.

@maggie44
Copy link
Contributor Author

maggie44 commented Oct 4, 2023

No problem, was worth a look.

Here is the output of the linter on the current master branch, probably worth a look, although I know master is a bit behind dev now so not sure how many are still relevant:

autobahn_test.go:132:22: Error return value of `wstest.Process.Kill` is not checked (errcheck)
                wstest.Process.Kill()
                                   ^
conn_test.go:162:17: Error return value of `n1.SetDeadline` is not checked (errcheck)
                n1.SetDeadline(d)
                              ^
conn_test.go:163:17: Error return value of `n1.SetDeadline` is not checked (errcheck)
                n1.SetDeadline(time.Time{})
                              ^
autobahn_test.go:95:25: SA4009: argument ctx is overwritten before first use (staticcheck)
func wstestClientServer(ctx context.Context) (url string, closeFn func(), err error) {
                        ^
autobahn_test.go:113:2: SA4009(related information): assignment to ctx (staticcheck)
        ctx, cancel := context.WithTimeout(context.Background(), time.Minute*15)
        ^
compress_notjs.go:103:27: Error return value of `(github.com/klauspost/compress/flate.Resetter).Reset` is not checked (errcheck)
        fr.(flate.Resetter).Reset(r, dict)
                                 ^
netconn.go:145:20: Error return value of `c.SetWriteDeadline` is not checked (errcheck)
        c.SetWriteDeadline(t)
                          ^
netconn.go:146:19: Error return value of `c.SetReadDeadline` is not checked (errcheck)
        c.SetReadDeadline(t)
                         ^
read.go:60:11: Error return value of `c.Reader` is not checked (errcheck)
                c.Reader(ctx)
                        ^
write.go:385:14: Error return value of `bw.WriteByte` is not checked (errcheck)
        bw.WriteByte(0)
                    ^
accept.go:278:6: func `acceptWebkitDeflate` is unused (unused)
func acceptWebkitDeflate(w http.ResponseWriter, ext websocketExtension, mode CompressionMode) (*compressionOptions, error) {
     ^
netconn.go:154:22: S1024: should use time.Until instead of t.Sub(time.Now()) (gosimple)
                c.writeTimer.Reset(t.Sub(time.Now()))
                                   ^
netconn.go:163:21: S1024: should use time.Until instead of t.Sub(time.Now()) (gosimple)
                c.readTimer.Reset(t.Sub(time.Now()))
                                  ^
compress_notjs.go:159:26: SA6002: argument should be pointer-like to avoid allocations (staticcheck)
        swPool[cap(sw.buf)].Put(sw.buf)
                                ^
accept.go:83:9: SA4001: &*x will be simplified to x. It will not copy x. (staticcheck)
        opts = &*opts
               ^
dial.go:73:9: SA4001: &*x will be simplified to x. It will not copy x. (staticcheck)
        opts = &*opts
               ^
dial.go:246:10: SA4001: &*x will be simplified to x. It will not copy x. (staticcheck)
        copts = &*copts
                ^
frame_test.go:57:11: SA4030: (*math/rand.Rand).Intn(n) generates a random value 0 <= x < n; that is, the generated values don't include n; r.Intn(1) therefore always returns 0 (staticcheck)
                        return r.Intn(1) == 0
``

@nhooyr
Copy link
Owner

nhooyr commented Oct 4, 2023

Some good ones in there, I'll get them in #256. Definitely not against running them occasionally to filter the false positives. Thanks @maggie44

nhooyr added a commit that referenced this pull request Oct 10, 2023
Credits to @maggie44 for making me add staticcheck.
See #407

Co-authored-by: maggie0002 <64841595+maggie0002@users.noreply.github.com>
@nhooyr
Copy link
Owner

nhooyr commented Oct 10, 2023

Ended up adding staticcheck at least in #256. Thanks again @maggie44

nhooyr added a commit that referenced this pull request Oct 10, 2023
Credits to @maggie44 for making me add staticcheck.
See #407

Co-authored-by: maggie0002 <64841595+maggie0002@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants